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

feat(rome_analyzer): ARIA query #3832

Merged
merged 4 commits into from
Nov 28, 2022
Merged

feat(rome_analyzer): ARIA query #3832

merged 4 commits into from
Nov 28, 2022

Conversation

ematipico
Copy link
Contributor

@ematipico ematipico commented Nov 23, 2022

Summary

This is the first step towards having some APIs to get information about ARIA properties and ARIA roles.

What this PR does

The amount of metadata around ARIA properties and ARIA roles is very big, and it can get bigger with time, here's what we need to eventually support:

This is a list of the metadata that I once created when I implemented the ARIA rules in rome classic. This metadata was copied from the existing eslint-plugin-jsx-a11y, which was a crafted data list from https://www.w3.org/TR/wai-aria-1.1

Considering the amount of information, I thought we could opt-in this metadata using the same query system we use for the semantic model. Fortunately, rules that involve ARIA should not involve the semantic model, so this works perfectly for us.

There's some generated code in an attempt to make the metadata small, but if you have some enhancements, please share!

I could plan to add the generated code using build.rs, but not in this PR (I tried before but I had some issues at compile time).

I also added a new API to the RuleDiagnostic, called note_list. You can see the results in the diagnostics.

This PR adds new rules that use the newly added APIs:

The two rules are not exhaustive and that's the plan for this PR. I wanted to at least add a couple of failing cases to get a feeling of how the APIs should be crafted.

What this PR doesn't do

  • it doesn't attempt to add ALL the metadata of all the ARIA roles and ARIA properties;
  • it doesn't attempt to document the newly added rules;
  • it doesn't attempt to make the newly added rules exhaustive;
  • it doesn't attempt to try to add all possible APIs that we might need in the future, the intent is the crate new APIs with time while we implement new rules;

Test Plan

There are some doc tests to make sure that we don't overflow the stack, considering that we directly access an array. I also added snapshots for the new rules.

@ematipico ematipico requested a review from a team November 23, 2022 10:55
@netlify
Copy link

netlify bot commented Nov 23, 2022

Deploy Preview for docs-rometools canceled.

Name Link
🔨 Latest commit a672521
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/6384d7bcc5a1f600091b2bdf

@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45879 45879 0
Passed 44936 44936 0
Failed 943 943 0
Panics 0 0 0
Coverage 97.94% 97.94% 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 1757 1757 0
Failed 4189 4189 0
Panics 0 0 0
Coverage 29.55% 29.55% 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 12397 12397 0
Failed 3860 3860 0
Panics 0 0 0
Coverage 76.26% 76.26% 0.00%

properties: Arc<AriaProperties>,
}

impl AriaServices {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative to having a service could be to use lazy_static and expose the roles and properties globally.

crates/rome_aria/src/generated.rs Outdated Show resolved Hide resolved
crates/rome_analyze/src/registry.rs Outdated Show resolved Hide resolved
@ematipico ematipico requested a review from a team as a code owner November 28, 2022 14:22
@ematipico ematipico force-pushed the feat/aria-roles branch 2 times, most recently from 5eb67c0 to d4eba05 Compare November 28, 2022 14:35
crates/rome_analyze/src/rule.rs Outdated Show resolved Hide resolved
crates/rome_aria/src/lib.rs Outdated Show resolved Hide resolved
crates/rome_aria/src/properties.rs Outdated Show resolved Hide resolved
crates/rome_js_analyze/src/aria_services.rs Outdated Show resolved Hide resolved
crates/rome_analyze/src/registry.rs Outdated Show resolved Hide resolved
@ematipico ematipico merged commit c4f6d5a into main Nov 28, 2022
@ematipico ematipico deleted the feat/aria-roles branch November 28, 2022 16:17
@ematipico ematipico added the A-Linter Area: linter label Nov 28, 2022
@ematipico ematipico added this to the 11.0.0 milestone Nov 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Linter Area: linter
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants