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

Commit

Permalink
fix(rome_diagnostics): fix unknown rule warning false positive (#3412)
Browse files Browse the repository at this point in the history
* fix(rome_diagnostics): fix unknown rule warning false positive

* address PR review
  • Loading branch information
leops authored Oct 13, 2022
1 parent 4d107e2 commit 10cbc0e
Show file tree
Hide file tree
Showing 15 changed files with 319 additions and 216 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

26 changes: 17 additions & 9 deletions crates/rome_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ pub use crate::matcher::{InspectMatcher, MatchQueryParams, QueryMatcher, RuleKey
pub use crate::options::{AnalyzerConfiguration, AnalyzerOptions, AnalyzerRules};
pub use crate::query::{Ast, QueryKey, QueryMatch, Queryable};
pub use crate::registry::{
LanguageRoot, Phase, Phases, RegistryRuleMetadata, RuleRegistry, RuleSuppressions,
LanguageRoot, MetadataRegistry, Phase, Phases, RegistryRuleMetadata, RegistryVisitor,
RuleRegistry, RuleRegistryBuilder, RuleSuppressions,
};
pub use crate::rule::{
CategoryLanguage, GroupCategory, GroupLanguage, Rule, RuleAction, RuleDiagnostic, RuleGroup,
Expand Down Expand Up @@ -57,6 +58,8 @@ use rome_rowan::{
pub struct Analyzer<'analyzer, L: Language, Matcher, Break> {
/// List of visitors being run by this instance of the analyzer for each phase
phases: BTreeMap<Phases, Vec<Box<dyn Visitor<Language = L> + 'analyzer>>>,
/// Holds the metadata for all the rules statically known to the analyzer
metadata: &'analyzer MetadataRegistry,
/// Executor for the query matches emitted by the visitors
query_matcher: Matcher,
/// Language-specific suppression comment parsing function
Expand All @@ -81,12 +84,14 @@ where
/// Construct a new instance of the analyzer with the given rule registry
/// and suppression comment parser
pub fn new(
metadata: &'analyzer MetadataRegistry,
query_matcher: Matcher,
parse_suppression_comment: SuppressionParser,
emit_signal: SignalHandler<'analyzer, L, Break>,
) -> Self {
Self {
phases: BTreeMap::new(),
metadata,
query_matcher,
parse_suppression_comment,
emit_signal,
Expand All @@ -106,6 +111,7 @@ where
pub fn run(self, mut ctx: AnalyzerContext<L>) -> Option<Break> {
let Self {
phases,
metadata,
mut query_matcher,
parse_suppression_comment,
mut emit_signal,
Expand All @@ -118,6 +124,7 @@ where
let runner = PhaseRunner {
phase,
visitors: &mut visitors,
metadata,
query_matcher: &mut query_matcher,
signal_queue: BinaryHeap::new(),
parse_suppression_comment,
Expand Down Expand Up @@ -165,6 +172,8 @@ struct PhaseRunner<'analyzer, 'phase, L: Language, Matcher, Break> {
phase: Phases,
/// List of visitors being run by this instance of the analyzer for each phase
visitors: &'phase mut [Box<dyn Visitor<Language = L> + 'analyzer>],
/// Holds the metadata for all the rules statically known to the analyzer
metadata: &'analyzer MetadataRegistry,
/// Executor for the query matches emitted by the visitors
query_matcher: &'phase mut Matcher,
/// Queue for pending analyzer signals
Expand Down Expand Up @@ -398,11 +407,10 @@ where
});

let key = match group_rule {
None => self.query_matcher.find_group(rule).map(RuleFilter::from),
Some((group, rule)) => self
.query_matcher
.find_rule(group, rule)
.map(RuleFilter::from),
None => self.metadata.find_group(rule).map(RuleFilter::from),
Some((group, rule)) => {
self.metadata.find_rule(group, rule).map(RuleFilter::from)
}
};

if let Some(key) = key {
Expand Down Expand Up @@ -571,12 +579,12 @@ pub struct AnalysisFilter<'a> {

impl<'analysis> AnalysisFilter<'analysis> {
/// Return `true` if the category `C` matches this filter
fn match_category<C: GroupCategory>(&self) -> bool {
pub fn match_category<C: GroupCategory>(&self) -> bool {
self.categories.contains(C::CATEGORY.into())
}

/// Return `true` if the group `G` matches this filter
fn match_group<G: RuleGroup>(&self) -> bool {
pub fn match_group<G: RuleGroup>(&self) -> bool {
self.match_category::<G::Category>()
&& self.enabled_rules.map_or(true, |enabled_rules| {
enabled_rules.iter().any(|filter| filter.match_group::<G>())
Expand All @@ -589,7 +597,7 @@ impl<'analysis> AnalysisFilter<'analysis> {
}

/// Return `true` if the rule `R` matches this filter
fn match_rule<R>(&self) -> bool
pub fn match_rule<R>(&self) -> bool
where
R: Rule,
{
Expand Down
44 changes: 8 additions & 36 deletions crates/rome_analyze/src/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@ use crate::{
/// [QueryMatch] instances emitted by the various [Visitor](crate::Visitor)
/// and push signals wrapped in [SignalEntry] to the signal queue
pub trait QueryMatcher<L: Language> {
/// Return a unique identifier for a rule group if it's known by this query matcher
fn find_group(&self, group: &str) -> Option<GroupKey>;
/// Return a unique identifier for a rule if it's known by this query matcher
fn find_rule(&self, group: &str, rule: &str) -> Option<RuleKey>;

/// Execute a single query match
fn match_query(&mut self, params: MatchQueryParams<L>);
}
Expand Down Expand Up @@ -54,7 +49,7 @@ impl From<GroupKey> for RuleFilter<'static> {
}

/// Opaque identifier for a single rule
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct RuleKey {
group: &'static str,
rule: &'static str,
Expand Down Expand Up @@ -150,14 +145,6 @@ where
F: FnMut(&MatchQueryParams<L>),
I: QueryMatcher<L>,
{
fn find_group(&self, group: &str) -> Option<GroupKey> {
self.inner.find_group(group)
}

fn find_rule(&self, group: &str, rule: &str) -> Option<RuleKey> {
self.inner.find_rule(group, rule)
}

fn match_query(&mut self, params: MatchQueryParams<L>) {
(self.func)(&params);
self.inner.match_query(params);
Expand All @@ -174,34 +161,15 @@ mod tests {

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

use super::{GroupKey, MatchQueryParams};
use super::MatchQueryParams;

struct SuppressionMatcher;

impl QueryMatcher<RawLanguage> for SuppressionMatcher {
// Recognize group name "group" and rule name "rule"
fn find_group(&self, group: &str) -> Option<GroupKey> {
if group == "group" {
Some(GroupKey::new("group"))
} else {
None
}
}

fn find_rule(&self, group: &str, rule: &str) -> Option<RuleKey> {
match group {
"group" => match rule {
"rule" => Some(RuleKey::new("group", "rule")),
_ => None,
},
_ => None,
}
}

/// Emits a warning diagnostic for all literal expressions
fn match_query(&mut self, params: MatchQueryParams<RawLanguage>) {
let node = match params.query {
Expand Down Expand Up @@ -346,7 +314,11 @@ mod tests {
.collect()
}

let mut metadata = MetadataRegistry::default();
metadata.insert_rule("group", "rule");

let mut analyzer = Analyzer::new(
&metadata,
SuppressionMatcher,
parse_suppression_comment,
&mut emit_signal,
Expand Down
Loading

0 comments on commit 10cbc0e

Please sign in to comment.