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.

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

feat(eslint-plugin): [no-unsafe-call][no-unsafe-member-access] improve report messages for this for noImplicitThis #3199

Merged
merged 4 commits into from Apr 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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(
JounQin marked this conversation as resolved.
Show resolved Hide resolved
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,
},
],
},
],
});