microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
99.01k stars 12.29k forks source link

JSDoc @import not detecting use in @callback #58368

Open turadg opened 2 months ago

turadg commented 2 months ago

πŸ”Ž Search Terms

jsdoc import @import callback lsp

πŸ•— Version & Regression Information

5.5 beta

⏯ Playground Link

No response

πŸ’» Code

/**
 * @import {Checker, Passable} from '@endo/pass-style';
 */

/**
 * @callback GetChecker
 * @param {Passable} payload
 * @returns {Checker}
 */

/**
 * @typedef {object} Misc
 * @property {Passable} passable
 */

πŸ™ Actual behavior

The Checker import is dim in an IDE and hovering says,

'Checker' is declared but never used.ts(6196)

πŸ™‚ Expected behavior

LSP detects that Checker is in the return of GetChecker.

Additional information about the issue

The use is detected in @typedef and @type. E.g. as with Passable in the example code.

a-tarasyuk commented 2 months ago

I attempted to reproduce this issue, but it appears to be working correctly. Could you please provide additional details?

Screenshot 2024-05-01 at 01 18 40
turadg commented 2 months ago

Thanks for trying. Looking into it further, in my case it's reporting ts(6133) until I navigate to the definition. It resets again to thinking it's unused when I restart TS.

Here is a video demonstrating in VS Code.

Version: 1.88.1
Commit: e170252f762678dec6ca2cc69aba1570769a5d39
Date: 2024-04-10T17:43:08.196Z
Electron: 28.2.8
ElectronBuildId: 27744544
Chromium: 120.0.6099.291
Node.js: 18.18.2
V8: 12.0.267.19-electron.0
OS: Darwin arm64 23.4.0
turadg commented 1 month ago

More info: I think this only happens when the type is available by ambients. Then the imported type reports that it's not used.

This may not be a bug per current spec but it is a bit of a UX hazard because ambient resolution can differ by build order etc. So a type that's "not used" at coding time may be necessary at build time. The message that says it's not used suggests it can be removed, but that breaks the build.

RyanCavanaugh commented 2 weeks ago

Looks like a bug, but I can't repro this either. We can investigate if there's a way to reproduce it

michaeljaltamirano commented 1 week ago

@RyanCavanaugh I am having trouble putting together a reproduction, but the details of my case somewhat match up with this issue. However, I noticed that if I move the JSDoc import statement to happen before the last non-JSDoc import, it removes the ts(6133) error:

Before:

Screenshot 2024-06-20 at 4 36 46 PM

After:

Screenshot 2024-06-20 at 4 37 15 PM

I only saw this "unused import" error in some files. Not sure what the root cause might be, there was no pattern (relative imports vs. absolute imports, etc.) I could see in the cases I went through.

remcohaszing commented 1 week ago

Here’s a reproduction: playground

It’s based on this commit, where it happens in packages/language-service/lib/mdast-utils.js and packages/language-service/lib/tsconfig.js in seemingly arbritrary situations.