[macro apis] The hasAbstract getter doesn't make sense for functions, should exist for fields

[jakemac53]

The abstract keyword isn't allowed for functions/getters/setters, but it is allowed for at least fields (but not top level variables). I think this was messed up during the transition from isAbstract => hasAbstract so we should clean it up.

[scheglov]

Alternatively, add isAbstract (not hasAbstract) for methods, but remove hasBody. The reason is that we consider external methods not abstract even though they don't have Dart bodies.

[jakemac53]

I think hasBody + hasExternal is ok at least for now, it allows macro authors to make a choice about how to treat members that are marked external? Granted, it means they also have to think about external members explicitly, but that might be a good thing.

[jakemac53]

Also semantically, I am not sure it would be correct to describe a method which has no body, but also whose surrounding class is not marked as abstract, as an abstract method. Because it isn't actually abstract it just isn't filled in yet (but will be).

[scheglov]

I agree that hasBody + hasExternal might work without isAbstract.

Your comment about "isn't filled in yet" made me look at the documentation for hasBody, and it seems to state not exactly what I expected for external.

  /// Note that for external functions, this may return `false` even though
  /// there is actually a body that is filled in later by another tool.

First, why "may"? I expected the defined behavior. If there are additional conditions, we probably should describe them.

Second, as you said above, it will be filled. So, I expected true when hasExternal is true.

[jakemac53]

Today, hasBody is only meant to represent if there is an explicit (Dart) body today. By the time a macro is running, there may not yet be any explicit body filled in, either as a truly external function or some augmentation or patch file filling in a Dart implementation.

Probably the wording should be more clear, saying will instead of may.

We could harden the definition of external such that it guarantees a body filled in by a compiler, and not just a normal augmentation. And any augmented reference in an augmentation always refers to whatever the compiler filled in (even if that was filled in later). And then similarly we could change this to say that for external members hasBody is always true?

[lrhn]

Also semantically, I am not sure it would be correct to describe a method which has no body, but also whose surrounding class is not marked as abstract, as an abstract method. Because it isn't actually abstract it just isn't filled in yet (but will be).

That's allowed today, if an implantation is inherited.

class C {
  int foo()=> 42;
}
class D extends C {
  /// The Foo of a D is special.
  int foo();
}

You can do this if all you need is to add docs or metadata. (Or add covariant.)

There should be no functional difference between abstract final int x; and int get x;, they are semantically equivalent ways to define an abstract getter.

That means that the way to augment an abstract final int x; is with an augment int get x => 1;, not an augment final int x = 1;. (Not unless the latter can be used as an augment for int get x; too.)

[jakemac53]

That's allowed today, if an implantation is inherited.

TIL 🤣. I think that is enough of a reason to keep hasAbstract separate from hasBody.

There should be no functional difference between abstract final int x; and int get x;, they are semantically equivalent ways to define an abstract getter.

That means that the way to augment an abstract final int x; is with an augment int get x => 1;, not an augment final int x = 1;. (Not unless the latter can be used as an augment for int get x; too.)

I will look into each of these and make sure we have a coherent solution.

[jakemac53]

This is likely obsolete, moving to no milestone/release and we can close when we transition to dart_model.

[munificent]

This is definitely obsolete now. :)