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

feature(rome_analyzer): Rule context #2714

Merged
merged 7 commits into from
Jun 16, 2022
Merged

feature(rome_analyzer): Rule context #2714

merged 7 commits into from
Jun 16, 2022

Conversation

xunilrj
Copy link
Contributor

@xunilrj xunilrj commented Jun 15, 2022

PR for this discussion: #2603

The heart of the PR is the type RuleContext. It acts as a simple façade for everything that we want to offer to lint rules. What we offer varies by language. To see an example look at JsRuleContext, which specifies which methods we allow to be called directly on the context (like extension methods), and impl LanguageSpecificServiceBag for JsLanguage which specifies which services are stored inside the context for each language.

The RuleContextServiceBag is typed and not a usual ServiceLocator to avoid all get(...) methods returning Option<...>. This comes with a little bit of complexity but also guarantees that we will not see any runtime errors when linting.

Another nice side effect is that lint rules for other languages will not compile until someone specifies which services these languages need.

Rules now receive just the context as a parameter

fn run(ctx: &RuleContext<Self>) -> Option<Self::State> {
    ...
}

@xunilrj xunilrj temporarily deployed to aws June 15, 2022 14:33 Inactive
@github-actions
Copy link

github-actions bot commented Jun 15, 2022

@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45878 45878 0
Passed 44938 44938 0
Failed 940 940 0
Panics 0 0 0
Coverage 97.95% 97.95% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 350 350 0
Failed 5596 5596 0
Panics 0 0 0
Coverage 5.89% 5.89% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12393 12393 0
Failed 3864 3864 0
Panics 0 0 0
Coverage 76.23% 76.23% 0.00%

@cloudflare-pages
Copy link

cloudflare-pages bot commented Jun 15, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: acf1dce
Status: ✅  Deploy successful!
Preview URL: https://9b3c3c8e.tools-8rn.pages.dev
Branch Preview URL: https://feature-rule-context-c88f.tools-8rn.pages.dev

View logs

@xunilrj xunilrj temporarily deployed to aws June 15, 2022 14:57 Inactive
@MichaReiser MichaReiser requested a review from leops June 16, 2022 10:05
crates/rome_analyze/src/context.rs Outdated Show resolved Hide resolved
crates/rome_analyze/src/context.rs Outdated Show resolved Hide resolved
@xunilrj xunilrj temporarily deployed to aws June 16, 2022 15:44 Inactive
@xunilrj xunilrj merged commit 9a09452 into main Jun 16, 2022
@xunilrj xunilrj deleted the feature/rule-context branch June 16, 2022 17:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants