feat(rome_js_analyze): new rule noCondAssign
Deploy Preview for docs-rometools ready!
Built without sensitive environment variables
| Name | Link |
|---|---|
| Latest commit | cfea4dc52414f2829ad731ac352b1c42ffaa75bc |
| Latest deploy log | https://app.netlify.com/sites/docs-rometools/deploys/6374c964eaa9040009cea249 |
| Deploy Preview | https://deploy-preview-3750--docs-rometools.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
Should this rule be named noConditionalAssignment to avoid abbreviation
Utilize verbosity when naming commands and flags. No unnecessary and confusing abbreviations. source
CC: @ematipico @leops
!bench_analyzer
Analyzer Benchmark Results
group main pr
----- ---- --
analyzer/css.js 1.00 2.1±0.02ms 5.5 MB/sec 1.00 2.1±0.02ms 5.5 MB/sec
analyzer/index.js 1.00 6.1±0.03ms 5.4 MB/sec 1.00 6.1±0.07ms 5.4 MB/sec
analyzer/lint.ts 1.00 3.0±0.03ms 14.1 MB/sec 1.00 3.0±0.03ms 14.0 MB/sec
analyzer/parser.ts 1.00 7.3±0.04ms 6.7 MB/sec 1.00 7.2±0.10ms 6.7 MB/sec
analyzer/router.ts 1.00 5.1±0.02ms 11.9 MB/sec 1.00 5.1±0.02ms 11.9 MB/sec
analyzer/statement.ts 1.00 7.1±0.01ms 5.0 MB/sec 1.00 7.1±0.05ms 5.0 MB/sec
analyzer/typescript.ts 1.00 10.7±0.03ms 5.1 MB/sec 1.00 10.7±0.07ms 5.1 MB/sec
Given the nature of the rule, I think we could provide a suggested fix to replace the assignment expression with a
===binary expression
It could be incorrect in some cases. For example, traversing a linked list, you could have
do {} while (node = node.next)
Suggesting the below seem bad:
do {} while (node === node.next)
Given the nature of the rule, I think we could provide a suggested fix to replace the assignment expression with a
===binary expressionIt could be incorrect in some cases
I think that's acceptable as long it's only a "suggested fix" with Applicability::MaybeIncorrect, as the name indicates it's allowed to be incorrect and should not be applied automatically without a manual review by the user unlike a "safe fix" with Applicability::Always
@leops Is it ok if I take it up in a followup PR?
Is it ok if I take it up in a followup PR?
Sure yes let's get this merged first