From 7a2d97eb650c4f3101c6b6fa3a86295540853bad Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 6 Mar 2023 19:11:42 +0000 Subject: [PATCH] feat(rome_js_analyzer): read globals from analyzer options (#4261) --- crates/rome_analyze/src/context.rs | 8 ++++ crates/rome_analyze/src/lib.rs | 7 +++- crates/rome_analyze/src/matcher.rs | 9 +++-- crates/rome_analyze/src/registry.rs | 13 ++++--- crates/rome_analyze/src/rule.rs | 3 +- crates/rome_analyze/src/signals.rs | 9 ++++- crates/rome_analyze/src/syntax.rs | 7 ++-- crates/rome_analyze/src/visitor.rs | 2 + crates/rome_cli/tests/commands/check.rs | 37 +++++++++++++++++++ .../ignore_configured_globals.snap | 27 ++++++++++++++ crates/rome_js_analyze/src/lib.rs | 8 +++- .../correctness/no_undeclared_variables.rs | 4 ++ 12 files changed, 116 insertions(+), 18 deletions(-) create mode 100644 crates/rome_cli/tests/snapshots/main_commands_check/ignore_configured_globals.snap diff --git a/crates/rome_analyze/src/context.rs b/crates/rome_analyze/src/context.rs index 209f365d18a..ca02be01b7d 100644 --- a/crates/rome_analyze/src/context.rs +++ b/crates/rome_analyze/src/context.rs @@ -16,6 +16,7 @@ where root: &'a RuleRoot, bag: &'a ServiceBag, services: RuleServiceBag, + globals: &'a [&'a str], } impl<'a, R> RuleContext<'a, R> @@ -26,6 +27,7 @@ where query_result: &'a RuleQueryResult, root: &'a RuleRoot, services: &'a ServiceBag, + globals: &'a [&'a str], ) -> Result { let rule_key = RuleKey::rule::(); Ok(Self { @@ -33,6 +35,7 @@ where root, bag: services, services: FromServices::from_services(&rule_key, services)?, + globals, }) } @@ -89,6 +92,11 @@ where .unwrap(); options } + + /// Checks whether the provided text belongs to globals + pub fn is_global(&self, text: &str) -> bool { + self.globals.contains(&text) + } } impl<'a, R> Deref for RuleContext<'a, R> diff --git a/crates/rome_analyze/src/lib.rs b/crates/rome_analyze/src/lib.rs index 8954fdb36d1..f336cb4896d 100644 --- a/crates/rome_analyze/src/lib.rs +++ b/crates/rome_analyze/src/lib.rs @@ -76,7 +76,7 @@ pub struct AnalyzerContext<'a, L: Language> { pub root: LanguageRoot, pub services: ServiceBag, pub range: Option, - pub options: &'a AnalyzerOptions, + pub globals: &'a [&'a str], } impl<'analyzer, L, Matcher, Break, Diag> Analyzer<'analyzer, L, Matcher, Break, Diag> @@ -140,6 +140,7 @@ where root: &ctx.root, services: &ctx.services, range: ctx.range, + globals: ctx.globals, apply_suppression_comment, }; @@ -217,6 +218,8 @@ struct PhaseRunner<'analyzer, 'phase, L: Language, Matcher, Break, Diag> { services: &'phase ServiceBag, /// Optional text range to restrict the analysis to range: Option, + /// Options passed to the analyzer + globals: &'phase [&'phase str], } /// Single entry for a suppression comment in the `line_suppressions` buffer @@ -275,6 +278,7 @@ where query_matcher: self.query_matcher, signal_queue: &mut self.signal_queue, apply_suppression_comment: self.apply_suppression_comment, + globals: self.globals, }; visitor.visit(&node_event, ctx); @@ -299,6 +303,7 @@ where query_matcher: self.query_matcher, signal_queue: &mut self.signal_queue, apply_suppression_comment: self.apply_suppression_comment, + globals: self.globals, }; visitor.visit(&event, ctx); diff --git a/crates/rome_analyze/src/matcher.rs b/crates/rome_analyze/src/matcher.rs index bf8c25d5d69..7edae7110ce 100644 --- a/crates/rome_analyze/src/matcher.rs +++ b/crates/rome_analyze/src/matcher.rs @@ -26,6 +26,7 @@ pub struct MatchQueryParams<'phase, 'query, L: Language> { pub services: &'phase ServiceBag, pub signal_queue: &'query mut BinaryHeap>, pub apply_suppression_comment: SuppressionCommentEmitter, + pub globals: &'phase [&'phase str], } /// Wrapper type for a [QueryMatch] @@ -206,9 +207,9 @@ mod tests { use crate::SuppressionKind; use crate::{ - signals::DiagnosticSignal, Analyzer, AnalyzerContext, AnalyzerOptions, AnalyzerSignal, - ControlFlow, MetadataRegistry, Never, Phases, QueryMatcher, RuleKey, ServiceBag, - SignalEntry, SyntaxVisitor, + signals::DiagnosticSignal, Analyzer, AnalyzerContext, AnalyzerSignal, ControlFlow, + MetadataRegistry, Never, Phases, QueryMatcher, RuleKey, ServiceBag, SignalEntry, + SyntaxVisitor, }; use super::MatchQueryParams; @@ -379,7 +380,7 @@ mod tests { root, range: None, services: ServiceBag::default(), - options: &AnalyzerOptions::default(), + globals: &[], }; let result: Option = analyzer.run(ctx); diff --git a/crates/rome_analyze/src/registry.rs b/crates/rome_analyze/src/registry.rs index 8dd65837d9c..c445be1de65 100644 --- a/crates/rome_analyze/src/registry.rs +++ b/crates/rome_analyze/src/registry.rs @@ -223,6 +223,7 @@ impl RegistryVisitor for RuleRegistryBuilder phase.rule_states.push(RuleState::default()); let rule_key = RuleKey::rule::(); + let deserialized = if let Some(options) = self.options.configuration.rules.get_rule(&rule_key) { let value = options.value(); @@ -425,11 +426,12 @@ impl RegistryRule { // if the query doesn't match let query_result = params.query.downcast_ref().unwrap(); let query_result = ::unwrap_match(params.services, query_result); - - let ctx = match RuleContext::new(&query_result, params.root, params.services) { - Ok(ctx) => ctx, - Err(error) => return Err(error), - }; + let ctx = + match RuleContext::new(&query_result, params.root, params.services, params.globals) + { + Ok(ctx) => ctx, + Err(error) => return Err(error), + }; for result in R::run(&ctx) { let text_range = @@ -443,6 +445,7 @@ impl RegistryRule { result, params.services, params.apply_suppression_comment, + params.globals, )); params.signal_queue.push(SignalEntry { diff --git a/crates/rome_analyze/src/rule.rs b/crates/rome_analyze/src/rule.rs index 8c68910d160..37ead1e88f9 100644 --- a/crates/rome_analyze/src/rule.rs +++ b/crates/rome_analyze/src/rule.rs @@ -232,7 +232,8 @@ impl_group_language!( T00, T01, T02, T03, T04, T05, T06, T07, T08, T09, T10, T11, T12, T13, T14, T15, T16, T17, T18, T19, T20, T21, T22, T23, T24, T25, T26, T27, T28, T29, T30, T31, T32, T33, T34, T35, T36, T37, T38, T39, T40, T41, T42, T43, T44, T45, T46, T47, T48, T49, T50, T51, T52, T53, T54, T55, T56, - T57, T58, T59 + T57, T58, T59, T60, T61, T62, T63, T64, T65, T66, T67, T68, T69, T70, T71, T72, T73, T74, T75, + T76, T77, T78, T79, T80, T81, T82, T83, T84, T85, T86, T87, T88, T89 ); // pub trait DeserializableRuleOptions: Default + DeserializeOwned + Sized { diff --git a/crates/rome_analyze/src/signals.rs b/crates/rome_analyze/src/signals.rs index 28b524e5b60..cce1458d853 100644 --- a/crates/rome_analyze/src/signals.rs +++ b/crates/rome_analyze/src/signals.rs @@ -243,6 +243,8 @@ pub(crate) struct RuleSignal<'phase, R: Rule> { services: &'phase ServiceBag, /// An optional action to suppress the rule. apply_suppression_comment: SuppressionCommentEmitter>, + /// A list of strings that are considered "globals" inside the analyzer + globals: &'phase [&'phase str], } impl<'phase, R> RuleSignal<'phase, R> @@ -257,6 +259,7 @@ where apply_suppression_comment: SuppressionCommentEmitter< <::Query as Queryable>::Language, >, + globals: &'phase [&'phase str], ) -> Self { Self { root, @@ -264,6 +267,7 @@ where state, services, apply_suppression_comment, + globals, } } } @@ -273,13 +277,14 @@ where R: Rule + 'static, { fn diagnostic(&self) -> Option { - let ctx = RuleContext::new(&self.query_result, self.root, self.services).ok()?; + let ctx = + RuleContext::new(&self.query_result, self.root, self.services, self.globals).ok()?; R::diagnostic(&ctx, &self.state).map(AnalyzerDiagnostic::from) } fn actions(&self) -> AnalyzerActionIter> { - let ctx = RuleContext::new(&self.query_result, self.root, self.services).ok(); + let ctx = RuleContext::new(&self.query_result, self.root, self.services, self.globals).ok(); if let Some(ctx) = ctx { let mut actions = Vec::new(); if let Some(action) = R::action(&ctx, &self.state) { diff --git a/crates/rome_analyze/src/syntax.rs b/crates/rome_analyze/src/syntax.rs index 2f8176517f0..3d79325b9e7 100644 --- a/crates/rome_analyze/src/syntax.rs +++ b/crates/rome_analyze/src/syntax.rs @@ -100,9 +100,8 @@ mod tests { use std::convert::Infallible; use crate::{ - matcher::MatchQueryParams, registry::Phases, Analyzer, AnalyzerContext, AnalyzerOptions, - AnalyzerSignal, ControlFlow, MetadataRegistry, Never, QueryMatcher, ServiceBag, - SyntaxVisitor, + matcher::MatchQueryParams, registry::Phases, Analyzer, AnalyzerContext, AnalyzerSignal, + ControlFlow, MetadataRegistry, Never, QueryMatcher, ServiceBag, SyntaxVisitor, }; #[derive(Default)] @@ -165,7 +164,7 @@ mod tests { root, range: None, services: ServiceBag::default(), - options: &AnalyzerOptions::default(), + globals: &[], }; let result: Option = analyzer.run(ctx); diff --git a/crates/rome_analyze/src/visitor.rs b/crates/rome_analyze/src/visitor.rs index f3080c52e46..c1d6d200c4a 100644 --- a/crates/rome_analyze/src/visitor.rs +++ b/crates/rome_analyze/src/visitor.rs @@ -17,6 +17,7 @@ pub struct VisitorContext<'phase, 'query, L: Language> { pub(crate) query_matcher: &'query mut dyn QueryMatcher, pub(crate) signal_queue: &'query mut BinaryHeap>, pub apply_suppression_comment: SuppressionCommentEmitter, + pub globals: &'phase [&'phase str], } impl<'phase, 'query, L: Language> VisitorContext<'phase, 'query, L> { @@ -28,6 +29,7 @@ impl<'phase, 'query, L: Language> VisitorContext<'phase, 'query, L> { services: self.services, signal_queue: self.signal_queue, apply_suppression_comment: self.apply_suppression_comment, + globals: self.globals, }) } } diff --git a/crates/rome_cli/tests/commands/check.rs b/crates/rome_cli/tests/commands/check.rs index 78ae85761bf..9b3685abfd6 100644 --- a/crates/rome_cli/tests/commands/check.rs +++ b/crates/rome_cli/tests/commands/check.rs @@ -1544,3 +1544,40 @@ fn top_level_not_all_down_level_all() { result, )); } + +#[test] +fn ignore_configured_globals() { + let mut fs = MemoryFileSystem::default(); + let mut console = BufferConsole::default(); + + let rome_json = r#"{ + "javascript": { + "globals": ["foo", "bar"] + } + }"#; + + // style/useSingleVarDeclarator + let code = r#"foo.call(); bar.call();"#; + + let file_path = Path::new("fix.js"); + fs.insert(file_path.into(), code.as_bytes()); + + let config_path = Path::new("rome.json"); + fs.insert(config_path.into(), rome_json.as_bytes()); + + let result = run_cli( + DynRef::Borrowed(&mut fs), + &mut console, + Arguments::from_vec(vec![OsString::from("check"), file_path.as_os_str().into()]), + ); + + assert!(result.is_ok(), "run_cli returned {result:?}"); + + assert_cli_snapshot(SnapshotPayload::new( + module_path!(), + "ignore_configured_globals", + fs, + console, + result, + )); +} diff --git a/crates/rome_cli/tests/snapshots/main_commands_check/ignore_configured_globals.snap b/crates/rome_cli/tests/snapshots/main_commands_check/ignore_configured_globals.snap new file mode 100644 index 00000000000..bc6e27f5e83 --- /dev/null +++ b/crates/rome_cli/tests/snapshots/main_commands_check/ignore_configured_globals.snap @@ -0,0 +1,27 @@ +--- +source: crates/rome_cli/tests/snap_test.rs +expression: content +--- +## `rome.json` + +```json +{ + "javascript": { + "globals": ["foo", "bar"] + } +} +``` + +## `fix.js` + +```js +foo.call(); bar.call(); +``` + +# Emitted Messages + +```block +Checked 1 file(s) in