test(rome_js_analyze): every rule has its folder #4318
Conversation
✅ Deploy Preview for docs-rometools ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Assuming that orphan snapshots is just an issue of What's the change around JSON? I can't understand the argument around the diagnostics |
In my view it makes new contributor entrance easier because there is a single way to start writing tests. Moreover, it makes easier to identify in a tree view where the tests are located since everything is in a folder.
Baically with a js file you get the following snpshot: # Input
```js
let x = 1; foo(x)
let x = 1; x++;
for (let i in [1,2,3]) { foo(i); }
```
# Diagnostics
```
invalid.js:1:1 lint/style/useConst FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This 'let' declares a variable which is never re-assigned.
> 1 │ let x = 1; foo(x);
│ ^^^
i 'x' is never re-assigned.
> 1 │ let x = 1; foo(x);
│ ^
invalid.js:5:6 lint/style/useConst FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This 'let' declares a variable which is never re-assigned.
> 3 │ for (let i in [1,2,3]) { foo(i); }
│ ^^^
i 'i' is never re-assigned.
> 1 │ for (let i in [1,2,3]) { foo(i); }
│ ^
```While with jsonc, you get the following snapshot: # Input
```js
let x = 1; foo(x);
```
# Diagnostics
```
invalid.jsonc:1:1 lint/style/useConst FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This 'let' declares a variable which is never re-assigned.
> 1 │ let x = 1; foo(x);
│ ^^^
i 'x' is never re-assigned.
> 1 │ let x = 1; foo(x);
│ ^
```
# Input
```js
let x = 1; x++;
```
# Input
```js
for (let i in [1,2,3]) { foo(i); }
```
# Diagnostics
```
invalid.jsonc:1:6 lint/style/useConst FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This 'let' declares a variable which is never re-assigned.
> 1 │ for (let i in [1,2,3]) { foo(i); }
│ ^^^
i 'i' is never re-assigned.
> 1 │ for (let i in [1,2,3]) { foo(i); }
│ ^
```It is easier to catch in the second form that an input does not trigger the rule, and thus the test is either in the wrong file (in |
|
I think there's something wrong with the code blocks, both have diagnostics coming from a I would argue that having a JS file is better with IDE experience, because you can see if you write wrong code. |
Fixed. They are "hand-made" examples. |
Summary
This PR normalizes the folder structure of linter tests: every rule has a dedicated folder.
Moreover, I caught a copy-paste error:
correctness/noDuplicateObjectKeys.js.snaphas been moved tosuspicious/noDuplicateObjectKeys.js.snap. However, the original file was not removed. This kind of error should be easier to catch with the new structure.I also turn some files into JSONC. This is easier to catch a potential missing diagnostic since the code and the diagnostic are close.
I also updated the CONTRIBUTING file to encourage the use of folder. By the way, we could change the test script to require it...
Test Plan
Documentation
CONTRIBUTING updated.