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

Report error for duplicate @type declaration in @typedef #17442

Closed
ghost opened this issue Jul 26, 2017 · 11 comments · Fixed by #38340
Closed

Report error for duplicate @type declaration in @typedef #17442

ghost opened this issue Jul 26, 2017 · 11 comments · Fixed by #38340
Labels
Bug A bug in TypeScript Domain: JavaScript The issue relates to JavaScript specifically Domain: JSDoc Relates to JSDoc parsing and type generation Good First Issue Well scoped, documented and has the green light Help Wanted You can do this
Milestone

Comments

@ghost
Copy link

ghost commented Jul 26, 2017

TypeScript Version: master

Code

/**
 * @typedef Name
 * @type {string}
 * @type {Oops} - this is dropped and we stop parsing the @typedef
 */

Expected behavior:

Error.

Actual behavior:

No error.

@mhegazy mhegazy added the Bug A bug in TypeScript label Aug 23, 2017
@mhegazy mhegazy added Salsa Domain: JSDoc Relates to JSDoc parsing and type generation labels Aug 23, 2017
@mhegazy mhegazy added the Help Wanted You can do this label Apr 12, 2018
@mhegazy mhegazy added this to the Community milestone Apr 12, 2018
@mhegazy mhegazy added the Good First Issue Well scoped, documented and has the green light label Apr 12, 2018
@noamyogev84
Copy link

Hi @mhegazy can I try that one?

@sandersn
Copy link
Member

sandersn commented Oct 2, 2018

@noamyogev84 Go for it!

@noamyogev84
Copy link

Thanks @sandersn. can you please share some code pointers?

@sandersn
Copy link
Member

sandersn commented Oct 2, 2018

You'll want to start in parseTypedefTag in parser.ts. There is a section of code that checks for the child @type inside @typedef:

while (child = tryParse(() => parseChildPropertyTag(indent))) {
    // ...
    if (child.kind === SyntaxKind.JSDocTypeTag) {
        if (childTypeTag) {
            break;
        }
        else {
            childTypeTag = child;
        }
    }
    // ...
}

As you can see, in case of duplicate @type tags, the code just breaks out of the loop with no error. Add a call to parseErrorAtCurrentToken before breaking out of the loop. DiagnosticMessages.Unexpected_token is probably good enough for the message, but you can add a new one in diagnosticMessages.json if you want. Maybe "Unexpected tag '{0}'." or "Duplicate tag '{0}'."

For adding tests, follow the directions in CONTRIBUTING, except that you should add your test in ts/tests/cases/conformance/jsdoc, and you will need to explicitly allow JS at the top:

// @allowJS: true
// @checkJS: true
// @noEmit: true
// @Filename: bug17442.js
// ... actual test goes here ...

@DanielRosenwasser any opinions on what the error message should be?

@noamyogev84
Copy link

Great I'm on it.

@noamyogev84
Copy link

Hi @sandersn,
Following your directions does not produce the wanted result, or maybe i'm missing something. (No error message is outputted by the compiler)
I'm debugging tsc.js and it seems like parseErrorAtCurrentToken does not affect the rest of the method.

"Planting" an arbitrary syntax error in the above example does produce error, but I still can't figure out where it's being handled. Do you have any idea what's missing?

@noamyogev84
Copy link

noamyogev84 commented Oct 13, 2018

@sandersn, @DanielRosenwasser
Pinging you to get your thoughts on this... .

@weswigham weswigham added Domain: JavaScript The issue relates to JavaScript specifically and removed Salsa labels Nov 29, 2018
@nisha-kaushik
Copy link

Can I try this one?

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jan 18, 2019

A JSDoc '@typedef' comment may not contain multiple '@type' tags.

Bonus points on providing a related span on the first tag if it's not too hard:

The tag was first specified here.

@DanielRosenwasser
Copy link
Member

@nisha-kaushik go for it.

Not sure if @noamyogev84 is still looking at this but the issue they were running into might've just been running tests in a .ts file or an unchecked .js file.

@noamyogev84
Copy link

@nisha-kaushik go ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: JavaScript The issue relates to JavaScript specifically Domain: JSDoc Relates to JSDoc parsing and type generation Good First Issue Well scoped, documented and has the green light Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants