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.

Sign up for GitHub

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

Improvement to type relation system #3291

Closed
timotheeguerin opened this issue May 6, 2024 · 0 comments · Fixed by #4357
Closed

Improvement to type relation system #3291

timotheeguerin opened this issue May 6, 2024 · 0 comments · Fixed by #4357
Assignees
@timotheeguerin
Labels
triaged:core
Milestone
Backlog

Comments

@timotheeguerin
Copy link
Member

Currently we do not do a great job of reporting unassignable errors. Those are just thrown at the top level with some more or less useful message.

We should:

  • whenever possible to report the error at the most precise location.
  • otherwise show a stacktrace of unassignable message like TypeScript but target the root node

For this i think we need to change a few thing:

  • the whole type relation pipeline needs to not report diagnostics directly but pass some intermediate message so we can build the stacktrace
  • we should have something to detect if the preceise unassignable target is relevant (probably check if it is a syntax chidren of the passed diagnostic target)
@markcowl markcowl changed the title Improvment to type relation system Improvement to type relation system May 7, 2024
@markcowl markcowl added the MQ label May 7, 2024
@markcowl markcowl added this to the Backlog milestone May 7, 2024
@bterlson bterlson removed the MQ label May 10, 2024
@timotheeguerin timotheeguerin mentioned this issue Sep 5, 2024
Merged
@timotheeguerin timotheeguerin self-assigned this Sep 5, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 6, 2024
@timotheeguerin
Improvements to type relation errors (#4357)
03d4fca 
fix #3291

Changes: 
1. Figure out the most accurate location for the diagnostic
2. If diagnostic target a child node of the base diagnostic target then
emit diagnostic directly there
3. Otherwise emit back at the root(or closest child node) and build
stack of error message

Example the following would now emit the error on a
```ts
 const b = #{ prop: #{a: "abc"}};
 const a: {prop: {}} = b;
```

```
Type '{ prop: { a: "abc" } }' is not assignable to type '{ prop: {} }'
  Type '{ a: "abc" }' is not assignable to type '{}'
    Object value may only specify known properties, and 'a' does not exist in type '{}'.
```

Previously the error would have been in the complete wrong place 
<img width="271" alt="image"
src="https://github.com/user-attachments/assets/c403d1ec-3611-4ad6-87b0-2e0a075dc1c5">
sarangan12 pushed a commit to sarangan12/typespec that referenced this issue Sep 16, 2024
@timotheeguerin @sarangan12
Improvements to type relation errors (microsoft#4357)
71d9774 
fix microsoft#3291

Changes: 
1. Figure out the most accurate location for the diagnostic
2. If diagnostic target a child node of the base diagnostic target then
emit diagnostic directly there
3. Otherwise emit back at the root(or closest child node) and build
stack of error message

Example the following would now emit the error on a
```ts
 const b = #{ prop: #{a: "abc"}};
 const a: {prop: {}} = b;
```

```
Type '{ prop: { a: "abc" } }' is not assignable to type '{ prop: {} }'
  Type '{ a: "abc" }' is not assignable to type '{}'
    Object value may only specify known properties, and 'a' does not exist in type '{}'.
```

Previously the error would have been in the complete wrong place 
<img width="271" alt="image"
src="https://github.com/user-attachments/assets/c403d1ec-3611-4ad6-87b0-2e0a075dc1c5">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@timotheeguerin timotheeguerin

Labels
triaged:core
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

Improvements to type relation errors timotheeguerin/typespec
3 participants
@bterlson @timotheeguerin @markcowl