This repository has been archived by the owner on Aug 31, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 659
refactor(rome_analyze): refactor how Visitors, Queryables and QueryMatches are related #4063
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
3d613a8
refactor(rome_analyze): allow queryables to register their own visitor
leops fa28a62
replace the `QueryMatch` enum with `Box<dyn Any>`
leops e5e6a0f
store visitors in a BTreeMap to register them in a stable order
leops 7615912
fix CI checks
leops 82f408d
move calculatiung the text range of query matches to lazy evaluation
leops 12fc36b
introduce a `Query` type to enforce the correct downcasting of query …
leops d3bc27a
add documentation
leops File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,91 +1,58 @@ | ||
use rome_control_flow::ControlFlowGraph; | ||
use rome_rowan::{AstNode, Language, SyntaxKindSet, SyntaxNode, TextRange}; | ||
use std::any::TypeId; | ||
|
||
use crate::{ | ||
registry::{NodeLanguage, Phase}, | ||
services::FromServices, | ||
ServiceBag, | ||
}; | ||
use rome_rowan::{Language, SyntaxKindSet, TextRange}; | ||
|
||
/// Trait implemented for all types, for example lint rules can query them to emit diagnostics or code actions. | ||
use crate::{registry::Phase, services::FromServices, Phases, ServiceBag, Visitor}; | ||
|
||
/// Trait implemented for types that lint rules can query in order to emit diagnostics or code actions. | ||
pub trait Queryable: Sized { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose this trait should have more documentation now |
||
type Input: QueryMatch; | ||
type Output; | ||
|
||
type Language: Language; | ||
type Services: FromServices + Phase; | ||
|
||
/// Statically declares which [QueryMatch] variant is matched by this | ||
/// [Queryable] type. For instance the [Ast] queryable matches on | ||
/// [QueryMatch::Syntax], so its key is defined as [QueryKey::Syntax] | ||
const KEY: QueryKey<Self::Language>; | ||
/// Registers one or more [Visitor] that will emit `Self::Input` query | ||
/// matches during the analyzer run | ||
fn build_visitor( | ||
analyzer: &mut impl AddVisitor<Self::Language>, | ||
root: &<Self::Language as Language>::Root, | ||
); | ||
|
||
/// Returns the type of query matches this [Queryable] expects as inputs | ||
/// | ||
/// Unless your custom queryable needs to match on a specific | ||
/// [SyntaxKindSet], you should not override the default implementation of | ||
/// this method | ||
fn key() -> QueryKey<Self::Language> { | ||
QueryKey::TypeId(TypeId::of::<Self::Input>()) | ||
} | ||
|
||
/// Unwrap an instance of `Self` from a [QueryMatch]. | ||
/// | ||
/// ## Panics | ||
/// | ||
/// If the [QueryMatch] variant of `query` doesn't match `Self::KEY` | ||
fn unwrap_match(services: &ServiceBag, query: &QueryMatch<Self::Language>) -> Self::Output; | ||
fn unwrap_match(services: &ServiceBag, query: &Self::Input) -> Self::Output; | ||
} | ||
|
||
/// Enumerate all the types of [Queryable] analyzer visitors may emit | ||
#[derive(Clone, Debug)] | ||
pub enum QueryMatch<L: Language> { | ||
Syntax(SyntaxNode<L>), | ||
SemanticModel(TextRange), | ||
ControlFlowGraph(ControlFlowGraph<L>, TextRange), | ||
/// This trait is implemented on all types that supports the registration of [Visitor] | ||
pub trait AddVisitor<L: Language> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documentation? |
||
/// Registers a [Visitor] for a given `phase` | ||
fn add_visitor<F, V>(&mut self, phase: Phases, visitor: F) | ||
where | ||
F: FnOnce() -> V, | ||
V: Visitor<Language = L> + 'static; | ||
} | ||
|
||
impl<L: Language> QueryMatch<L> { | ||
pub fn text_range(&self) -> TextRange { | ||
match self { | ||
QueryMatch::Syntax(node) => node.text_trimmed_range(), | ||
QueryMatch::SemanticModel(range) | QueryMatch::ControlFlowGraph(_, range) => *range, | ||
} | ||
} | ||
/// Marker trait implemented for all the types analyzer visitors may emit | ||
pub trait QueryMatch: 'static { | ||
fn text_range(&self) -> TextRange; | ||
} | ||
|
||
/// Mirrors the variants of [QueryMatch] to statically compute which queries a | ||
/// given [Queryable] type can match | ||
/// Represents which type a given [Queryable] type can match, either a specific | ||
/// subset of syntax node kinds or any generic type | ||
pub enum QueryKey<L: Language> { | ||
Syntax(SyntaxKindSet<L>), | ||
ControlFlowGraph, | ||
SemanticModel, | ||
} | ||
|
||
/// Query type usable by lint rules to match on specific [AstNode] types | ||
#[derive(Clone)] | ||
pub struct Ast<N>(pub N); | ||
|
||
impl<N> Queryable for Ast<N> | ||
where | ||
N: AstNode + 'static, | ||
{ | ||
type Output = N; | ||
type Language = NodeLanguage<N>; | ||
type Services = (); | ||
|
||
/// Match on [QueryMatch::Syntax] if the kind of the syntax node matches | ||
/// the kind set of `N` | ||
const KEY: QueryKey<Self::Language> = QueryKey::Syntax(N::KIND_SET); | ||
|
||
fn unwrap_match(_: &ServiceBag, query: &QueryMatch<Self::Language>) -> Self::Output { | ||
match query { | ||
QueryMatch::Syntax(node) => N::unwrap_cast(node.clone()), | ||
_ => panic!("tried to unwrap unsupported QueryMatch kind, expected Syntax"), | ||
} | ||
} | ||
} | ||
|
||
impl<L: Language> Queryable for ControlFlowGraph<L> { | ||
type Output = Self; | ||
type Language = L; | ||
type Services = (); | ||
|
||
const KEY: QueryKey<Self::Language> = QueryKey::ControlFlowGraph; | ||
|
||
fn unwrap_match(_: &ServiceBag, query: &QueryMatch<Self::Language>) -> Self::Output { | ||
match query { | ||
QueryMatch::ControlFlowGraph(cfg, _) => cfg.clone(), | ||
_ => panic!("tried to unwrap unsupported QueryMatch kind, expected Syntax"), | ||
} | ||
} | ||
TypeId(TypeId), | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's time to document this public API. We started using it more and more... I suppose some description is due.