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

test(rome_js_analyze): every rule has its folder #4318

Merged
merged 1 commit into from
Mar 27, 2023
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
11 changes: 2 additions & 9 deletions crates/rome_analyze/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,9 @@ _**your test cases are placed inside the wrong group, you won't see any diagnost

Since each new rule will start from `nursery`, that's where we start.
If you used `just new-lintrule`, a folder that use the name of the rule should exist.
Otherwise, create a folder called `myRuleName/`, and then create one or more files where you want to create different cases.

Otherwise, you have two options:

- Create a single file called like the rule name, e.g. `myRuleName.js`, `myRuleName.tsx`, etc.
The extension of the file matters based on which kind of file you need to test;

- Create a folder called `myRuleName/`, and then create various files where you want to create different cases.
These options are useful if your rules target different super languages, or you want to split your cases among different files.

If you chose to create a folder, a common pattern is to create files prefixed by `invalid` or `valid`.
A common pattern is to create files prefixed by `invalid` or `valid`.
The files prefixed by `invalid` contain code that are reported by the rule.
The files prefixed by `valid` contain code that are not reported by the rule.

Expand Down
36 changes: 17 additions & 19 deletions crates/rome_js_analyze/tests/spec_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,14 @@ fn run_test(input: &'static str, _: &str, _: &str, _: &str) {

let input_file = Path::new(input);
let file_name = input_file.file_name().and_then(OsStr::to_str).unwrap();
let input_code = read_to_string(input_file)
.unwrap_or_else(|err| panic!("failed to read {:?}: {:?}", input_file, err));

let (group, rule) = parse_test_path(input_file);

if rule == "specs" || rule == "suppression" {
panic!("the test file must be placed in the {rule}/<group-name>/<rule-name>/ directory");
}
if group == "specs" || group == "suppression" {
panic!("the test file must be placed in the {group}/{rule}/<rule-name>/ directory");
}
if rome_js_analyze::metadata().find_rule(group, rule).is_none() {
panic!("could not find rule {group}/{rule}");
}
Expand All @@ -55,6 +58,8 @@ fn run_test(input: &'static str, _: &str, _: &str, _: &str) {
let mut snapshot = String::new();
let extension = input_file.extension().unwrap_or_default();

let input_code = read_to_string(input_file)
.unwrap_or_else(|err| panic!("failed to read {:?}: {:?}", input_file, err));
let quantity_diagnostics = if let Some(scripts) = scripts_from_json(extension, &input_code) {
for script in scripts {
write_analysis_to_snapshot(
Expand Down Expand Up @@ -211,25 +216,18 @@ pub(crate) fn write_analysis_to_snapshot(
}

/// The test runner for the analyzer is currently designed to have a
/// one-to-one mapping between test case and analyzer rules, so each testing
/// file will be run through the analyzer with only the rule corresponding
/// to the file name (or the name of the parent directory if it's not "specs")
/// enabled, eg. `style/useWhile.js` and `style/useWhile/test.js` will be analyzed with
/// just the `style/useWhile` rule
/// one-to-one mapping between test case and analyzer rules.
/// So each testing file will be run through the analyzer with only the rule
/// corresponding to the directory name. E.g., `style/useWhile/test.js`
/// will be analyzed with just the `style/useWhile` rule.
fn parse_test_path(file: &Path) -> (&str, &str) {
let file_stem = file.file_stem().unwrap();

let ancestor_0 = file.parent().unwrap();
let name_0 = ancestor_0.file_name().unwrap();
let rule_folder = file.parent().unwrap();
let rule_name = rule_folder.file_name().unwrap();

let ancestor_1 = ancestor_0.parent().unwrap();
let name_1 = ancestor_1.file_name().unwrap();
let group_folder = rule_folder.parent().unwrap();
let group_name = group_folder.file_name().unwrap();

if matches!(name_1.to_str().unwrap(), "specs" | "suppression") {
(name_0.to_str().unwrap(), file_stem.to_str().unwrap())
} else {
(name_1.to_str().unwrap(), name_0.to_str().unwrap())
}
(group_name.to_str().unwrap(), rule_name.to_str().unwrap())
}

fn markup_to_string(markup: Markup) -> String {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<input type="submit" accessKey="s" value="Submit" />;
<button accessKey="n">Next</button>;
Original file line number Diff line number Diff line change
@@ -1,62 +1,49 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: noAccessKey.jsx
expression: invalid.jsx
---
# Input
```js
// invalid
<input type="submit" accessKey="s" value="Submit" />;
<button accessKey="n">Next</button>;

// valid
<input accessKey={undefined} />;
<input accessKey={null} />;
<button>Next</button>;
<Input accessKey="s" />;
<Button accessKey="n" />;
<RadioGroup.Radio accessKey="a" />;
<context.Provider accessKey={key} />;

```

# Diagnostics
```
noAccessKey.jsx:2:22 lint/a11y/noAccessKey FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:1:22 lint/a11y/noAccessKey FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid the accessKey attribute to reduce inconsistencies between keyboard shortcuts and screen reader keyboard comments.

1 │ // invalid
> 2 │ <input type="submit" accessKey="s" value="Submit" />;
> 1 │ <input type="submit" accessKey="s" value="Submit" />;
│ ^^^^^^^^^^^^^
3 │ <button accessKey="n">Next</button>;
4
2 │ <button accessKey="n">Next</button>;
3

i Assigning keyboard shortcuts using the accessKey attribute leads to inconsistent keyboard actions across applications.

i Suggested fix: Remove the accessKey attribute.

2 │ <input·type="submit"·accessKey="s"·value="Submit"·/>;
1 │ <input·type="submit"·accessKey="s"·value="Submit"·/>;
│ --------------

```

```
noAccessKey.jsx:3:9 lint/a11y/noAccessKey FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:2:9 lint/a11y/noAccessKey FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid the accessKey attribute to reduce inconsistencies between keyboard shortcuts and screen reader keyboard comments.

1 │ // invalid
2 │ <input type="submit" accessKey="s" value="Submit" />;
> 3 │ <button accessKey="n">Next</button>;
1 │ <input type="submit" accessKey="s" value="Submit" />;
> 2 │ <button accessKey="n">Next</button>;
│ ^^^^^^^^^^^^^
4 │
5 │ // valid
3 │

i Assigning keyboard shortcuts using the accessKey attribute leads to inconsistent keyboard actions across applications.

i Suggested fix: Remove the accessKey attribute.

3 │ <button·accessKey="n">Next</button>;
2 │ <button·accessKey="n">Next</button>;
│ -------------

```
Expand Down
7 changes: 7 additions & 0 deletions crates/rome_js_analyze/tests/specs/a11y/noAccessKey/valid.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<input accessKey={undefined} />;
<input accessKey={null} />;
<button>Next</button>;
<Input accessKey="s" />;
<Button accessKey="n" />;
<RadioGroup.Radio accessKey="a" />;
<context.Provider accessKey={key} />;
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
// invalid
<input type="submit" accessKey="s" value="Submit" />;
<button accessKey="n">Next</button>;

// valid
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: valid.jsx
---
# Input
```js
<input accessKey={undefined} />;
<input accessKey={null} />;
<button>Next</button>;
<Input accessKey="s" />;
<Button accessKey="n" />;
<RadioGroup.Radio accessKey="a" />;
<context.Provider accessKey={key} />;

```


Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: noAutoFocusInvalid.jsx
expression: invalid.jsx
---
# Input
```js
Expand All @@ -19,7 +19,7 @@ expression: noAutoFocusInvalid.jsx

# Diagnostics
```
noAutoFocusInvalid.jsx:2:13 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:2:13 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid the autoFocus attribute.

Expand All @@ -37,7 +37,7 @@ noAutoFocusInvalid.jsx:2:13 lint/a11y/noAutofocus FIXABLE ━━━━━━
```

```
noAutoFocusInvalid.jsx:3:12 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:3:12 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid the autoFocus attribute.

Expand All @@ -56,7 +56,7 @@ noAutoFocusInvalid.jsx:3:12 lint/a11y/noAutofocus FIXABLE ━━━━━━
```

```
noAutoFocusInvalid.jsx:4:12 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:4:12 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid the autoFocus attribute.

Expand All @@ -75,7 +75,7 @@ noAutoFocusInvalid.jsx:4:12 lint/a11y/noAutofocus FIXABLE ━━━━━━
```

```
noAutoFocusInvalid.jsx:5:12 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:5:12 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid the autoFocus attribute.

Expand All @@ -94,7 +94,7 @@ noAutoFocusInvalid.jsx:5:12 lint/a11y/noAutofocus FIXABLE ━━━━━━
```

```
noAutoFocusInvalid.jsx:6:12 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:6:12 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid the autoFocus attribute.

Expand All @@ -113,7 +113,7 @@ noAutoFocusInvalid.jsx:6:12 lint/a11y/noAutofocus FIXABLE ━━━━━━
```

```
noAutoFocusInvalid.jsx:7:12 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:7:12 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid the autoFocus attribute.

Expand All @@ -137,7 +137,7 @@ noAutoFocusInvalid.jsx:7:12 lint/a11y/noAutofocus FIXABLE ━━━━━━
```

```
noAutoFocusInvalid.jsx:8:24 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:8:24 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid the autoFocus attribute.

Expand All @@ -156,7 +156,7 @@ noAutoFocusInvalid.jsx:8:24 lint/a11y/noAutofocus FIXABLE ━━━━━━
```

```
noAutoFocusInvalid.jsx:9:12 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:9:12 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid the autoFocus attribute.

Expand All @@ -175,7 +175,7 @@ noAutoFocusInvalid.jsx:9:12 lint/a11y/noAutofocus FIXABLE ━━━━━━
```

```
noAutoFocusInvalid.jsx:10:24 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.jsx:10:24 lint/a11y/noAutofocus FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Avoid the autoFocus attribute.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<>
<a href='http://external.link' target='_blank'>child</a>
<a href='http://external.link' target='_BLank'>child</a>
<a href={dynamicLink} target='_blank'>child</a>
<a href="http://external.link" target="_blank">child</a>
<a href="http://external.link" target="_blank" rel="noopener">child</a>
<a {...spread} href="http://external.link" target="_blank" rel="noopener">child</a>
</>
Loading