Skip to content
Permalink
Browse files

feat: support ESTree optional chaining representation (#2308)

  • Loading branch information
bradzacher committed Aug 29, 2020
1 parent 3854d6c commit e9d2ab638b6767700b52797e74b814ea059beaae
Showing with 3,980 additions and 2,896 deletions.
  1. +1 −1 package.json
  2. +2 −4 packages/eslint-plugin-internal/src/rules/no-poorly-typed-ts-props.ts
  3. +32 −0 packages/eslint-plugin-internal/tests/rules/no-poorly-typed-ts-props.test.ts
  4. +0 −1 packages/eslint-plugin/src/rules/consistent-type-assertions.ts
  5. +2 −6 packages/eslint-plugin/src/rules/func-call-spacing.ts
  6. +2 −6 packages/eslint-plugin/src/rules/no-array-constructor.ts
  7. +2 −2 packages/eslint-plugin/src/rules/no-extra-non-null-assertion.ts
  8. +5 −2 packages/eslint-plugin/src/rules/no-inferrable-types.ts
  9. +1 −5 packages/eslint-plugin/src/rules/no-misused-promises.ts
  10. +61 −24 packages/eslint-plugin/src/rules/no-non-null-asserted-optional-chain.ts
  11. +38 −42 packages/eslint-plugin/src/rules/no-non-null-assertion.ts
  12. +1 −1 packages/eslint-plugin/src/rules/no-require-imports.ts
  13. +15 −21 packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
  14. +1 −1 packages/eslint-plugin/src/rules/no-unsafe-call.ts
  15. +4 −6 packages/eslint-plugin/src/rules/no-unsafe-member-access.ts
  16. +2 −1 packages/eslint-plugin/src/rules/no-unused-expressions.ts
  17. +10 −9 packages/eslint-plugin/src/rules/no-var-requires.ts
  18. +2 −4 packages/eslint-plugin/src/rules/prefer-for-of.ts
  19. +16 −20 packages/eslint-plugin/src/rules/prefer-includes.ts
  20. +20 −27 packages/eslint-plugin/src/rules/prefer-optional-chain.ts
  21. +5 −6 packages/eslint-plugin/src/rules/prefer-reduce-type-parameter.ts
  22. +83 −79 packages/eslint-plugin/src/rules/prefer-string-starts-ends-with.ts
  23. +2 −2 packages/eslint-plugin/src/rules/require-array-sort-compare.ts
  24. +4 −8 packages/eslint-plugin/src/rules/unbound-method.ts
  25. +2 −3 packages/eslint-plugin/src/util/explicitReturnTypeUtils.ts
  26. +5 −2 packages/eslint-plugin/tests/rules/indent/indent.test.ts
  27. +0 −1 packages/experimental-utils/src/ast-utils/eslint-utils/astUtilities.ts
  28. +4 −14 packages/experimental-utils/src/ast-utils/predicates.ts
  29. +1 −2 packages/experimental-utils/src/ts-eslint/Rule.ts
  30. +0 −23 packages/parser/src/analyze-scope.ts
  31. +34 −8 packages/parser/tests/lib/__snapshots__/typescript.ts.snap
  32. +2 −18 packages/scope-manager/src/referencer/Referencer.ts
  33. +1 −2 packages/types/src/ast-node-types.ts
  34. +8 −27 packages/types/src/ts-estree.ts
  35. +60 −64 packages/typescript-estree/src/convert.ts
  36. +7 −10 packages/typescript-estree/src/node-utils.ts
  37. +4 −4 packages/typescript-estree/src/ts-estree/estree-to-ts-node-types.ts
  38. +1 −1 packages/typescript-estree/tests/ast-fixtures.test.ts
  39. +389 −253 ...-estree/tests/snapshots/typescript/basics/optional-chain-call-with-non-null-assertion.src.ts.shot
  40. +405 −252 ...s/typescript-estree/tests/snapshots/typescript/basics/optional-chain-call-with-parens.src.ts.shot
  41. +474 −321 packages/typescript-estree/tests/snapshots/typescript/basics/optional-chain-call.src.ts.shot
  42. +422 −286 ...sts/snapshots/typescript/basics/optional-chain-element-access-with-non-null-assertion.src.ts.shot
  43. +366 −264 ...pt-estree/tests/snapshots/typescript/basics/optional-chain-element-access-with-parens.src.ts.shot
  44. +377 −275 ...ges/typescript-estree/tests/snapshots/typescript/basics/optional-chain-element-access.src.ts.shot
  45. +208 −140 ...cript-estree/tests/snapshots/typescript/basics/optional-chain-with-non-null-assertion.src.ts.shot
  46. +352 −250 packages/typescript-estree/tests/snapshots/typescript/basics/optional-chain-with-parens.src.ts.shot
  47. +304 −219 packages/typescript-estree/tests/snapshots/typescript/basics/optional-chain.src.ts.shot
  48. +188 −154 ...estree/tests/snapshots/typescript/expressions/optional-call-expression-type-arguments.src.ts.shot
  49. +3 −2 packages/visitor-keys/package.json
  50. +6 −0 packages/visitor-keys/src/get-keys.ts
  51. +1 −0 packages/visitor-keys/src/index.ts
  52. +0 −2 packages/visitor-keys/src/visitor-keys.ts
  53. +45 −21 yarn.lock
@@ -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",
@@ -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) {
@@ -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(),
},
],
},
],
},
],
});
@@ -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)
) {
@@ -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(
@@ -175,7 +172,6 @@ export default util.createRule<Options, MessageIds>({

return {
CallExpression: checkSpacing,
OptionalCallExpression: checkSpacing,
NewExpression: checkSpacing,
};
},
@@ -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,
@@ -60,7 +57,6 @@ export default util.createRule({

return {
CallExpression: check,
OptionalCallExpression: check,
NewExpression: check,
};
},
@@ -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,
};
},
});
@@ -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
);
@@ -71,7 +71,6 @@ export default util.createRule<Options, 'conditional' | 'voidReturn'>({

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

@@ -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);
@@ -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>({
@@ -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
@@ -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({
@@ -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({

0 comments on commit e9d2ab6

Please sign in to comment.