Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore(type-utils): reuse newly added "is builtin symbol like" logic#8287

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 5 additions & 17 deletionspackages/eslint-plugin/src/rules/no-implied-eval.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,7 @@ import { getScope } from '@typescript-eslint/utils/eslint-utils';
import * as tsutils from 'ts-api-utils';
import * as ts from 'typescript';

import { createRule, getParserServices } from '../util';
import { createRule, getParserServices, isFunctionSimilar } from '../util';

const FUNCTION_CONSTRUCTOR = 'Function';
const GLOBAL_CANDIDATES = new Set(['global', 'window', 'globalThis']);
Expand DownExpand Up@@ -78,15 +78,8 @@ export default createRule({
return true;
}

// eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison
if (symbol && symbol.escapedName === FUNCTION_CONSTRUCTOR) {
const declarations = symbol.getDeclarations() ?? [];
for (const declaration of declarations) {
const sourceFile = declaration.getSourceFile();
if (services.program.isSourceFileDefaultLibrary(sourceFile)) {
return true;
}
}
if (isFunctionSimilar(services.program, type)) {
return true;
}

const signatures = checker.getSignaturesOfType(
Expand DownExpand Up@@ -143,13 +136,8 @@ export default createRule({
const type = services.getTypeAtLocation(node.callee);
const symbol = type.getSymbol();
if (symbol) {
const declarations = symbol.getDeclarations() ?? [];
for (const declaration of declarations) {
const sourceFile = declaration.getSourceFile();
if (services.program.isSourceFileDefaultLibrary(sourceFile)) {
context.report({ node, messageId: 'noFunctionConstructor' });
return;
}
if (isFunctionSimilar(services.program, type)) {
context.report({ node, messageId: 'noFunctionConstructor' });
}
} else {
context.report({ node, messageId: 'noFunctionConstructor' });
Expand Down
12 changes: 12 additions & 0 deletionspackages/type-utils/src/builtinSymbolLikes.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,6 +69,18 @@ export function isReadonlyTypeLike(
);
});
}
/**
* let F = new Function("foo");
* ^ FunctionSimilar
* let I = (callback: Function) => {}
* ^ FunctionSimilar
*/
export function isFunctionSimilar(program: ts.Program, type: ts.Type): boolean {
return (
isBuiltinSymbolLike(program, type, 'Function') ||
isBuiltinSymbolLike(program, type, 'FunctionConstructor')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The original rule didn't check for symbols with nameFunctionConstructor. Should we add tests that contain symbols with nameFunctionConstructor? But that is probably not within the scope of this pr. IMO we shouldn't change the rule logic in this pr

Copy link
ContributorAuthor

@arkapratimcarkapratimcJan 22, 2024•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I just want to ask a small question -

This rule is checking FunctionConstructor right ? Like, in this case, the escapedName of the symbol is 'FunctionConstructor'.

code:"const fn = new Function('a', 'b', 'return a + b');",
errors:[
{
messageId:'noFunctionConstructor',
line:1,
column:12,
},
],

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

You're rightFunction here hasFunctionConstructor type. I missed this because in the source code of rule both symbol name and callee identifier's name are compared toFUNCTION_CONSTRUCTOR

Then +1 on renamingisFunctionLike... Not sure what's the best name for it. MaybeisFunctionTypeLike or something like this?

Also we should consider that#8094 most likely will introduce changes toisBuiltinSymbolLike.
As per#8094 (comment):
isBuiltinSymbolLike(program, type, 'Function') || isBuiltinSymbolLike(program, type, 'FunctionConstructor') recursively visits all subtypes twice, we can optimize to not do the same job twice.#8094 does the following:

isBuiltinSymbolLike(services.program,calleeType,name=>name==='PromiseConstructor'||name==='Promise')

Perhaps we can wait until#8094 is merged and then do the same

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yes, sure.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

But that is probably not within the scope of this pr. IMO we shouldn't change the rule logic in this pr

Yeah this is just a refactor (chore). If you want to change how it works that'd be a separate issue.

👍 I'll mark this as blocked on#8094. Thanks :)

arkapratimc reacted with thumbs up emoji
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldbergApr 23, 2024•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

#8094 is closed - so we can now pretend it doesn't exist*. cc @arka1002, I'm un-blocking this PR. Sorry for the long wait!

*(though if you end up taking in code from it, we'll need aCo-authored-by: Timothy Moore <mtimothy984@gmail.com> in the PR description)

);
}
export function isBuiltinTypeAliasLike(
program: ts.Program,
type: ts.Type,
Expand Down
[8]ページ先頭
©2009-2025 Movatter.jp