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

refactor(rome_analyze): automatically derive the rule category in declare_rule! #3321

Merged
merged 5 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions crates/rome_analyze/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Let's say we want to create a new rule called `useAwesomeTricks`, which uses the
2. run the cargo alias `cargo codegen analyzer`, this command will update the file called `nursery.rs`
inside the `semantic_analyzers` folder
3. from there, use the [`declare_rule`](#declare_rule) macro to create a new type
```rust
```rust,ignore
ematipico marked this conversation as resolved.
Show resolved Hide resolved
use rome_analyze::declare_rule;

declare_rule! {
Expand Down Expand Up @@ -160,7 +160,7 @@ for more information about it;
# Example

The macro itself expect the following syntax:
```rust
```rust,ignore
use rome_analyze::declare_rule;

declare_rule! {
Expand All @@ -185,7 +185,7 @@ use rome_analyze::declare_rule;
blocks in Rust doc-comments are assumed to be written in Rust by default
the language of the test must be explicitly specified, for instance:

```rust
```rust,ignore
use rome_analyze::declare_rule;
declare_rule! {
/// Disallow the use of `var`
Expand All @@ -206,7 +206,7 @@ declare_rule! {
Additionally, it's possible to declare that a test should emit a diagnostic
by adding `expect_diagnostic` to the language metadata:

```rust
```rust,ignore
use rome_analyze::declare_rule;
declare_rule! {
/// Disallow the use of `var`
Expand Down Expand Up @@ -235,7 +235,7 @@ use rome_analyze::declare_rule;

In order to do, the macro allows to add additional field to add the reason for deprecation

```rust
```rust,ignore
use rome_analyze::declare_rule;

declare_rule! {
Expand Down
51 changes: 37 additions & 14 deletions crates/rome_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ pub use crate::registry::{
LanguageRoot, Phase, Phases, RegistryRuleMetadata, RuleRegistry, RuleSuppressions,
};
pub use crate::rule::{
GroupLanguage, Rule, RuleAction, RuleDiagnostic, RuleGroup, RuleMeta, RuleMetadata,
CategoryLanguage, GroupCategory, GroupLanguage, Rule, RuleAction, RuleDiagnostic, RuleGroup,
RuleMeta, RuleMetadata,
};
pub use crate::services::{CannotCreateServicesError, FromServices, ServiceBag};
use crate::signals::DiagnosticSignal;
Expand Down Expand Up @@ -524,15 +525,24 @@ pub enum RuleFilter<'a> {
}

impl RuleFilter<'_> {
/// Return `true` if the group `G` matches this filter
fn match_group<G: RuleGroup>(self) -> bool {
match self {
RuleFilter::Group(group) => group == G::NAME,
RuleFilter::Rule(group, _) => group == G::NAME,
}
}

/// Return `true` if the rule `R` matches this filter
pub fn match_rule<G, R>(self) -> bool
fn match_rule<R>(self) -> bool
where
G: RuleGroup,
R: Rule,
{
match self {
RuleFilter::Group(group) => group == G::NAME,
RuleFilter::Rule(group, rule) => group == G::NAME && rule == R::METADATA.name,
RuleFilter::Group(group) => group == <R::Group as RuleGroup>::NAME,
RuleFilter::Rule(group, rule) => {
group == <R::Group as RuleGroup>::NAME && rule == R::METADATA.name
}
}
}
}
Expand All @@ -552,22 +562,35 @@ pub struct AnalysisFilter<'a> {
}

impl<'analysis> AnalysisFilter<'analysis> {
/// Return `true` if the category `C` matches this filter
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 {
self.match_category::<G::Category>()
&& self.enabled_rules.map_or(true, |enabled_rules| {
enabled_rules.iter().any(|filter| filter.match_group::<G>())
})
&& self.disabled_rules.map_or(true, |disabled_rules| {
!disabled_rules
.iter()
.any(|filter| filter.match_group::<G>())
})
}

/// Return `true` if the rule `R` matches this filter
pub fn match_rule<G, R>(&self) -> bool
fn match_rule<R>(&self) -> bool
where
G: RuleGroup,
R: Rule,
{
self.categories.contains(R::CATEGORY.into())
self.match_group::<R::Group>()
&& self.enabled_rules.map_or(true, |enabled_rules| {
enabled_rules
.iter()
.any(|filter| filter.match_rule::<G, R>())
enabled_rules.iter().any(|filter| filter.match_rule::<R>())
})
&& self.disabled_rules.map_or(true, |disabled_rules| {
!disabled_rules
.iter()
.any(|filter| filter.match_rule::<G, R>())
!disabled_rules.iter().any(|filter| filter.match_rule::<R>())
})
}

Expand Down
4 changes: 2 additions & 2 deletions crates/rome_analyze/src/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ impl RuleKey {
Self { group, rule }
}

pub fn rule<G: RuleGroup, R: Rule>() -> Self {
Self::new(G::NAME, R::METADATA.name)
pub fn rule<R: Rule>() -> Self {
Self::new(<R::Group as RuleGroup>::NAME, R::METADATA.name)
}
}

Expand Down
36 changes: 23 additions & 13 deletions crates/rome_analyze/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use crate::{
matcher::{GroupKey, MatchQueryParams},
query::{QueryKey, QueryMatch, Queryable},
signals::RuleSignal,
AnalysisFilter, QueryMatcher, Rule, RuleGroup, RuleKey, RuleMetadata, SignalEntry,
AnalysisFilter, GroupCategory, QueryMatcher, Rule, RuleGroup, RuleKey, RuleMetadata,
SignalEntry,
};

/// Defines all the phases that the [RuleRegistry] supports.
Expand Down Expand Up @@ -60,22 +61,33 @@ struct PhaseRules<L: Language> {
}

impl<L: Language + Default> RuleRegistry<L> {
pub fn push_category<C: GroupCategory<Language = L>>(&mut self, filter: &AnalysisFilter) {
if filter.match_category::<C>() {
C::push_groups(self, filter);
}
}

pub fn push_group<G: RuleGroup<Language = L>>(&mut self, filter: &AnalysisFilter) {
G::push_rules(self, filter);
if filter.match_group::<G>() {
G::push_rules(self, filter);
}
}

/// Add the rule `R` to the list of rules stores in this registry instance
pub fn push<G, R>(&mut self)
pub fn push_rule<R>(&mut self, filter: &AnalysisFilter)
where
G: RuleGroup<Language = L> + 'static,
R: Rule + 'static,
R::Query: Queryable<Language = L>,
<R::Query as Queryable>::Output: Clone,
{
if !filter.match_rule::<R>() {
return;
}

let phase = R::phase() as usize;
let phase = &mut self.phase_rules[phase];

let rule = RegistryRule::new::<G, R>(phase.rule_states.len());
let rule = RegistryRule::new::<R>(phase.rule_states.len());

match <R::Query as Queryable>::KEY {
QueryKey::Syntax(key) => {
Expand Down Expand Up @@ -107,7 +119,7 @@ impl<L: Language + Default> RuleRegistry<L> {

self.metadata.insert(
MetadataKey {
inner: (G::NAME, R::METADATA.name),
inner: (<R::Group as RuleGroup>::NAME, R::METADATA.name),
},
R::METADATA,
);
Expand Down Expand Up @@ -240,19 +252,17 @@ impl<L: Language> RuleSuppressions<L> {
type RuleExecutor<L> = fn(&mut MatchQueryParams<L>, &mut RuleState<L>);

impl<L: Language + Default> RegistryRule<L> {
fn new<G, R>(state_index: usize) -> Self
fn new<R>(state_index: usize) -> Self
where
G: RuleGroup<Language = L> + 'static,
R: Rule + 'static,
R::Query: Queryable<Language = L> + 'static,
<R::Query as Queryable>::Output: Clone,
{
/// Generic implementation of RuleExecutor for any rule type R
fn run<G, R>(
fn run<R>(
params: &mut MatchQueryParams<RuleLanguage<R>>,
state: &mut RuleState<RuleLanguage<R>>,
) where
G: RuleGroup + 'static,
R: Rule + 'static,
R::Query: 'static,
<R::Query as Queryable>::Output: Clone,
Expand All @@ -277,7 +287,7 @@ impl<L: Language + Default> RegistryRule<L> {

R::suppressed_nodes(&ctx, &result, &mut state.suppressions);

let signal = Box::new(RuleSignal::<G, R>::new(
let signal = Box::new(RuleSignal::<R>::new(
params.file_id,
params.root,
query_result.clone(),
Expand All @@ -287,14 +297,14 @@ impl<L: Language + Default> RegistryRule<L> {

params.signal_queue.push(SignalEntry {
signal,
rule: RuleKey::rule::<G, R>(),
rule: RuleKey::rule::<R>(),
text_range,
});
}
}

Self {
run: run::<G, R>,
run: run::<R>,
state_index,
}
}
Expand Down
65 changes: 57 additions & 8 deletions crates/rome_analyze/src/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ impl RuleMetadata {
}

pub trait RuleMeta {
type Group: RuleGroup;
const METADATA: RuleMetadata;
}

Expand All @@ -55,7 +56,7 @@ pub trait RuleMeta {
///
/// The macro itself expect the following syntax:
///
/// ```rust
/// ```rust,ignore
///use rome_analyze::declare_rule;
///
/// declare_rule! {
Expand All @@ -81,6 +82,7 @@ macro_rules! declare_rule {
$vis enum $id {}

impl $crate::RuleMeta for $id {
type Group = super::Group;
const METADATA: $crate::RuleMetadata =
$crate::RuleMetadata::new($version, $name, concat!( $( $doc, "\n", )* )) $( .$key($value) )*;
}
Expand All @@ -102,9 +104,10 @@ macro_rules! declare_rule {
/// disabled at once
pub trait RuleGroup {
type Language: Language;
type Category: GroupCategory;
/// The name of this group, displayed in the diagnostics emitted by its rules
const NAME: &'static str;
/// Register all the rules belonging to this group into `registry` if they match `filter`
/// Register all the rules belonging to this group into `registry`
fn push_rules(registry: &mut RuleRegistry<Self::Language>, filter: &AnalysisFilter);
}

Expand All @@ -117,14 +120,17 @@ macro_rules! declare_group {

impl $crate::RuleGroup for $id {
type Language = <( $( $( $rule )::* , )* ) as $crate::GroupLanguage>::Language;
type Category = super::Category;

const NAME: &'static str = $name;

fn push_rules(registry: &mut $crate::RuleRegistry<Self::Language>, filter: &$crate::AnalysisFilter) {
$( if filter.match_rule::<Self, $( $rule )::*>() { registry.push::<Self, $( $rule )::*>(); } )*
$( registry.push_rule::<$( $rule )::*>(filter); )*
}
}

pub(self) use $id as Group;

// Declare a `group_category!` macro in the context of this module (and
// all its children). This macro takes the name of a rule as a string
// literal token and expands to the category of the lint rule with this
Expand All @@ -142,7 +148,38 @@ macro_rules! declare_group {
};
}

/// This trait is implemented for tuples of [Rule] types of size 1 to 20 if the
/// A group category is a collection of rule groups under a given category ID,
/// serving as a broad classification on the kind of diagnostic or code action
/// these rule emit, and allowing whole categories of rules to be disabled at
/// once depending on the kind of analysis being performed
pub trait GroupCategory {
type Language: Language;
/// The category ID used for all groups and rule belonging to this category
const CATEGORY: RuleCategory;
/// Register all the groups belonging to this category into `registry`
fn push_groups(registry: &mut RuleRegistry<Self::Language>, filter: &AnalysisFilter);
}

#[macro_export]
macro_rules! declare_category {
( $vis:vis $id:ident { kind: $kind:ident, groups: [ $( $( $group:ident )::* , )* ] } ) => {
$vis enum $id {}

impl $crate::GroupCategory for $id {
type Language = <( $( $( $group )::* , )* ) as $crate::CategoryLanguage>::Language;

const CATEGORY: $crate::RuleCategory = $crate::RuleCategory::$kind;

fn push_groups(registry: &mut $crate::RuleRegistry<Self::Language>, filter: &$crate::AnalysisFilter) {
$( registry.push_group::<$( $group )::*>(filter); )*
}
}

pub(self) use $id as Category;
};
}

/// This trait is implemented for tuples of [Rule] types of size 1 to 29 if the
/// query type of all the rules in the tuple share the same associated
/// [Language] (which is then aliased as the `Language` associated type on
/// [GroupLanguage] itself). It is used to ensure all the rules in a given
Expand All @@ -151,6 +188,15 @@ pub trait GroupLanguage {
type Language: Language;
}

/// This trait is implemented for tuples of [Rule] types of size 1 to 29 if the
/// language of all the groups in the tuple share the same associated
/// [Language] (which is then aliased as the `Language` associated type on
/// [CategoryLanguage] itself). It is used to ensure all the groups in a given
/// category are all querying the same underlying language
pub trait CategoryLanguage {
type Language: Language;
}

/// Helper macro for implementing [GroupLanguage] on a large number of tuple types at once
macro_rules! impl_group_language {
( $head:ident $( , $rest:ident )* ) => {
Expand All @@ -161,6 +207,13 @@ macro_rules! impl_group_language {
type Language = RuleLanguage<$head>;
}

impl<$head $( , $rest )*> CategoryLanguage for ($head, $( $rest ),*)
where
$head: RuleGroup $( , $rest: RuleGroup<Language = <$head as RuleGroup>::Language> )*
{
type Language = <$head as RuleGroup>::Language;
}

impl_group_language!( $( $rest ),* );
};

Expand All @@ -176,10 +229,6 @@ impl_group_language!(
/// and a callback function to be executed on all nodes matching the query to possibly
/// raise an analysis event
pub trait Rule: RuleMeta {
/// The category this rule belong to, this is used for broadly filtering
/// rules when running the analyzer
const CATEGORY: RuleCategory;

/// The type of AstNode this rule is interested in
type Query: Queryable;
/// A generic type that will be kept in memory between a call to `run` and
Expand Down
Loading