@typedef: Improve error spans from declaration emit #42501
Conversation
This is a proof-of-concept fix. I think it could be expanded for all of jsdoc, but I only set it up for jsdoc type aliases. It could use a lot of polish too.
typescript-bot
added
Author: Team
For Uncommitted Bug
Remove trueErrorNode
| @@ -4699,7 +4700,7 @@ namespace ts { | |||
|
|
|||
| function createMappedTypeNodeFromType(type: MappedType) { | |||
| Debug.assert(!!(type.flags & TypeFlags.Object)); | |||
| const readonlyToken = type.declaration.readonlyToken ? <ReadonlyToken | PlusToken | MinusToken>factory.createToken(type.declaration.readonlyToken.kind) : undefined; | |||
| const readonlyToken = type.declaration.readonlyToken ? <ReadonlyKeyword | PlusToken | MinusToken>factory.createToken(type.declaration.readonlyToken.kind) : undefined; | |||
There was a problem hiding this comment.
drive-by deprecation fix
| @@ -6183,7 +6184,7 @@ namespace ts { | |||
| tracker: { | |||
| ...oldcontext.tracker, | |||
| trackSymbol: (sym, decl, meaning) => { | |||
| const accessibleResult = isSymbolAccessible(sym, decl, meaning, /*computeALiases*/ false); | |||
| const accessibleResult = isSymbolAccessible(sym, decl, meaning, /*computeAliases*/ false); | |||
| /** @typedef {import('./base')} BaseFactory */ | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| !!! error TS4081: Exported type alias 'BaseFactory' has or is using private name 'Base'. |
There was a problem hiding this comment.
Do you think we should add a Exported type alias 'BaseFactory' has or is using private name 'Base' from module "tests/cases/conformance/jsdoc/declarations/base". formatted error?
There was a problem hiding this comment.
In the general case, yes -- in this case, with the import('./base') sitting right there, it's not so valuable.
| !!! error TS4081: Exported type alias 'BaseFactoryFactory' has or is using private name 'Base'. | ||
| /** @enum {import('./base')} BaseFactoryFactoryRegistry */ | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| !!! error TS4081: Exported type alias 'Base' has or is using private name '"tests/cases/conformance/jsdoc/declarations/base"'. |
There was a problem hiding this comment.
This one seems less like the other two.
There was a problem hiding this comment.
uhhhhhhhh yeah, I didn't notice that. It should be ${0}=BaseFactoryFactoryRegistry, ${1}='Base'.
| /** @typedef {import('./base')} BaseFactory */ | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| !!! error TS4081: Exported type alias 'BaseFactory' has or is using private name 'Base'. |
There was a problem hiding this comment.
In the general case, yes -- in this case, with the import('./base') sitting right there, it's not so valuable.
| !!! error TS4081: Exported type alias 'BaseFactoryFactory' has or is using private name 'Base'. | ||
| /** @enum {import('./base')} BaseFactoryFactoryRegistry */ | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| !!! error TS4081: Exported type alias 'Base' has or is using private name '"tests/cases/conformance/jsdoc/declarations/base"'. |
There was a problem hiding this comment.
uhhhhhhhh yeah, I didn't notice that. It should be ${0}=BaseFactoryFactoryRegistry, ${1}='Base'.
Since enums don't have a name property, you *have* to call `getNameOfDeclaration` to go looking through the AST for one.
|
Done. Improving the error forced me to fix the |
This is a proof-of-concept fix. I think it could be expanded for all of jsdoc, but I only set it up for jsdoc type aliases.
It could use a lot of polish too.
(The silly prefix on
trueErrorNodeshould remind me of that.)Fixes rest of #41672 for jsdoc type aliases:
@typedef,@callback,@enum. But not for commonplace tags like@param,@return,@type, etc.