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

Commit

Permalink
refactor(rome_analyze): deserialize options early (#4541)
Browse files Browse the repository at this point in the history
* refactor(rome_analyze): deserialize options early

* chore: add test case

* chore: code generation

* changelog

* fix file name in diagnostic

* chore: ignore deps

* chore: remove deps
  • Loading branch information
ematipico authored Jun 10, 2023
1 parent 665bb9d commit 7b65492
Show file tree
Hide file tree
Showing 64 changed files with 1,016 additions and 470 deletions.
54 changes: 54 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,60 @@
### Editors
### Formatter
### Linter

#### Other changes

- The rules [`useExhaustiveDependencies`](https://docs.rome.tools/lint/rules/useexhaustivedependencies/) and [`useHookAtTopLevel`](https://docs.rome.tools/lint/rules/usehookattoplevel/) accept a different
shape of options

Old configuration

```json
{
"linter": {
"rules": {
"nursery": {
"useExhaustiveDependencies": {
"level": "error",
"options": {
"hooks": [
["useMyEffect", 0, 1]
]
}
}
}
}
}
}
```

New configuration

```json
{
"linter": {
"rules": {
"nursery": {
"useExhaustiveDependencies": {
"level": "error",
"options": {
"hooks": [
{
"name": "useMyEffect",
"closureIndex": 0,
"dependenciesIndex": 1
}
]
}
}
}
}
}
}
```



### Parser
### VSCode
### JavaScript APIs
Expand Down
6 changes: 4 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,6 @@ rome_json_factory = { version = "0.0.1", path = "./crates/rome_json_factory" }
tests_macros = { path = "./crates/tests_macros" }
rome_formatter_test = { path = "./crates/rome_formatter_test" }
rome_js_analyze = { path = "./crates/rome_js_analyze" }
schemars = { version = "0.8.10" }


2 changes: 0 additions & 2 deletions crates/rome_analyze/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ license.workspace = true
rome_rowan = { workspace = true }
rome_console = { workspace = true }
rome_diagnostics = { workspace = true }
rome_json_parser = { workspace = true }
rome_deserialize = { workspace = true}
bitflags.workspace = true
rustc-hash = { workspace = true }
serde = { version = "1.0.136", features = ["derive"] }
Expand Down
12 changes: 4 additions & 8 deletions crates/rome_analyze/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ use std::path::Path;
type RuleQueryResult<R> = <<R as Rule>::Query as Queryable>::Output;
type RuleServiceBag<R> = <<R as Rule>::Query as Queryable>::Services;

#[derive(Clone)]
pub struct ServiceBagRuleOptionsWrapper<R: Rule>(pub R::Options);

pub struct RuleContext<'a, R>
where
R: ?Sized + Rule,
Expand All @@ -19,6 +16,7 @@ where
services: RuleServiceBag<R>,
globals: &'a [&'a str],
file_path: &'a Path,
options: &'a R::Options,
}

impl<'a, R> RuleContext<'a, R>
Expand All @@ -31,6 +29,7 @@ where
services: &'a ServiceBag,
globals: &'a [&'a str],
file_path: &'a Path,
options: &'a R::Options,
) -> Result<Self, Error> {
let rule_key = RuleKey::rule::<R>();
Ok(Self {
Expand All @@ -40,6 +39,7 @@ where
services: FromServices::from_services(&rule_key, services)?,
globals,
file_path,
options,
})
}

Expand Down Expand Up @@ -90,11 +90,7 @@ where
/// }
/// ```
pub fn options(&self) -> &R::Options {
let ServiceBagRuleOptionsWrapper(options) = self
.bag
.get_service::<ServiceBagRuleOptionsWrapper<R>>()
.unwrap();
options
self.options
}

/// Checks whether the provided text belongs to globals
Expand Down
20 changes: 6 additions & 14 deletions crates/rome_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use std::cmp::Ordering;
use std::collections::{BTreeMap, BinaryHeap};
use std::fmt::{Debug, Display, Formatter};
use std::ops;
use std::path::Path;

mod categories;
pub mod context;
Expand Down Expand Up @@ -43,7 +42,6 @@ pub use crate::services::{FromServices, MissingServicesDiagnostic, ServiceBag};
pub use crate::signals::{AnalyzerAction, AnalyzerSignal, DiagnosticSignal};
pub use crate::syntax::{Ast, SyntaxVisitor};
pub use crate::visitor::{NodeVisitor, Visitor, VisitorContext, VisitorFinishContext};
pub use rule::DeserializableRuleOptions;

use rome_console::markup;
use rome_diagnostics::{category, Applicability, Diagnostic, DiagnosticExt, DiagnosticTags};
Expand Down Expand Up @@ -77,8 +75,7 @@ pub struct AnalyzerContext<'a, L: Language> {
pub root: LanguageRoot<L>,
pub services: ServiceBag,
pub range: Option<TextRange>,
pub globals: &'a [&'a str],
pub file_path: &'a Path,
pub options: &'a AnalyzerOptions,
}

impl<'analyzer, L, Matcher, Break, Diag> Analyzer<'analyzer, L, Matcher, Break, Diag>
Expand Down Expand Up @@ -142,9 +139,8 @@ where
root: &ctx.root,
services: &ctx.services,
range: ctx.range,
globals: ctx.globals,
apply_suppression_comment,
file_path: ctx.file_path,
options: ctx.options,
};

// The first phase being run will inspect the tokens and parse the
Expand Down Expand Up @@ -221,10 +217,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],
/// The [Path] of the current file
file_path: &'phase Path,
/// Analyzer options
options: &'phase AnalyzerOptions,
}

