Sitelet https://web.archive.org/web/20201112010740/https://github.com/microsoft/TypeScript/pull/33473
Skip to content

/ TypeScript

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

Jump to bottom

Introduce flattened error reporting for properties, call signatures, and construct signatures #33473

Merged
merged 6 commits into from Sep 23, 2019
Merged

Introduce flattened error reporting for properties, call signatures, and construct signatures #33473

merged 6 commits into from Sep 23, 2019

Conversation

@weswigham
Copy link
Member

@weswigham weswigham commented Sep 17, 2019 •
edited

Fixes #33361

Before:
image

After:
image
@weswigham
Introduce flattened error reporting for properties, call signatures, …
Verified
This commit was signed with a verified signature.
weswigham Wesley Wigham
GPG key ID: D59F87F60C5400C9 Learn about signing commits
Loading status checks…
06908f8 
…and construct signatures
@weswigham
Copy link
Member Author

@weswigham weswigham commented Sep 17, 2019

@typescript-bot test this
@typescript-bot pack this
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Sep 17, 2019 •
edited

Heya @weswigham, I've started to run the extended test suite on this PR at 06908f8. You can monitor the build here. It should now contribute to this PR's status checks.
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Sep 17, 2019 •
edited

Heya @weswigham, I've started to run the tarball bundle task on this PR at 06908f8. You can monitor the build here. It should now contribute to this PR's status checks.
@weswigham
Copy link
Member Author

@weswigham weswigham commented Sep 17, 2019

cc @DanielRosenwasser @RyanCavanaugh because the assign reviewers thing is stuck
@DanielRosenwasser
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented Sep 17, 2019

Love it!

I'm okay with rephrasing to

The types of '{0}' are incompatible between these types.

Additionally, if there is a way to track that there are no parameters, I would cut out the ... in function calls.
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Sep 17, 2019

Hey @weswigham, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/44341/artifacts?artifactName=tgz&fileId=931EC1DB45107D732E234F7803AF1DFA079F596466EA8B7A51CCFC7E7182C8A702&fileName=/typescript-3.7.0-insiders.20190917.tgz"
    }
}

and then running npm install.
@weswigham
Update message, specialize output for argument-less signatures
Verified
This commit was signed with a verified signature.
weswigham Wesley Wigham
GPG key ID: D59F87F60C5400C9 Learn about signing commits
Loading status checks…
d4fcff2
@weswigham
Copy link
Member Author

@weswigham weswigham commented Sep 17, 2019

@typescript-bot test this
@typescript-bot pack this
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Sep 17, 2019 •
edited

Heya @weswigham, I've started to run the extended test suite on this PR at d4fcff2. You can monitor the build here. It should now contribute to this PR's status checks.
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Sep 17, 2019 •
edited

Heya @weswigham, I've started to run the tarball bundle task on this PR at d4fcff2. You can monitor the build here. It should now contribute to this PR's status checks.
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Sep 17, 2019

Hey @weswigham, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/44358/artifacts?artifactName=tgz&fileId=16AD9D8D3F4E7874755206CF2ACFDB9A7CE0178F8CE287BB364592B12001C41B02&fileName=/typescript-3.7.0-insiders.20190917.tgz"
    }
}

and then running npm install.
@weswigham
Copy link
Member Author

@weswigham weswigham commented Sep 17, 2019

@DanielRosenwasser feel free to look over the RWC changes, they seem pretty OK to me~
weswigham added 2 commits Sep 18, 2019
@weswigham
Skip leading signature incompatability flattening
Verified
This commit was signed with a verified signature.
weswigham Wesley Wigham
GPG key ID: D59F87F60C5400C9 Learn about signing commits
Loading status checks…
4b95ba7
@weswigham
Merge branch 'master' into flattened-path-based-diagnostics
Verified
This commit was signed with a verified signature.
weswigham Wesley Wigham
GPG key ID: D59F87F60C5400C9 Learn about signing commits
Loading status checks…
f5dc527
@weswigham
Copy link
Member Author

@weswigham weswigham commented Sep 18, 2019

@typescript-bot test this
@typescript-bot pack this

@DanielRosenwasser your in-person request of skipping leading signatures is now in~
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Sep 18, 2019 •
edited

Heya @weswigham, I've started to run the tarball bundle task on this PR at f5dc527. You can monitor the build here. It should now contribute to this PR's status checks.
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Sep 18, 2019 •
edited

Heya @weswigham, I've started to run the extended test suite on this PR at f5dc527. You can monitor the build here. It should now contribute to this PR's status checks.
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Sep 18, 2019

Hey @weswigham, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/44555/artifacts?artifactName=tgz&fileId=2DC7273910A525A7F23534E26497A6AAD73AF1DB601DC45ACBB828762A83C5E102&fileName=/typescript-3.7.0-insiders.20190918.tgz"
    }
}

and then running npm install.
@weswigham
Copy link
Member Author

@weswigham weswigham commented Sep 19, 2019

@DanielRosenwasser @sandersn whenever you get a chance, give this a review, so we can start testing it in the nightly to see what people think about it. It's fairly simple to introduce a toggle back to the old style of elaboration, if need be.
@orta
Copy link
Member

