Skip to content

Commit

Permalink
perf(linter): disable lint rules by file type (#4380)
Browse files Browse the repository at this point in the history
### TL;DR

Added a `should_run` function to multiple lint rules to determine if a rule should be executed based on the source type. This change optimizes the linting process by avoiding unnecessary rule checks.

### What changed?

1. **New Method**: Introduced the `should_run` method in the `Rule` trait.
2. **Implementation**: Implemented the `should_run` method for various lint rules, particularly those related to React and TypeScript.
3. **Usage**: Updated the `Linter` to use the `should_run` method to filter rules before execution.
4. **Macro Update**: Modified the `declare_all_lint_rules` macro to incorporate the `should_run` method.

### How to test?

1. Run the linter on a project containing React and TypeScript files.
2. Verify that only relevant rules are executed based on the file type (e.g., JSX rules for React files).

### Why make this change?

This change improves the performance of the linter by ensuring that only applicable rules are run for a given file type, reducing unnecessary computation and potential false positives.

---
  • Loading branch information
DonIsaac committed Jul 21, 2024
1 parent 44a10c4 commit f7da22d
Show file tree
Hide file tree
Showing 56 changed files with 229 additions and 11 deletions.
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,10 @@ impl Linter {
let ctx = self.create_ctx(path, semantic);
let semantic = Rc::clone(ctx.semantic());

let ctx = ctx.with_fix(self.options.fix).with_eslint_config(&self.eslint_config);
let rules = self
.rules
.iter()
.filter(|rule| rule.should_run(&ctx))
.map(|rule| {
let rule_name = rule.name();
let plugin_name = self.map_jest(rule.plugin_name(), rule_name);
Expand Down
11 changes: 11 additions & 0 deletions crates/oxc_linter/src/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@ pub trait Rule: Sized + Default + fmt::Debug {

/// Run only once. Useful for inspecting scopes and trivias etc.
fn run_once(&self, _ctx: &LintContext) {}

/// Check if a rule should be run at all.
///
/// You usually do not need to implement this function. If you do, use it to
/// enable rules on a file-by-file basis. Do not check if plugins are
/// enabled/disabled; this is handled by the [`linter`].
///
/// [`linter`]: crate::Linter
fn should_run(&self, _ctx: &LintContext) -> bool {
true
}
}

pub trait RuleMeta {
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/button_has_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ impl Rule for ButtonHasType {
.unwrap_or(true),
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

impl ButtonHasType {
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/jsx_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ impl Rule for JsxKey {
_ => {}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

enum InsideArrayOrIterator {
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/jsx_no_comment_textnodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ impl Rule for JsxNoCommentTextnodes {
ctx.diagnostic(jsx_no_comment_textnodes_diagnostic(jsx_text.span));
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

fn control_patterns(pattern: &str) -> bool {
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/jsx_no_duplicate_props.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ impl Rule for JsxNoDuplicateProps {
}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/jsx_no_target_blank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,10 @@ impl Rule for JsxNoTargetBlank {
.unwrap_or(false),
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

fn check_is_external_link(link: &str) -> bool {
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/jsx_no_undef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ impl Rule for JsxNoUndef {
}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/jsx_no_useless_fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ impl Rule for JsxNoUselessFragment {
_ => {}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

impl JsxNoUselessFragment {
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/no_children_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ impl Rule for NoChildrenProp {
_ => {}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/no_danger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ impl Rule for NoDanger {
_ => {}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/no_direct_mutation_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ impl Rule for NoDirectMutationState {
_ => {}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

// check current node is this.state.xx
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/no_find_dom_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ impl Rule for NoFindDomNode {
};
ctx.diagnostic(no_find_dom_node_diagnostic(span));
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/no_is_mounted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ impl Rule for NoIsMounted {
}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/no_render_return_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ impl Rule for NoRenderReturnValue {
}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/no_set_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ impl Rule for NoSetState {

ctx.diagnostic(no_set_state_diagnostic(call_expr.callee.span()));
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/no_string_refs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ impl Rule for NoStringRefs {
_ => {}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/no_unescaped_entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ impl Rule for NoUnescapedEntities {
}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

pub const DEFAULTS: Map<char, &'static [&'static str]> = phf_map! {
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/no_unknown_property.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,10 @@ impl Rule for NoUnknownProperty {
);
});
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/prefer_es6_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ impl Rule for PreferEs6Class {
));
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[derive(Debug, Default, Clone)]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/react_in_jsx_scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ impl Rule for ReactInJsxScope {
ctx.diagnostic(react_in_jsx_scope_diagnostic(node_span));
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react/require_render_return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ impl Rule for RequireRenderReturn {
};
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[derive(Clone, Copy, Debug, Default, PartialEq)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ impl Rule for VoidDomElementsNoChildren {
_ => {}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/react_perf/jsx_no_jsx_as_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ impl Rule for JsxNoJsxAsProp {
check_jsx_element(jsx_elem, ctx);
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

fn check_jsx_element<'a>(jsx_elem: &JSXElement<'a>, ctx: &LintContext<'a>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ impl Rule for JsxNoNewArrayAsProp {
check_jsx_element(jsx_elem, ctx);
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

fn check_jsx_element<'a>(jsx_elem: &JSXElement<'a>, ctx: &LintContext<'a>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ impl Rule for JsxNoNewFunctionAsProp {
check_jsx_element(jsx_elem, ctx);
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

fn check_jsx_element<'a>(jsx_elem: &JSXElement<'a>, ctx: &LintContext<'a>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ impl Rule for JsxNoNewObjectAsProp {
check_jsx_element(jsx_elem, ctx);
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_jsx()
}
}

fn check_jsx_element<'a>(jsx_elem: &JSXElement<'a>, ctx: &LintContext<'a>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,10 @@ impl Rule for AdjacentOverloadSignatures {
_ => {}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_typescript()
}
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/typescript/array_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ impl Rule for ArrayType {
_ => {}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_typescript()
}
}

fn check(
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/typescript/ban_ts_comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,10 @@ impl Rule for BanTsComment {
}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_typescript()
}
}

impl BanTsComment {
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_linter/src/rules/typescript/ban_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ impl Rule for BanTypes {
_ => {}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_typescript()
}
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,10 @@ impl Rule for ConsistentIndexedObjectStyle {
}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_typescript()
}
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ impl Rule for ConsistentTypeDefinitions {
_ => {}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_typescript()
}
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,10 @@ impl Rule for ConsistentTypeImports {
);
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_typescript()
}
}

// Given an array of words, returns an English-friendly concatenation, separated with commas, with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,10 @@ impl Rule for ExplicitFunctionReturnType {
_ => {}
}
}

fn should_run(&self, ctx: &LintContext) -> bool {
ctx.source_type().is_typescript()
}
}

impl ExplicitFunctionReturnType {
Expand Down
Loading

0 comments on commit f7da22d

Please sign in to comment.