Unexpected parameter hint showing up for log in js file  #33565

Activity

1. Ryan Cavanaugh (RyanCavanaugh)

   added

   BugA bug in TypeScriptA bug in TypeScript

   Domain: LS: Quick Infoe.g. hover text, tool-tips, and tooltips.e.g. hover text, tool-tips, and tooltips.

   on Sep 25, 2019
2. Ryan Cavanaugh (RyanCavanaugh)

   added this to the Backlog milestone on Sep 25, 2019
3. RyanCavanaugh commented on Sep 25, 2019

   

   Ryan Cavanaugh (RyanCavanaugh)

   on Sep 25, 2019

   Member

   More actions

   wat
4. a-tarasyuk commented on Jan 28, 2020

   

   Oleksandr Tarasiuk (a-tarasyuk)

   on Jan 28, 2020

   Last edited by a-tarasyuk

   Contributor

   More actions

   I investigated the issue, and this error happens because

   TypeScript/src/services/signatureHelp.ts

   Line 125 in 1bb6ea0

   return name === undefined ? undefined : firstDefined(program.getSourceFiles(), sourceFile =>

   returns es5.d.ts source file and

   TypeScript/src/services/signatureHelp.ts

   Line 126 in 1bb6ea0

   firstDefined(sourceFile.getNamedDeclarations().get(name), declaration => {

   returns sourceFile.getNamedDeclarations() all declarations and ignore namespaces. For instance log it is a name from Math object :).

   Is there any reason why sourceFile.getNamedDeclarations() is used here?

   cc Daniel Rosenwasser (@DanielRosenwasser) Ryan Cavanaugh (@RyanCavanaugh)
5. DanielRosenwasser commented on Jan 28, 2020

   

   Daniel Rosenwasser (DanielRosenwasser)

   on Jan 28, 2020

   Last edited by DanielRosenwasser

   Member

   More actions

   I think the idea is to try a best-effort signature help given that we're in JavaScript and we won't always have accurate types around.

   If you're in ts-check, this behavior is probably extremely surprising because you're also getting an error that the identifier can't be found. Maybe the solution is to turn it off just there.
6. a-tarasyuk commented on Jan 29, 2020

   

   Oleksandr Tarasiuk (a-tarasyuk)

   on Jan 29, 2020

   Contributor

   More actions

   Daniel Rosenwasser (@DanielRosenwasser) Do you mean disable/remove createJSSignatureHelpItems?
7. DanielRosenwasser commented on Jan 29, 2020

   

   Daniel Rosenwasser (DanielRosenwasser)

   on Jan 29, 2020

   Last edited by DanielRosenwasser

   Member

   More actions

   Yeah, if

   • the current file belongs to a tsconfig.json
   • the current file is checked
   • the current file belongs to a project with checkJs

   I think we should probably disable the heuristic. Does that sound reasonable?
8. a-tarasyuk commented on Jan 31, 2020

   

   Oleksandr Tarasiuk (a-tarasyuk)

   on Jan 31, 2020

   Contributor

   More actions

   Daniel Rosenwasser (@DanielRosenwasser) It seems in this case, users who use JavaScript without TS checks will get weird suggestions. Or am I wrong?

   TypeScript/src/services/signatureHelp.ts

   Line 48 in ad8feb5

   const candidateInfo = getCandidateOrTypeInfo(argumentInfo, typeChecker, sourceFile, startingToken, onlyUseSyntacticOwners);

   Returns correct info for global functions (parseInt, eval, etc), or Objects (Math.log, Date etc). I'm trying to understand where this heuristic can help 😃 👀
9. mjbvz commented on Feb 1, 2020

   

   Matt Bierner (mjbvz)

   on Feb 1, 2020

   Author

   More actions

   Would it be ok to only return signature help when we can properly resolve a symbol? This is what we do for quickInfo for example.

   The majority of VS Code users are on untyped JS so I’m wary of a fix that is specific to checkJs

   👍React with 👍1Reacted by Oleksandr Tarasiuk
10. Mine Starks (minestarks)

    added

    Domain: LS: Signature HelpInformation in editor tooltips when invoking a function callInformation in editor tooltips when invoking a function call

    on Feb 7, 2020
11. a-tarasyuk commented on Apr 10, 2020

    

    Oleksandr Tarasiuk (a-tarasyuk)

    on Apr 10, 2020

    Last edited by a-tarasyuk

    Contributor

    More actions

    createJSSignatureHelpItems was added in this PR #2641, and changed in this #26025. Both PRs don't have tests - and it is difficult to understand which cases these changes cover, however, without createJSSignatureHelpItems tests work fine.
12. DanielRosenwasser commented on Apr 14, 2020

    

    Daniel Rosenwasser (DanielRosenwasser)

    on Apr 14, 2020

    Member

    More actions

    My personal take is that in a checked JS file, or in a TypeScript project, we should not apply the heuristic. Otherwise, I think the heuristic is fine.
13. DanielRosenwasser commented on Apr 14, 2020

    

    Daniel Rosenwasser (DanielRosenwasser)

    on Apr 14, 2020

    Member

    More actions

    Ah, I just looked up and saw that reread the convo.

    Uh, maybe this is something we should try to understand better - would it be easy to conduct a user study on this? Let's chat about this tomorrow Matt Bierner (@mjbvz)

    👀React with 👀1Reacted by Oleksandr Tarasiuk
14. mjbvz commented on Apr 14, 2020

    

    Matt Bierner (mjbvz)

    on Apr 14, 2020

    Author

    More actions

    Sure. I'm still of the opinion that the behavior of parameter hints should match hover. It seems confusing to show parameter hints on symbols that we can't resolve

    👍React with 👍1Reacted by Oleksandr Tarasiuk
15. Ryan Cavanaugh (RyanCavanaugh)

    assigned

    Andrew Branch (andrewbranch)

    on Apr 15, 2020
16. Ryan Cavanaugh (RyanCavanaugh)

    modified the milestones: Backlog, TypeScript 4.0 on Apr 15, 2020
17. RyanCavanaugh commented on Apr 15, 2020

    

    Ryan Cavanaugh (RyanCavanaugh)

    on Apr 15, 2020

    Member

    More actions

    Notes from meeting today:

    • Disable this behavior entirely for bare identifiers
    • Add some description of which declaration the description is coming from
18. a-tarasyuk commented on Apr 16, 2020

    

    Oleksandr Tarasiuk (a-tarasyuk)

    on Apr 16, 2020

    Contributor

    More actions

    Ryan Cavanaugh (@RyanCavanaugh) Thanks for the update.

    What does mean bare identifiers? 😕

    Add some description of which declaration the description is coming from

    Do you mean to add an additional field to the server response?
19. DanielRosenwasser commented on Apr 17, 2020

    

    Daniel Rosenwasser (DanielRosenwasser)

    on Apr 17, 2020

    Last edited by DanielRosenwasser

    Member

    More actions

    Oleksandr Tarasiuk (@a-tarasyuk) Ryan can comment on the second part.

    To answer your first question, the idea is foo should not try to search for members named foo anywhere, but foo.bar should still look for declarations named bar. That seems fairly reasonable to me.
20. Andrew Branch (andrewbranch)

    modified the milestones: TypeScript 4.0, Typescript 4.0.1 on Jun 23, 2020
21. Andrew Branch (andrewbranch)

    closed this as completedin #39352on Jun 30, 2020
22. microsoft

    locked as resolved and limited conversation to collaborators on Oct 21, 2025