feat(lint): rule noUnnecessaryConditions#7084
Conversation
🦋 Changeset detectedLatest commit: 721a2ed The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
A-CLI
ematipico
force-pushed
the
feat/no-unnecessary-contidtions
branch
from
f7006a8 to
0beda5b
Compare
|
cc @arendjr for some reason the type resolver doesn't work in this rule when running the |
CodSpeed Performance ReportMerging #7084 will not alter performanceComparing Summary
|
ematipico
force-pushed
the
feat/no-unnecessary-contidtions
branch
from
0beda5b to
7295f3f
Compare
There was a problem hiding this comment.
nit: revert quick test changes
| let text = literal.text_trimmed(); | ||
| // Remove quotes to get actual string content | ||
| if text.len() >= 2 { | ||
| let content = &text[1..text.len() - 1]; // Remove surrounding quotes |
There was a problem hiding this comment.
You probably want str_expr.inner_string_text.
Is this still an issue? If so, how can I see the issue? |
|
Still an issue. Remove the ignore from the code blocks and run |
| @@ -0,0 +1,141 @@ | |||
| // should generate diagnostics | |||
|
|
|||
There was a problem hiding this comment.
we need some tests to cover:
declare const b1: false;
declare const b2: boolean;
if (b1 && b2) {
}
const b1 = 22;
declare const b2: boolean;
if (b1 && b2) {
}
and the more funky ones:
((string & { __brandA: string }) | (number & { __brandB: string })) & ""
((string & { __brandA: string }) | (number & { __brandB: string })) & ("foo" | 123)
(123 | true) & { __brand: string }
("" | false) & { __brand: string }
("foo" | "bar") & { __brand: string }
"" & {}
"" & { __brand: string }
for instance:
declare const b1: ((string & { __brandA: string }) | (number & { __brandB: string })) & "";
declare const b2: boolean;
if (b1 && b2) {
}
etc.
| @@ -0,0 +1,183 @@ | |||
| // should not generate diagnostics | |||
|
|
|||
There was a problem hiding this comment.
some more funky tests can be added around declare const here too:
declare const arr2: Array<{ x: { y: { z: object } } }>;
arr2[42]?.x.y?.z;
declare const arr: string[];
if (arr[0]) {
arr[0] ?? 'foo';
}
declare const arr: object[];
if (arr[42] && arr[42]) {
}
declare function isString(x: unknown): x is string;
declare const a: string;
isString('fa' + 'lafel');
declare function isString(x: unknown): x is string;
declare const a: string;
isString(a);
declare function assertsString(x: unknown): asserts x is string;
declare const a: string;
assertsString(a);
declare function assert(x: unknown): asserts x;
assert({});
declare function assert(x: unknown, y: unknown): asserts x;
assert(true, Math.random() > 0.5);
type Foo = { [key: string]: () => number } | null;
declare const foo: Foo;
foo?.['bar']?.()?.toExponential();
also these operators:
declare let foo: null;
foo ||= null;
foo &&= null;
foo ??= null;
etc...
|
conditional.is_truthy and conditional.is_falsy I found may need some more tweaking for more complex ts cases - see my other comments on tests. |
We have the nursery category for these reasons. The rule doesn't need to be perfect on the first try |
|
@vladimir-ivanov can you please add those use cases in the main issue so we can track them? |
ematipico
force-pushed
the
feat/no-unnecessary-contidtions
branch
from
d4d06e7 to
bdc74ce
Compare
|
@ematipico Re: I pushed a commit doing the same here, |
|
Just trying it out and it's making a lot of false positives, flagging switch statements, etc! 
|
|
@radiosilence please follow up here #6611 |
5 participants
Summary
Implements the rule
noUnnecessaryConditionsusing our inference engine.Part of #6611
Test Plan
I used the tests from typescript-eslint
Docs