@orta orta commented Sep 20, 2019

This PR has a playground so you can try it out on the site.
@orta
orta reviewed Sep 20, 2019
View changes
tests/baselines/reference/extendAndImplementTheSameBaseType2.errors.txt Show resolved Hide resolved
tests/baselines/reference/interfaceWithMultipleBaseTypes.errors.txt
Types of property 'b' are incompatible.
Type 'number' is not assignable to type 'string'.
The types of 'x.b' are incompatible between these types.
Type 'number' is not assignable to type 'string'.

This comment has been minimized.

Sign in to view
@orta

orta Sep 20, 2019
Member

Does The types of 'x.b' are incompatible between these types. feels grammatically odd to you?
Maybe The types of 'x.b' are incompatible.

Though I guess it could be because it's going to mention it on the next line: in which case:

The types of 'x.b' are incompatible between these types: maybe?

This comment has been minimized.

Sign in to view
@weswigham

weswigham Sep 23, 2019
Author Member

The period vs colon argument could be had for every line of our pyramid of elaborations - we've settled on period right now 🤷‍♂
tests/baselines/reference/invariantGenericErrorElaboration.errors.txt
!!! related TS2728 tests/cases/compiler/invariantGenericErrorElaboration.ts:12:3: 'tag' is declared here.
!!! error TS2322: The types of 'constraint.constraint.constraint' are incompatible between these types.
!!! error TS2322: Type 'Constraint<Constraint<Constraint<Num>>>' is not assignable to type 'Constraint<Constraint<Constraint<Runtype<any>>>>'.
!!! error TS2322: Type 'Constraint<Runtype<any>>' is not assignable to type 'Constraint<Num>'.
const Foo = Obj({ foo: Num })

This comment has been minimized.

Sign in to view
@orta

orta Sep 20, 2019
Member

so good
weswigham added 2 commits Sep 23, 2019
@weswigham
Add return type specialized message
Verified
This commit was signed with a verified signature.
weswigham Wesley Wigham
GPG key ID: D59F87F60C5400C9 Learn about signing commits
Loading status checks…
8d3038a
@weswigham
Merge branch 'master' into flattened-path-based-diagnostics
Verified
This commit was signed with a verified signature.
weswigham Wesley Wigham
GPG key ID: D59F87F60C5400C9 Learn about signing commits
Loading status checks…
4fdabf1
@weswigham
Copy link
Member Author

@weswigham weswigham commented Sep 23, 2019

@DanielRosenwasser I've added the specialization for construct/call-ending chains - would you care to add more feedback/👍?
@weswigham weswigham requested a review from DanielRosenwasser Sep 23, 2019
@DanielRosenwasser
DanielRosenwasser reviewed Sep 23, 2019
View changes
tests/baselines/reference/nestedRecursiveArraysOrObjectsError01.errors.txt
!!! error TS2322: Object literal may only specify known properties, and 'jj' does not exist in type 'Style'.
!!! error TS2322: The types returned by 'pop()' are incompatible between these types.
!!! error TS2322: Type '{ foo: string; jj: number; }[][]' is not assignable to type 'Style'.
!!! error TS2322: Type '{ foo: string; jj: number; }[][]' is not assignable to type 'StyleArray'.

This comment has been minimized.

Sign in to view
@DanielRosenwasser

DanielRosenwasser Sep 23, 2019
Member

wtf
@DanielRosenwasser
DanielRosenwasser approved these changes Sep 23, 2019
View changes
@weswigham weswigham merged commit 26caa37 into microsoft:master Sep 23, 2019
5 checks passed
5 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
node10 Build #45263 succeeded
Details
node12 Build #45261 succeeded
Details
node8 Build #45262 succeeded
Details
@weswigham weswigham deleted the weswigham:flattened-path-based-diagnostics branch Sep 23, 2019
@DanielRosenwasser
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented Sep 23, 2019

@typescript-bot pack this
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Sep 23, 2019 •
edited

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 4fdabf1. You can monitor the build here. It should now contribute to this PR's status checks.
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Sep 23, 2019

Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/45298/artifacts?artifactName=tgz&fileId=ACC28539FC46D6D95320549F7058F22881709BAB6ABC498A3F905B2F51FE20DF02&fileName=/typescript-3.7.0-insiders.20190923.tgz"
    }
}

and then running npm install.
@typescript-bot
Copy link
Collaborator

@typescript-bot typescript-bot commented Sep 23, 2019

Hey @DanielRosenwasser, something went wrong when looking for the build artifact. (You can check the log here).
@orta orta mentioned this pull request Sep 25, 2019
Closed
@DanielRosenwasser DanielRosenwasser mentioned this pull request Sep 25, 2019
Open
@amcasey amcasey mentioned this pull request Feb 1, 2020
Open
@sheetalkamat sheetalkamat mentioned this pull request May 17, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orta orta

@DanielRosenwasser DanielRosenwasser

Assignees

@sandersn sandersn

@DanielRosenwasser DanielRosenwasser

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Provide flattened.error.reporting(...).for(...).nested.incompatible.types
5 participants
@weswigham @typescript-bot @DanielRosenwasser @orta @sandersn
You can’t perform that action at this time.