8000 [no-unnecessary-type-arguments] Flags a violation for identically-named but different types · Issue #4489 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]
Skip to content

[no-unnecessary-type-arguments] Flags a violation for identically-named bu 8000 t different types #4489

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
3 tasks done
NickHeiner opened this issue Jan 27, 2022 · 5 comments · Fixed by #4543
Closed
3 tasks done

[no-unnecessary-type-arguments] Flags a violation for identically-named but different types #4489

NickHeiner opened this issue Jan 27, 2022 · 5 comments · Fixed by #4543
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@NickHeiner
Copy link
Contributor
  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have read the FAQ and my problem is not listed.

Repro

Playground

type DefaultE = {foo: string};
type T<E = DefaultE> = {box: E};

type G = T<DefaultE>;

declare module 'bar' {
    type DefaultE = {somethingElse: true};
    // This is flagged as an error. The name `DefaultE` is the same, but the type is different.
    type G = T<DefaultE>;
}

Expected Result
The rule only flags violations when the type argument is identical to the default.

Actual Result
The rule flags violations when the type argument is different from the default, but has the same name.
@NickHeiner NickHeiner added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jan 27, 2022
@bradzacher
Copy link
Member

The rule currently does a name comparison

typescript-eslint/packages/eslint-plugin/src/rules/no-unnecessary-type-arguments.ts

Line 54 in d4768c0

param.default.getText() !== sourceCode.getText(arg)

It should probably just compare the types by reference instead?
Unsure why it did a text comparison

@bradzacher bradzacher added accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working and removed triage Waiting for team members to take a look labels Jan 27, 2022
@NickHeiner
Copy link
Contributor Author

Yeah, by reference seems better. 😄

@islandryu islandryu mentioned this issue Feb 12, 2022
Merged
3 tasks
@JoshuaKGoldberg
Copy link
Member

Yup, this is yet another reason why microsoft/TypeScript#9879 would be very beneficial for us and other TypeScript static analysis libraries.

@NickHeiner
Copy link
Contributor Author

Thanks!

Are there other TS static analysis libraries you have in mind? I might be interested in using them as well.

@JoshuaKGoldberg
Copy link
Member

https://github.com/JoshuaKGoldberg/TypeStat is the big one for me, personally. 😄

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2022
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 bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Milestone
No milestone
3 participants
@NickHeiner @JoshuaKGoldberg @bradzacher
2AB2
0