Skip to content

[Class Modifiers] Add induced modifiers to analyzer element model #51671

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.

Sign up for GitHub

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

Jump to bottom
Closed
kallentu opened this issue Mar 8, 2023 · 1 comment
Closed

[Class Modifiers] Add induced modifiers to analyzer element model #51671

kallentu opened this issue Mar 8, 2023 · 1 comment
Assignees
@kallentu
Labels
dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec legacy-area-analyzer Use area-devexp instead. P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Milestone
Dart 3 beta 3

Comments

@kallentu
Copy link
Member

kallentu commented Mar 8, 2023

Implement a new algorithm that takes advantage of the existing tree-traversal in strong_mode.dart to mark induced modifiers on classes and mixins.

Potentially also store an explicit final element and an inducing final element to avoid another traversal later for linting/error reporting.
@kallentu kallentu added this to the Dart 3 beta 3 milestone Mar 8, 2023
@kallentu kallentu mentioned this issue Mar 8, 2023
Closed
@srawlins srawlins added P1 A high priority bug; for example, a single project is unusable or has many test failures legacy-area-analyzer Use area-devexp instead. type-enhancement A request for a change that isn't a bug dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec labels Mar 8, 2023
copybara-service bot pushed a commit that referenced this issue Mar 20, 2023
@kallentu
[analyzer] Mark induced modifiers on sealed types.
fde66e9 
Changed the classes to store induced modifiers on `isBase`, `isFinal`, and `isInterface`. If one of these getters are true and `isSealed` is true, then we assume it's an induced modifier. (Since a class or mixin cannot be both sealed and another modifier).

There's a subtle change here where the induced modifiers are only marked on sealed classes which means that classes that subclass/subtype non-sealed classes won't get any base or final errors. This will also affect how lints will be reported -- at the next erroneous subtype and not downstream from that type.

Also, the context messages now point to the class with the explicit modifier inciting the error.

Bug: #51671

Change-Id: I4c7541d956382dcb3917a4b8d73756840880c52f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/289406
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Phil Quitslund <[email protected]>
@kallentu
Copy link
Member Author

If an element's isSealed is true and one of isBase, isInterface, or isFinal is true, then the element has an induced (non-sealed) modifier.
If an element's isSealed is false and one of isBase, isInterface, or isFinal is true, then the element has an explicit (non-sealed) modifier.

We ended up not using more space to store induced information as these are pretty rare edge cases and most elements won't have them.

The work for this is done though. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kallentu kallentu

Labels
dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec legacy-area-analyzer Use area-devexp instead. P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Projects
None yet
Milestone
Dart 3 beta 3
Development

No branches or pull requests
2 participants
@srawlins @kallentu