Skip to content

Repo: reuse newly added "is builtin symbol like" logic from type-utils #8234

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
Closed
auvred opened this issue Jan 9, 2024 · 3 comments · Fixed by #9462
Closed

Repo: reuse newly added "is builtin symbol like" logic from type-utils #8234

auvred opened this issue Jan 9, 2024 · 3 comments · Fixed by #9462
Labels
accepting prs Go ahead, send a pull request that resolves this issue good first issue Good for newcomers locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. refactor PRs that refactor code only repo maintenance things to do with maintenance of the repo, and not with code/docs

Comments

@auvred
Copy link
Member

auvred commented Jan 9, 2024

Suggestion

Followup for #8011 (comment)

We've added few utility functions to the packages/type-utils/src/builtinSymbolLikes.ts

Basically they contain the logic for determining whether a given type is a type from the TS default library or not

These files already contain similar logic, it'd be cool if we dedupe it!

typescript-eslint/packages/eslint-plugin/src/rules/no-implied-eval.ts

Lines 82 to 90 in 705370a

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;
}
}
}

typescript-eslint/packages/eslint-plugin/src/rules/no-implied-eval.ts

Lines 145 to 153 in 705370a

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;
}
}

Let's label this issue as good first issue? Ref: #8011 (comment)
@auvred auvred added repo maintenance things to do with maintenance of the repo, and not with code/docs triage Waiting for team members to take a look labels Jan 9, 2024
@JoshuaKGoldberg JoshuaKGoldberg added accepting prs Go ahead, send a pull request that resolves this issue good first issue Good for newcomers refactor PRs that refactor code only and removed triage Waiting for team members to take a look labels Jan 20, 2024
@nishit-prasad
Copy link

@auvred @JoshuaKGoldberg

New contributor here:

  • I see a utility function defined for this called isSymbolFromDefaultLibrary().

typescript-eslint/packages/type-utils/src/isSymbolFromDefaultLibrary.ts

Lines 3 to 20 in 920f909

export function isSymbolFromDefaultLibrary(
program: ts.Program,
symbol: ts.Symbol | undefined,
): boolean {
if (!symbol) {
return false;
}
const declarations = symbol.getDeclarations() ?? [];
for (const declaration of declarations) {
const sourceFile = declaration.getSourceFile();
if (program.isSourceFileDefaultLibrary(sourceFile)) {
return true;
}
}
return false;
}

Shall I use this instead or move this function to packages/type-utils/src/builtinSymbolLikes.ts and then use it?

arkapratimc added a commit to arkapratimc/typescript-eslint that referenced this issue Jan 22, 2024
@arkapratimc
chore(type-utils): reuse newly added "is builtin symbol like" logic
d004c95 
fixes: typescript-eslint#8234
@arkapratimc arkapratimc mentioned this issue Jan 22, 2024
Closed
3 tasks
@JoshuaKGoldberg
Copy link
Member

Oop sorry, just seeing the question now - that does look like a good start! (#8287 continues the discussion)

@YutongZhuu
Copy link
Contributor

Hey, I see arka1002 opened a PR but failed to merge it in. I wonder failed to understand why, but I would like to give it a try. However, I am new to open source projects, so I will certainly need some time and help.

@YutongZhuu YutongZhuu mentioned this issue Jul 1, 2024
Merged
3 tasks
@github-actions github-actions bot added the locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. label Jul 13, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
accepting prs Go ahead, send a pull request that resolves this issue good first issue Good for newcomers locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. refactor PRs that refactor code only repo maintenance things to do with maintenance of the repo, and not with code/docs
Projects
None yet
Milestone
No milestone
4 participants
@JoshuaKGoldberg @auvred @nishit-prasad @YutongZhuu