Flag unintended field over field overrides

[kasperl]

There are concerns about the usability of field over field overrides. It seems too easy to accidentally hide fields and in most cases field over field overrides are unintended.

Three options have been discussed:

• Make it a static error or a warning to override a field with a field.
• Introduce a lint rule that flags all field over field overrides.
• Use existing overridden_fields lint rule to flag field over field overrides that are not marked with @OverRide.

[kasperl]

CC: @vsmenon @leafpetersen @floitschG

[zoechi]

Sounds like #28117, #28601, #28589, #28119

[leafpetersen]

Where do folks stand on this? cc @munificent @floitschG @lrhn @eernstg

[lrhn]

It's not an error, but I'm agnostic to lints since I never use them.
As long as they are completely optional, I don't care which lints people use for their own projects.

[eernstg]

Sounds like #28117, #28601, #28589, #28119.

There's certainly an overlap, but those issues are about allowing field overrides, e.g., setter-overrides-field (say, to perform some validation on the values assigned to that field), or getter-overrides-field (say, to log usages), etc. Even though field-overrides-field is a case which is difficult to justify (it's very likely to be a mistake which just creates some confusion and wastes some space), it is not singled out in the work done for those issues, and in particular it's not an error nor a warning.

I think we should keep a rather tight budget on making any notion of "stupid code" an error, it's much better to take a softer approach like a lint on that type of situation. The problem is that (1) there is no end to the number of situations we could cover if we start squeezing stupid code out of the language per se, and (2) some times we will get it wrong and outlaw some really useful corner cases.

As far as I can see, the overridden_fields lint, does actually check for field-overrides-field, but its documentation is sufficiently vague to allow several other interpretations: the description is a plain 'Do not override fields', and the reader can't really generalize from the examples.

So I think the existing lint is a good solution, but it needs to have better documentation.

[leafpetersen]

Sorry, I wasn't clear enough. Lints have nothing to do with the language team, other than as evidence of demand for features that we have chosen not to provide (e.g. @override).

The question is whether or not we want to make field/field overrides a language level error (as they are in swift, for example). The argument for doing so is that it's a performance and correctness trap, and is essentially never what the user intended.

It sounds like @eernstg and @lrhn are against adding this as a language error. Anyone else have thoughts?

[lrhn]

Indeed.
There should be no difference between a getter and a final field. If there is a difference, then you can't refactor between them, and the entire idea of getters is to allow you to back and forth without worry.

[munificent]

I agree with @lhrn, and for the stated reasons. That being said, I think we have clearly set a usability trap for our users since we see that they do seem to override fields with fields even though it virtually never does what they want.

I believe that's because the syntax for declaring an "abstract field" is painfully long and redundant. A better syntax for that, or properties in general, might help a lot.

[lrhn]

Better syntax would be great.
If you could just write:

int get set x;

to extend your abstract getter int get x; with a same-typed abstract setter, then it would be almost as short as int x; and won't allocate storage.

Or we could move to a C#-like syntax like int x { get; set; }. That's longer, but you can write the non-abstract version with less repetition:

int _num; 
String str { 
  get => "$_num"; 
  set => numx = int.parse(_);  // Using implicit argument for the => function!
}

[munificent]

Or we could move to a C#-like syntax like int x { get; set; }.

I didn't get enough time to polish it off, but I started working on a proposal in this direction. It gets complex when you consider all of the different flavors of "property" a user might want to define, but I think there's something promising there. Not having to repeat the name and type between the getter and setter would be fantastic.

[eernstg]

We could of course add something like int x { get; set; } in order to make pairs of declarations a more concise (that's not much, though) and a more connected (which I think would be good for code comprehension, and which would be enforced as opposed to putting the getter and the setter right next to each other as a convention).

However, we can also go pretty far using the current syntactic style. Lasse already mentioned the abbreviation ..get set... On top of that, the repeated type would go away in many cases if we add one more kind of signature inference: set foo(x).. infers the argument type T if there is an accessible getter named foo returning T (it could declared in the same scope or in a supertype, with an explicit declaration or induced by a variable declaration).

[leafpetersen]

Ok, so it seems like there is no consensus for moving forward on this. Closing as not planned.