abrahamguo
commented
3 months ago Closed abrahamguo closed 2 weeks ago
abrahamguo
commented
3 months ago 
Josh-Cena
commented
3 months ago I'm not familiar with eslint-plugin-unicorn so it took me slightly longer to look at some of the rules (in fact I'm surprised we have this plugin at all).
Tentatively, I'm +1 on (as in, I believe this coding style will benefit us):
e or err, not error—I actively hate full names)prefer-for-of?)I'm +0/-0.5 on (as in, I believe this coding style is desirable, but it's either not useful or may take away some expressiveness unnecessarily):
Boolean is not a predicate, but I'm open to testing because it looks like they aren't used as predicates anyway.)I'm -1 on (as in, this actively goes against my coding preferences or may not work with other tools):
sindresorhus
commented
3 months ago no-array-for-each
There are many benefit with for-of loops: https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-array-for-each.md
no-object-as-default-parameter
This one is catching actual mistakes: https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-object-as-default-parameter.md
Josh-Cena
commented
3 months ago no-array-for-each
I would prefer to leave this discretion to the author. The performance is not a concern in our case; readability is debatable, especially when the array expression is long; ability to skip iterations is also present with forEach, so the only real benefit seems to be ability to exit early, but that's rarely needed, if you look at the places it's reporting on.
no-object-as-default-parameter
The only place it's reporting on has only one property, if the rule's only motivation is "default options with more than one option". And in general I don't think that's a bug vector, since the type information would accurately tell us what each property can and cannot be.
abrahamguo
commented
3 months ago @Josh-Cena another benefit of prefer-for-each that might be more relevant for us:
Additionally, using for…of has great benefits if you are using TypeScript, because it does not cause a function boundary to be crossed. This means that type-narrowing earlier on in the current scope will work properly while inside of the loop (without having to re-type-narrow). Furthermore, any mutated variables inside of the loop will picked up on for the purposes of determining if a variable is being used.
abrahamguo
commented
3 months ago no-object-as-default-parameter
The only place it's reporting on has only one property
That's actually not correct; that file has
options: VisitPatternOptions = { processRightHandNodes: false },and earlier in that same file,
interface VisitPatternOptions extends PatternVisitorOptions {
processRightHandNodes?: boolean;
}and following the chain of interfaces, there are actually two more options:
interface VisitorOptions {
childVisitorKeys?: VisitorKeys | null;
visitChildrenEvenIfSelectorExists?: boolean;
}I agree for...of loops have many benefits and I'm not against using it, but I don't think it's worthwhile to crystalize it in a rule.
Apologies, I misread the motivation for the no-object-as-default-parameter rule. This rule also prevents the case where someone passes an empty object and overrides the default which has one property. Still my point stands: this property is already typed to be nullable, so it doesn't matter if it's guaranteed to be present at runtime, since we have to write code to guard against this case anyway. If we change the type definition then TS will force the callsites to change too. I don't find it useful in a fully typed codebase.
JoshuaKGoldberg
commented
3 months ago (but we should either use
eorerr, noterror—I actively hate full names)
I actively hate abbreviations and think we should use error, not e or err. 😂
I think we should lint them with
eslint-plugin-regexp(which I use a lot), which should be able to provide the same capabilities
Strong +1. Potentially hot take (someone please yell at me if it's not good): any universally useful rules from eslint-plugin-unicorn that don't exist in eslint-plugin-regexp should IMO be filed as feature requests on eslint-plugin-regexp. IMO there's no need to have them in a catch-all general community plugin when a standard plugin for regexps exists.
abrahamguo
commented
3 months ago @JoshuaKGoldberg makes sense! and yes, the functionality from unicorn's one regexp rule can all be done by eslint-plugin-regexp.
abrahamguo
commented
3 months ago Responding to a few of @Josh-Cena's thoughts from earlier:
- no-unused-properties (not sure how reliable it is)
At a quick glance at some of the reports, they certainly look correct. I use this rule at work and have not had any false positives from it. I'm guessing the rule is pretty conservative and bails out right away if the whole object is used somewhere.
- prefer-native-coercion-functions (this is a funny one because Boolean is not a predicate, but I'm open to testing because it looks like they aren't used as predicates anyway.)
Yeah, inferred filter predicates just landed in TypeScript anyways, and you have to write your callbacks in very specific ways in order to use the inferred predicate feature. But certainly worth looking whether using real TypeScript predicates would be beneficial.
- no-array-method-this-argument
Three of those reports are understable to keep that way. However, this one is certainly a mistake:
Do not use the this argument in Array#findIndex().
website/plugins/generated-rule-docs/insertions/insertWhenNotToUseIt.ts:18
- no-array-reduce
Note that this rule does allow using reduce for array summations. I will want to look at the specific reports, because in at least some of them, it does seem like there is a more idiomatic method to use.
Josh-Cena
commented
3 months ago However, this one is certainly a mistake:
I'm surprised and I don't even remember what it's supposed to look like!
I will want to look at the specific reports, because in at least some of them, it does seem like there is a more idiomatic method to use.
Sounds good 👍 I'm also pro-remove-reduce, but want to allow the freedom in case it is useful.
kirkwaiblinger
commented
3 months ago @abrahamguo Just wanna say real quick I really like how neatly you've created the issue report and included the links and everything. Super helpful ❤️
I'll preface all of the following with saying that I haven't personally used any of these rules, so my opinions are only as good as my understanding of the rule's intent, my ignorance as to edge cases, and the limited amount of time I've spent thinking about each one....
| rule | score | words |
|---|---|---|
| better-regex | +/- 0 | others seem to know more |
| catch-error-name | +1 | I significantly prefer unabbreviated error too |
| custom-error-definition | +1? | I don't understand this well, but it seems sensible. This report does seem like a false positive/bug in the rule though 🤔 |
| error-message | +0.5 | The one report is a special case we'd want to disable, but the rule seems reasonable |
| no-array-for-each | +1.5 | I am a die-hard True Believer in for...of. I'll spare the manifesto for now, but I'll mention that I mostly disagree with the assessment in https://github.com/typescript-eslint/typescript-eslint/pull/9629#discussion_r1689040294; just use an intermediate variable. |
| no-array-method-this-argument | +1 | |
| no-array-push-push | -1 | just not a fan |
| no-array-reduce | -0.5 | Checked a few of the report examples, and they were heinous IMO. However, I think this is a concern that is the domain of human code review, not linting |
| no-console-spaces | +0.5 | |
| no-for-loop | +0.5 | I like. But it's pretty opinionated. |
| no-hex-escape | +0.5? | I have no prior knowledge of this, but the rule description sounds sensible |
| no-lonely-if | -1 (edited) | ~how is this different from the eslint core rule?~ This rule is even worse than the core rule. Booooo |
| no-object-as-default-parameter | +0.1 | description sounds plausible, but haven't thought about this hard |
| no-unused-properties | -0.5? | This seems not useful when using TS. Note that the rule description even says, (paraphrasing) to overcome to the limitations of this rule, use TS. |
| no-useless-spread | +1 | |
| no-useless-switch-case | -0.1 | It's useful to be explicit sometimes.... A glance at the reports didn't seem too suspicious. |
| no-useless-undefined | -100 | Actively, strongly disagree with the premise of this rule (particularly in the context of TS) |
| number-literal-case | -1 (edited) | ~Does other tooling (i.e. prettier) already handle this?~ Conflicts with other tooling |
| prefer-add-event-listener | +0.5? | |
| prefer-array-flat | +1 | |
| prefer-array-some | -0.5? | See also https://github.com/typescript-eslint/typescript-eslint/issues/8378 🤔 |
| prefer-at | -0.5? | See also https://github.com/typescript-eslint/typescript-eslint/issues/6401 🤔 |
| prefer-code-point | +/- 0 | can't be bothered to read up on this one |
| prefer-dom-node-append | +/-0 | 🤷 |
| prefer-export-from | +0.5 | |
| prefer-includes | -1 | we should be dogfooding https://typescript-eslint.io/rules/prefer-includes/ instead |
| prefer-math-trunc | +0.5 | 3/4 cases are 1 << 0 in a flags enum, which should stay as-is. The remaining 1 case looks like an actual bug |
| prefer-native-coercion-functions | +/-0 | don't have the attention span to think hard about this one right now, but I'm wary of it (and no-implicit-coercion is my main concern anyway) |
| prefer-negative-index | -1? | related to prefer-at, see #6401 🤔 |
| prefer-node-protocol | +0.1 | |
| prefer-regexp-test | +0.1 | |
| prefer-spread | +0.5 | |
| prefer-string-replace-all | +0.1 | |
| prefer-string-slice | +0.5 | |
| prefer-switch | -1 | |
| prefer-ternary | -0.5 | |
| prefer-top-level-await | +1 | This one is quite intriguing to me, and may be helpful to keep in mind for users with pains around no-floating-promises |
| prefer-type-error | -0.5 | This feels like a JS practice for validation that we avoid having to do in TS due to the type system, but I could be wrong |
| text-encoding-identifier-case | +/-0? | does TS type system itself not enforce/encourage this? |
EDIT - Added second batch of rules
Josh-Cena
commented
3 months ago just use an intermediate variable.
Needless to say my biggest principle is concise code, so "no" to full names, "no" to redundant braces, and "no" to intermediate variables, but I can see how I'm in the minority especially among linter maintainers :P
how is this different from the eslint core rule?
The core rule only reports on else { if {} }. This one reports on if { if {} }, which I personally think is fine.
Does other tooling (i.e. prettier) already handle this?
Yes.
kirkwaiblinger
commented
3 months ago Needless to say my biggest principle is concise code, so "no" to full names, "no" to redundant braces, and "no" to intermediate variables, but I can see how I'm in the minority especially among linter maintainers :P
Most concise != least entropy 😁
how is this different from the eslint core rule?
The core rule only reports on
else { if {} }. This one reports onif { if {} }, which I personally think is fine.
Gotcha. I'm -1 on both 👍
Does other tooling (i.e. prettier) already handle this?
Yes.
Cool, sounds like we should omit it then 👍
kirkwaiblinger
commented
3 months ago Throwing this out there, I think we should automatically reject adding any rule from unicorn that has a near-identical form in typescript-eslint, or has a proposal in typescript-eslint that is "accepting PRs".
If it exists in our plugin, we should use it, and if it's inadequate, we should fix it.
If it doesn't yet exist, but we have accepted it, it either means
Or, we weren't aware of the unicorn version. In that case, and if it's adequate without type information, then we should close/"wontfix" the proposal in typescript-eslint, and use the unicorn version. Otherwise, we should just dogfood our own version once it's released.
I've noted one or two rules with near-identical forms or proposals thereof in typescript-eslint, but there may be more. Let's be sure to double check before proceeding with the unicorn rules.
meszaros-lajos-gyorgy
commented
3 months ago Hey guys, this will probably be a bit off topic, so sorry about that in advance, but I'm reading that you guys are adding rules from unicorn to the project. How do you mix together the 2 repos?
I tried adding it like this:
but the no-unsafe-assignment doesn't let it pass through:
I thought it was all about the missing d.ts file, but today they've released a new version including a d.ts, but it didn't work.
github-actions[bot]
commented
3 months ago Uh oh! @meszaros-lajos-gyorgy, the image you shared is missing helpful alt text. Check https://github.com/typescript-eslint/typescript-eslint/issues/9623#issuecomment-2250096674.
Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.
Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.
🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.
JoshuaKGoldberg
commented
3 months ago @meszaros-lajos-gyorgy you're right: that is off topic and not what this issue is about 🙂. https://typescript-eslint.io/contributing/issues#questions-and-support-requests has links to places you can ask for help.
fisker
commented
3 months ago No one likes prefer-string-raw?
Josh-Cena
commented
3 months ago It depends. String.raw is 10 characters long so the rule is only beneficial if there are more than 10 backslashes in the string.
bradzacher
commented
3 months ago so the rule is only beneficial if there are more than 10 backslashes in the string
I don't think the point of String.raw is conciseness 😅
It's about readability - you don't need to think about escaping backslashes so it's easier to read and understand such strings.
eg "C:\\windows\\style\\path\\to\\file.js" is less clear than String.raw`C:\windows\style\path\to\file.js`
Josh-Cena
commented
3 months ago I find them equally readable, FWIW, especially with proper syntax highlighting that tells you what is escaped and what is not. I'm not strongly opposed to this rule though.
bradzacher
commented
3 months ago | rule | score | words |
|---|---|---|
| better-regex | +1 | |
| catch-error-name | -1 | I personally dislike restrictions on names -- I think that they are bikeshedding stylistic rules that don't actually improve readability |
| custom-error-definition | +1 | |
| error-message | 0 | I don't think it actually will catch a bug for us |
| no-array-for-each | 0 | I'm in favour of "standardising the codebase", but our codebase has about 50/50 forEach vs for..of. IDK I guess we could standardise on for..of? |
| no-array-method-this-argument | +1 | |
| no-array-push-push | -1e9999 | Many of the reports I very purposely wrote like that because it causes the code to be laid out in an easy to read fashion. I don't think this nitpick is valuable, personally -- seems weird to be in their recommended set. |
| no-array-reduce | -1e9999 | reduce has its uses and banning it outright isn't good. I'm gobsmacked that such an opinionated rule is in the recommended set. |
| no-console-spaces | +1 | |
| no-for-loop | -1 | The idea is great but the implementation seems wrong. For example this loop uses its i for things other than array index, or both this loop and this loop uses the i for accessing two different arrays at once. |
| no-hex-escape | -1 | IDK if one is clearer than the other, IMO. And the one case it's reporting on is clearer with hex codes cos it's console colour escapes. |
| no-lonely-if | +1 | |
| no-object-as-default-parameter | 0 | Sure I guess but IDC either way |
| no-unused-properties | +1 | |
| no-useless-spread | +1 | |
| no-useless-switch-case | -1e9999 | I prefer being explicitly exhaustive and listing conditions out to make it clear that they were considered and handled as opposed to leaving it implicit. |
| no-useless-undefined | -1 | I don't think this makes code any better - an explicit undefined can make code easier to understand |
| number-literal-case | 0 | I don't think this makes anything clearer, personally but whatever |
| prefer-add-event-listener | 0 | We're not a webapp mostly and we don't use event listeners - I don't think it's worth turning on |
| prefer-array-flat | +0.5 | |
| prefer-array-some | 0 | Meh IDC |
| prefer-at | +1 | |
| prefer-code-point | -1 | Just cos the only reports are from code that was synced from TS |
| prefer-dom-node-append | 0 | Ditto for add event listener |
| prefer-export-from | +0.5 | |
| prefer-includes | +1 | |
| prefer-math-trunc | -1 | cos it's flagging << 0 which we commonly use in flag enums for the first member |
| prefer-native-coercion-functions | +1 | |
| prefer-negative-index | +1 | |
| prefer-node-protocol | +1 | |
| prefer-regexp-test | +1 | |
| prefer-spread | -1 | I don't think that [...x] is clearer or better than Array.from(x) |
| prefer-string-replace-all | +1 | |
| prefer-string-slice | 0 | Almost a -1 just cos of the rule's description "String#substr() and String#substring() are the two lesser known legacy ways to slice a string" -- like "lesser known"? WHAT? |
| prefer-switch | -1 | I don't think a switch is always the better construct -- I think that an if/else can make code clearer sometimes. |
| prefer-ternary | -1 | I don't think conciseness is better or clearer. And looking at some of those reports it would actively harm the code clarity. |
| prefer-top-level-await | -♾ | We don't use MJS so we can't use TLA always. Also this case should NOT use TLA because the await is in a nested scope. So the rule is wrong. |
| prefer-type-error | -1e9999 | It's misclassifying many of our conditions as "type check conditions". So it's just wrong? |
| text-encoding-identifier-case | +1 |
Josh-Cena
commented
3 months ago better-regex +1
This rule may become deprecated and is certainly buggy: https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2409 and better alternatives exist (as proposed above), so I hope you are not too married to it.
no-array-reduce -1e9999
Could you give a valid reduce use case that cannot, or should not, be refactored to a for...of loop for better performance and readability? This would be helpful to be added to the MDN docs too, since I scratched my head to come up with two (2) valid use cases: summation (and other related operations), function piping (and its async version).
no-for-loop -1
The suggestion is to use entries(). I don't have strong opinions but I slightly prefer C-style for loops.
prefer-add-event-listener 0
We have a web app: the website.
fisker
commented
3 months ago Also this case should NOT use TLA because the await is in a nested scope. So the rule is wrong.
You misunderstood, it reports not because there is an await, but because prettierConfig is a Promise
bradzacher
commented
3 months ago We have a web app: the website.
Sure but it's react - so we don't delve into the imperative APIs much. And it's also minimal amounts of code. Hence "pretty low value for our codebase".
The suggestion is to use entries().
I don't see how
for (const [i, paramA] of Object.entries(paramsA)) {
const paramB = paramsB[i]; Is in any way clearer than
for (let i = 0; i < paramsA.length; ++i) {
const paramA = paramsA[i];
const paramB = paramsB[i]; It's worse in every way, IMO.
So will update my vote to -1e9999
Could you give a valid reduce use case that cannot, or should not, be refactored to a for...of loop for better performance and readability?
It's an opinionated stylistic thing to choose one over the other. In your opinion a for loop is more readable.
For example I disagree that
const result = {} as ParsedOptions;
for (const k of getEnumNames(Selectors)) {
result[k] = createValidator(k, context, normalizedOptions);
}Is necessarily clearer than
const result = getEnumNames(Selectors).reduce((acc, k) => {
acc[k] = createValidator(k, context, normalizedOptions);
return acc;
}, {} as ParsedOptions);Being functional, reduce promotes isolation and encapsulation - it makes you think "I don't want to mutate values outside of the loop closure" - which is good for minimising side-effects of the loop and making code more predictable.
But the for..of version is made to explicitly mutate a value outside the loop closure. It purposely invites you to add side-effects to loops.
I'm firmly against this rule. I understand some people don't like reduce - but it's ultimately an opinionated, stylistic choice to use one over the other.
Some people are very loudly and vehemently against using enums but that doesn't mean they're right or that everyone should migrate off of them. I see this as the same class of opinionated, stylistic debate that codebases can choose to lint for if they want.
better-regex +1 This rule may become deprecated
🤷♂️ I didn't look into it too much. I noticed you guys were discussing it above. I was just weighing in that I'm good with the idea of linting to improve our regex.
alfaproject
commented
3 months ago I accidentally stumbled upon this thread while checking the stylistic typed recommended rules. We also use unicorn and some of the rules look nearly identical but in the documentation they are not mentioned so it's not obvious to me whether I should keep using the unicorn ones that look identical since they don't need types, or I should instead prefer the ones from here.
I see above that there is awareness of the overlap, but have you considered adding to the docs why the ones in this repo are better (nor not) than the ones from unicorn?
bradzacher
commented
3 months ago Often times the overlap is that the two projects grew independently and there weren't open lines of communication to keep track of what each other was working on.
Sometimes it because we offered a type-aware version (more correct but slower and limited to TS codebases) and they chose a pure-syntactic version (faster, chance of false positives, applies to non-TS codebases).
Which rules are you talking about specifically?
alfaproject
commented
3 months ago Some examples:
There's probably more, I only had a quick look at the stylish ones like I mentioned.
I understand that both projects grow independently but I was just wondering if it would be helpful to add in the documentation like you do for base eslint rules, something like, if you enable this rule, please bear in mind that it might conflict with this other rule from unicorn (or some other popular plugin).
In terms of documentation I've seen other plugin authors doing the same for popular plugins which I think unicorn qualifies. This can be done by the community ofc if that's desired. For instance in this plugin: https://github.com/lydell/eslint-plugin-simple-import-sort
Make sure not to use other sorting rules at the same time:
JoshuaKGoldberg
commented
2 weeks ago @abrahamguo were there any other rules you wanted to enable here? Just checking before we close this out. 🙂
abrahamguo
commented
2 weeks ago Should be good to close!
Suggestion
Following up on #9523, I was looking through the
eslint-plugin-unicornrules that we do not have turned on internally, and I found these rules that might be beneficial to enable. Thoughts?docs
/([`$``])/g can be optimized to /([$```])/g. eslint-plugin/src/rules/no-unnecessary-template-expression.ts:150 eslint-plugin/src/rules/no-useless-template-literals.ts:151
/(^|[^``])"(?:``.|[^``"`r`n])`"(?!`s`:)/ can be optimized to /(^|[^``])"(?:``.|[^`n`r"``])`"(?!`s`:)/. website/src/prism/language/jsonc.js:11
/(^|[^``])"(?:``.|[^``"`r`n])`"(?=`s`:)/ can be optimized to /(^|[^``])"(?:``.|[^`n`r"``])`"(?=`s`:)/. website/src/prism/language/jsonc.js:6
/[{}[`],]/ can be optimized to /[,[`]{}]/. website/src/prism/language/jsonc.js:20
/[``/]`error`[``/]/ can be optimized to /[/``]`error`[/``]/. ast-spec/tests/fixtures.test.ts:75
/[``^$.`+?()[`]{}|]/g can be optimized to /[$()`+.?[```]^{|}]/g. eslint-plugin/src/util/escapeRegExp.ts:5
/`/`/.`|`/``[`s`S]`?(?:```/|$)/ can be optimized to /`/`/.`|`/``[`S`s]`?(?:```/|$)/. website/src/prism/language/jsonc.js:16
/``(?!["])/gm can be optimized to /``(?!")/gm. typescript-estree/tests/lib/parse.test.ts:60
/`$`{/g can be optimized to /`${/g. eslint-plugin-internal/src/rules/plugin-test-formatting.ts:51
/`r`n|[`r`n`u2028`u2029]/ can be optimized to /`r`n|[`n`r`u2028`u2029]/. utils/src/ast-utils/misc.ts:3
/&(?:#`d+|#x[`da-fA-F]+|[0-9a-zA-Z]+);/g can be optimized to /&(?:#`d+|#x[`dA-Fa-f]+|[`dA-Za-z]+);/g. typescript-estree/src/node-utils.ts:450
/^([ ]`)/ can be optimized to /^( `)/. eslint-plugin-internal/src/rules/plugin-test-formatting.ts:49
/^(`/?|)([`s`S]`?)((?:`.{1,2}|[^/]+?|)(`.[^./]`|))(?:[/]`)$/ can be optimized to /^(`/?|)([`S`s]`?)((?:`.{1,2}|[^/]+?|)(`.[^./]`|))`/`$/. website-eslint/src/mock/path.js:55
/^"'['"]$/ can be optimized to /^"'["']$/. scope-manager/tests/fixtures.test.ts:39
/^[(`w+:)``/~]/ can be optimized to /^[`w()+/:``~]/. typescript-estree/src/create-program/describeFilePath.ts:18
/^[`([]/ can be optimized to /^[([`]/. eslint-plugin/src/util/getWrappingFixer.ts:70
/^[=!]=/ can be optimized to /^[!=]=/. eslint-plugin/src/rules/prefer-string-starts-ends-with.ts:16
/^[=?:]$/ can be optimized to /^[:=?]$/. eslint-plugin/src/rules/space-infix-ops.ts:74
/^`/`/`/`/[ ]+@(`w+)[ ]`=[ ]`(.+)$/ can be optimized to /^`/{4} +@(`w+) `= `(.+)$/. scope-manager/tests/fixtures.test.ts:38
/^`/`s`<reference`s`(types|path|lib)`s`=`s`"|'["|']/ can be optimized to /^`/`s`<reference`s`(types|path|lib)`s`=`s`"'|["'|]/. eslint-plugin/src/rules/triple-slash-reference.ts:98
/^`s`(?:`/|``)``s`@ts-(?expect-error|ignore)(?.`)/ can be optimized to /^`s`[`/]``s`@ts-(?expect-er{2}or|ignore)(?.`)/.
eslint-plugin/src/rules/ban-ts-comment.ts:117
/^`s`(?:`/|``)``s`@ts-ignore/ can be optimized to /^`s`[`/]``s`@ts-ignore/. eslint-plugin/src/rules/prefer-ts-expect-error.ts:28
/Cannot read file .+semanticInfo'/i can be optimized to /cannot read file .+semanticinfo'/i. typescript-estree/tests/lib/semanticInfo.test.ts:286
/getParserServices(`(`s`[^,`s)]+)`s`(,`s`false`s`)?`)/ can be optimized to /getParserServices(`(`s`[^`s),]+)`s`(,`s`false`s`)?`)/. eslint-plugin/tests/docs.test.ts:512
/import`s``{`s`TypeScriptWorker`s`}`s`from`s`['"].`/tsWorker['"];/ can be optimized to /import`s`{`s`TypeScriptWorker`s`}`s`from`s`["'].`/tsWorker["'];/. website/tools/generate-website-dts.ts:75
docs
The catch parameter
eshould be namederror. ast-spec/tests/fixtures.test.ts:140 parser/tests/lib/tsx.test.ts:15 repo-tools/src/generate-lib.mts:310 rule-schema-to-typescript-types/src/index.ts:51 scope-manager/tests/fixtures.test.ts:146 scope-manager/tests/fixtures.test.ts:160 typescript-estree/src/semantic-or-syntactic-errors.ts:42 website-eslint/build.ts:166 website/src/components/ESQueryFilter.tsx:25 website/src/components/ast/ASTViewer.tsx:31 website/src/components/hooks/useHashState.ts:43 website/src/components/hooks/useHashState.ts:102 website/src/components/hooks/useHashState.ts:132 website/src/components/hooks/useHashState.ts:172 website/src/components/lib/json.ts:19 website/src/components/lib/parseConfig.ts:23 website/src/components/lib/parseConfig.ts:47 website/src/components/lib/parseConfig.ts:77 website/src/components/linter/createLinter.ts:82 website/src/components/linter/createLinter.ts:149 website/src/components/linter/createLinter.ts:161 website/src/components/typeDetails/TypeInfo.tsx:86The catch parameter
errshould be namederror. rule-tester/src/RuleTester.ts:622 rule-tester/src/utils/config-validator.ts:116 rule-tester/src/utils/config-validator.ts:197 website/plugins/generated-rule-docs/addESLintHashToCodeBlocksMeta.ts:57 website/src/components/editor/useSandboxServices.ts:137The catch parameter
exshould be namederror. eslint-plugin-internal/src/rules/plugin-test-formatting.ts:171 integration-tests/tools/integration-test-base.ts:155docs
The
nameproperty should be set toAssertionError. website-eslint/src/mock/assert.js:27The
nameproperty should be set toNotSupportedError. rule-schema-to-typescript-types/src/errors.ts:2The
nameproperty should be set toTSError. typescript-estree/src/node-utils.ts:702The
nameproperty should be set toUnexpectedError. rule-schema-to-typescript-types/src/errors.ts:14docs
Pass a message to the
Errorconstructor. website-eslint/src/mock/assert.js:45docs
Use
for…ofinstead of.forEach(…). ast-spec/tests/fixtures.test.ts:336 eslint-plugin/src/rules/adjacent-overload-signatures.ts:135 eslint-plugin/src/rules/ban-ts-comment.ts:186 eslint-plugin/src/rules/ban-tslint-comment.ts:37 eslint-plugin/src/rules/class-literal-property-style.ts:99 eslint-plugin/src/rules/comma-spacing.ts:180 eslint-plugin/src/rules/consistent-generic-constructors.ts:123 eslint-plugin/src/rules/consistent-type-definitions.ts:104 eslint-plugin/src/rules/lines-around-comment.ts:41 eslint-plugin/src/rules/lines-around-comment.ts:438 eslint-plugin/src/rules/member-delimiter-style.ts:340 eslint-plugin/src/rules/member-ordering.ts:320 eslint-plugin/src/rules/member-ordering.ts:354 eslint-plugin/src/rules/member-ordering.ts:359 eslint-plugin/src/rules/member-ordering.ts:657 eslint-plugin/src/rules/member-ordering.ts:700 eslint-plugin/src/rules/member-ordering.ts:872 eslint-plugin/src/rules/member-ordering.ts:1001 eslint-plugin/src/rules/naming-convention-utils/parse-options.ts:21 eslint-plugin/src/rules/naming-convention-utils/parse-options.ts:24 eslint-plugin/src/rules/naming-convention.ts:289 eslint-plugin/src/rules/naming-convention.ts:370 eslint-plugin/src/rules/naming-convention.ts:377 eslint-plugin/src/rules/naming-convention.ts:405 eslint-plugin/src/rules/no-duplicate-enum-values.ts:43 eslint-plugin/src/rules/no-inferrable-types.ts:253 eslint-plugin/src/rules/no-mixed-enums.ts:62 eslint-plugin/src/rules/no-type-alias.ts:344 eslint-plugin/src/rules/no-unnecessary-parameter-property-assignment.ts:147 eslint-plugin/src/rules/no-unnecessary-template-expression.ts:125 eslint-plugin/src/rules/no-unnecessary-type-parameters.ts:293 eslint-plugin/src/rules/no-unnecessary-type-parameters.ts:298 eslint-plugin/src/rules/no-use-before-define.ts:328 eslint-plugin/src/rules/no-use-before-define.ts:376 eslint-plugin/src/rules/no-useless-template-literals.ts:126 eslint-plugin/src/rules/prefer-enum-initializers.ts:29 eslint-plugin/src/rules/prefer-function-type.ts:157 eslint-plugin/src/rules/prefer-promise-reject-errors.ts:140 eslint-plugin/src/rules/prefer-readonly.ts:452 eslint-plugin/src/rules/prefer-readonly.ts:456 eslint-plugin/src/rules/prefer-ts-expect-error.ts:54 eslint-plugin/src/rules/return-await.ts:378 eslint-plugin/src/rules/return-await.ts:389 eslint-plugin/src/rules/space-infix-ops.ts:135 eslint-plugin/src/rules/triple-slash-reference.ts:65 eslint-plugin/src/rules/triple-slash-reference.ts:102 eslint-plugin/src/rules/unbound-method.ts:191 eslint-plugin/src/util/collectUnusedVariables.ts:460 eslint-plugin/src/util/collectUnusedVariables.ts:483 eslint-plugin/src/util/collectUnusedVariables.ts:500 eslint-plugin/tests/configs.test.ts:98 eslint-plugin/tests/docs.test.ts:223 eslint-plugin/tests/rules/indent/indent.test.ts:611 eslint-plugin/tests/rules/naming-convention/cases/createTestCases.ts:276 eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts:39 eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts:55 eslint-plugin/tests/util.test.ts:23 eslint-plugin/tests/util.test.ts:33 parser/tests/lib/services.test.ts:36 rule-tester/src/RuleTester.ts:378 rule-tester/src/RuleTester.ts:437 rule-tester/src/RuleTester.ts:458 rule-tester/src/RuleTester.ts:847 rule-tester/src/RuleTester.ts:1001 rule-tester/src/RuleTester.ts:1008 rule-tester/src/utils/SourceCodeFixer.ts:83 rule-tester/src/utils/config-validator.ts:143 rule-tester/src/utils/config-validator.ts:169 rule-tester/src/utils/config-validator.ts:193 rule-tester/src/utils/freezeDeeply.ts:7 rule-tester/src/utils/validationHelpers.ts:117 rule-tester/src/utils/validationHelpers.ts:118 scope-manager/src/ScopeManager.ts:55 scope-manager/src/ScopeManager.ts:56 scope-manager/src/ScopeManager.ts:58 scope-manager/src/referencer/ClassVisitor.ts:52 scope-manager/src/referencer/ClassVisitor.ts:70 scope-manager/src/referencer/ClassVisitor.ts:209 scope-manager/src/referencer/ClassVisitor.ts:251 scope-manager/src/referencer/ClassVisitor.ts:265 scope-manager/src/referencer/ClassVisitor.ts:330 scope-manager/src/referencer/PatternVisitor.ts:59 scope-manager/src/referencer/PatternVisitor.ts:84 scope-manager/src/referencer/Referencer.ts:74 scope-manager/src/referencer/Referencer.ts:270 scope-manager/src/referencer/TypeVisitor.ts:191 scope-manager/src/referencer/Visitor.ts:40 scope-manager/src/scope/ScopeBase.ts:364 scope-manager/src/scope/WithScope.ts:26 scope-manager/tests/eslint-scope/es6-default-parameters.test.ts:7 scope-manager/tests/eslint-scope/references.test.ts:450 scope-manager/tests/eslint-scope/references.test.ts:462 scope-manager/tests/eslint-scope/references.test.ts:484 scope-manager/tests/eslint-scope/references.test.ts:496 scope-manager/tests/eslint-scope/references.test.ts:520 scope-manager/tests/eslint-scope/references.test.ts:536 scope-manager/tests/fixtures.test.ts:174 typescript-eslint/tests/configs.test.ts:103 typescript-estree/src/convert.ts:553 typescript-estree/src/convert.ts:1047 typescript-estree/src/convert.ts:1083 typescript-estree/src/convert.ts:1647 typescript-estree/src/convert.ts:3206 typescript-estree/src/create-program/createProjectProgram.ts:65 typescript-estree/src/create-program/getWatchProgramsForProjects.ts:149 typescript-estree/src/create-program/getWatchProgramsForProjects.ts:410 typescript-estree/src/create-program/getWatchProgramsForProjects.ts:495 typescript-estree/src/node-utils.ts:679 typescript-estree/tests/lib/persistentParse.test.ts:32 typescript-estree/tests/lib/semanticInfo.test.ts:44 utils/src/eslint-utils/applyDefault.ts:25 website/src/components/editor/LoadedEditor.tsx:249 website/src/components/lib/createEventsBinder.ts:10 website/src/components/linter/bridge.ts:47 website/src/components/linter/bridge.ts:49 website/src/components/linter/createLinter.ts:67 website/src/components/linter/createLinter.ts:167 website/src/theme/prism-include-languages.js:10docs
Do not use the
thisargument inArray#findIndex(). website/plugins/generated-rule-docs/insertions/insertWhenNotToUseIt.ts:18Do not use the
thisargument inArray#forEach(). scope-manager/src/referencer/PatternVisitor.ts:59 scope-manager/src/referencer/TypeVisitor.ts:191 scope-manager/src/referencer/Visitor.ts:40docs
Do not call
Array#push()multiple times. ast-spec/tests/util/serializers/Node.ts:64 ast-spec/tests/util/serializers/Node.ts:76 ast-spec/tests/util/serializers/Node.ts:77 ast-spec/tests/util/serializers/Node.ts:87 eslint-plugin/src/rules/consistent-type-definitions.ts:61 eslint-plugin/src/rules/consistent-type-definitions.ts:96 eslint-plugin/src/rules/member-ordering.ts:578 eslint-plugin/src/rules/member-ordering.ts:582 eslint-plugin/src/rules/member-ordering.ts:594 eslint-plugin/src/rules/member-ordering.ts:598 eslint-plugin/src/rules/prefer-optional-chain-utils/gatherLogicalOperands.ts:341 eslint-plugin/tests/rules/no-unsafe-assignment.test.ts:33 eslint-plugin/tests/rules/semi.test.ts:274 eslint-plugin/tests/rules/semi.test.ts:1092 repo-tools/src/generate-contributors.mts:129 repo-tools/src/generate-contributors.mts:130 repo-tools/src/generate-contributors.mts:131 repo-tools/src/generate-contributors.mts:132 repo-tools/src/generate-contributors.mts:133 repo-tools/src/generate-contributors.mts:134 repo-tools/src/generate-contributors.mts:137 repo-tools/src/generate-lib.mts:281docs
Array#reduce()is not allowed eslint-plugin/src/rules/member-ordering.ts:317 eslint-plugin/src/rules/naming-convention-utils/parse-options.ts:85 eslint-plugin/src/rules/no-duplicate-type-constituents.ts:112 eslint-plugin/src/rules/no-type-alias.ts:328 eslint-plugin/src/rules/semi.ts:60 eslint-plugin/tests/configs.test.ts:20 eslint-plugin/tests/rules/func-call-spacing.test.ts:330 eslint-plugin/tests/rules/indent/indent.test.ts:596 eslint-plugin/tests/rules/no-base-to-string.test.ts:49 eslint-plugin/tests/rules/no-explicit-any.test.ts:1200 eslint-plugin/tests/rules/no-inferrable-types.test.ts:14 eslint-plugin/tests/rules/no-unsafe-assignment.test.ts:18 eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts:37 eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts:53 eslint-plugin/tests/rules/semi.test.ts:269 eslint-plugin/tests/rules/semi.test.ts:1082 eslint-plugin/tests/rules/type-annotation-spacing.test.ts:6567 eslint-plugin/tests/rules/type-annotation-spacing.test.ts:6609 parser/src/parser.ts:45 repo-tools/src/generate-configs.mts:75 repo-tools/src/generate-configs.mts:278 repo-tools/src/generate-configs.mts:396 repo-tools/src/generate-sponsors.mts:121 repo-tools/src/generate-sponsors.mts:132 typescript-eslint/tests/configs.test.ts:23 typescript-estree/src/parseSettings/resolveProjectList.ts:60 utils/src/eslint-utils/deepMerge.ts:25 website/src/components/config/ConfigTypeScript.tsx:35 website/src/components/lib/jsonSchema.ts:179docs
Do not use trailing space between
console.logparameters. repo-tools/src/apply-canary-version.mts:35docs
Use a
for-ofloop instead of thisforloop. eslint-plugin/src/rules/no-unsafe-argument.ts:59 eslint-plugin/src/rules/prefer-includes.ts:82 eslint-plugin/src/rules/prefer-string-starts-ends-with.ts:139 eslint-plugin/tests/docs.test.ts:66 rule-schema-to-typescript-types/src/index.ts:33 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:536 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:543 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:642 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:656 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:814 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:950 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:1099 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:1127 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:1140 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:1169 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:1183 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:1212 scope-manager/tests/eslint-scope/es6-destructuring-assignments.test.ts:1229 type-utils/src/isUnsafeAssignment.ts:98 typescript-estree/src/node-utils.ts:780 website/src/components/ast/selectedRange.ts:53docs
Use Unicode escapes instead of hexadecimal escapes. repo-tools/src/generate-configs.mts:22 repo-tools/src/generate-configs.mts:22 repo-tools/src/generate-configs.mts:23 repo-tools/src/generate-configs.mts:23 repo-tools/src/generate-configs.mts:24 repo-tools/src/generate-configs.mts:24 repo-tools/src/generate-configs.mts:25 repo-tools/src/generate-configs.mts:25 repo-tools/src/generate-configs.mts:26 repo-tools/src/generate-configs.mts:26
docs
Unexpected
ifas the only statement in aifblock withoutelse. eslint-plugin/src/rules/consistent-type-imports.ts:229 eslint-plugin/src/rules/consistent-type-imports.ts:559 eslint-plugin/src/rules/lines-around-comment.ts:403 eslint-plugin/src/rules/lines-around-comment.ts:407 eslint-plugin/src/rules/no-confusing-void-expression.ts:261 eslint-plugin/src/rules/no-confusing-void-expression.ts:273 eslint-plugin/src/rules/no-confusing-void-expression.ts:281 eslint-plugin/src/rules/no-confusing-void-expression.ts:291 eslint-plugin/src/rules/no-confusing-void-expression.ts:297 eslint-plugin/src/rules/no-confusing-void-expression.ts:300 eslint-plugin/src/rules/no-empty-interface.ts:62 eslint-plugin/src/rules/no-magic-numbers.ts:167 eslint-plugin/src/rules/no-misused-new.ts:102 eslint-plugin/src/rules/no-misused-promises.ts:513 eslint-plugin/src/rules/no-type-alias.ts:218 eslint-plugin/src/rules/no-unnecessary-parameter-property-assignment.ts:168 eslint-plugin/src/rules/prefer-optional-chain-utils/gatherLogicalOperands.ts:83 eslint-plugin/src/util/collectUnusedVariables.ts:284 eslint-plugin/src/util/getWrappingFixer.ts:64 rule-tester/src/utils/serialization.ts:34 typescript-estree/src/convert.ts:232 typescript-estree/src/convert.ts:3564 website/src/components/ast/selectedRange.ts:57 website/src/components/ast/tsUtils.ts:27docs
Do not use an object literal as default for parameter
options. scope-manager/src/referencer/Visitor.ts:31docs
Property
emitDecoratorMetadatais defined but never used. scope-manager/src/analyze.ts:75Property
libis defined but never used. scope-manager/src/analyze.ts:73docs
Spread an object literal in object literal is unnecessary. eslint-plugin/src/rules/naming-convention-utils/schema.ts:149 eslint-plugin/src/rules/type-annotation-spacing.ts:49 eslint-plugin/src/rules/type-annotation-spacing.ts:54
docs
Useless case in switch statement. typescript-estree/src/convert.ts:651 typescript-estree/src/convert.ts:652 typescript-estree/src/node-utils.ts:591 typescript-estree/src/node-utils.ts:592 typescript-estree/src/node-utils.ts:593 typescript-estree/src/node-utils.ts:594
docs
Do not use useless
undefined. eslint-plugin/src/rules/no-restricted-imports.ts:101 eslint-plugin/src/rules/no-restricted-imports.ts:109 eslint-plugin/src/rules/no-restricted-imports.ts:136 parser/tests/test-utils/test-utils.ts:25 typescript-estree/src/create-program/createIsolatedProgram.ts:54 typescript-estree/src/simple-traverse.ts:91 typescript-estree/tests/lib/createProjectService.test.ts:26 typescript-estree/tests/lib/createProjectService.test.ts:35 typescript-estree/tests/lib/createProjectService.test.ts:60 typescript-estree/tests/lib/createProjectService.test.ts:78 typescript-estree/tests/lib/createProjectService.test.ts:92 typescript-estree/tests/lib/createProjectService.test.ts:105 typescript-estree/tests/lib/useProgramFromProjectService.test.ts:211 website/src/components/ESQueryFilter.tsx:24 website/src/components/ast/DataRenderer.tsx:68 website/src/components/ast/DataRenderer.tsx:200 website/src/components/typeDetails/TypeInfo.tsx:71docs
Invalid number literal casing. eslint-plugin/tests/rules/no-magic-numbers.test.ts:146 eslint-plugin/tests/rules/no-magic-numbers.test.ts:150 eslint-plugin/tests/rules/no-magic-numbers.test.ts:807 eslint-plugin/tests/rules/no-magic-numbers.test.ts:821 typescript-estree/src/node-utils.ts:457
docs
Prefer
addEventListeneroveronload. website/src/components/editor/loadSandbox.ts:20