Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
feat(rome_js_analyzer): read globals from analyzer options (#4261)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Mar 10, 2023
1 parent cb49a39 commit 6451441
Show file tree
Hide file tree
Showing 12 changed files with 116 additions and 18 deletions.
8 changes: 8 additions & 0 deletions crates/rome_analyze/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ where
root: &'a RuleRoot<R>,
bag: &'a ServiceBag,
services: RuleServiceBag<R>,
globals: &'a [&'a str],
}

impl<'a, R> RuleContext<'a, R>
Expand All @@ -26,13 +27,15 @@ where
query_result: &'a RuleQueryResult<R>,
root: &'a RuleRoot<R>,
services: &'a ServiceBag,
globals: &'a [&'a str],
) -> Result<Self, Error> {
let rule_key = RuleKey::rule::<R>();
Ok(Self {
query_result,
root,
bag: services,
services: FromServices::from_services(&rule_key, services)?,
globals,
})
}

Expand Down Expand Up @@ -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>
Expand Down
7 changes: 6 additions & 1 deletion crates/rome_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub struct AnalyzerContext<'a, L: Language> {
pub root: LanguageRoot<L>,
pub services: ServiceBag,
pub range: Option<TextRange>,
pub options: &'a AnalyzerOptions,
pub globals: &'a [&'a str],
}

impl<'analyzer, L, Matcher, Break, Diag> Analyzer<'analyzer, L, Matcher, Break, Diag>
Expand Down Expand Up @@ -140,6 +140,7 @@ where
root: &ctx.root,
services: &ctx.services,
range: ctx.range,
globals: ctx.globals,
apply_suppression_comment,
};

Expand Down Expand Up @@ -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<TextRange>,
/// Options passed to the analyzer
globals: &'phase [&'phase str],
}

/// Single entry for a suppression comment in the `line_suppressions` buffer
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions crates/rome_analyze/src/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub struct MatchQueryParams<'phase, 'query, L: Language> {
pub services: &'phase ServiceBag,
pub signal_queue: &'query mut BinaryHeap<SignalEntry<'phase, L>>,
pub apply_suppression_comment: SuppressionCommentEmitter<L>,
pub globals: &'phase [&'phase str],
}

/// Wrapper type for a [QueryMatch]
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -379,7 +380,7 @@ mod tests {
root,
range: None,
services: ServiceBag::default(),
options: &AnalyzerOptions::default(),
globals: &[],
};

let result: Option<Never> = analyzer.run(ctx);
Expand Down
13 changes: 8 additions & 5 deletions crates/rome_analyze/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ impl<L: Language + Default + 'static> RegistryVisitor<L> for RuleRegistryBuilder
phase.rule_states.push(RuleState::default());

let rule_key = RuleKey::rule::<R>();

let deserialized =
if let Some(options) = self.options.configuration.rules.get_rule(&rule_key) {
let value = options.value();
Expand Down Expand Up @@ -425,11 +426,12 @@ impl<L: Language + Default> RegistryRule<L> {
// if the query doesn't match
let query_result = params.query.downcast_ref().unwrap();
let query_result = <R::Query as Queryable>::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 =
Expand All @@ -443,6 +445,7 @@ impl<L: Language + Default> RegistryRule<L> {
result,
params.services,
params.apply_suppression_comment,
params.globals,
));

params.signal_queue.push(SignalEntry {
Expand Down
3 changes: 2 additions & 1 deletion crates/rome_analyze/src/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 7 additions & 2 deletions crates/rome_analyze/src/signals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<RuleLanguage<R>>,
/// A list of strings that are considered "globals" inside the analyzer
globals: &'phase [&'phase str],
}

impl<'phase, R> RuleSignal<'phase, R>
Expand All @@ -257,13 +259,15 @@ where
apply_suppression_comment: SuppressionCommentEmitter<
<<R as Rule>::Query as Queryable>::Language,
>,
globals: &'phase [&'phase str],
) -> Self {
Self {
root,
query_result,
state,
services,
apply_suppression_comment,
globals,
}
}
}
Expand All @@ -273,13 +277,14 @@ where
R: Rule + 'static,
{
fn diagnostic(&self) -> Option<AnalyzerDiagnostic> {
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<RuleLanguage<R>> {
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) {
Expand Down
7 changes: 3 additions & 4 deletions crates/rome_analyze/src/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -165,7 +164,7 @@ mod tests {
root,
range: None,
services: ServiceBag::default(),
options: &AnalyzerOptions::default(),
globals: &[],
};

let result: Option<Never> = analyzer.run(ctx);
Expand Down
2 changes: 2 additions & 0 deletions crates/rome_analyze/src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub struct VisitorContext<'phase, 'query, L: Language> {
pub(crate) query_matcher: &'query mut dyn QueryMatcher<L>,
pub(crate) signal_queue: &'query mut BinaryHeap<SignalEntry<'phase, L>>,
pub apply_suppression_comment: SuppressionCommentEmitter<L>,
pub globals: &'phase [&'phase str],
}

impl<'phase, 'query, L: Language> VisitorContext<'phase, 'query, L> {
Expand All @@ -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,
})
}
}
Expand Down
37 changes: 37 additions & 0 deletions crates/rome_cli/tests/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1548,3 +1548,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,
));
}
Original file line number Diff line number Diff line change
@@ -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 <TIME>
```


8 changes: 7 additions & 1 deletion crates/rome_js_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,18 @@ where

services.insert_service(Arc::new(AriaRoles::default()));
services.insert_service(Arc::new(AriaProperties::default()));
let globals: Vec<_> = options
.configuration
.globals
.iter()
.map(|global| global.as_str())
.collect();
(
analyzer.run(AnalyzerContext {
root: root.clone(),
range: filter.range,
services,
options,
globals: globals.as_slice(),
}),
diagnostics,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ impl Rule for NoUndeclaredVariables {
let token = identifier.value_token().ok()?;
let text = token.text_trimmed();

if ctx.is_global(text) {
return None;
}

// Typescript Const Assertion
if text == "const" && under_as_expression {
return None;
Expand Down

0 comments on commit 6451441

Please sign in to comment.