Add related error spans for getter/setters with different types

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/set accessor's types not matching:

Code:

let x = {
  get foo() { return 100; }
  set foo(value: string): { }
}

Current error:

'get' and 'set' accessor must have the same type.

We'd like to give a better error message. For example:

Primary span:

A 'get-' and 'set-' accessor must have the same type, but this 'get' accessor has the type '{0}'.

Related span:

The respective 'set' accessor has the type '{0}'.

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
bashdx

EDIT:

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 2380 to 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

see 4459730#diff-e119cc28df9582f9e1ca0d41720ea142

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.

microsoft#25002: removed unneccessary newlines

Open to ideas here that have some concrete improvement

There is no restriction on get set in ECMA specification, so adhering to the standard, shouldn't this restriction be lifted? I mean "'get' and 'set' accessor must have the same type".

Ryan Cavanaugh (@RyanCavanaugh)

setters 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)

Ryan Cavanaugh (@RyanCavanaugh)

To 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)

Martin Heidegger (@martinheidegger) that limitation is not per ECMA standards and only serves as additional gatekeeping.

Agastya Chandrakant (@acagastya)

I completely agree with Agastya Chandrakant (@acagastya) and sirian. Different setters/getters are very useful when doing normalization.

/cc Sindre Sorhus (@sindresorhus)

Navigation Menu

Repository navigation

Add related error spans for getter/setters with different types #25002

5 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Add related error spans for getter/setters with different types #25002

Description

Activity

bashdx commented on Jun 30, 2018

mhegazy commented on Jul 2, 2018

bashdx commented on Jul 2, 2018

mhegazy commented on Jul 2, 2018

bashdx commented on Jul 2, 2018

mhegazy commented on Jul 2, 2018

bashdx commented on Jul 2, 2018

mhegazy commented on Jul 2, 2018

mhegazy commented on Jul 2, 2018

bashdx commented on Jul 2, 2018

5 remaining items

RyanCavanaugh commented on Jan 8, 2020

acagastya commented on Feb 6, 2020

sirian commented on Mar 21, 2020

martinheidegger commented on Sep 3, 2020

acagastya commented on Sep 3, 2020

szmarczak commented on Nov 14, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions