Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intellisense can't be overridden by JSDoc in normal Javascript #18229

Closed
jamietre opened this issue Jan 6, 2017 · 9 comments
Closed

Intellisense can't be overridden by JSDoc in normal Javascript #18229

jamietre opened this issue Jan 6, 2017 · 9 comments
Assignees
Labels
javascript JavaScript support issues

Comments

@jamietre
Copy link

jamietre commented Jan 6, 2017

  • VSCode Version: 1.8.1 vommit ee428b
  • OS Version: Windows 10

Steps to Reproduce:

  1. Create a function that returns something that TS thinks it can figure out a signature for
  2. Add jsdoc with @returns
  3. Use the function elsewhere

TSServer does a great job with signatures when using plain javascript - sometimes. But often I want to just use JSDoc to explicitly provide a signature, because it's indetermine or TS can't figure it out correctly.

e.g.

image

Using this function:

image

I would expect to see the signature I defined in @returns. ("bind" actually seems to have another issue too -- I would think it knows to return the same sig of what you invoked "bind" on, but that's a different story, in this case I just want it to use my explicitly described signature).

This seems to happen if Typescript thinks it knows the signature of what I'm returning. It doesn't happen if I return something it truly doesn't know about - e.g. a property of an object that's not defined yet; in that case it always uses my JSDoc.

I would think that JSDoc should always supercede what TS Server thinks the signature is, if present.

@waderyan
Copy link

waderyan commented Jan 9, 2017

@jamietre thank you for opening this issue. Can you help us?

  1. Share the code so we can copy and paste it easily.
  2. Disable all extensions to ensure the error is not caused by an extension.

Please see the contribution guidelines for more info on opening issues.

@jamietre
Copy link
Author

jamietre commented Jan 9, 2017

Launched vscode with --disable-extensions and no change. But in trying to create a simple case, I'm realizing the issue is not exactly what I thought it was.

The problem actually seems to be that if vscode/tsserver doesn't recognize the syntax of the signature in my @returns, that it falls back on the inferred signature, instead of reporting an error somehow.

I would expect that if I provide a signature (erroneous or not) it would just use it verbatim (accepting it as an unknown syntax) or fail. If I make an error, falling back on the inferred signature means it's not especially likely I would ever realize there was an error in my jsdoc. It seems unlikely that this would be desirable in any situation - I just wouldn't have documented it if my intent was to use the inferred signature.

For example:

function foo() {
    return 'foo';
}

/**
 * A test
 *
 * @returns {()=>any} An instance of the ref manager
 **/
function bar() {
    return foo()
}

let baz = bar()

Hovering over .. = bar() shows function bar(): string which it inferred from the function defintion for foo. If I change the @returns to something else that is apparently understood, e.g. number, then it correctly returns number.

The other qeuestion is how come I can't use valid TS syntax in jsdoc, and maybe this is just a collision of worlds situation :) but I'm more concerned about how invalid syntax is handled.

@mjbvz mjbvz added the javascript JavaScript support issues label Jan 9, 2017
@mjbvz
Copy link
Collaborator

mjbvz commented Jan 9, 2017

@jamietre Can you try using function():anyin the @returns instead? This code seems to show the correct behavior:

function foo() {
    return 'foo';
}

/**
 * A test
 *
 * @returns {function():any} An instance of the ref manager
 **/
function bar() {
    return foo()
}

let baz = bar()

screen shot 2017-01-09 at 11 57 02 am

I'll look into whether you can use arrow function in jsdocs.

@mjbvz mjbvz closed this as completed Jan 9, 2017
@jamietre
Copy link
Author

jamietre commented Jan 9, 2017

Re: closed -- does this mean the behavior of falling back on the inferred signature when the jsdoc is not understood isn't a bug? To me that is the real problem.

@mjbvz mjbvz reopened this Jan 9, 2017
@mjbvz
Copy link
Collaborator

mjbvz commented Jan 9, 2017

Wrong button. Reopening. Sorry for closing early.

We were tracking support for generating JSdoc errors in JS files here microsoft/TypeScript#6658 which has since been rolled into microsoft/TypeScript#6802

Also, arrow function signatures do not appear to be supported at the top level. I think you have to use the function syntax instead

@jamietre
Copy link
Author

jamietre commented Jan 9, 2017

Good to know on the TS syntax. I can live with that, but for the other thing, the root issue is that JSDoc errors are not generated in JS files? Is this some kind of integration problem resulting from that - e.g. as far as the intellisense service is concerned, it can't distinguish between JSdoc that has errors and JSDoc that doesn't exist?

@mjbvz
Copy link
Collaborator

mjbvz commented Jan 9, 2017

@jamietre We are intentionally very conservative with producing JS errors and warnings. It is difficult to understand all the usage patterns that may be encountered in JavaScript (for example, is missing type in a JSDoc actually an error or is it coming from some global that we don't know about?) And since the JS is not being compiled, we've chosen to play it on the safe side and not generate false positive errors and warnings that may annoy users.

We know this is an area that could be improved and are tracking some of these improvements with microsoft/TypeScript#6802

@mjbvz mjbvz closed this as completed Jan 9, 2017
@jamietre
Copy link
Author

jamietre commented Jan 9, 2017

To be clear - the issue isn't being conservative about generating errors. Actually this is exactly how I would expect the system to work: just treat my jsdoc as valid. But it's not treating it as valid - it's throwing it out, but not telling anyone there's an error.

Either of these seem acceptable:

  1. Ignore errors in JSDoc, and just treat anything unknown as valid
  2. Report errors

Right now what's happening is that errors are ignored by the parser, and then the erroneus signature is also ignored, causing the wrong signature (fall through from the underlying function) to be reported in intellisense.

If for some reason it's impossible to treat something invalid as valid -- because structured output must be provided by the parser (I don't know much about the internals) -- then there should be some way that at least it's not treated by intellisense as if there were no jsdoc at all. It just creates an "error swallowing" condition, so i end up using intellisense that's wrong, because of an error I didn't know about. Far better for intellisense to report "unknown" or some indicator that it couldn't parse my jsdoc (or simply the entire string it couldn't parse)

@mjbvz
Copy link
Collaborator

mjbvz commented Jan 9, 2017

I've opened microsoft/TypeScript#13371 to explicitly track reporting syntax errors in JSDoc type annotations. This seems like a fairly conservative error that TypeScript should alert us about

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
javascript JavaScript support issues
Projects
None yet
Development

No branches or pull requests

3 participants