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): automatically derive the rule category in `de…
Browse files Browse the repository at this point in the history
…clare_rule!` (#3321)

* automatically derive the rule category in `declare_rule!`

* link the rules to their group and category at the type system level

* fixup after rebase

* run codegen

* Update crates/rome_analyze/src/rule.rs

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
  • Loading branch information
leops and ematipico authored Oct 5, 2022
1 parent 21fa143 commit a0a520f
Show file tree
Hide file tree
Showing 65 changed files with 294 additions and 274 deletions.
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
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

0 comments on commit a0a520f

Please sign in to comment.