Skip to content

Commit

Permalink
perf: optimize is_binding_suppressed() (#105)
Browse files Browse the repository at this point in the history
  • Loading branch information
arendjr authored Mar 28, 2024
1 parent 7509491 commit 2290dde
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 57 deletions.
4 changes: 1 addition & 3 deletions crates/core/src/pattern/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,7 @@ impl<'a> State<'a> {
if let ResolvedPattern::Binding(bindings) = value {
for binding in bindings.iter() {
bindings_count += 1;
if is_binding_suppressed(binding, lang, current_name)
.unwrap_or_default()
{
if is_binding_suppressed(binding, lang, current_name) {
suppressed_count += 1;
continue;
}
Expand Down
76 changes: 40 additions & 36 deletions crates/core/src/suppress.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,22 @@
use itertools::{EitherOrBoth, Itertools};
use marzano_language::{
language::Language,
parent_traverse::{ParentTraverse, TreeSitterParentCursor},
};
use tree_sitter::{Node, Range};

use crate::binding::Binding;
use crate::resolve;
use anyhow::Result;

pub(crate) fn is_binding_suppressed(
binding: &Binding,
lang: &impl Language,
current_name: Option<&str>,
) -> Result<bool> {
) -> bool {
let (src, node) = match binding {
Binding::Node(src, node) => (src, node),
Binding::String(_, _) => return Ok(false),
Binding::List(src, node, _) => (src, node),
Binding::Empty(src, node, _) => (src, node),
Binding::FileName(_) => return Ok(false),
Binding::ConstantRef(_) => return Ok(false),
Binding::Node(src, node) | Binding::List(src, node, _) | Binding::Empty(src, node, _) => {
(src, node)
}
Binding::String(_, _) | Binding::FileName(_) | Binding::ConstantRef(_) => return false,
};
let target_range = node.range();
for n in
Expand All @@ -34,13 +31,13 @@ pub(crate) fn is_binding_suppressed(
if !(lang.is_comment(c.kind_id()) || lang.is_comment_wrapper(&c)) {
continue;
}
if is_suppress_comment(&c, src, &target_range, current_name, lang)? {
return Ok(true);
if is_suppress_comment(&c, src, &target_range, current_name, lang) {
return true;
}
}
}

Ok(false)
false
}

fn is_suppress_comment(
Expand All @@ -49,71 +46,78 @@ fn is_suppress_comment(
target_range: &Range,
current_name: Option<&str>,
lang: &impl Language,
) -> Result<bool> {
) -> bool {
let child_range = comment_node.range();
let text = comment_node.utf8_text(src.as_bytes())?;
let Ok(text) = comment_node.utf8_text(src.as_bytes()) else {
return false;
};
let inline_suppress = child_range.end_point().row() >= target_range.start_point().row()
&& child_range.end_point().row() <= target_range.end_point().row();
if !inline_suppress {
let pre_suppress = comment_applies_to_range(comment_node, target_range, lang, src)?
&& comment_occupies_entire_line(text.as_ref(), &comment_node.range(), src)?;
let pre_suppress = comment_applies_to_range(comment_node, target_range, lang, src)
&& comment_occupies_entire_line(text.as_ref(), &comment_node.range(), src);
if !pre_suppress {
return Ok(false);
return false;
}
}
if !text.contains("grit-ignore") {
return Ok(false);
return false;
}
let comment_text = text.trim();
let ignore_spec = match comment_text.split("grit-ignore").collect::<Vec<_>>().get(1) {
Some(s) => match s.split(':').next() {
let ignore_spec = match comment_text.split_once("grit-ignore") {
Some((_, s)) => match s.split(':').next() {
Some(s) => s.trim(),
None => return Ok(true),
None => return true,
},
None => return Ok(true),
None => return true,
};
if ignore_spec.is_empty()
|| ignore_spec
.chars()
.next()
.is_some_and(|c| !c.is_alphanumeric() && c != '_')
{
return Ok(true);
return true;
}
let Some(current_name) = current_name else {
return Ok(false);
return false;
};
let ignored_rules = ignore_spec.split(',').map(|s| s.trim()).collect::<Vec<_>>();
Ok(ignored_rules.contains(&current_name))
ignore_spec
.split(',')
.map(str::trim)
.contains(&current_name)
}

fn comment_applies_to_range(
comment_node: &Node,
range: &Range,
lang: &impl Language,
src: &str,
) -> Result<bool> {
let mut applicable = resolve!(comment_node.next_named_sibling());
) -> bool {
let Some(mut applicable) = comment_node.next_named_sibling() else {
return false;
};
while let Some(next) = applicable.next_named_sibling() {
if !lang.is_comment(applicable.kind_id())
&& !lang.is_comment_wrapper(&applicable)
// Some languages have significant whitespace; continue until we find a non-whitespace non-comment node
&& !applicable.utf8_text(src.as_bytes())?.trim().is_empty()
&& !applicable.utf8_text(src.as_bytes()).map_or(true, |text| text.trim().is_empty())
{
break;
}
applicable = next;
}
let applicable_range = applicable.range();
Ok(applicable_range.start_point().row() == range.start_point().row())
applicable_range.start_point().row() == range.start_point().row()
}

fn comment_occupies_entire_line(text: &str, range: &Range, src: &str) -> Result<bool> {
let code = src
.lines()
fn comment_occupies_entire_line(text: &str, range: &Range, src: &str) -> bool {
src.lines()
.skip(range.start_point().row() as usize)
.take((range.end_point().row() - range.start_point().row() + 1) as usize)
.collect::<Vec<_>>()
.join("\n");
Ok(code.trim() == text.trim())
.zip_longest(text.split("\n"))

Check failure on line 118 in crates/core/src/suppress.rs

View workflow job for this annotation

GitHub Actions / clippy

single-character string constant used as pattern

error: single-character string constant used as pattern --> crates/core/src/suppress.rs:118:33 | 118 | .zip_longest(text.split("\n")) | ^^^^ help: consider using a `char`: `'\n'` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern = note: `-D clippy::single-char-pattern` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::single_char_pattern)]`
.all(|zipped| match zipped {
EitherOrBoth::Both(src_line, text_line) => src_line.trim() == text_line.trim(),
_ => false,

Check failure on line 121 in crates/core/src/suppress.rs

View workflow job for this annotation

GitHub Actions / clippy

wildcard match will also match any future added variants

error: wildcard match will also match any future added variants --> crates/core/src/suppress.rs:121:13 | 121 | _ => false, | ^ help: try: `EitherOrBoth::Left(_) | EitherOrBoth::Right(_)` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#wildcard_enum_match_arm note: the lint level is defined here --> crates/core/src/lib.rs:1:9 | 1 | #![deny(clippy::wildcard_enum_match_arm)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
})
}
28 changes: 10 additions & 18 deletions crates/core/src/text_unparser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,38 +28,30 @@ pub(crate) fn apply_effects<'a>(
current_name: Option<&str>,
logs: &mut AnalysisLogs,
) -> Result<(String, Option<Vec<Range<usize>>>)> {
let mut our_effects = Vec::new();
for effect in effects {
let disabled = is_binding_suppressed(&effect.binding, language, current_name)?;
if !disabled {
our_effects.push(effect);
}
}
if our_effects.is_empty() {
let effects: Vec<_> = effects
.into_iter()
.filter(|effect| !is_binding_suppressed(&effect.binding, language, current_name))
.collect();
if effects.is_empty() {
return Ok((code.to_string(), None));
}
let mut memo: HashMap<CodeRange, Option<String>> = HashMap::new();
let (from_inline, ranges) = linearize_binding(
language,
&our_effects,
&effects,
files,
&mut memo,
code,
CodeRange::new(0, code.len() as u32, code),
language.should_pad_snippet().then_some(0),
logs,
)?;
for effect in our_effects.iter() {
for effect in effects.iter() {
if let Binding::FileName(c) = effect.binding {
if std::ptr::eq(c, the_filename) {
let snippet = effect.pattern.linearized_text(
language,
&our_effects,
files,
&mut memo,
false,
logs,
)?;
let snippet = effect
.pattern
.linearized_text(language, &effects, files, &mut memo, false, logs)?;
*new_filename = PathBuf::from(snippet.to_string());
}
}
Expand Down

0 comments on commit 2290dde

Please sign in to comment.