Repository navigation
Add related error spans for getter/setters with different types #25002
Description
Activity
Hello guys,
if you could point me in the correct direction (where do I find the error messages) I will take a look at it.
Thank you and regards
bashdxEDIT:
If I got it right it revolves around src/compiler/diagnosticMessages.json - there you have a bunch of JS objects like
"'get' and 'set' accessor must have the same type.": { "category": "Error", "code": 2380 },The build will generate the appropriate JS in for example tsc.js?
How do you inject the type information in there if you had format parameter in the string?The issue in the OP is about adding additional information to the error message. We have recently allowed an error message to include child messages (to add clarifications), we call these related spans. See #25359 for a similar change.
Thank you for your reply. I built the compiler and tried the following with your code snippet from above:
`$ node tsc.js --target "ES5" test_accessors.ts
test_accessors.ts:4:3 - error TS2322: Type '100' is not assignable to type 'string'.4 return 100;
~~~~~~~~~~~test_accessors.ts:6:26 - error TS1005: '{' expected.
6 set foo(value: string):{}
~
`I can't get error
2380to be invoked. Could you please provide a fitting code snippet. Using a class instead of an object resulted in a similar result.Thank you and regards
Try this instead:
let x = { get foo(): number { return 100; }, set foo(value: string) { } }
That did it! I am getting closer - could you please tell me, how to create a "blank" node? The current result looks like this (please ignore the unrelated error message about generators - this was solely for testing). Where would I create the message for the related span?
built/local/test_accessors.ts:2:9 - error TS2380: 'get' and 'set' accessor must have the same type, but this 'get' accessor has the type 'number'. 2 get foo(): number { return 100; }, ~~~ built/local/test_accessors.ts:3:9 3 set foo(value: string) { } ~~~ Generators are not allowed in an ambient context. built/local/test_accessors.ts:3:9 - error TS2380: 'get' and 'set' accessor must have the same type, but this 'get' accessor has the type 'string'. 3 set foo(value: string) { } ~~~ built/local/test_accessors.ts:2:9 2 get foo(): number { return 100; }, ~~~ Generators are not allowed in an ambient context.not sure why you are getting the error
Generators are not allowed in an ambient context...This was just a random error message I picked to test the behavior of my changes.
Ahh.. take a look at #25377 then.
To add a new error message, you need to edit https://github.com/Microsoft/TypeScript/blob/master/src/compiler/diagnosticMessages.json
The rest of the output can stay like this? This very format? Also it seems the error handler is invoked twice, while the former aims for the getter (inteded) and the latter aims for the setter, but uses the getter error message. I will check it out ASAP.
5 remaining items
- added a commit that references this issue
on Jul 7, 2018 Open to ideas here that have some concrete improvement
There is no restriction on
getsetin ECMA specification, so adhering to the standard, shouldn't this restriction be lifted? I mean "'get' and 'set' accessor must have the same type".Reacted by Mateusz Turcza, Martin Heidegger, Szymon Marczak, Aleksey Kliger (λgeek) and MatheusLeitaosetters and getters MAY HAVE different types
get zIndex(): string set zIndex(value: string | number | null | undefined)
Another example:
class MY_URL { get query(): Record<string, string> set query(value: string | Record<string, string | number | null |undefined>); } const url = new MY_URL(); url.query = "foo=1&bar=2" // string setter url.query = {foo: 1, bar: "2"} // Record<string, ...> setter assert("1" === url.query.foo && "2" === url.query.bar)
Reacted by Mateusz Turcza, Cole Palm, Martin Heidegger, Szymon Marczak and Alex YangTo me its a reasonable limitation that getter/setter should not have different type but this should imo. be handled by a linter rather than the compiler.
What I have been thinking is that this might be related to the writeonly proposal #21759. Maybe having a type declaration such as the following could help to get a mental model for how getters/setters have different types:
interface IMY_URL { readonly query: Record<string, string> writeonly query: string | Record <string, string | number | null | undefined> }
Another real-life example I have for this is the sketch api which does C mappings and has properties that can be set with partial objects but the getters will return filled-out instances.
Martin Heidegger (@martinheidegger) that limitation is not per ECMA standards and only serves as additional gatekeeping.
I completely agree with Agastya Chandrakant (@acagastya) and sirian. Different setters/getters are very useful when doing normalization.
Reacted by Sindre Sorhus, Alex Yang and Aleksey Kliger (λgeek)
Now that we support multiple related spans for errors (#10489, #22789, #24548), we'd like to improve an existing error message.
Currently, we provide a diagnostic for a pair of
get/setaccessor's types not matching:Code:
Current error:
We'd like to give a better error message. For example:
Primary span:
Related span: