dart-lang
Should we disambiguate abstract and incomplete functions?
With augmentations, we need to be able to express that a function is "incomplete": it has a declared signature but the body will be filled in separately by an augmentation. What is the syntax for that?
The current proposal is to use the same syntax as abstract functions (and generalizes that to allow ; bodies in places where an abstract function isn't meaningful).
That means that in a context where you can have an abstract member, it's unclear to a reader whether the given member is abstract or incomplete (and filled in by a later augmentation) or both (if the augmentation adds metadata but still not a body).
For example, say a reader looks at:
part 'thing_stuff.dart';
abstract class Thing {
void foo();
void bar();
}
They want to define their own class that implements Thing. Which of these is a correct implementation?
import 'thing.dart';
class MyThing1 implements Thing {}
class MyThing1 implements Thing {
void foo() {}
}
class MyThing1 implements Thing {
void bar() {}
}
class MyThing1 implements Thing {
void foo() {}
void bar() {}
}
The answer depends on what's in "thing_stuff.dart" and whether an augmentation in there fills in foo(), bar(), or both.
Now, of course, in practice, the analyzer will tell the user what to do. So if we're comfortable relying on static analysis, it's OK that just reading the text doesn't provide a complete picture. But it's still potentially a usability problem.
Should we do anything about this? Some options:
Keep the current proposal
We could stick with the current behavior where ; is overloaded to mean both and you can't tell if a member is actually abstract until all augmentations have been applied and we see which ones are still incomplete.
This is terse. It minimizes ceremony both for abstract and incomplete members.
Note that in contexts where a member can't be abstract (top level functions, static members), the syntax isn't ambiguous and can only mean "incomplete".
A problem with this approach that @johhniwinther raised is that it means we won't be able to use augmentations to replace the core library's current use of patch files. In the patch file system, a compiler applies a patch file to the core library when compiling code, so it knows which members do get filled in by the patch file and which ones are left abstract.
But the analyzer doesn't apply any patch files because it's trying to show users the superposition of all possible platforms. Because of that, it doesn't know which "abstract or incomplete" members will ultimately get patched, so it can't tell if a member is actually abstract or not.
We could probably workaround that using some kind of metadata annotation, but I do like the idea of designing an augmentation feature sufficiently powerful and clear that we can use it for our own core libraries.
An explicit syntax for incomplete functions
Instead of overloading ; to mean abstract and incomplete, we could introduce a second syntax for the latter. It would probably be another kind of function body syntax (since it's a property of the body). Maybe like:
abstract class Thing {
void foo();
void bar() incomplete;
}
Now it's clear that foo() is actually abstract and bar() is not.
Or we could use augmented:
abstract class Thing {
void foo();
void bar() augmented;
}
I don't hate how it looks. It's explicit and clear, which is nice. But it does mean that a user hand-writing code to be completed by a code generator has more ceremony to deal with.
One of the main motivations of augmentations is to allow hand-authored code and generated code to be woven together in a way that doesn't require the human to explicitly weave things in.
For example, right now, if you're using built_value, the author has to delegate to the generated code as in:
abstract class SimpleValue implements Built<SimpleValue, SimpleValueBuilder> {
static Serializer<SimpleValue> get serializer => _$simpleValueSerializer;
int get anInt;
String? get aString;
factory SimpleValue([void Function(SimpleValueBuilder) updates]) =
_$SimpleValue;
SimpleValue._();
}
One of the goals of augmentations is to be able to eliminate boilerplate like the => _$simpleValueSerializer; and = _$SimpleValue;.
In this case, an augmentation could completely define these members (and maybe use a metadata annotation to opt in to serialization), like:
@serialized
abstract class SimpleValue implements Built<SimpleValue, SimpleValueBuilder> {
int get anInt;
String? get aString;
SimpleValue._();
}
For other use cases, I don't know how common it would be for the hand-authored code to need to declare a member and have the code generator fill in the body, versus the code generator defining the member from scratch. If most use cases are the latter, then a little ceremony like incomplete; is probably tolerable.
If we do take this approach, then I'd like to consider also moving external to be a body syntax and not a modifier. There's a decent amount of irregularity in the grammar to handle external. The presence of that modifier makes it a syntax error to have a non-; body and the grammar has to deal with that. Making it a body syntax would fix that:
void someFunction() external;
Also, conceptually being external is really a property of the implementation of the function and not its declaration. So it makes sense to me to make it a body syntax.
An explicit syntax for abstract methods
We could flip the default and say that ; unconditionally means "incomplete". If you want an abstract member, it has to be explicitly marked such:
abstract class Thing {
abstract void foo();
void bar();
}
I'm fairly certain that there will be many more abstract members in the wild than incomplete ones, so this is probably the wrong default. Also, this would require a large migration.
It does have the minor advantage of making abstract methods, getters, and setters be consistent with abstract variables.
Use a metadata annotation to disambiguate
We could use the same ; body syntax for both abstract and incomplete functions, but then have a metadata annotation that specifies whether the function is abstract or incomplete.
With this approach, we have to decide whether the metadata annotation is mandatory or optional. And whether you annotate abstract members, incomplete ones, or both.
I'm guessing we wouldn't want to mandate annotating abstract members since they are so common and it would be seriously breaking. So probably something like:
abstract class Thing {
void foo();
@incomplete
void bar();
}
Personally, I don't find this very appealing. Metadata doesn't really have semantics and if this is information that's important enough to write down, I'd rather it be real syntax.
Opinion
We discussed this at the last language meeting, but didn't come to a resolution (which is why I'm writing up this issue). Personally, I lean towards a body syntax for incomplete functions, either incomplete or augmented. Leaning slightly towards the latter because it clearly sends a signal that augmentations are involved.
I think the right answer here hinges on how often users will have to write the syntax. That in turn depends on how often code generators fill in a hand-authored declaration versus adding entirely new declarations to a type. I suspect that most use cases are the latter but I don't have concrete data. If it's worth it, I can try to dig in.
If anyone is familiar with some of the widely used code generators and wants to chime with whether they would require a user to write incomplete functions here, that would be very helpful.
I feel like this feels more familiar and easier to spot when you are looking over a class and need to understand which of the functions are abstract and which are augmented.
abstract class Thing {
void foo();
augmented void bar();
}
FWIW, I like void foo() incomplete; body syntax, it is consistent with just void foo(); as if it were void foo() abstract;.
I feel like this feels more familiar and easier to spot when you are looking over a class and need to understand which of the functions are abstract and which are augmented.
abstract class Thing { void foo(); augmented void bar(); }
Yes, we could make it a modifier like this (and then require the body to be ;), but I think that sends the wrong signal. Modifiers are usually placed first because they tell the user of the declaration something important about it. For example, final before a variable tells you you can't assign to it, base before a class says you can't implement it.
From the user's perspective, whether the body of a function is provided right there at the declaration or filled in later by an augmentation doesn't matter. It's an implementation detail. That leads me to feel like it makes more sense after the parameter signature in place of the body.
I admit that it looks a little unusual to see a keyword there, though we have prior art with things like sync and async (which are also implementation details of the function and not relevant to callers).
Something is deeply irking me about making this distinction. I think it's the feeling of "having to write extra words". That's a strong feeling to go up against! (No, I won't use final, it's two more letters than var! I'm a programmer, not a speed typist.)
What I would prefer is that I write only enough to give the information needed by the code generator, which is (in this case) the method signature. Which is just the abstract method declaration.
Anything more than that is unnecessary. We write it only to tell the human reader that something will be added later. The compiler will know that, it can look ahead. The DartDoc too. The hoover over the member in the IDE can tell the full story. Basically, anything other than looking at the first declaration of a multi-part class will give you the information.
We can no longer assume that looking at
abstract class C {
void peel();
}
tells us everything about C.
That declaration doesn't tell use whether C implements more interfaces, has super-classes or mixins, whether it has more members than peel, static, instance or constructors. Basically nothing.
Not being able to tell whether foo is has an implementation is small stuff in comparison.
And that was never the case anyway:
abstract class C with Banana {
void peel();
}
Does C have an implementation of foo? I don't know, that depends on the Banana.
And with augmentations, even if we introduce a different way to say that a method will be augmented, you still can't trust that to tell the whole story.
abstract class C {
int foo(); // Really means abstract-and-stay-abstract!
}
augment abstract class C with Banan {} // Banana.peel() is a thing!
Then C has a concrete peel method, even if the declaration was marked as abstract and was never augmented.
So even if we add verbosity to distinguish between "is abstract and stays abstract" and "is abstract but will be augmented to concrete", you still can't trust that. So why bother?
I say: Let the compiler determine whether something is abstract or not, don't bother telling it one way or another. If it's still abstract at the end of the library, and the class isn't abstract, you have an error.
Stop thinking that the first declaration is somehow special. You can start out with
class C {}
and add everything using later augmentations. All that class C {} says is that there is a class named C. The full declaration stack, and only the full declaration stack, will tell you what C does.
That's also why I argue that the augment keyword isn't needed. The first declaration isn't special, it's just first. We can have the keyword, and treat it like @override in that you don't need it, but if you do use it, you'll get an error if you are not actually overriding something, but we don't actually need it for anything. A declaration with the same name is either a valid augmentation, with or without augment in front, or it's not, and in the latter case it's an error. It would catch it if you remove the first declaration, but the remedy is to remove the augment from the new first class. And nothing else changes.
The one thing a redundant keyword can do, is to check if its consistent. That's what redundancy is good for - if it gets out of sync, and is no longer redundant, the compiler can tell you. I don't think it's worth it. It'll just require moving around keywords. If you have:
class C {}
augment class C {
int foo() => 42;
}
and you add int foo(); to the first C declaration, which is just repeating the interface signature of foo, you now have to go to the second C declaration and add an augment modifier.
I had an idea for another approach: We could combine the current approach and an explicit syntax for incomplete functions.
We give you an optional syntax for incomplete function bodies. Maybe incomplete; or augmented;. If you use that, then the function is unambiguously not abstract and is incomplete. It is a compile error if a function has this marker body and doesn't get a body filled in by an augmentation.
But you can also simply omit the marker and use ;. If you do, you get the current behavior where in some contexts the function may be either abstract or incomplete. The answer isn't known until after augmentations are applied.
This means:
-
In all of the places where a function can't be abstract (top-level, static methods), you can just use
;because it's unambiguous anyway. -
The core libraries can use
augmented;diligently so that the analyzer can reliably tell which instance methods are abstract and which are incomplete without having to look at any patch files. The compiler will report an if a patch file fails to fill in some incomplete function instead of silently assuming it's abstract. -
If you have a library package with a public API containing a mixture of abstract and incomplete functions you can use this if you want it to be clearer to readers which methods are abstract. (But you don't have to.)
-
If you want the compiler to remind you if you have some intended-incomplete function and you forget to fill in a body in an augmentation, you can opt in to
augmented;at the introductory declaration to get stricter checking. -
If you're just a user of a code generator writing application code, you can happily write
;with minimum ceremony and everything works. -
Over time, we can see whether users generally prefer to be explicit or implicit about incomplete functions because we give them the choice. If a clear best practice emerges, we can add a lint to encourage that.
-
A potential source of confusion is that if users get used to seeing
augmented;, they may start to assume that;only means abstract. That's a downside.
I kind of liked the idea of literally incomplete functions.
abstract class Foo {
// Subclasses don't need to implement it, known for a fact.
String bar() =>;
}
No need for keywords, and it's visibly clear that it's incomplete.
I get that augmentations and mixins and such can implement it and that's that, but you may not be at that point yet.
Since we usually don't control what code generators produce, we can't actually guarantee that it gets implemented.
This is fine for a normal class, as it will tell you that it's not implemented, but that's terrible for an abstract class that's intended to be extended, or for a mixin, as the public API for the type is expected to have it already implemented.
Consumers should not need to implement the method. It is a breaking change to require that later, which may happen between code gen versions without our knowledge.
Therefore, an incomplete method, regardless of specific syntax, exists to enforce this invairent.
"At this point, bar is guaranteed to be implemented, so subclasses won't be forced to implement it"
To be very clear, usually it's not necessary in today's code, since we can see all mixins and such, but now that we can't see all of the code in one place, we don't actually know if it's implemented. That's bad.
Imagine forgetting an annotation, and not realizing until it becomes a problem?
Tests would help with this of course, but we like type safety.
literally incomplete functions:
abstract class Foo { // Subclasses don't need to implement it, known for a fact. String bar() =>; }
It has an issue: void bar () =>; looks wrong. Don't return values from a void function. (That's my windmill to tilt at. Void functions should not use =>, damnit!)
Generally, I think any signature intended for a code generatator should be plain <signature>;.
That allows the code generator to augment the function with an implementation, but it also allows it to implement it by augmenting the class with a mixin application, which will not augment the declaration.
If you use String bar() =>; and that requires the member to be augmented, then that implementation option goes away.
Having the option of either void foo();, void foo() abstract; or void foo() late;, where abstract must not be augmented with an implementation, and late must be augmented with an implementation (wouldn't use augmented for that, because you can augment without adding an implementation), and the plain ; can be either, then I think we do have the ability to make things explicit in hand-written code, if you want to, and keep things open for code generators.
If you use
String bar() =>;and that requires the member to be augmented, then that implementation option goes away
No, it just requires that it be implemented.
That said; String bar() late; isn't bad. The semantics are not dissimilar to late variables - the implementation comes _late_er, but there is an implementation.
And it's not too verbose - abstract and incomplete are a bit long, but late is nicely concise.
I think most people will likely continue using normal abstract declarations in most cases, but having this feature is really nice for, like I said, when you have an abstract class and you want to indicate that no, you don't need to implement it - there's an implementation already.
It's also nice for generated code, if it's done in stages, or if one generator expects a different generator to fill it out - the invairent for that generator being that that member is implemented.
Whatever we do, I'd rather not have language keyword here.
We use @override, so I don't see why we would have incomplete be a language keyword.
As for my opinion, I'd go with no keyword at all.
For starter, code-generation is rarely applied on abstract classes. I least, I don't think I ever did that.
The idea of asking all "hooks" for code-generation to add an extra keyword for a niche readability issue doesn't sound right.
That sounds like optimizing for the 1%, in the detriment of the 99%
There are also cases where asking for an extra keyword adds no value: constructors or top-level functions.
When we do:
class Foo {
factory Foo.fromJson(String);
}
It's kinda obvious that this is a target for augmentation. After all, constructors can't be abstract.
Same with top-level functions.
That's not a bad point. Having an annotation we can opt into using could be enough.
That said, we don't need to introduce new keywords if we reuse late
Plus, while simply using the normal ; in most cases is enough, having explicit syntax for "this will be implemented" can be extra useful for some cases where you might use UnimplementedError. Sometimes, you just don't feel like implementing it now. Usually ; is enough, but not for abstract classes. Plus, explicitly marking it as to-be-implemented can be useful semantically.
I suppose it could just be an annotation with a lint that's an error rather than a warning, if we want. It could effectively override the meaning of the usual missing body error too, to explicitly state that it's not implemented yet
That said, using a keyword means we could have it effectively fill in the body with an UnimplementedError in debug mode, if we ignore he warning. Basically a mini macro I suppose.
Lasse makes the point that an augmentation may choose to complete a method by filling in a body directly or by applying a mixin that implements the method. That means the ambiguity between "abstract" and "incomplete" is useful because it gives the augmentation more freedom.
I'm also sensitive to an increasing proliferation of modifier keywords. Given that, I think we should stick with the current proposal where you can give any declaration an empty body with ; and then an augmentation can fill it in or not. Then we compile the resulting declaration and interpret the members appropriately. By that, I mean it becomes an error for a non-instance member to not end up having a body, and it becomes abstract otherwise.
I'm going to close this issue, but if others want to keep discussing, I can re-open it.