Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: support ESTree optional chaining representation #2308

Merged
merged 3 commits into from Jul 28, 2020
Merged

feat: support ESTree optional chaining representation #2308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2411e6f
feat: support ESTree optional chaining representation
bradzacher Jul 19, 2020
0ac0901
Merge branch 'v4' into new-optional-chain
bradzacher Jul 27, 2020
7582ee9
bump eslint version to 7.5.0
bradzacher Jul 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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