Dart-code: Override suggestion not offered using quickfix if @override option deleted from build

[atreeon]

If you type 'ini' above the @override statement below you get the initState suggestion. If the override statement is deleted then you don't get the suggestion. I heard Filip on the Boring Show say the @override element isn't necessary. Should we always specify @override on our overridden funcions or is there another way of getting the suggestion to work when @override has been removed?

class _TestWidgetState extends State<TestWidget> {
  @override
  Widget build(BuildContext context) => Text("blah");
}

[DanTup]

I think this may be a dupe of https://github.com/dart-lang/sdk/issues/32677. Whether you see override completions seems to depend a lot on the structured of the AST around where the cursor is.

[atreeon]

Hi Danny
I tried it with the cursor being in the exact same place and I had the same result. If the override meta tag is specified directly above the build method it works, if the override meta tag is removed from the build method then I don't get the suggestion. I think dart-lang/sdk#32677 will provide another way of getting the suggestion.

[DanTup]

Hmmm, it seems when I tested this, I ended up debugging a different issue :)

The issue I found was because of a missing return type on my second method, so it though what I was typing was the return type (and therefore didn't show override completions). However, re-testing it with a return type I see strange behaviour:

class Base {
  void initState() {}
  void build() => '';
}

class Foo extends Base {
  init // works fine here

  void build() => '';
}

But if I replace both voids with String then it no longer works.

class Base {
  void initState() {}
  String build() => '';
}

class Foo extends Base {
  init // works fine here

  String build() => '';
}

The difference in the AST seems to be that when the return type on the following method is void, then the completion target is init; as a FieldDeclaration, but when the return type is String then it gets init String; as a FieldDeclaration and this code fails because init is in the type field, and this code looks for a null type.

@bwilkerson is it correct that the AST differs in this way between having void vs String? It might be tricky to handle this in the override contributor because the AST seems a bit misleading (it contains a FieldDeclaration for init String when String is actually the return type of the following method.

[bwilkerson]

is it correct that the AST differs in this way between having void vs String?

The parser, which is where the AST is produced, is currently fairly greedy. So yes, when it sees two identifiers together at the beginning of a class member it decides that the first is a type and the second is the name of a field that's being declared. It then assumes that there's a missing semicolon, inserts it, and moves on to the next class member.

The reason void doesn't have that behavior is because it isn't an identifier that can be declared. When it sees void it decides to end the current member prior to the second identifier rather than after it.

We've had discussions over the years about doing better recovery in cases like these. The primary suggestion is to look at the whitespace and decide that because they first and second identifiers are on different lines and it's valid to interpret the second as a return type for the method that that would be the better way to recover. Unfortunately, that hasn't happened yet.

[DanTup]

@bwilkerson thanks for the details, all makes sense!

@twistedinferno to answer your original question - this is probably a reason that having the annotations works out better (there's a lint you can enable if you want to enforce this) - though it won't help if you're trying to add an override immediately before a non-override method.

If the parser is improved to handle this differently in the future it will just start working as you'd like without any changes here, so I don't think there's anything to do in the VS Code extension (nor can I think of any reasonable workarounds in the meantime I'm afraid!).