Skip to content

Commit

Permalink
feat: support ESTree optional chaining representation (#2308)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bradzacher
bradzacher committed Aug 29, 2020
1 parent 3854d6c commit e9d2ab6
Show file tree
Hide file tree
Showing 53 changed files with 3,980 additions and 2,896 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Expand Up @@ -79,7 +79,7 @@
"cspell": "^4.0.61",
"cz-conventional-changelog": "^3.2.0",
"downlevel-dts": "^0.4.0",
"eslint": "^7.2.0",
"eslint": "^7.5.0",
"eslint-plugin-eslint-comments": "^3.1.2",
"eslint-plugin-eslint-plugin": "^2.2.1",
"eslint-plugin-import": "^2.20.2",
Expand Down
6 changes: 2 additions & 4 deletions packages/eslint-plugin-internal/src/rules/no-poorly-typed-ts-props.ts
View file
Expand Up @@ -59,10 +59,8 @@ export default createRule({
const checker = program.getTypeChecker();

return {
':matches(MemberExpression, OptionalMemberExpression)[computed = false]'(
node:
| TSESTree.MemberExpressionNonComputedName
| TSESTree.OptionalMemberExpressionNonComputedName,
'MemberExpression[computed = false]'(
node: TSESTree.MemberExpressionNonComputedName,
): void {
for (const banned of BANNED_PROPERTIES) {
if (node.property.name !== banned.property) {
Expand Down
32 changes: 32 additions & 0 deletions packages/eslint-plugin-internal/tests/rules/no-poorly-typed-ts-props.test.ts
View file
Expand Up @@ -90,5 +90,37 @@ thing.getSymbol();
},
],
},
{
code: `
import ts from 'typescript';
declare const thing: ts.Type;
thing?.symbol;
`.trimRight(),
errors: [
{
messageId: 'doNotUseWithFixer',
data: {
type: 'Type',
property: 'symbol',
fixWith: 'getSymbol()',
},
line: 4,
suggestions: [
{
messageId: 'suggestedFix',
data: {
type: 'Type',
fixWith: 'getSymbol()',
},
output: `
import ts from 'typescript';
declare const thing: ts.Type;
thing?.getSymbol();
`.trimRight(),
},
],
},
],
},
],
});
1 change: 0 additions & 1 deletion packages/eslint-plugin/src/rules/consistent-type-assertions.ts
View file
Expand Up @@ -139,7 +139,6 @@ export default util.createRule<Options, MessageIds>({
node.parent &&
(node.parent.type === AST_NODE_TYPES.NewExpression ||
node.parent.type === AST_NODE_TYPES.CallExpression ||
node.parent.type === AST_NODE_TYPES.OptionalCallExpression ||
node.parent.type === AST_NODE_TYPES.ThrowStatement ||
node.parent.type === AST_NODE_TYPES.AssignmentPattern)
) {
Expand Down
8 changes: 2 additions & 6 deletions packages/eslint-plugin/src/rules/func-call-spacing.ts
View file
Expand Up @@ -77,12 +77,9 @@ export default util.createRule<Options, MessageIds>({
* @private
*/
function checkSpacing(
node:
| TSESTree.CallExpression
| TSESTree.OptionalCallExpression
| TSESTree.NewExpression,
node: TSESTree.CallExpression | TSESTree.NewExpression,
): void {
const isOptionalCall = util.isOptionalOptionalCallExpression(node);
const isOptionalCall = util.isOptionalCallExpression(node);

const closingParenToken = sourceCode.getLastToken(node)!;
const lastCalleeTokenWithoutPossibleParens = sourceCode.getLastToken(
Expand Down Expand Up @@ -175,7 +172,6 @@ export default util.createRule<Options, MessageIds>({

return {
CallExpression: checkSpacing,
OptionalCallExpression: checkSpacing,
NewExpression: checkSpacing,
};
},
Expand Down
8 changes: 2 additions & 6 deletions packages/eslint-plugin/src/rules/no-array-constructor.ts
View file
Expand Up @@ -27,17 +27,14 @@ export default util.createRule({
* @param node node to evaluate
*/
function check(
node:
| TSESTree.CallExpression
| TSESTree.OptionalCallExpression
| TSESTree.NewExpression,
node: TSESTree.CallExpression | TSESTree.NewExpression,
): void {
if (
node.arguments.length !== 1 &&
node.callee.type === AST_NODE_TYPES.Identifier &&
node.callee.name === 'Array' &&
!node.typeParameters &&
!util.isOptionalOptionalCallExpression(node)
!util.isOptionalCallExpression(node)
) {
context.report({
node,
Expand All @@ -60,7 +57,6 @@ export default util.createRule({

return {
CallExpression: check,
OptionalCallExpression: check,
NewExpression: check,
};
},
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/rules/no-extra-non-null-assertion.ts
View file
Expand Up @@ -32,8 +32,8 @@ export default util.createRule({

return {
'TSNonNullExpression > TSNonNullExpression': checkExtraNonNullAssertion,
'OptionalMemberExpression[optional = true] > TSNonNullExpression': checkExtraNonNullAssertion,
'OptionalCallExpression[optional = true] > TSNonNullExpression.callee': checkExtraNonNullAssertion,
'MemberExpression[optional = true] > TSNonNullExpression': checkExtraNonNullAssertion,
'CallExpression[optional = true] > TSNonNullExpression.callee': checkExtraNonNullAssertion,
};
},
});
7 changes: 5 additions & 2 deletions packages/eslint-plugin/src/rules/no-inferrable-types.ts
View file
Expand Up @@ -53,9 +53,12 @@ export default util.createRule<Options, MessageIds>({
init: TSESTree.Expression,
callName: string,
): boolean {
if (init.type === AST_NODE_TYPES.ChainExpression) {
return isFunctionCall(init.expression, callName);
}

return (
(init.type === AST_NODE_TYPES.CallExpression ||
init.type === AST_NODE_TYPES.OptionalCallExpression) &&
init.type === AST_NODE_TYPES.CallExpression &&
init.callee.type === AST_NODE_TYPES.Identifier &&
init.callee.name === callName
);
Expand Down
6 changes: 1 addition & 5 deletions packages/eslint-plugin/src/rules/no-misused-promises.ts
View file
Expand Up @@ -71,7 +71,6 @@ export default util.createRule<Options, 'conditional' | 'voidReturn'>({

const voidReturnChecks: TSESLint.RuleListener = {
CallExpression: checkArguments,
OptionalCallExpression: checkArguments,
NewExpression: checkArguments,
};

Expand All @@ -94,10 +93,7 @@ export default util.createRule<Options, 'conditional' | 'voidReturn'>({
}

function checkArguments(
node:
| TSESTree.CallExpression
| TSESTree.OptionalCallExpression
| TSESTree.NewExpression,
node: TSESTree.CallExpression | TSESTree.NewExpression,
): void {
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
const voidParams = voidFunctionParams(checker, tsNode);
Expand Down
85 changes: 61 additions & 24 deletions packages/eslint-plugin/src/rules/no-non-null-asserted-optional-chain.ts
View file
Expand Up @@ -3,15 +3,18 @@ import {
TSESLint,
AST_NODE_TYPES,
} from '@typescript-eslint/experimental-utils';
import { version } from 'typescript';
import * as ts from 'typescript';
import * as semver from 'semver';
import * as util from '../util';

type MessageIds = 'noNonNullOptionalChain' | 'suggestRemovingNonNull';

const is3dot9 = !semver.satisfies(
version,
'< 3.9.0 || < 3.9.1-rc || < 3.9.0-beta',
const is3dot9 = semver.satisfies(
ts.version,
`>= 3.9.0 || >= 3.9.1-rc || >= 3.9.0-beta`,
{
includePrerelease: true,
},
);

export default util.createRule<[], MessageIds>({
Expand All @@ -34,28 +37,62 @@ export default util.createRule<[], MessageIds>({
},
defaultOptions: [],
create(context) {
const sourceCode = context.getSourceCode();

function isValidFor3dot9(node: TSESTree.ChainExpression): boolean {
if (!is3dot9) {
return false;
}

// TS3.9 made a breaking change to how non-null works with optional chains.
// Pre-3.9, `x?.y!.z` means `(x?.y).z` - i.e. it essentially scrubbed the optionality from the chain
// Post-3.9, `x?.y!.z` means `x?.y!.z` - i.e. it just asserts that the property `y` is non-null, not the result of `x?.y`.
// This means that for > 3.9, x?.y!.z is valid!
// NOTE: these cases are still invalid:
// - x?.y.z!
// - (x?.y)!.z

const parent = util.nullThrows(
node.parent,
util.NullThrowsReasons.MissingParent,
);
const grandparent = util.nullThrows(
parent.parent,
util.NullThrowsReasons.MissingParent,
);

if (
grandparent.type !== AST_NODE_TYPES.MemberExpression &&
grandparent.type !== AST_NODE_TYPES.CallExpression
) {
return false;
}

const lastChildToken = util.nullThrows(
sourceCode.getLastToken(parent, util.isNotNonNullAssertionPunctuator),
util.NullThrowsReasons.MissingToken('last token', node.type),
);
if (util.isClosingParenToken(lastChildToken)) {
return false;
}

const tokenAfterNonNull = sourceCode.getTokenAfter(parent);
if (
tokenAfterNonNull != null &&
util.isClosingParenToken(tokenAfterNonNull)
) {
return false;
}

return true;
}

return {
'TSNonNullExpression > :matches(OptionalMemberExpression, OptionalCallExpression)'(
node:
| TSESTree.OptionalCallExpression
| TSESTree.OptionalMemberExpression,
'TSNonNullExpression > ChainExpression'(
node: TSESTree.ChainExpression,
): void {
if (is3dot9) {
// TS3.9 made a breaking change to how non-null works with optional chains.
// Pre-3.9, `x?.y!.z` means `(x?.y).z` - i.e. it essentially scrubbed the optionality from the chain
// Post-3.9, `x?.y!.z` means `x?.y!.z` - i.e. it just asserts that the property `y` is non-null, not the result of `x?.y`.
// This means that for > 3.9, x?.y!.z is valid!
// NOTE: these cases are still invalid:
// - x?.y.z!
// - (x?.y)!.z
const nnAssertionParent = node.parent?.parent;
if (
nnAssertionParent?.type ===
AST_NODE_TYPES.OptionalMemberExpression ||
nnAssertionParent?.type === AST_NODE_TYPES.OptionalCallExpression
) {
return;
}
if (isValidFor3dot9(node)) {
return;
}

// selector guarantees this assertion
Expand Down
80 changes: 38 additions & 42 deletions packages/eslint-plugin/src/rules/no-non-null-assertion.ts
View file
Expand Up @@ -59,60 +59,56 @@ export default util.createRule<[], MessageIds>({
};
}

if (node.parent) {
if (
(node.parent.type === AST_NODE_TYPES.MemberExpression ||
node.parent.type === AST_NODE_TYPES.OptionalMemberExpression) &&
node.parent.object === node
) {
if (!node.parent.optional) {
if (node.parent.computed) {
// it is x![y]?.z
suggest.push({
messageId: 'suggestOptionalChain',
fix: convertTokenToOptional('?.'),
});
} else {
// it is x!.y?.z
suggest.push({
messageId: 'suggestOptionalChain',
fix: convertTokenToOptional('?'),
});
}
if (
node.parent?.type === AST_NODE_TYPES.MemberExpression &&
node.parent.object === node
) {
if (!node.parent.optional) {
if (node.parent.computed) {
// it is x![y]?.z
suggest.push({
messageId: 'suggestOptionalChain',
fix: convertTokenToOptional('?.'),
});
} else {
if (node.parent.computed) {
// it is x!?.[y].z
suggest.push({
messageId: 'suggestOptionalChain',
fix: removeToken(),
});
} else {
// it is x!?.y.z
suggest.push({
messageId: 'suggestOptionalChain',
fix: removeToken(),
});
}
// it is x!.y?.z
suggest.push({
messageId: 'suggestOptionalChain',
fix: convertTokenToOptional('?'),
});
}
} else if (
(node.parent.type === AST_NODE_TYPES.CallExpression ||
node.parent.type === AST_NODE_TYPES.OptionalCallExpression) &&
node.parent.callee === node
) {
if (!node.parent.optional) {
// it is x.y?.z!()
} else {
if (node.parent.computed) {
// it is x!?.[y].z
suggest.push({
messageId: 'suggestOptionalChain',
fix: convertTokenToOptional('?.'),
fix: removeToken(),
});
} else {
// it is x.y.z!?.()
// it is x!?.y.z
suggest.push({
messageId: 'suggestOptionalChain',
fix: removeToken(),
});
}
}
} else if (
node.parent?.type === AST_NODE_TYPES.CallExpression &&
node.parent.callee === node
) {
if (!node.parent.optional) {
// it is x.y?.z!()
suggest.push({
messageId: 'suggestOptionalChain',
fix: convertTokenToOptional('?.'),
});
} else {
// it is x.y.z!?.()
suggest.push({
messageId: 'suggestOptionalChain',
fix: removeToken(),
});
}
}

context.report({
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin/src/rules/no-require-imports.ts
View file
Expand Up @@ -18,7 +18,7 @@ export default util.createRule({
defaultOptions: [],
create(context) {
return {
':matches(CallExpression, OptionalCallExpression) > Identifier[name="require"]'(
'CallExpression > Identifier[name="require"]'(
node: TSESTree.Identifier,
): void {
context.report({
Expand Down

0 comments on commit e9d2ab6

Please sign in to comment.