-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fasta allows unsound abstract override #32014
Comments
The check for missing implementations in https://dart-review.googlesource.com/c/sdk/+/43660 fails to detect this case, where a concrete method is overridden by (via a subclass or implemented interface) an abstract method with a more general signature (i.e. adding optional parameters). |
Note that in this case, noSuchMethod forwarders are not generated, so this is an error whether or not the class has a noSuchMethod implementation different from the one in Object. |
…ixin." This reverts commit 03c7184. Reason for revert: Flutter doesn't build with after this CL because the following (kind of) program abstract class RenderObject { String toString({int minLevel}) => "foo"; } abstract class ContainerRenderObjectMixin extends RenderObject {} abstract class RenderBoxContainerDefaultsMixin implements ContainerRenderObjectMixin {} is wrongfully rejected. In patching this, I stumbled upon what seems to be bug in Fasta where some concrete classes arising from mixed in applications are marked as abstract, which causes the patch to produce false-positives. This needs some more investigation. Original change's description: > Enables arity check for overridden methods inherited from a mixin. > > The return type and parameter types checks are still disabled for > mixins as there is an issue with type parameters being uninstantiated > in mixin applications which causes the two checks to produce > false-positives. As a result the SDK fails to compile with the two > checks enabled because the libraries (such as collections) makes > extensive use of mixin application with generic types. I've opened a > separate ticket to track this issue [c.f. #34285]. > > Furthermore this CL abstracts the arity check, return type check, and > method parameter type check in checkMethodOverride to make it easier > to understand the logic of the method. > > Closes #34235 and closes #32014 > > Change-Id: Iae224926c2e99e6e89ccc3c19ec4bc7919ee48a5 > Reviewed-on: https://dart-review.googlesource.com/71781 > Commit-Queue: Daniel Hillerström <hillerstrom@google.com> > Reviewed-by: Aske Simon Christensen <askesc@google.com> TBR=askesc@google.com,hillerstrom@google.com Change-Id: Idee6f53c2d6a17ea8a4103872b1183c01e4d30ce No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/72108 Reviewed-by: Daniel Hillerström <hillerstrom@google.com> Commit-Queue: Daniel Hillerström <hillerstrom@google.com>
Partially fixed in 40864cc (only checks for difference in arity; misses checks for named parameters and types). The partial fix reuses the error message for missing members, which is misleading (especially the tip). The fix for the remaining checks should add one or more error messages to use for all mismatch situations. |
Fixed in https://dart-review.googlesource.com/c/sdk/+/74642, but blocked on dart-lang/dartdoc#1755 being rolled into the SDK deps. |
This is the same missing error message that DDC and the analyzer has. See #30568 for details.
The text was updated successfully, but these errors were encountered: