Skip to content

Commit

Permalink
perf(linter): no-fallthrough: Use string matching instead of Regex …
Browse files Browse the repository at this point in the history
…for default comment pattern (#6008)

Profiling has shown this rule to be a consistent slow point, and in particular, the Regex construction is slow.

<img width="1323" alt="Screenshot 2024-09-23 at 7 12 58 PM" src="https://github.com/user-attachments/assets/1d9b367d-eeda-4b19-b0a3-463e54ac4334">

This PR improves the situation in two ways:

1. A `Regex` is only constructed when there is a custom comment pattern (which is likely the minority of cases)
2. For the default comment pattern (when no custom pattern is passed), we now use a simple string matcher function, instead of a full-blown regex matcher. I believe this should be faster since it involves much less work, though can still allocate a `String`.
  • Loading branch information
camchenry committed Sep 24, 2024
1 parent 5a0d17c commit 5ae3f36
Showing 1 changed file with 25 additions and 9 deletions.
34 changes: 25 additions & 9 deletions crates/oxc_linter/src/rules/eslint/no_fallthrough.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::ops::Range;

use cow_utils::CowUtils;
use itertools::Itertools;
use oxc_ast::{
ast::{Statement, SwitchCase, SwitchStatement},
Expand Down Expand Up @@ -35,11 +36,12 @@ fn no_unused_fallthrough_diagnostic(span: Span) -> OxcDiagnostic {
.with_label(span)
}

const DEFAULT_FALLTHROUGH_COMMENT_PATTERN: &str = r"falls?\s?through";

#[derive(Debug, Clone)]
struct Config {
comment_pattern: Regex,
/// The custom comment pattern to match against. If set to None, the rule
/// will use the default pattern. Otherwise, if this is Some, the rule will
/// use the provided pattern.
comment_pattern: Option<Regex>,
allow_empty_case: bool,
report_unused_fallthrough_comment: bool,
}
Expand All @@ -53,9 +55,9 @@ impl NoFallthrough {
allow_empty_case: Option<bool>,
report_unused_fallthrough_comment: Option<bool>,
) -> Self {
let comment_pattern = comment_pattern.unwrap_or(DEFAULT_FALLTHROUGH_COMMENT_PATTERN);
Self(Box::new(Config {
comment_pattern: Regex::new(format!("(?iu){comment_pattern}").as_str()).unwrap(),
comment_pattern: comment_pattern
.map(|pattern| Regex::new(format!("(?iu){pattern}").as_str()).unwrap()),
allow_empty_case: allow_empty_case.unwrap_or(false),
report_unused_fallthrough_comment: report_unused_fallthrough_comment.unwrap_or(false),
}))
Expand Down Expand Up @@ -387,10 +389,7 @@ impl NoFallthrough {
.last()
.map(str::trim);

comment.is_some_and(|comment| {
(!comment.starts_with("oxlint-") && !comment.starts_with("eslint-"))
&& self.0.comment_pattern.is_match(comment)
})
comment.is_some_and(|comment| self.is_comment_fall_through(comment))
};

let (start, end) = possible_fallthrough_comment_span(case);
Expand All @@ -409,6 +408,23 @@ impl NoFallthrough {
None
}
}

fn is_comment_fall_through(&self, comment: &str) -> bool {
if comment.starts_with("oxlint-") || comment.starts_with("eslint-") {
return false;
}
if let Some(custom_pattern) = &self.0.comment_pattern {
custom_pattern.is_match(comment)
} else {
// We are doing a quick check here to see if it starts with the expected "falls" comment,
// so that we don't need to initialize the pattern matcher if we don't need it.
let comment = comment.trim().cow_to_ascii_lowercase();
comment == "falls through"
|| comment == "fall through"
|| comment == "fallsthrough"
|| comment == "fallthrough"
}
}
}

/// Get semantic information about a switch cases and its exit point.
Expand Down

0 comments on commit 5ae3f36

Please sign in to comment.