/// Single entry for a suppression comment in the `line_suppressions` buffer
Expand Down Expand Up @@ -283,8 +277,7 @@ where
query_matcher: self.query_matcher,
signal_queue: &mut self.signal_queue,
apply_suppression_comment: self.apply_suppression_comment,
globals: self.globals,
file_path: self.file_path,
options: self.options,
};

visitor.visit(&node_event, ctx);
Expand All @@ -309,8 +302,7 @@ where
query_matcher: self.query_matcher,
signal_queue: &mut self.signal_queue,
apply_suppression_comment: self.apply_suppression_comment,
globals: self.globals,
file_path: self.file_path,
options: self.options,
};

visitor.visit(&event, ctx);
Expand Down
29 changes: 11 additions & 18 deletions crates/rome_analyze/src/matcher.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use crate::{
AnalyzerSignal, Phases, QueryMatch, Rule, RuleFilter, RuleGroup, ServiceBag,
AnalyzerOptions, AnalyzerSignal, Phases, QueryMatch, Rule, RuleFilter, RuleGroup, ServiceBag,
SuppressionCommentEmitter,
};
use rome_rowan::{Language, TextRange};
use std::path::Path;
use std::{
any::{Any, TypeId},
cmp::Ordering,
Expand All @@ -27,8 +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],
pub file_path: &'phase Path,
pub options: &'phase AnalyzerOptions,
}

/// Wrapper type for a [QueryMatch]
Expand Down Expand Up @@ -198,24 +196,20 @@ where

#[cfg(test)]
mod tests {
use std::convert::Infallible;
use std::path::Path;

use super::MatchQueryParams;
use crate::{
signals::DiagnosticSignal, Analyzer, AnalyzerContext, AnalyzerSignal, ControlFlow,
MetadataRegistry, Never, Phases, QueryMatcher, RuleKey, ServiceBag, SignalEntry,
SyntaxVisitor,
};
use crate::{AnalyzerOptions, SuppressionKind};
use rome_diagnostics::{category, DiagnosticExt};
use rome_diagnostics::{Diagnostic, Severity};
use rome_rowan::{
raw_language::{RawLanguage, RawLanguageKind, RawLanguageRoot, RawSyntaxTreeBuilder},
AstNode, SyntaxNode, TextRange, TextSize, TriviaPiece, TriviaPieceKind,
};

use crate::SuppressionKind;
use crate::{
signals::DiagnosticSignal, Analyzer, AnalyzerContext, AnalyzerSignal, ControlFlow,
MetadataRegistry, Never, Phases, QueryMatcher, RuleKey, ServiceBag, SignalEntry,
SyntaxVisitor,
};

use super::MatchQueryParams;
use std::convert::Infallible;

struct SuppressionMatcher;

Expand Down Expand Up @@ -383,8 +377,7 @@ mod tests {
root,
range: None,
services: ServiceBag::default(),
globals: &[],
file_path: Path::new(""),
options: &AnalyzerOptions::default(),
};

let result: Option<Never> = analyzer.run(ctx);
Expand Down
50 changes: 38 additions & 12 deletions crates/rome_analyze/src/options.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,28 @@
use crate::RuleKey;
use serde::Deserialize;
use crate::{Rule, RuleKey};
use std::any::{Any, TypeId};
use std::collections::HashMap;
use std::fmt::Debug;
use std::path::PathBuf;

/// A convenient new type data structure to store the options that belong to a rule
#[derive(Debug, Default, Deserialize)]
pub struct RuleOptions(String);
#[derive(Debug)]
pub struct RuleOptions((TypeId, Box<dyn Any>));

impl RuleOptions {
/// It returns the deserialized rule option
pub fn value(&self) -> &String {
&self.0
pub fn value<O: 'static>(&self) -> &O {
let (type_id, value) = &self.0;
let current_id = TypeId::of::<O>();
debug_assert_eq!(type_id, &current_id);
// SAFETY: the code should fail when asserting the types.
// If the code throws an error here, it means that the developer didn't test
// the rule with the options
value.downcast_ref::<O>().unwrap()
}

/// Creates a new [RuleOptions]
pub fn new(options: String) -> Self {
Self(options)
pub fn new<O: 'static>(options: O) -> Self {
Self((TypeId::of::<O>(), Box::new(options)))
}
}

Expand All @@ -26,13 +32,13 @@ pub struct AnalyzerRules(HashMap<RuleKey, RuleOptions>);

impl AnalyzerRules {
/// It tracks the options of a specific rule
pub fn push_rule(&mut self, rule_key: RuleKey, options: String) {
self.0.insert(rule_key, RuleOptions::new(options));
pub fn push_rule(&mut self, rule_key: RuleKey, options: RuleOptions) {
self.0.insert(rule_key, options);
}

/// It retrieves the options of a stored rule, given its name
pub fn get_rule(&self, rule_key: &RuleKey) -> Option<&RuleOptions> {
self.0.get(rule_key)
pub fn get_rule_options<O: 'static>(&self, rule_key: &RuleKey) -> Option<&O> {
self.0.get(rule_key).map(|o| o.value::<O>())
}
}

Expand All @@ -57,3 +63,23 @@ pub struct AnalyzerOptions {
/// The file that is being analyzed
pub file_path: PathBuf,
}
impl AnalyzerOptions {
pub fn globals(&self) -> Vec<&str> {
self.configuration
.globals
.iter()
.map(|global| global.as_str())
.collect()
}

pub fn rule_options<R: 'static>(&self) -> Option<R::Options>
where
R: Rule,
R::Options: Clone,
{
self.configuration
.rules
.get_rule_options::<R::Options>(&RuleKey::rule::<R>())
.map(R::Options::clone)
}
}
Loading

0 comments on commit 7b65492

Please sign in to comment.