Skip to content

Commit

Permalink
feat(eslint-plugin): [no-unsafe-call][no-unsafe-member-access] improv…
Browse files Browse the repository at this point in the history
…e report messages for `this` for `noImplicitThis` (#3199)
  • Loading branch information
JounQin committed Apr 2, 2021
1 parent b1aa7dc commit b1b26c4
Show file tree
Hide file tree
Showing 11 changed files with 276 additions and 18 deletions.
39 changes: 34 additions & 5 deletions packages/eslint-plugin/src/rules/no-unsafe-assignment.ts
Expand Up @@ -2,8 +2,10 @@ import {
TSESTree,
AST_NODE_TYPES,
} from '@typescript-eslint/experimental-utils';
import * as tsutils from 'tsutils';
import * as ts from 'typescript';
import * as util from '../util';
import { getThisExpression } from '../util';

const enum ComparisonType {
/** Do no assignment comparison */
Expand All @@ -25,20 +27,29 @@ export default util.createRule({
requiresTypeChecking: true,
},
messages: {
anyAssignment: 'Unsafe assignment of an any value.',
unsafeArrayPattern: 'Unsafe array destructuring of an any array value.',
anyAssignment: 'Unsafe assignment of an `any` value.',
anyAssignmentThis: [
'Unsafe assignment of an `any` value. `this` is typed as `any`.',
'You can try to fix this by turning on the `noImplicitThis` compiler option, or adding a `this` parameter to the function.',
].join('\n'),
unsafeArrayPattern: 'Unsafe array destructuring of an `any` array value.',
unsafeArrayPatternFromTuple:
'Unsafe array destructuring of a tuple element with an any value.',
'Unsafe array destructuring of a tuple element with an `any` value.',
unsafeAssignment:
'Unsafe assignment of type {{sender}} to a variable of type {{receiver}}.',
unsafeArraySpread: 'Unsafe spread of an any value in an array.',
unsafeArraySpread: 'Unsafe spread of an `any` value in an array.',
},
schema: [],
},
defaultOptions: [],
create(context) {
const { program, esTreeNodeToTSNodeMap } = util.getParserServices(context);
const checker = program.getTypeChecker();
const compilerOptions = program.getCompilerOptions();
const isNoImplicitThis = tsutils.isStrictCompilerOptionEnabled(
compilerOptions,
'noImplicitThis',
);

// returns true if the assignment reported
function checkArrayDestructureHelper(
Expand Down Expand Up @@ -243,9 +254,27 @@ export default util.createRule({
return false;
}

let messageId: 'anyAssignment' | 'anyAssignmentThis' = 'anyAssignment';

if (!isNoImplicitThis) {
// `var foo = this`
const thisExpression = getThisExpression(senderNode);
if (
thisExpression &&
util.isTypeAnyType(
util.getConstrainedTypeAtLocation(
checker,
esTreeNodeToTSNodeMap.get(thisExpression),
),
)
) {
messageId = 'anyAssignmentThis';
}
}

context.report({
node: reportingNode,
messageId: 'anyAssignment',
messageId,
});
return true;
}
Expand Down
34 changes: 32 additions & 2 deletions packages/eslint-plugin/src/rules/no-unsafe-call.ts
@@ -1,7 +1,13 @@
import { TSESTree } from '@typescript-eslint/experimental-utils';
import * as tsutils from 'tsutils';
import * as util from '../util';
import { getThisExpression } from '../util';

type MessageIds = 'unsafeCall' | 'unsafeNew' | 'unsafeTemplateTag';
type MessageIds =
| 'unsafeCall'
| 'unsafeCallThis'
| 'unsafeNew'
| 'unsafeTemplateTag';

export default util.createRule<[], MessageIds>({
name: 'no-unsafe-call',
Expand All @@ -14,7 +20,11 @@ export default util.createRule<[], MessageIds>({
requiresTypeChecking: true,
},
messages: {
unsafeCall: 'Unsafe call of an any typed value.',
unsafeCall: 'Unsafe call of an `any` typed value.',
unsafeCallThis: [
'Unsafe call of an `any` typed value. `this` is typed as `any`.',
'You can try to fix this by turning on the `noImplicitThis` compiler option, or adding a `this` parameter to the function.',
].join('\n'),
unsafeNew: 'Unsafe construction of an any type value.',
unsafeTemplateTag: 'Unsafe any typed template tag.',
},
Expand All @@ -24,6 +34,11 @@ export default util.createRule<[], MessageIds>({
create(context) {
const { program, esTreeNodeToTSNodeMap } = util.getParserServices(context);
const checker = program.getTypeChecker();
const compilerOptions = program.getCompilerOptions();
const isNoImplicitThis = tsutils.isStrictCompilerOptionEnabled(
compilerOptions,
'noImplicitThis',
);

function checkCall(
node: TSESTree.Node,
Expand All @@ -34,6 +49,21 @@ export default util.createRule<[], MessageIds>({
const type = util.getConstrainedTypeAtLocation(checker, tsNode);

if (util.isTypeAnyType(type)) {
if (!isNoImplicitThis) {
// `this()` or `this.foo()` or `this.foo[bar]()`
const thisExpression = getThisExpression(node);
if (
thisExpression &&
util.isTypeAnyType(
util.getConstrainedTypeAtLocation(
checker,
esTreeNodeToTSNodeMap.get(thisExpression),
),
)
) {
messageId = 'unsafeCallThis';
}
}
context.report({
node: reportingNode,
messageId: messageId,
Expand Down
36 changes: 34 additions & 2 deletions packages/eslint-plugin/src/rules/no-unsafe-member-access.ts
Expand Up @@ -2,7 +2,9 @@ import {
TSESTree,
AST_NODE_TYPES,
} from '@typescript-eslint/experimental-utils';
import * as tsutils from 'tsutils';
import * as util from '../util';
import { getThisExpression } from '../util';

const enum State {
Unsafe = 1,
Expand All @@ -21,7 +23,11 @@ export default util.createRule({
},
messages: {
unsafeMemberExpression:
'Unsafe member access {{property}} on an any value.',
'Unsafe member access {{property}} on an `any` value.',
unsafeThisMemberExpression: [
'Unsafe member access {{property}} on an `any` value. `this` is typed as `any`.',
'You can try to fix this by turning on the `noImplicitThis` compiler option, or adding a `this` parameter to the function.',
].join('\n'),
unsafeComputedMemberAccess:
'Computed name {{property}} resolves to an any value.',
},
Expand All @@ -31,6 +37,11 @@ export default util.createRule({
create(context) {
const { program, esTreeNodeToTSNodeMap } = util.getParserServices(context);
const checker = program.getTypeChecker();
const compilerOptions = program.getCompilerOptions();
const isNoImplicitThis = tsutils.isStrictCompilerOptionEnabled(
compilerOptions,
'noImplicitThis',
);
const sourceCode = context.getSourceCode();

const stateCache = new Map<TSESTree.Node, State>();
Expand Down Expand Up @@ -58,9 +69,30 @@ export default util.createRule({

if (state === State.Unsafe) {
const propertyName = sourceCode.getText(node.property);

let messageId: 'unsafeMemberExpression' | 'unsafeThisMemberExpression' =
'unsafeMemberExpression';

if (!isNoImplicitThis) {
// `this.foo` or `this.foo[bar]`
const thisExpression = getThisExpression(node);

if (
thisExpression &&
util.isTypeAnyType(
util.getConstrainedTypeAtLocation(
checker,
esTreeNodeToTSNodeMap.get(thisExpression),
),
)
) {
messageId = 'unsafeThisMemberExpression';
}
}

context.report({
node,
messageId: 'unsafeMemberExpression',
messageId,
data: {
property: node.computed ? `[${propertyName}]` : `.${propertyName}`,
},
Expand Down
38 changes: 33 additions & 5 deletions packages/eslint-plugin/src/rules/no-unsafe-return.ts
Expand Up @@ -2,8 +2,9 @@ import {
TSESTree,
AST_NODE_TYPES,
} from '@typescript-eslint/experimental-utils';
import { isExpression } from 'tsutils';
import * as tsutils from 'tsutils';
import * as util from '../util';
import { getThisExpression } from '../util';

export default util.createRule({
name: 'no-unsafe-return',
Expand All @@ -16,16 +17,25 @@ export default util.createRule({
requiresTypeChecking: true,
},
messages: {
unsafeReturn: 'Unsafe return of an {{type}} typed value',
unsafeReturn: 'Unsafe return of an `{{type}}` typed value.',
unsafeReturnThis: [
'Unsafe return of an `{{type}}` typed value. `this` is typed as `any`.',
'You can try to fix this by turning on the `noImplicitThis` compiler option, or adding a `this` parameter to the function.',
].join('\n'),
unsafeReturnAssignment:
'Unsafe return of type {{sender}} from function with return type {{receiver}}.',
'Unsafe return of type `{{sender}}` from function with return type `{{receiver}}`.',
},
schema: [],
},
defaultOptions: [],
create(context) {
const { program, esTreeNodeToTSNodeMap } = util.getParserServices(context);
const checker = program.getTypeChecker();
const compilerOptions = program.getCompilerOptions();
const isNoImplicitThis = tsutils.isStrictCompilerOptionEnabled(
compilerOptions,
'noImplicitThis',
);

function getParentFunctionNode(
node: TSESTree.Node,
Expand Down Expand Up @@ -74,7 +84,7 @@ export default util.createRule({
// so we have to use the contextual typing in these cases, i.e.
// const foo1: () => Set<string> = () => new Set<any>();
// the return type of the arrow function is Set<any> even though the variable is typed as Set<string>
let functionType = isExpression(functionTSNode)
let functionType = tsutils.isExpression(functionTSNode)
? util.getContextualType(checker, functionTSNode)
: checker.getTypeAtLocation(functionTSNode);
if (!functionType) {
Expand All @@ -100,10 +110,28 @@ export default util.createRule({
}
}

let messageId: 'unsafeReturn' | 'unsafeReturnThis' = 'unsafeReturn';

if (!isNoImplicitThis) {
// `return this`
const thisExpression = getThisExpression(returnNode);
if (
thisExpression &&
util.isTypeAnyType(
util.getConstrainedTypeAtLocation(
checker,
esTreeNodeToTSNodeMap.get(thisExpression),
),
)
) {
messageId = 'unsafeReturnThis';
}
}

// If the function return type was not unknown/unknown[], mark usage as unsafeReturn.
return context.report({
node: reportingNode,
messageId: 'unsafeReturn',
messageId,
data: {
type: anyType === util.AnyType.Any ? 'any' : 'any[]',
},
Expand Down
24 changes: 24 additions & 0 deletions packages/eslint-plugin/src/util/getThisExpression.ts
@@ -0,0 +1,24 @@
import {
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';

export function getThisExpression(
node: TSESTree.Node,
): TSESTree.ThisExpression | undefined {
while (node) {
if (node.type === AST_NODE_TYPES.CallExpression) {
node = node.callee;
} else if (node.type === AST_NODE_TYPES.ThisExpression) {
return node;
} else if (node.type === AST_NODE_TYPES.MemberExpression) {
node = node.object;
} else if (node.type === AST_NODE_TYPES.ChainExpression) {
node = node.expression;
} else {
break;
}
}

return;
}
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/util/index.ts
Expand Up @@ -4,6 +4,7 @@ export * from './astUtils';
export * from './collectUnusedVariables';
export * from './createRule';
export * from './getFunctionHeadLoc';
export * from './getThisExpression';
export * from './getWrappingFixer';
export * from './isTypeReadonly';
export * from './misc';
Expand Down
@@ -0,0 +1,6 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"noImplicitThis": false
}
}
17 changes: 16 additions & 1 deletion packages/eslint-plugin/tests/rules/no-unsafe-assignment.test.ts
Expand Up @@ -68,7 +68,7 @@ function assignmentTest(
const ruleTester = new RuleTester({
parser: '@typescript-eslint/parser',
parserOptions: {
project: './tsconfig.json',
project: './tsconfig.noImplicitThis.json',
tsconfigRootDir: getFixturesRootDir(),
},
});
Expand Down Expand Up @@ -347,5 +347,20 @@ declare function Foo(props: Props): never;
},
],
},
{
code: `
function foo() {
const bar = this;
}
`,
errors: [
{
messageId: 'anyAssignmentThis',
line: 3,
column: 9,
endColumn: 19,
},
],
},
],
});
31 changes: 30 additions & 1 deletion packages/eslint-plugin/tests/rules/no-unsafe-call.test.ts
Expand Up @@ -9,7 +9,7 @@ import {
const ruleTester = new RuleTester({
parser: '@typescript-eslint/parser',
parserOptions: {
project: './tsconfig.json',
project: './tsconfig.noImplicitThis.json',
tsconfigRootDir: getFixturesRootDir(),
},
});
Expand Down Expand Up @@ -148,5 +148,34 @@ function foo(x: { tag: any }) { x.tag\`foo\` }
},
],
}),
{
code: noFormat`
const methods = {
methodA() {
return this.methodB()
},
methodB() {
return true
},
methodC() {
return this()
}
};
`,
errors: [
{
messageId: 'unsafeCallThis',
line: 4,
column: 12,
endColumn: 24,
},
{
messageId: 'unsafeCallThis',
line: 10,
column: 12,
endColumn: 16,
},
],
},
],
});

0 comments on commit b1b26c4

Please sign in to comment.