Hovering over JSDoc annotation resolves differently to Intellisense

[brendanarnold]

Issue Type: Bug

It's possible when hovering over the JSDoc annotation for it to resolve to a different object to what Intellisense will autocomplete to.

To re-create ...

// account.js

/**
 * @class
 */
class Account {
  constructor () {
    this.foo = 'foo'
  }
}

module.exports = {
  Account
}

// index.js

const { account } = require('./account.js')

/**
 * @param {Account} a
 */
const main = (a) => {
  a.foo = ''
}

Hovering over the JSDoc will resolve it to the lib.d.ts Account interface but when typing the a.foo in main it will autocomplete to the correct Account class.

VS Code version: Code 1.36.1 (2213894ea0415ee8c85c5eea0d0ff81ecc191529, 2019-07-08T22:56:38.504Z)
OS version: Darwin x64 18.6.0

System Info

Item	Value
CPUs	Intel(R) Core(TM) i7-6820HQ CPU @ 2.70GHz (8 x 2700)
GPU Status	2d_canvas: enabled flash_3d: enabled flash_stage3d: enabled flash_stage3d_baseline: enabled gpu_compositing: enabled multiple_raster_threads: enabled_on native_gpu_memory_buffers: enabled oop_rasterization: disabled_off protected_video_decode: unavailable_off rasterization: enabled skia_deferred_display_list: disabled_off skia_renderer: disabled_off surface_synchronization: enabled_on video_decode: enabled viz_display_compositor: disabled_off webgl: enabled webgl2: enabled
Load (avg)	2, 2, 2
Memory (System)	16.00GB (1.87GB free)
Process Argv
Screen Reader	no
VM	0%

Extensions (10)

Extension	Author (truncated)	Version
scratchpad	awe	0.1.0
insert-unicode	bru	0.6.0
vscode-standardjs	che	1.2.3
gitlens	eam	9.8.5
nunjucks-template	ese	0.1.2
vscode-docker	ms-	0.7.0
addDocComments	ste	0.0.8
code-spell-checker	str	1.7.17
vscodeintellicode	Vis	1.1.8
vscode-todo-highlight	way	1.0.4

[minestarks]

@brendanarnold I'm trying to repro this now and having trouble with this part:

but when typing the a.foo in main it will autocomplete to the correct Account class.

I'm just not seeing that autocomplete?

In my observation, the Account type from account.js isn't visible from index.js so it makes sense that it resolves to the one from lib.dom.d.ts. This all seems by design to me.

[brendanarnold]

@minestarks

Yeah I just revisited the test case setup and cannot reproduce myself now either with 1.42.0.

What I find instead is that if you name the Account class something else that Intellisense resolves the class in main.js but when you name the class back to Account the built-in reference takes precedence.

Happy to close the issue.

[minestarks]

Well, it may still be a bug. Let me write up a clearer repro of what I think is the problem.

// account.js

// @ts-check

/**
 * @class
 */
class Account {
    constructor() {
        this.foo = 'foo'
    }
}

/**
 * @class
 */
class OtherName {
    constructor() {
        this.foo = 'foo'
    }
}

module.exports = {
    Account,
    OtherName
}

// bar.js

// @ts-check

const { Account, OtherName } = require('./account.js')

/**
 * @type {Account}
 */
let a;
a.foo // error: Property 'foo' does not exist on type 'Account'

/**
 * @type {OtherName}
 */
let b;
b.foo // OK

Expected: Account resolves to the imported type, Actual: the lib.dom.d.ts type is preferred.

This could potentially be by design, but I suspect not since the TypeScript "equivalent" favors the imported Account type:

// account.ts

/**
 * @class
 */
class Account {
    foo: number
}

/**
 * @class
 */
class OtherName {
    foo: number
}

export { Account, OtherName }

// b.ts

import { Account, OtherName } from './account'

let a: Account
a.foo // OK

let b: OtherName
b.foo // OK

[minestarks]

@sandersn any comment on the JSDoc behavior in #32845 (comment) ? Expected?

[sandersn]

It is not expected. It's a bug in the commonjs require AND the commonjs module.exports handling; your correct Typescript example works exactly the same way in JS if you just change the extensions (and use foo = 1 instead of foo: number).

Basically, it looks like require fails to import the type meaning of Account, so type reference resolution finds interface Account from lib.dom.d.ts first. OtherName, on the hand, also doesn't import the type meaning, but falls back to JS-style value resolution after the compiler can't find a type named OtherName.

There might be an existing issue tracking this. #25533 is basically right as far as I remember.