refactor(linter): use the same miette graphical reporter for tests #2217
Assignees
Comments
|
If it's not urgent, can I have a go at that? |
Yes of course! |
Boshen
pushed a commit
that referenced
this issue
Feb 1, 2024
Fixes #2217 Previously we were using the miette default, but given ours is a fork, and for example now prints slightly more relevant line- and column-numbers vs miette, we should dog-food our own have the tests tell us if/when the output changes. I did actually scan all the snapshot deltas and all look correct to me. One funny one I noticed was this ```diff diff --git a/crates/oxc_linter/src/snapshots/no_empty_file.snap b/crates/oxc_linter/src/snapshots/no_empty_file.snap index cfc53e1c..6f001fbd 100644 --- a/crates/oxc_linter/src/snapshots/no_empty_file.snap +++ b/crates/oxc_linter/src/snapshots/no_empty_file.snap @@ -2,6 +2,7 @@ source: crates/oxc_linter/src/tester.rs expression: no_empty_file --- + ⚠ eslint-plugin-unicorn(no-empty-file): Empty files are not allowed. ╭─[no_empty_file.tsx:1:1] ╰──── @@ -29,7 +30,7 @@ expression: no_empty_file help: Delete this file or add some code to it. ⚠ eslint-plugin-unicorn(no-empty-file): Empty files are not allowed. - ╭─[no_empty_file.tsx:1:1] + ╭─[no_empty_file.tsx:0:1] 0 │ · ▲ ╰──── @@ -149,4 +150,3 @@ expression: no_empty_file ╰──── help: Delete this file or add some code to it. - ``` ...which I suppose is technically correct but also a bit confusing perhaps? Should we make the line **minimum 1**? If so I can create another PR for that. There is a subtle change in whitespace too - each file gains a newline at the start but looses one at the end. My assumption is that oxc's reporter is adding a newline at the start of each report (compared to miette's), plus I removed the extra newline in `tester.rs` or else the snapshot diffs would have been even larger. Finally there are no changes to reports with *multi-line* annotations like this: ``` ⚠ typescript-eslint(ban-types): Prefer explicitly define the object shape ╭─[ban_types.tsx:1:1] 1 │ ╭─▶ const emptyObj: { 2 │ │ 3 │ ╰─▶ } = {foo: "bar"}; ╰──── help: This type means "any non-nullish value", which is slightly better than 'unknown', but it's still a broad type ``` Again I can create a separate PR to improve those and we should see snapshot diffs when I make that change 😄 I'd appreciate a quick review on this one if at all possible, given the high chance of conflict.
IWANABETHATGUY
pushed a commit
to IWANABETHATGUY/oxc
that referenced
this issue
May 29, 2024
Fixes oxc-project#2217 Previously we were using the miette default, but given ours is a fork, and for example now prints slightly more relevant line- and column-numbers vs miette, we should dog-food our own have the tests tell us if/when the output changes. I did actually scan all the snapshot deltas and all look correct to me. One funny one I noticed was this ```diff diff --git a/crates/oxc_linter/src/snapshots/no_empty_file.snap b/crates/oxc_linter/src/snapshots/no_empty_file.snap index cfc53e1c..6f001fbd 100644 --- a/crates/oxc_linter/src/snapshots/no_empty_file.snap +++ b/crates/oxc_linter/src/snapshots/no_empty_file.snap @@ -2,6 +2,7 @@ source: crates/oxc_linter/src/tester.rs expression: no_empty_file --- + ⚠ eslint-plugin-unicorn(no-empty-file): Empty files are not allowed. ╭─[no_empty_file.tsx:1:1] ╰──── @@ -29,7 +30,7 @@ expression: no_empty_file help: Delete this file or add some code to it. ⚠ eslint-plugin-unicorn(no-empty-file): Empty files are not allowed. - ╭─[no_empty_file.tsx:1:1] + ╭─[no_empty_file.tsx:0:1] 0 │ · ▲ ╰──── @@ -149,4 +150,3 @@ expression: no_empty_file ╰──── help: Delete this file or add some code to it. - ``` ...which I suppose is technically correct but also a bit confusing perhaps? Should we make the line **minimum 1**? If so I can create another PR for that. There is a subtle change in whitespace too - each file gains a newline at the start but looses one at the end. My assumption is that oxc's reporter is adding a newline at the start of each report (compared to miette's), plus I removed the extra newline in `tester.rs` or else the snapshot diffs would have been even larger. Finally there are no changes to reports with *multi-line* annotations like this: ``` ⚠ typescript-eslint(ban-types): Prefer explicitly define the object shape ╭─[ban_types.tsx:1:1] 1 │ ╭─▶ const emptyObj: { 2 │ │ 3 │ ╰─▶ } = {foo: "bar"}; ╰──── help: This type means "any non-nullish value", which is slightly better than 'unknown', but it's still a broad type ``` Again I can create a separate PR to improve those and we should see snapshot diffs when I make that change 😄 I'd appreciate a quick review on this one if at all possible, given the high chance of conflict.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is the forked graphical reporter, the testers are using the one from miette. Let me clean this up a little bit.
The text was updated successfully, but these errors were encountered: