Semantic Tokens For Getter-Setters

[Number-3434]

Add semantic tokenisation for getter-setters.

This will be separate from fields / properties, allowing getter-setters to be distinguished.

[DanTup]

I'm not sure that this distinction works in Dart and trying to do this could be misleading. Consider the following:

void main() {
  printA(B());
}

void printA(A a) {
  print(a.a); // <-- Is this a field or a getter?
}

class A {
  String a = 'a';
}

class B extends A {
  @override
  String get a {
    print('I am a getter');
    return 'b';
  }
}

The commented line here statically looks like A.a which is a field and would be coloured as one (not a getter). However at runtime that line prints "I am a getter" so clearly it executed code and could have side-effects.

(@bwilkerson interested in your thoughts.. I feel like this has come up before, but I couldn't find any issue here or in the SDK tracker that seemed to discuss this)

[Number-3434]

Okay, but I think the suggest widget already distinguishes between fields and getter-setters.

Also, in your example above, A.a should be coloured as a field (it would actually be misleading if it was coloured as a getter, since it is not possible to tell what type A is expecting).

[bwilkerson]

Every non-local variable (which includes both static and instance fields as well as top-level variables) induces a getter and, unless the variable is final or const, a setter. See section "8.1 Implicitly Induced Getters and Setters" in the spec.

There is almost never a reference to a field. Almost everything that appears to be a reference to a field is actually a reference to (invocation of) either the getter or setter [1]. That means that every reference to an instance field actually performs a dynamic look-up, based on the actual class of the object, of the getter.

That's how the language supports the override in B: the getter in B is overriding the induced getter in A. When a.a is evaluated a dynamic look-up is performed to find the getter a. If the value of the parameter a is an instance of class A, then the getter induced by the field a will be found. If the value of the parameter a is an instance of class B then the explicit getter will be found.

The commented line here statically looks like A.a which is a field and would be coloured as one (not a getter).

Technically, that's also a reference to the induced getter, not a reference to the field.

I feel like this has come up before ...

It comes up frequently. This is a confusing concept for many users.

We've gone back and forth several times on whether it's more useful to users to represent the actual semantics of the language (that every reference is really to a getter or setter) or whether it's better to represent the user's mental model (that most references are references to fields). I suspect that we're currently using the second option.

The problem with the second option, of course, is that there's no precise specification of what the behavior ought to be, which leads to questions like this one. I wish we had a better answer, but both options lead to problems of one kind or another. Perhaps someday we'll think of a better solution, but for now I think we're stuck trying to make the best of an imperfect situation.

[1] The only exceptions I can think of are an initializing formal parameter in a constructor (of the form this.f) or a field initializer in a constructor (of the form f = someValue). In that case the field is assigned directly without performing any kind of lookup to invoke a getter. Those semantics are required in order to keep initialization sane and allow for const constructors.

[Number-3434]

Personally, I would say that only declared getter-setters (i.e. non-implicit) should have a token of getter-setter.

Otherwise, it should have a semantic token type of field.

This is because, as a design principle, the user writing the method doesn't know if they're receiving a class that inherits from a class they declared, so logically, it should be coloured as fields.

Simplicity is best.

This principle is also known as "Occam's razor" in science, and states that when there are multiple solutions to a problem, the simplest solution is the correct one.

In this instance, it would be to represent the user's model.

[DanTup]

Okay, but I think the suggest widget already distinguishes between fields and getter-setters.

I didn't realise this, but you're right - the icons are different:

I'm not certain this is the right decision for the reasons above (or at least, I think we should be consistent in these places).

Simplicity is best.

In this instace, it would be to represent the user's model.

I think this is probably a little subjective. From an implementation standpoint, showing everything as a getter(/property) is simpler, because that's how it's defined and modelled in the language/AST. Showing implicit getters as fields requires specific handling - potentially in multiple places (such as completion, hovers, semantic tokens) that may grow over time.

But, if we're already making this distinction in some places that might be an argument to also do it here. We can use this issue to collect feedback and/or 👍's to help prioritise.

[Number-3434]

@DanTup

Probably a little subjective

Yeah, I agree.

Since these are semantic tokens, could there be a setting to toggle this on and off?

[bwilkerson]

While I agree that simplicity (minimizing complexity) is an important design principle in UX design, it's not the only design principle that needs to be considered here.

The conflicting design principle, in this case, is to minimize confusion. This is hard in this case because there is potential for confusion whichever model we choose to use. You've already experienced the confusion of having something that you thought was a reference to a field be colored as a reference to a getter. But there's an equal chance of someone being confused because something they thought was a reference to a field and was colored that way doesn't behave that way at runtime. (I'm assuming that most users don't think of fields as things that can be overridden.)

So, from a UX design perspective, the question is: is it more confusing to misrepresent the semantics of the language, or is it more confusing to reveal to users a corner of the semantics that they frequently don't need to understand?

[Number-3434]

@bwilkerson

is it more confusing to reveal to users a corner of the semantics that they frequently don't need to understand?

I'm pretty sure that would be the case.

If the user doesn't need to know / understand something, then it's not worthwhile trying to do it that way.

[DanTup]

If the user doesn't need to know / understand something, then it's not worthwhile trying to do it that way.

While I agree it might not be so important for users to understand that fields create implicit getters/setters, I do think that users probably should understand that their "fields" might be overridden with getters. Accessing a "field" multiple twice could be expensive if overridden by a subclass to perform some computation.

So I worry that if we colour "fields" (getters that appear implicit statically) as fields that might give a false indication to users that accessing that "field" is cheap, when it might not be.

Maybe if Dart gets stable getters, that would be an appropriate distinction to colour getter/setters differently (and accurately)?

(Edit: Actually, maybe this doesn't help, because the distinction here was about mutable fields that aren't running any code besides setting a value, whereas stable getters are entirely immutable and wouldn't cover assignable fields)

[Number-3434]

@DanTup Thanks for the link.

I still think that we should colour it as the user thinks, since the semantic token colouriser is supposed to represent what the user thinks is happening.

It should be highlighted specific to the current context.

Currently, the user could be unsure as to whether it is a field or a getter-setter at all, because there is no highlighting.

To clarify:

• The class containing the field / getter-setter should be coloured as the user is led to believe.
• If the user accesses a getter-setter overriding a field, then the it should be coloured as a getter-setter in that specific instance.

class Original {
  // Colour this as a field
  int myNum = 24;
}

class Inheritor extends Original {
  late int _myNum = super.myNum;

  // When viewing this inside the class definition,
  // or when this is invoked, colour this as a getter
  @override
  int get myNum {
    print("Getter");
    return _myNum;
  }

  // When viewing this inside the class definition,
  // or when this is invoked, colour as a setter
  @override
  set myNum(int value) {
    print("Setter");
    _myNum = value;
  }
}

class Main {
  static void main() {
    final original = Original();
    // Coloured as a field
    original.myNum = 42;
    // Also coloured as a field
    print(original.myNum);

    final inheritor = Inheritor();
    // Coloured as a getter
    inheritor.myNum = 42;
    // Coloured as a setter
    print(inheritor.myNum);
  }
}

P.S. This could be an option (that defaults to colourising the way it actually works).

[bwilkerson]

If the user doesn't need to know / understand something, then it's not worthwhile trying to do it that way.

There are times when what you don't know can hurt you. I think that this might be one of those times. Whether for good or bad, fields in Dart don't work the way fields in most other languages work, and the difference is sometimes important.

If the user accesses a getter-setter overriding a field, then the it should be coloured as a getter-setter in that specific instance.

Unfortunately, there's no way for the tooling to know whether the reference to an induced getter-setter will be a reference to an explicitly written overriding getter-setter.

Because it's (almost) always possible for the reference to be overwritten, we use the highlight color of a getter-setter to make this clear to users.

[Number-3434]

@bwilkerson

Unfortunately, there's no way for the tooling to know whether the reference to an induced getter-setter will be a reference to an explicitly written overriding getter-setter.

Ahh, I didn't know that.

@DanTup How does it distinguish this in the suggest widget? (Since the suggest widget uses different icons to distinguish between getter-setters and fields)

Because it's (almost) always possible for the reference to be overwritten, we use the highlight color of a getter-setter to make this clear to users.

Okay, thanks for that clarification.

In JavaScript, I believe it also uses the highlight colour of a getter-setter.

In C#, it can distinguish between fields and getter-setters.

This led me to believe that C# was unusual in that it had a weird colouring scheme (I thought JavaScript and Dart were coloured normally).

---

P.S. Could you give an example of an implicit getter-setter ("field") having a reference to an explicit getter-setter?

[DanTup]

If the user accesses a getter-setter overriding a field, then the it should be coloured as a getter-setter in that specific instance.

We can only base the colouring on the static type, and I worry that colouring something "like a field" which might actually be a getter at runtime could be confusing to users.

Consider this code:

class A {
  String foo = '';
}

void doStuff(A a) {
  if (a.foo.isNotEmpty) {
    print(a.foo);
  }
}

If we coloured foo here as a field (and not a getter), you might assume that a) the value of foo does not change between the isNotEmpty check and the print and b) not assigning a.foo to a local variable does not incur any real cost. These assumptions are not true, but the different colouring may lead users to believe that they are. To illustrate why they are not sound, consider if I add on the following to the example above:

class B implements A {
  String get foo => expensiveOperationToComputeFoo();
}

void main() {
  doStuff(B());
}

With this code, the assumptions in doStuff() are not correct. Maybe expensiveOperationToComputeFoo takes 5 seconds (so now you're doubling it by calling foo twice, and maybe it returns a different value the second time it is called so the isNotEmpty check doesn't work as expected.

@DanTup How does it distinguish this in the suggest widget? (Since the suggest widget uses different icons to distinguish between getter-setters and fields)

The completion kind is just based on what is declared on the static type. It's not guaranteed that because something shows as a Field in the completion that at runtime that's what it is. I think there's a reasonable argument that we probably shouldn't have this distinction here either, but I think the implications of it are much lesser than the different colouring.

In JavaScript, I believe it also uses the highlight colour of a getter-setter.

As far as I can tell, they're treated the same in JavaScript (at least for me in VS Code):

I tried in TypeScript, but it actually forbids overriding a field with a getter:

'foo' is defined as a property in class 'A', but is overridden here in 'B' as an accessor.

In C#, it can distinguish between fields and getter-setters.

C# also does not allow overriding a field with a getter ("cannot override because 'A.foo' is not a property"), so again, the static type in C# showing a field is guaranteed that at runtime it's also always a field.

I don't know of any other languages that support overriding "fields" with getters as Dart does, so unfortunately I don't know of a good comparison.

P.S. Could you give an example of an implicit getter-setter ("field") having a reference to an explicit getter-setter?

The example included in this comment above shows this. The class A has a field with an implicit getter/setter, and class B overrides with an explicit getter.

[Number-3434]

Okay, maybe there could be three options:

1. Definitely a field
2. Field or getter-setter
3. Definitely a getter-setter

[DanTup]

According to the spec, everything will fall into "3. Definitely a getter-setter".

If what you mean is "an explicit getter-setter", then (besides some possible exceptions with final/sealed classes) everything will fall into "2. Field or explicit getter-setter".

Maybe it would be useful to state exactly what your goal is. What are you hoping to infer from having different colouring (I gave an assumption above for what I would infer from them, but I don't know if that's the same as what you're hoping for).

My feeling is that there is probably always a case where this could be misleading. Dart just doesn't work the same as other languages that make this distinction, as noted above.

[bwilkerson]

If what you mean is "an explicit getter-setter", then (besides some possible exceptions with final/sealed classes) everything will fall into "2. Field or explicit getter-setter".

To expand a bit, you can also override an inherited getter-setter with a field (or technically with the getter-setter induced by the field. So it goes both ways.

[Number-3434]

To Clarify

@DanTup @bwilkerson I understand your points of view.

I know that it is intended to help the user better unsderstand how the Dart language works.

However, is it valid?

There could be a higher change of misinterpretation by colouring the fields the way they are,
than by colouring them the way the user thinks it behaves. (For example, I thought it was just a weird quirk of the syntax highlighting).

Furthermore, the user could just hover over the symbol, and the hover will tell them if the declared type uses implicit or explicit getter-setters anyway.

I don't need syntax highlighting

I don't need the actual server to syntax highlight the fields / getter-setters differently.

All I need is for the server to provide a semantic scope / modifier, so then I can manually colour it differently myself.

For example, assuming the current scope is variable.readwrite.other.property, you could use variable.readwrite.other.property.explicit and variable.readwrite.other.property.implicit to indicate that it is explicit or implicit.

This way, it will still retain the current colouring by default, but will allow the user to assign a different colour if they wish so.

My Use Case

As mentioned above, you can already check if the current symbol is explicit or implicit in the context of the base / super type, by hovering over it.

This therefore makes it ineffective to try and explain to the user that it might be a getter-setter through the colouring.

There is only one instance in my entire project that I have overriden a field with a getter.

I would like there to be some sort of way to highlight that a symbol is declared as a field or a getter-setter in the base class.

I only need there to be different semantic tokens for fields and getter-setters.

If you do it correctly (like described above), it is possible for the default highlighting to still remain the same, whilst allowing the user to manually colour them differently.

Thank you for your patience!

[DanTup]

There could be a higher change of misinterpretation by colouring the fields the way they are,

I'm not sure I understand the possible misinterpretation here. Currently, all of these fields are marked as "properties" and that's really what they always are. At development time, we don't know whether at runtime they getter/setter will be implicit or explicit. IMO, showing them as fields when they are not guaranteed to be could be misleading.

I remembered that C# has "auto-properties" which are probably the closest thing to fields in Dart. You declare them like this:

public string Foo { get; set; }

This creates a field with an implicit getter/setter body that just reads/writes that field. Like Dart, these can be overridden with explicit getter/setters in subclasses (whereas the original field syntax cannot).

I checked in VS Code, and these are also marked as "property" even in the case where the getter/setter does nothing more than return the value of the field. It's true that this is a little more explicit in C#, although I suspect that's mainly for compatibility reasons (changing the original field syntax to compile like a getter/setter would be a breaking change to the compiled DLLs).

you could use variable.readwrite.other.property.explicit and variable.readwrite.other.property.implicit

But being an implicit getter in the static type does not guarantee that it's an implicit getter at runtime. I think making this distinction at all is only going to make a concept that is already a little confusing, more confusing.

IMO we should try to bring the users understanding of the language and how the language actually works closer together, but I feel that this may go in the opposite direction - reinforcing something that is misunderstood/overlooked.

Going back to a question I asked further up - I'm interested in why you want to make this distinction. If those modifiers existed today, what would you use that information for or do differently? And would it be a problem if the choices made were not sound?

[Number-3434]

@DanTup

Basically since you can already hover over a symbol to figure out whether it is an implicit or explicit getter-setter, I thought it would be useful to have a scope for it (which is not coloured differently by the server), (which would allow you to manually colourise it yourself).

But being an implicit getter in the static type does not guarantee that it's an implicit getter at runtime. I think making this distinction at all is only going to make a concept that is already a little confusing, more confusing.

As I mentioned earlier,

the fact that you can hover over a symbol to figure out if it is a "field" / implicit getter-setter or an explicit getter-setter basically defeats the purpose of not colouring them differently

(i.e. the user will still think that it is a field or a getter-setter because the hover tells them so), so therefore the users must be currently fine with that distinction.

Note

Because the fields are all syntax-highlighted as properties, I misinterpreted it as being the syntax highlighter's fault (i.e. not using a consistent scope).

I don't think it is a good idea to try and warn users of the fact that it might be explicit from the syntax highlighting; I interpreted that as a lack of proper scopes from the server (hence why I created this issue).

That would be like not syntax-highlighting functions in JavaScript because they can be overriden with a field!

My Use Case

I just want to know if a given symbol on an instance of the type I declared is explicit or implicit in the actual declaration (i.e. as indicated in the hover).

I don't really mind that you can override it.

[DanTup]

Sorry for the delay in responding. We had some discussion about this yesterday and agree that we should make the behaviour across the tooling consistent with respect to properties and fields (this includes semantic tokens, hovers, code completion, etc.).

We think the best way to do is closely follow the spec and stop showing that getters are "fields" in some places as we do today. Only actual references to fields (for example in field formals, constructor initializers, and the field itself in document outline) should be marked as fields, and where a reference really is a getter, it should always be marked as such.

I opened an issue at dart-lang/sdk#55956 with some additional details and will review the places where we have this inconsistency.

I know this isn't the outcome you wanted, but having the tooling represent the semantics of the language accurately may help people better understand some of the nuances of the language that aren't always obvious (for example that a final field can be overridden by getter/setter with a mutable value!). Hope this makes sense!

[Number-3434]

@DanTup I understand.

Thank you.