Skip to content

feat(eslint-plugin): [prefer-optional-chain] support if statement as part of chain #10137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 51 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
fd1c08d
feat(eslint-plugin): [prefer-optional-chain] support if statement as …
May 4, 2024
387a3ab
Add docs mdx
Oct 12, 2024
a21c197
Order matters + snapshots
Oct 12, 2024
eaa203f
Handle semi colons
Oct 12, 2024
a7258b8
Coverage
Oct 12, 2024
0289162
Use `lastOperand` to fix false-negative coverage report
Oct 12, 2024
958d09d
fix comment placement
Oct 12, 2024
4490802
Support chaining in `if` body - only for logicals that end with `Call…
Oct 12, 2024
f3ad6bd
coverage
Oct 13, 2024
ed1ccbe
reformat a bit and test with `requireNullish: true`
Oct 13, 2024
36f074c
comment
Oct 13, 2024
2c21587
Handle `if` without braces
Oct 13, 2024
23943b1
better node type
Oct 13, 2024
5f3f9df
hit the missing coverage
Oct 18, 2024
0d4b539
Merge remote-tracking branch 'upstream/main' into feat-eslint-plugin-…
Oct 19, 2024
665e903
Merge branch 'main' into feat-eslint-plugin-issue-6309-prefer-optiona…
omril1 Oct 22, 2024
00c220f
Merge branch 'main' into feat-eslint-plugin-issue-6309-prefer-optiona…
omril1 Oct 25, 2024
3aab4ba
`insert option description`
Oct 25, 2024
bd7ee28
CR: change arroy function to method to reduce indent
Nov 5, 2024
353202f
CR: less specific type
Nov 5, 2024
e180f60
CR: combine early returns
Nov 5, 2024
5cdeb5e
CR: remove unncessary return
Nov 5, 2024
b6b3491
CR: rename `allowSuggestingOnIfStatements` to `allowIfStatements`
Nov 5, 2024
1beff72
CR: Fix title for mdx TabItem
Nov 5, 2024
7b12732
CR: Remove comments on test cases
Nov 5, 2024
3e25c65
Merge remote-tracking branch 'upstream/main' into feat-eslint-plugin-…
Nov 5, 2024
c2c6ae3
fix snapshot
Nov 5, 2024
98cb43d
CR: Make the rule work with comments to some degree
Nov 5, 2024
6a96898
Merge remote-tracking branch 'upstream/main' into feat-eslint-plugin-…
Nov 5, 2024
1a3bee2
Remove unncessary `suggestions: null`
Nov 9, 2024
bfbf55c
better comments
Nov 8, 2024
5d9ef8f
change to suggestion fixer when `if` contains comments between
Nov 9, 2024
6465402
CR: add types to tests
Nov 15, 2024
061022d
Merge branch 'main' into feat-eslint-plugin-issue-6309-prefer-optiona…
omril1 Nov 15, 2024
26095c4
Merge remote-tracking branch 'upstream/main' into feat-eslint-plugin-…
omril1 Jan 4, 2025
f3f98a9
recreate comments only if the exist in ranges that are being removed
omril1 Jan 11, 2025
20c6eea
Merge remote-tracking branch 'upstream/main' into feat-eslint-plugin-…
omril1 Jan 11, 2025
6671148
CR: Fix rule descrition
omril1 Jan 11, 2025
310b56e
CR: Fix ignored non-CallExpression
omril1 Jan 11, 2025
36e306f
CR: Revert change to `context.sourceCode` input
omril1 Jan 11, 2025
6ccb9be
fix coverage and bug with comment included twice
omril1 Jan 11, 2025
bf84196
update snapshot
omril1 Jan 11, 2025
bb2b2aa
change rule name, invert value and fix tests
omril1 Jan 11, 2025
c843a4d
Fix internal lint error
omril1 Jan 11, 2025
9b6a32e
Merge branch 'main' into feat-eslint-plugin-issue-6309-prefer-optiona…
omril1 Jan 18, 2025
f16358f
add non-null assertions because I couldn't find a way to make this co…
omril1 Jan 18, 2025
c8e7980
Merge branch 'main' into feat-eslint-plugin-issue-6309-prefer-optiona…
omril1 Feb 16, 2025
ec6389b
Update comment
omril1 Feb 17, 2025
b4fd091
Merge branch 'main' into feat-eslint-plugin-issue-6309-prefer-optiona…
omril1 Feb 17, 2025
6d2f69b
Merge branch 'main' into feat-eslint-plugin-issue-6309-prefer-optiona…
omril1 Mar 5, 2025
cbfb329
Merge branch 'main' into feat-eslint-plugin-issue-6309-prefer-optiona…
omril1 Mar 16, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions packages/eslint-plugin/docs/rules/prefer-optional-chain.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,32 @@ thing?.toString();
</TabItem>
</Tabs>

### `ignoreIfStatements`

{/* insert option description */}

Examples of code for this rule with `{ ignoreIfStatements: false }`:

<Tabs>

<TabItem value="❌ Incorrect">

```ts option='{ "ignoreIfStatements": false }'
if (foo) {
foo.bar();
}
```

</TabItem>
<TabItem value="✅ Correct">

```ts option='{ "ignoreIfStatements": true }'
foo?.bar();
```

</TabItem>
</Tabs>

### `requireNullish`

{/* insert option description */}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export type PreferOptionalChainMessageIds =
| 'preferOptionalChain';

export interface PreferOptionalChainOptions {
ignoreIfStatements?: boolean;
allowPotentiallyUnsafeFixesThatModifyTheReturnTypeIKnowWhatImDoing?: boolean;
checkAny?: boolean;
checkBigInt?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type {
SourceCode,
} from '@typescript-eslint/utils/ts-eslint';

import { AST_NODE_TYPES } from '@typescript-eslint/utils';
import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils';
import { unionTypeParts } from 'ts-api-utils';
import * as ts from 'typescript';

Expand Down Expand Up @@ -208,9 +208,13 @@ const analyzeOrChainOperand: OperandAnalyzer = (
*/
function getReportRange(
chain: ValidOperand[],
boundary: TSESTree.Range,
node: TSESTree.Node,
sourceCode: SourceCode,
): TSESTree.Range {
if (node.type === AST_NODE_TYPES.IfStatement) {
return node.range;
}
const boundary = node.range;
const leftNode = chain[0].node;
const rightNode = chain[chain.length - 1].node;
let leftMost = nullThrows(
Expand Down Expand Up @@ -324,7 +328,7 @@ function getReportDescriptor(
// 6) diff(`foo.bar.baz?.bam`, `foo.bar.baz.bam()`) = `()`
// 7) result = `foo?.bar?.baz?.bam?.()`

const parts = [];
const parts: FlattenedChain[] = [];
for (const current of chain) {
const nextOperand = flattenChainExpression(
sourceCode,
Expand Down Expand Up @@ -401,7 +405,85 @@ function getReportDescriptor(
newCode = `!${newCode}`;
}

const reportRange = getReportRange(chain, node.range, sourceCode);
const reportRange = getReportRange(chain, node, sourceCode);

if (node.type === AST_NODE_TYPES.IfStatement) {
const chainEndedWithSemicolon =
sourceCode.getTokenAfter(lastOperand.node)?.value === ';';
if (chainEndedWithSemicolon) {
newCode += ';';
}

const nodeBeforeTheComment = chainEndedWithSemicolon
? lastOperand.node.parent
: lastOperand.node;

const commentsBefore = sourceCode.getCommentsBefore(chain[1].node);
const commentsAfter = sourceCode.getCommentsAfter(nodeBeforeTheComment);
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const tokenAfterNodeTest = sourceCode.getTokenAfter(node.test)!; // if (foo) /* this */, there is always a token here
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const tokenBeforeNodeTest = sourceCode.getTokenBefore(node.test)!; // if /* this */ (foo), there is always a token here
const tokenAfterAfterNodeTest =
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
sourceCode.getTokenAfter(tokenAfterNodeTest)!; // if (foo) /* this */, there is always a token here
const commentsBeforeNodeTest =
sourceCode.getCommentsBefore(tokenBeforeNodeTest); // if /* this */ (foo)
const beforeNodeTestComments = sourceCode.getCommentsBefore(node.test); // if (/* this */ foo)
const afterNodeTestComments1 = sourceCode.getCommentsAfter(node.test); // if (foo /* this */)
const afterNodeTestComments2 =
tokenAfterAfterNodeTest.type === AST_TOKEN_TYPES.Punctuator
? sourceCode.getCommentsBefore(tokenAfterAfterNodeTest)
: []; // if (foo) /* this */

const commentsToReloacte = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Typo]

Suggested change
const commentsToReloacte = [
const commentsToRelocate = [

...commentsBeforeNodeTest,
...beforeNodeTestComments,
...afterNodeTestComments1,
...afterNodeTestComments2,
];

if (commentsToReloacte.length) {
commentsBefore.unshift(...commentsToReloacte);
useSuggestionFixer = true;
const indentationCount = node.loc.start.column;
const indentation = ' '.repeat(indentationCount);
const newLineIndentation = `\n${indentation}`;
function getTextFromCommentsArray(comments: TSESTree.Comment[]): string {
return comments
.map(({ type, value }) =>
type === AST_TOKEN_TYPES.Line ? `//${value}` : `/*${value}*/`,
)
.join(newLineIndentation);
}
if (commentsBefore.length > 0) {
const commentsText = getTextFromCommentsArray(commentsBefore);
newCode = `${commentsText}\n${indentation}${newCode}`;
}
if (commentsAfter.length > 0) {
const commentsText = getTextFromCommentsArray(commentsAfter);
newCode = `${newCode}\n${indentation}${commentsText}`;
}
} else if (commentsBefore.length || commentsAfter.length) {
const indentationCount = node.loc.start.column;
const indentation = ' '.repeat(indentationCount);
function getTextFromCommentsArray(comments: TSESTree.Comment[]): string {
return sourceCode
.getText()
.slice(comments[0].range[0], comments[comments.length - 1].range[1]);
}
if (commentsBefore.length > 0) {
const commentsTextBeforeWithoutIndent =
getTextFromCommentsArray(commentsBefore);
newCode = `${commentsTextBeforeWithoutIndent}\n${indentation}${newCode}`;
}
if (commentsAfter.length > 0) {
const commentsTextAfterWithoutIndent =
getTextFromCommentsArray(commentsAfter);
newCode = `${newCode}\n${indentation}${commentsTextAfterWithoutIndent}`;
}
}
}

const fix: ReportFixFunction = fixer =>
fixer.replaceTextRange(reportRange, newCode);
Expand Down
87 changes: 86 additions & 1 deletion packages/eslint-plugin/src/rules/prefer-optional-chain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { analyzeChain } from './prefer-optional-chain-utils/analyzeChain';
import { checkNullishAndReport } from './prefer-optional-chain-utils/checkNullishAndReport';
import {
gatherLogicalOperands,
NullishComparisonType,
OperandValidity,
} from './prefer-optional-chain-utils/gatherLogicalOperands';

Expand Down Expand Up @@ -82,6 +83,11 @@ export default createRule<
description:
'Check operands that are typed as `unknown` when inspecting "loose boolean" operands.',
},
ignoreIfStatements: {
type: 'boolean',
description:
'Whether to ignore `if` statements with a single expression in the consequent.',
},
requireNullish: {
type: 'boolean',
description:
Expand All @@ -100,6 +106,7 @@ export default createRule<
checkNumber: true,
checkString: true,
checkUnknown: true,
ignoreIfStatements: false,
requireNullish: false,
},
],
Expand All @@ -109,7 +116,84 @@ export default createRule<
const seenLogicals = new Set<TSESTree.LogicalExpression>();

return {
// specific handling for `(foo ?? {}).bar` / `(foo || {}).bar`
// specific handling for `if (foo) { foo.bar }` / `if (foo) foo.bar`
'IfStatement[consequent.type=BlockStatement][consequent.body.length=1], IfStatement[consequent.type=ExpressionStatement]'(
node: TSESTree.IfStatement,
): void {
if (options.ignoreIfStatements || node.alternate) {
return;
}
const ifBodyStatement =
node.consequent.type === AST_NODE_TYPES.BlockStatement
? node.consequent.body[0]
: node.consequent;

if (ifBodyStatement.type !== AST_NODE_TYPES.ExpressionStatement) {
return;
}

const ifBodyExpression = ifBodyStatement.expression;
const currentChain: ValidOperand[] = [
{
node: node.test,
type: OperandValidity.Valid,
comparedName: node.test,
comparisonType: NullishComparisonType.Boolean,
isYoda: false,
},
];
if (node.test.type === AST_NODE_TYPES.LogicalExpression) {
const { newlySeenLogicals, operands } = gatherLogicalOperands(
node.test,
parserServices,
context.sourceCode,
options,
);
for (const logical of newlySeenLogicals) {
seenLogicals.add(logical);
}
for (const operand of operands) {
if (operand.type === OperandValidity.Invalid) {
return;
}
currentChain.push(operand);
}
}
if (ifBodyExpression.type === AST_NODE_TYPES.LogicalExpression) {
const { newlySeenLogicals, operands } = gatherLogicalOperands(
ifBodyExpression,
parserServices,
context.sourceCode,
options,
);
for (const logical of newlySeenLogicals) {
seenLogicals.add(logical);
}
for (const operand of operands) {
if (operand.type === OperandValidity.Invalid) {
return;
}
currentChain.push(operand);
}
} else {
currentChain.push({
node: ifBodyExpression,
type: OperandValidity.Valid,
comparedName: ifBodyExpression,
comparisonType: NullishComparisonType.Boolean,
isYoda: false,
});
}
analyzeChain(
context,
parserServices,
options,
node,
'&&',
currentChain,
);
},

'LogicalExpression[operator!="??"]'(
node: TSESTree.LogicalExpression,
): void {
Expand Down Expand Up @@ -157,6 +241,7 @@ export default createRule<
}
},

// specific handling for `(foo ?? {}).bar` / `(foo || {}).bar`
'LogicalExpression[operator="||"], LogicalExpression[operator="??"]'(
node: TSESTree.LogicalExpression,
): void {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading