Skip to content
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

[Class modifiers] Linter implementation #3917

Closed
2 tasks done
pq opened this issue Dec 15, 2022 · 7 comments
Closed
2 tasks done

[Class modifiers] Linter implementation #3917

pq opened this issue Dec 15, 2022 · 7 comments
Labels
new-language-feature 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

Comments

@pq
Copy link
Member

pq commented Dec 15, 2022

New class modifiers

This issue tracks the linter implementation of the new class modifiers feature.

Specs

@pq pq added type-enhancement A request for a change that isn't a bug new-language-feature P1 A high priority bug; for example, a single project is unusable or has many test failures labels Dec 15, 2022
@pq pq added this to the Dart 3 beta 1 milestone Dec 15, 2022
@pq
Copy link
Member Author

pq commented Dec 15, 2022

The spec says sealed classes are implicitly abstract classes:

A class marked sealed is implicitly an abstract class with all of the existing restrictions and capabilities that implies.

and so we should look at lints that treat abstract classes specially.

One such lint is one_member_abstracts. That said, my initial reaction is that we do not want to extend it to lint one member sealed classes since sealed classes can't just be replaced with functions (as the lint advice suggests).

@munificent?

@srawlins
Copy link
Member

The spec says @sealed classes are implicitly abstract classes:

You mean sealed here, right? Not @sealed?

@pq
Copy link
Member Author

pq commented Dec 15, 2022

Ha! Yes, absolutely. 😬

EDIT: fixed. Thanks!

@munificent munificent changed the title [NCM] Linter implementation [Class modifiers] Linter implementation Dec 19, 2022
@munificent
Copy link
Member

That said, my initial reaction is that we do not want to extend it to lint one member sealed classes since sealed classes can't just be replaced with functions (as the lint advice suggests).

👍

I wonder if one_member_abstracts still carries its weight. I added that corresponding rule to "Effective Dart" back in the days when many programmers still wrote Java in whatever language they were using and weren't comfortable with closures. These days, it seems like everyone is pretty fluent with lambdas and aren't likely to think they need to write an entire interface just for a single operation any more.

@pq
Copy link
Member Author

pq commented Dec 19, 2022

I wonder if one_member_abstracts still carries its weight.

I wonder this too. I'm not sure it merits deprecation quite but just shy maybe?

In any event, the initial issue seems settled, we will not upgrade it to be sealed class aware.

Thanks for the feedback!

@bwilkerson
Copy link
Member

Actually, we'll need to have a test in order to not produce the diagnostic when the sealed modifier exists. It's not so much a case of making it work with the new syntax as it is a matter of preventing it from working when the new syntax is being used.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Jan 25, 2023
See: dart-lang/linter#3917

Change-Id: I9652fabcbac37644cfca89227b6830e32a79576d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279657
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Phil Quitslund <[email protected]>
@pq pq modified the milestones: Dart 3 beta 1, Dart 3 beta 2 Feb 2, 2023
@pq
Copy link
Member Author

pq commented Mar 16, 2023

Closing in favor of individual tracking issues.

@pq pq closed this as completed Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-language-feature 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
Development

No branches or pull requests

4 participants