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

Bug: [unified-signatures] Recommends merging overloads that would then allow incorrect call signatures #10962

Open
Labels
awaiting responseIssues waiting for a reply from the OP or another partybugSomething isn't workingpackage: eslint-pluginIssues related to @typescript-eslint/eslint-plugin
@requinix

Description

@requinix

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I havesearched for related issues and found none that matched my issue.
  • I haveread the FAQ and my problem is not listed.

Playground Link

https://typescript-eslint.io/play/#ts=5.8.2&fileType=.tsx&code=PTAEAZQQwJwcwK4FsCmA7ALgZwLACgAzBNAYwwEsB7NUAyygCgEoAuUAN0vIBMBufEKACMAWgBM0eMnTYANKAwALFLXIwsGUOSygYKAI4I1KbviKkK1WvQZQ2GmOTRx5AIwD89jI%2BesOXPnwzYjIqGjpGAH0oT1AHJxdQSI8vHzg-Th5QAG9QAF8gvEElFQI1DVAGcgJoNABPJlAkBArXFW4UMrQTMxtiDq6TJn5CPrQBpxN5fs7J7mHCwRFlxbBwcUlEVEwseSh60BJKJFcnKEsaShr451ruUBnB00IQi9BXWFtYm8SUuO8EhkAiNghYwu9PtFvgDnPJktC0kCsrkCngBGA6igoIo4gg9AplKp1JoSPt3ipHnN5EpzgByHSUADWUDq%2BA%2BMAYlO68xG7M541m3OmAqeCzwQA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6WJygM0sQBNaySgHMmAQ3yxoKdFADuY6E0jgwAXxBqgA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false

Repro Code

functionfoo():void;functionfoo(a:string,b?:string):void;//                      ^^^^^^^^^^

ESLint Config

module.exports={parser:"@typescript-eslint/parser",rules:{"@typescript-eslint/unified-signatures":"warn",},};

Expected Result

No lint message.

Actual Result

Lint says the two overloads should be combined.

Additional Info

tldr 1: yes, I'm talking aboutundefined, but (a) there's more to it than simply "NAB because optional== undefined", (b) the edits the lint rule wants me to make will cause code to allow something that wasn't allowed previously, and (c) the rule is reporting on a parameter that it doesn't make sense to report on.

tldr 2: some commentary in the Playground link


These two overloads

functionfoo():void;functionfoo(a:string,b?:string):void;

are supposed to be combined into

functionfoo(a?:string,b?:string):void;

The first problem with doing so is thatfoo(undefined) becomes a valid call signature when it wasn't before. Now yes, I know that a developer should understand that calling foo like that is almost* the same as justfoo(), so let's ignore that one. (It's also been brought up in a past report or two about this lint rule.)

The second problem is thatfoo(undefined, "bar") becomes valid as well. In that case the whole "basically the same" argument backfires because a developer would probably expect this call to be meaningful - the"bar" is meaningful - when the implementation forfoo is likely* something simple likeif (a === undefined) { ... } and so completely ignoring the value.

(A workaround I've used in other cases has been to enableignoreDifferentlyNamedParameters, which was really nice because it helped to point out that the meaning of the parameters was actually slightly different between the overloads, but obviously that doesn't work here.)

The main reason I would call this a bug and not simply "lol Javascript" is that Typescript will quite (un)happily complain about callingfoo(undefined, string) when using the two overloads but will not complain about it when using the combined one. And I'd think one of the main principles of linting is that following its suggestions/fixes should not make invalid code become valid, right?

There's also a second reason this seems to be a bug: it's triggering on the optionalb? parameter and I can't imagine why it should. There'sa lot of code to sift through, but I'd speculate that it's detecting "overload with N required args / overload with N+1 required args" and triggering on the last parameter in the signature, when I think it should be accounting for the presence of optional parameters before deciding they can be combined.

I think my point's been made, but here's an interesting thing. Consider this:

functionfoo():void;functionfoo(a:string):void;functionfoo(a:string,b:string):void;

unified-signatures will say 1+2 can combine and 2+3 can combine, so naturally you'd think 1+2+3 could combine too. And currently, if you did merge a pair then the lint rule would agree and say to merge once more. But if that's incorrect and lint rule is fixed, if you merge one pair then youwon't get a lint message to merge again. Which could be surprising.

Except itwas incorrect. You can safely combine 1+2 or 2+3 but youcan't combine 1+2+3. For the reasons I gave earlier, which I'll repeat here as:

// 1+2, 3functionfoo(a?:string):void;functionfoo(a:string,b:string):void;// foo(undefined, string) not okay// 1, 2+3functionfoo():void;functionfoo(a:string,b?:string):void;// foo(undefined, string) not okay// 1+2+3functionfoo(a?:string,b?:string):void;// foo(undefined, string) suddenly okay

* Additional backstory that may be more relevant than I think: for better or worse, I tend to code the implementation of such overloaded functions like

functionfoo(...args:|[]|[a:string,b?:string]):void{// branch on args.length

Merging the overloads then breaks their compatibility with the implementation, not just withargs but with the code - now I have to support aargs.length == 1 && args[0] == undefined case that didn't exist before. Yes, I know, "but what if someone calls the function incorrectly", but to that I say "sorry, not sorry".

Metadata

Metadata

Assignees

No one assigned

    Labels

    awaiting responseIssues waiting for a reply from the OP or another partybugSomething isn't workingpackage: eslint-pluginIssues related to @typescript-eslint/eslint-plugin

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions


      [8]ページ先頭

      ©2009-2025 Movatter.jp