tools icon indicating copy to clipboard operation
tools copied to clipboard

fix(rome_js_analyze): fix noShoutyConstants with lowercase and being exported

Open xunilrj opened this issue 2 years ago • 5 comments

Summary

Closes https://github.com/rome/tools/issues/3077.

Test Plan

> cargo test -p rome_js_analyze -- shouty

with two new cases

let foo = "foo";

const A = "A";
export default A;

xunilrj avatar Aug 17 '22 21:08 xunilrj

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 50c3ef7
Status: ✅  Deploy successful!
Preview URL: https://344729b1.tools-8rn.pages.dev
Branch Preview URL: https://fix-no-shouty-constants-lowe.tools-8rn.pages.dev

View logs

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 396 396 0
Failed 5550 5550 0
Panics 0 0 0
Coverage 6.66% 6.66% 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%

github-actions[bot] avatar Aug 17 '22 21:08 github-actions[bot]

!bench_analyzer

xunilrj avatar Aug 17 '22 21:08 xunilrj

Analyzer Benchmark Results

group                     main                                   pr
-----                     ----                                   --
analyzer/css.js           1.03      2.7±0.11ms     4.3 MB/sec    1.00      2.6±0.16ms     4.5 MB/sec
analyzer/index.js         1.00      6.9±0.34ms     4.7 MB/sec    1.00      6.9±0.37ms     4.7 MB/sec
analyzer/lint.ts          1.02      3.6±0.21ms    11.5 MB/sec    1.00      3.6±0.20ms    11.6 MB/sec
analyzer/parser.ts        1.01      8.5±0.47ms     5.7 MB/sec    1.00      8.5±0.41ms     5.8 MB/sec
analyzer/router.ts        1.02      5.9±0.22ms    10.4 MB/sec    1.00      5.8±0.28ms    10.5 MB/sec
analyzer/statement.ts     1.00      7.9±0.36ms     4.5 MB/sec    1.00      7.9±0.37ms     4.5 MB/sec
analyzer/typescript.ts    1.00     11.8±0.53ms     4.6 MB/sec    1.00     11.8±0.54ms     4.6 MB/sec

github-actions[bot] avatar Aug 17 '22 21:08 github-actions[bot]

A few suggestions:

  1. Would you mind adding some valid cases in our generated markdown? https://github.com/rome/tools/pull/3078/files#diff-fdf4fb740794b3a765f8604dee5b3939df2fa9a5bf566e910c31b7db591373bcR26-R31

IWANABETHATGUY avatar Aug 18 '22 06:08 IWANABETHATGUY

A few suggestions:

1. Would you mind adding some valid cases in our generated markdown? https://github.com/rome/tools/pull/3078/files#diff-fdf4fb740794b3a765f8604dee5b3939df2fa9a5bf566e910c31b7db591373bcR26-R31

We can also pull the valid cases from the original rule in Rome JS: https://github.com/rome/tools/blob/archived-js/website/src/docs/lint/rules/js/noShoutyConstants.md

leops avatar Aug 18 '22 07:08 leops

A few suggestions:

Done.

xunilrj avatar Aug 19 '22 08:08 xunilrj