Skip to content

Many errors on JSDoc use entire tag as errorNode #41974

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

Open
sandersn opened this issue Dec 15, 2020 · 7 comments
Open

Many errors on JSDoc use entire tag as errorNode #41974

sandersn opened this issue Dec 15, 2020 · 7 comments
Assignees
Labels
Bug A bug in TypeScript
Milestone

Comments

@sandersn
Copy link
Member

This is made more obvious by #41877, which corrects the spans of tags to their full spans, making the error spans even more ungainly. Here's one example; see the errors.txt diffs in the linked PR for more.

Expected behavior:

    /** @type {Gioconda} */
        ~~~~~~~~~~~~~~~~ // before #41877
        ~~~~~~~~~~~~~~~~~ // after #41877 (both are wrong)
!!! error TS8030: The type of a function declaration must match the function's signature.
    function monaLisa(sb) {
        return typeof sb === 'string' ? 1 : 2;
    }

Actual behavior:

    /** @type {Gioconda} */
               ~~~~~~~~
!!! error TS8030: The type of a function declaration must match the function's signature.
    function monaLisa(sb) {
        return typeof sb === 'string' ? 1 : 2;
    }
@sandersn sandersn self-assigned this Dec 15, 2020
@sandersn sandersn added Bug A bug in TypeScript Help Wanted You can do this PursuitFellowship Help wanted from Pursuit fellowship; others please avoid until Dec 19 Good First Issue Well scoped, documented and has the green light labels Dec 15, 2020
@Kingwl
Copy link
Contributor

Kingwl commented Dec 17, 2020

BTW: What's 'PursuitFellowship' meaning....

@sandersn
Copy link
Member Author

@Kingwl the TS team is running a workshop with the Pursuit Fellowship today, and we're going to work through the contribution process using this bug (among others).

@a-tarasyuk
Copy link
Contributor

@sandersn Does this mean that issues labeled with "PursuitFellowship" should not be considered for external contributors?

@sandersn
Copy link
Member Author

Yes, please, at least until tomorrow. But in general, they're simple issues that frequent contributors like you and @Kingwl should leave to new people.

sandersn added a commit that referenced this issue Dec 17, 2020
Previously, the error span was too large on @type errors on functions
when the type was not a function. The span covered the entire tag. This
PR changes the error node just to be the type of the type tag. In other words,
the error span was previously this:

```
@type {IncorrectType}
```

But is now just this:

```
IncorrectType
```

Fixes the first error from #41974, but not the other two.

Co-authored-by: Ashya Manning <[email protected]>
Co-authored-by: Nilber Remon <[email protected]>
@sandersn sandersn added this to the Backlog milestone Dec 17, 2020
@chenjigeng
Copy link
Contributor

Has this been fixed?

sandersn added a commit that referenced this issue Dec 30, 2020
Previously, the error span was too large on @type errors on functions
when the type was not a function. The span covered the entire tag. This
PR changes the error node just to be the type of the type tag. In other words,
the error span was previously this:

```
@type {IncorrectType}
```

But is now just this:

```
IncorrectType
```

Fixes the first error from #41974, but not the other two.

Co-authored-by: Ashya Manning <[email protected]>
Co-authored-by: Nilber Remon <[email protected]>

Co-authored-by: Ashya Manning <[email protected]>
Co-authored-by: Nilber Remon <[email protected]>
@sandersn
Copy link
Member Author

@DanielRosenwasser points out that the error message in the example is not very good, and should maybe have a related span:

This error still is a little strange because it doesn't actually highlight the function declaration itself. You'd think it would switch to that with a "the signature is declared here.

@chenjigeng The first bad error span is fixed; the other two are not.

Zzzen pushed a commit to Zzzen/TypeScript that referenced this issue Jan 16, 2021
Previously, the error span was too large on @type errors on functions
when the type was not a function. The span covered the entire tag. This
PR changes the error node just to be the type of the type tag. In other words,
the error span was previously this:

```
@type {IncorrectType}
```

But is now just this:

```
IncorrectType
```

Fixes the first error from microsoft#41974, but not the other two.

Co-authored-by: Ashya Manning <[email protected]>
Co-authored-by: Nilber Remon <[email protected]>

Co-authored-by: Ashya Manning <[email protected]>
Co-authored-by: Nilber Remon <[email protected]>
@andrewbranch
Copy link
Member

The other baselines that still show errors are (at least):

  • tests/baselines/reference/jsDeclarationsParameterTagReusesInputNodeInEmit1.errors.txt
  • tests/baselines/reference/jsdocImportTypeNodeNamespace.errors.txt
  • tests/baselines/reference/jsdocTypeTagCast.errors.txt
  • tests/baselines/reference/unusedTypeParameters_templateTag.errors.txt
  • tests/baselines/reference/unusedTypeParameters_templateTag2.errors.txt
  • tests/baselines/reference/enumTagCircularReference.errors.txt

@andrewbranch andrewbranch removed PursuitFellowship Help wanted from Pursuit fellowship; others please avoid until Dec 19 Good First Issue Well scoped, documented and has the green light Help Wanted You can do this labels May 24, 2021
@RyanCavanaugh RyanCavanaugh assigned iisaduan and unassigned sandersn May 26, 2021
iisaduan added a commit that referenced this issue Jun 2, 2021
* some fixes for 41974

* linted

* fixed todo messages for readability
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants