Skip to content

implicit_reopen loophole #59124

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

Open
eernstg opened this issue Apr 24, 2023 · 2 comments
Open

implicit_reopen loophole #59124

eernstg opened this issue Apr 24, 2023 · 2 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-false-negative P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@eernstg
Copy link
Member

eernstg commented Apr 24, 2023

Summary

It is possible for a class to inherit an instance member implementation from an interface class in another library without getting any lint messages from implicit_reopen. This should not happen.

Example

This behavior is exhibited with the following program (consisting of two user-written libraries 'lib.dart' and 'main.dart'):

// Library 'lib.dart'.

interface class A {
  void m() => print('Inherited from an `interface` class');
}

mixin M on A {}
class B extends A with M {} // No lint, currently.

// Library 'main.dart'.
import 'lib.dart';

class C extends B {}

void main() {
  C().m(); // Prints 'Inherited from an `interface` class'.
}

The program is accepted with no diagnostic messages when processed by dart analyze main.dart (e34daa8), with the following analysis_options.yaml:

linter:
  rules:
    - implicit_reopen

Analysis

The feature specification rules are as follows: An implicit_reopen lint, if enabled, should be emitted when a class C is not annotated with @reopen, and it satisfies any of the following:

  • it extends a class marked interface or final and is not itself marked interface or final, or
  • it extends a sealed class which itself transitively extends a class marked interface or final.

In the example, the superclass of B is A with M.

If you prefer to consider the situation in terms of implicitly induced class modifiers, the mixin application A with M is implicitly interface, because the interface property is propagated from the superclass to the mixin application (it would also be propagated from the mixin, except that a mixin can't be interface). So we need the lint because the superclass A with M is interface, but the class B itself is not.

If we insist that implicitly induced class modifiers do not exist then we will need to say that B extends A, not A with M, and we would then again require a @reopen annotation or an interface (or stronger) annotation on B, because A is interface and B is not.

In any case, there should be a diagnostic message from implicit_reopen in the case where a class (like C above) can inherit an instance member implementation from an interface (or stronger) class in a different library (like A above), and no @reopen annotation is encountered at the point where the interface property is dropped and the member is still inherited.

Expected behavior

A lint message is emitted at the declaration of the class B. Currently, no such message is emitted.

@eernstg eernstg added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Apr 24, 2023
@lrhn
Copy link
Member

lrhn commented Apr 24, 2023

The current specification of modifiers try to say nothing about the anonymous mixin applications, by declaring everything in terms of declarations. We should still be aware of the anonymous mixin application classes, but they should not affect anything relative to modifiers.

I usually try to introduce a name for the transitive-through-sealed properties. so:

  • A class or mixin declaration prohibits inheritance if it's marked final or interface, or if it's marked sealed and it extends a class declaration or mixies in a mixin declaration which prohibits inheritance.

Then a class declaration is "reopening" if it extends a class declaration or mixies in a mixin declaration which prohibits inheritance, and it is not itself marked interface, final or sealed. (Aka. It does not itself prohibit inheritance.)

The B class here is reopening the restriction from its declared superclass A.

@srawlins srawlins added the P2 A bug or feature request we're likely to work on label Apr 24, 2023
@eernstg
Copy link
Member Author

eernstg commented Apr 24, 2023

The current specification of modifiers try to say nothing about the anonymous mixin applications, by declaring everything in terms of declarations.

Yes, I know. I was referring to the wording of the lint rule, and it says 'extends'. So we'd probably just need to adjust that rule to something like "a class is not annotated with @reopen, and it satisfies any of the following:

  • its declared superclass is marked interface or final and ...
  • ..."

The word 'transitively' in the second bullet should perhaps also be reconsidered (there is no reopening if we're taking two or more superinterface steps above that sealed class).

@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-false-negative P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

5 participants