From 533a8d921c3e1d9e21180deefc26f67d4cd34e5b Mon Sep 17 00:00:00 2001 From: rzvxa Date: Tue, 18 Jun 2024 21:52:19 +0330 Subject: [PATCH 1/4] feat(linter): add `use_cfg` attribute. --- crates/oxc_linter/src/context.rs | 11 +++++++ crates/oxc_linter/src/rule.rs | 2 ++ .../src/rules/eslint/getter_return.rs | 6 ++-- .../src/rules/eslint/no_fallthrough.rs | 4 +-- .../src/rules/eslint/no_this_before_super.rs | 4 +-- .../src/rules/eslint/no_unreachable.rs | 4 +-- .../src/rules/react/require_render_return.rs | 4 +-- .../src/rules/react/rules_of_hooks.rs | 4 +-- crates/oxc_macros/src/declare_oxc_lint.rs | 32 +++++++++++-------- 9 files changed, 45 insertions(+), 26 deletions(-) diff --git a/crates/oxc_linter/src/context.rs b/crates/oxc_linter/src/context.rs index d16a590c142bb..1900c1bae9924 100644 --- a/crates/oxc_linter/src/context.rs +++ b/crates/oxc_linter/src/context.rs @@ -1,5 +1,6 @@ use std::{cell::RefCell, path::Path, rc::Rc, sync::Arc}; +use oxc_cfg::ControlFlowGraph; use oxc_diagnostics::{OxcDiagnostic, Severity}; use oxc_semantic::{AstNodes, JSDocFinder, ScopeTree, Semantic, SymbolTable}; use oxc_span::{SourceType, Span}; @@ -78,6 +79,16 @@ impl<'a> LintContext<'a> { &self.semantic } + /// # Panics + /// If rule doesn't have `#[use_cfg]` in it's declaration it would panic. + pub fn cfg(&self) -> &ControlFlowGraph { + if let Some(cfg) = self.semantic().cfg() { + cfg + } else { + unreachable!("for creating a control flow aware rule you have to add `#[use_cfg]` attribute to its `declare_oxc_lint` declaration"); + } + } + pub fn disable_directives(&self) -> &DisableDirectives<'a> { &self.disable_directives } diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 7183459fd7b97..4dd78dad132b6 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -29,6 +29,8 @@ pub trait RuleMeta { const CATEGORY: RuleCategory; + const USE_CFG: bool; + fn documentation() -> Option<&'static str> { None } diff --git a/crates/oxc_linter/src/rules/eslint/getter_return.rs b/crates/oxc_linter/src/rules/eslint/getter_return.rs index 56d56f09b757a..532ce110b27f9 100644 --- a/crates/oxc_linter/src/rules/eslint/getter_return.rs +++ b/crates/oxc_linter/src/rules/eslint/getter_return.rs @@ -48,14 +48,14 @@ declare_oxc_lint!( /// } /// } /// ``` + #[use_cfg] GetterReturn, - nursery + nursery, ); impl Rule for GetterReturn { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - // control flow dependant - let Some(cfg) = ctx.semantic().cfg() else { return }; + let cfg = ctx.cfg(); // https://eslint.org/docs/latest/rules/getter-return#handled_by_typescript if ctx.source_type().is_typescript() { diff --git a/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs b/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs index b6ec14f96efc7..e6d3c6e3135b8 100644 --- a/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs +++ b/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs @@ -73,6 +73,7 @@ declare_oxc_lint!( /// /// Disallow fallthrough of `case` statements /// + #[use_cfg] NoFallthrough, correctness ); @@ -89,8 +90,7 @@ impl Rule for NoFallthrough { } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - // control flow dependant - let Some(cfg) = ctx.semantic().cfg() else { return }; + let cfg = ctx.cfg(); let AstKind::SwitchStatement(switch) = node.kind() else { return }; diff --git a/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs b/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs index bc756b210c832..aac0f70883c0f 100644 --- a/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs +++ b/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs @@ -42,6 +42,7 @@ declare_oxc_lint!( /// } /// } /// ``` + #[use_cfg] NoThisBeforeSuper, correctness ); @@ -56,9 +57,8 @@ enum DefinitelyCallsThisBeforeSuper { impl Rule for NoThisBeforeSuper { fn run_once(&self, ctx: &LintContext) { + let cfg = ctx.cfg(); let semantic = ctx.semantic(); - // control flow dependant - let Some(cfg) = ctx.semantic().cfg() else { return }; // first pass -> find super calls and local violations let mut wanted_nodes = Vec::new(); diff --git a/crates/oxc_linter/src/rules/eslint/no_unreachable.rs b/crates/oxc_linter/src/rules/eslint/no_unreachable.rs index 2c71338ac9c0e..8727f1ecdfd3e 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unreachable.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unreachable.rs @@ -25,14 +25,14 @@ declare_oxc_lint!( /// /// Disallow unreachable code after `return`, `throw`, `continue`, and `break` statements /// + #[use_cfg] NoUnreachable, nursery ); impl Rule for NoUnreachable { fn run_once(&self, ctx: &LintContext) { - // control flow dependant - let Some(cfg) = ctx.semantic().cfg() else { return }; + let cfg = ctx.cfg(); let nodes = ctx.nodes(); let Some(root) = nodes.root_node() else { return }; diff --git a/crates/oxc_linter/src/rules/react/require_render_return.rs b/crates/oxc_linter/src/rules/react/require_render_return.rs index ea9364f2fe238..e565192dc903f 100644 --- a/crates/oxc_linter/src/rules/react/require_render_return.rs +++ b/crates/oxc_linter/src/rules/react/require_render_return.rs @@ -44,14 +44,14 @@ declare_oxc_lint!( /// } /// } /// ``` + #[use_cfg] RequireRenderReturn, nursery ); impl Rule for RequireRenderReturn { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - // control flow dependant - let Some(cfg) = ctx.semantic().cfg() else { return }; + let cfg = ctx.cfg(); if !matches!(node.kind(), AstKind::ArrowFunctionExpression(_) | AstKind::Function(_)) { return; diff --git a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs index 8ed55f68b08db..9cb85b22ee324 100644 --- a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs +++ b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs @@ -101,14 +101,14 @@ declare_oxc_lint!( /// /// /// + #[use_cfg] RulesOfHooks, nursery ); impl Rule for RulesOfHooks { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - // control flow dependant - let Some(cfg) = ctx.semantic().cfg() else { return }; + let cfg = ctx.cfg(); let AstKind::CallExpression(call) = node.kind() else { return }; diff --git a/crates/oxc_macros/src/declare_oxc_lint.rs b/crates/oxc_macros/src/declare_oxc_lint.rs index e0c735ba6926e..120f729c78c88 100644 --- a/crates/oxc_macros/src/declare_oxc_lint.rs +++ b/crates/oxc_macros/src/declare_oxc_lint.rs @@ -3,44 +3,48 @@ use proc_macro::TokenStream; use quote::quote; use syn::{ parse::{Parse, ParseStream}, - Attribute, Error, Expr, Ident, Lit, LitStr, Meta, Result, Token, + Attribute, Expr, Ident, Lit, LitStr, Meta, Result, Token, }; pub struct LintRuleMeta { name: Ident, category: Ident, documentation: String, + use_cfg: bool, pub used_in_test: bool, } impl Parse for LintRuleMeta { fn parse(input: ParseStream<'_>) -> Result { let mut documentation = String::new(); - for attr in input.call(Attribute::parse_outer)? { - if let Some(lit) = parse_attr(["doc"], &attr) { - let value = lit.value(); - let line = value.strip_prefix(' ').unwrap_or(&value); + let use_cfg = 'use_cfg: { + for attr in input.call(Attribute::parse_outer)? { + if let Some(lit) = parse_attr(["doc"], &attr) { + let value = lit.value(); + let line = value.strip_prefix(' ').unwrap_or(&value); - documentation.push_str(line); - documentation.push('\n'); - } else { - return Err(Error::new_spanned(attr, "unexpected attribute")); + documentation.push_str(line); + documentation.push('\n'); + } else { + break 'use_cfg parse_attr(["use_cfg"], &attr).is_some(); + } } - } + false + }; let struct_name = input.parse()?; - input.parse::()?; + input.parse::()?; let category = input.parse()?; // Ignore the rest input.parse::()?; - Ok(Self { name: struct_name, category, documentation, used_in_test: false }) + Ok(Self { name: struct_name, category, documentation, use_cfg, used_in_test: false }) } } pub fn declare_oxc_lint(metadata: LintRuleMeta) -> TokenStream { - let LintRuleMeta { name, category, documentation, used_in_test } = metadata; + let LintRuleMeta { name, category, documentation, use_cfg, used_in_test } = metadata; let canonical_name = name.to_string().to_case(Case::Kebab); let category = match category.to_string().as_str() { "correctness" => quote! { RuleCategory::Correctness }, @@ -67,6 +71,8 @@ pub fn declare_oxc_lint(metadata: LintRuleMeta) -> TokenStream { const CATEGORY: RuleCategory = #category; + const USE_CFG: bool = #use_cfg; + fn documentation() -> Option<&'static str> { Some(#documentation) } From 1272f27a535162469bb1cb72889a58afadad35f3 Mon Sep 17 00:00:00 2001 From: rzvxa Date: Tue, 18 Jun 2024 22:06:54 +0330 Subject: [PATCH 2/4] fix: generate cfg for lsp, wasm and linter benchmarks. --- crates/oxc_language_server/src/linter.rs | 1 + crates/oxc_wasm/src/lib.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/crates/oxc_language_server/src/linter.rs b/crates/oxc_language_server/src/linter.rs index 06d0722ed897f..685ec3a7af57e 100644 --- a/crates/oxc_language_server/src/linter.rs +++ b/crates/oxc_language_server/src/linter.rs @@ -288,6 +288,7 @@ impl IsolatedLintHandler { let program = allocator.alloc(ret.program); let semantic_ret = SemanticBuilder::new(javascript_source_text, source_type) .with_trivias(ret.trivias) + .with_cfg(true) .with_check_syntax_error(true) .build(program); diff --git a/crates/oxc_wasm/src/lib.rs b/crates/oxc_wasm/src/lib.rs index b9ee8b1afd0a1..86c4ad2d2dadc 100644 --- a/crates/oxc_wasm/src/lib.rs +++ b/crates/oxc_wasm/src/lib.rs @@ -181,6 +181,7 @@ impl Oxc { let semantic_ret = SemanticBuilder::new(source_text, source_type) .with_trivias(ret.trivias.clone()) + .with_cfg(true) .with_check_syntax_error(true) .build(program); From e9c4080791539cef2942a53ca8241bd6608eec8e Mon Sep 17 00:00:00 2001 From: rzvxa Date: Tue, 18 Jun 2024 22:56:33 +0330 Subject: [PATCH 3/4] chore: cleanup. --- .../src/rules/eslint/getter_return.rs | 2 +- .../oxc_macros/src/declare_all_lint_rules.rs | 6 ++++ crates/oxc_macros/src/declare_oxc_lint.rs | 28 +++++++++---------- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/getter_return.rs b/crates/oxc_linter/src/rules/eslint/getter_return.rs index 532ce110b27f9..556ade3d03578 100644 --- a/crates/oxc_linter/src/rules/eslint/getter_return.rs +++ b/crates/oxc_linter/src/rules/eslint/getter_return.rs @@ -50,7 +50,7 @@ declare_oxc_lint!( /// ``` #[use_cfg] GetterReturn, - nursery, + nursery ); impl Rule for GetterReturn { diff --git a/crates/oxc_macros/src/declare_all_lint_rules.rs b/crates/oxc_macros/src/declare_all_lint_rules.rs index 1df6b3b5e0370..e9ede7577565c 100644 --- a/crates/oxc_macros/src/declare_all_lint_rules.rs +++ b/crates/oxc_macros/src/declare_all_lint_rules.rs @@ -81,6 +81,12 @@ pub fn declare_all_lint_rules(metadata: AllLintRulesMeta) -> TokenStream { } } + pub fn use_cfg(&self) -> bool { + match self { + #(Self::#struct_names(_) => #struct_names::USE_CFG),* + } + } + pub fn documentation(&self) -> Option<&'static str> { match self { #(Self::#struct_names(_) => #struct_names::documentation()),* diff --git a/crates/oxc_macros/src/declare_oxc_lint.rs b/crates/oxc_macros/src/declare_oxc_lint.rs index 120f729c78c88..1687ff3ceed6a 100644 --- a/crates/oxc_macros/src/declare_oxc_lint.rs +++ b/crates/oxc_macros/src/declare_oxc_lint.rs @@ -3,7 +3,7 @@ use proc_macro::TokenStream; use quote::quote; use syn::{ parse::{Parse, ParseStream}, - Attribute, Expr, Ident, Lit, LitStr, Meta, Result, Token, + Attribute, Expr, Ident, Lit, Meta, Result, Token, }; pub struct LintRuleMeta { @@ -19,8 +19,7 @@ impl Parse for LintRuleMeta { let mut documentation = String::new(); let use_cfg = 'use_cfg: { for attr in input.call(Attribute::parse_outer)? { - if let Some(lit) = parse_attr(["doc"], &attr) { - let value = lit.value(); + if let Some(value) = parse_attr(["doc"], &attr) { let line = value.strip_prefix(' ').unwrap_or(&value); documentation.push_str(line); @@ -82,19 +81,20 @@ pub fn declare_oxc_lint(metadata: LintRuleMeta) -> TokenStream { TokenStream::from(output) } -fn parse_attr<'a, const LEN: usize>( - path: [&'static str; LEN], - attr: &'a Attribute, -) -> Option<&'a LitStr> { - if let Meta::NameValue(name_value) = &attr.meta { - let path_idents = name_value.path.segments.iter().map(|segment| &segment.ident); - if itertools::equal(path_idents, path) { - if let Expr::Lit(expr_lit) = &name_value.value { - if let Lit::Str(s) = &expr_lit.lit { - return Some(s); +fn parse_attr(path: [&'static str; LEN], attr: &Attribute) -> Option { + match &attr.meta { + Meta::NameValue(name_value) => { + let path_idents = name_value.path.segments.iter().map(|segment| &segment.ident); + if itertools::equal(path_idents, path) { + if let Expr::Lit(expr_lit) = &name_value.value { + if let Lit::Str(s) = &expr_lit.lit { + return Some(s.value()); + } } } + None } + Meta::Path(p) if p.is_ident(path[0]) => Some(path[0].to_string()), + _ => None, } - None } From 6198181683862978b80b9353dce513cefcb4454f Mon Sep 17 00:00:00 2001 From: rzvxa Date: Tue, 18 Jun 2024 23:25:23 +0330 Subject: [PATCH 4/4] chore: lower the diff. --- crates/oxc_wasm/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/oxc_wasm/src/lib.rs b/crates/oxc_wasm/src/lib.rs index 86c4ad2d2dadc..b9ee8b1afd0a1 100644 --- a/crates/oxc_wasm/src/lib.rs +++ b/crates/oxc_wasm/src/lib.rs @@ -181,7 +181,6 @@ impl Oxc { let semantic_ret = SemanticBuilder::new(source_text, source_type) .with_trivias(ret.trivias.clone()) - .with_cfg(true) .with_check_syntax_error(true) .build(program);