Skip to content

prefer_const_literals_to_create_immutables support for extension types #59276

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

Closed
Tracked by #58838
pq opened this issue Aug 25, 2023 · 4 comments
Closed
Tracked by #58838

prefer_const_literals_to_create_immutables support for extension types #59276

pq opened this issue Aug 25, 2023 · 4 comments
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-new-language-feature linter-set-flutter P2 A bug or feature request we're likely to work on

Comments

@pq
Copy link
Member

pq commented Aug 25, 2023

Related to #59275,

extension type E(List<int> i) { }

var e = E([1]);

should we treat E as immutable and produce a prefer_const_literals_to_create_immutables lint on [1]?

/fyi @goderbauer @a14n @eernstg

@pq pq added P2 A bug or feature request we're likely to work on linter-new-language-feature labels Aug 25, 2023
@bwilkerson
Copy link
Member

The documentation for the lint doesn't make it clear what the motivation for the lint is, and I don't remember it. Could you describe the motivation? It's hard to know how to answer the question without understanding what the lint is trying to accomplish.

@lrhn
Copy link
Member

lrhn commented Aug 26, 2023

Presumably, the @immutable annotations is intended to convey that the class instances should be deeply immutable.
By passing in only deeply immutable arguments to the constructor, that is more likely to stay true, and the only way to ensure that arguments are deeply immutable is that they are constant expressions. (Or, presumably, instances of other @immutable-annotated classes.)

The documentation for @immutable isn't that clear. I'm guessing it shows intent to be deeply immutable, but the only direct effect it has on the annotated class is to require all fields to be final. This lint is trying to force you towards full immutability.

@pq
Copy link
Member Author

pq commented Aug 28, 2023

Presumably, the @immutable annotations is intended to convey that the class instances should be deeply immutable.

I think this is right.

This lint is trying to force you towards full immutability.

And this.

copybara-service bot referenced this issue Aug 31, 2023
Closes: https://github.com/dart-lang/linter/issues/4722

Change-Id: Ifd60ab27242e802638c072e5957c5c35fe89c399
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/323682
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Phil Quitslund <[email protected]>
@pq
Copy link
Member Author

pq commented Aug 31, 2023

Fixed w/ 039640f.

@pq pq closed this as completed Aug 31, 2023
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 20, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-new-language-feature linter-set-flutter P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

4 participants