Skip to content

proposal: no constructor should give const suggestion? #57107

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
stephane-archer opened this issue Jul 6, 2024 · 9 comments
Closed

proposal: no constructor should give const suggestion? #57107

stephane-archer opened this issue Jul 6, 2024 · 9 comments
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead.

Comments

@stephane-archer
Copy link

stephane-archer commented Jul 6, 2024

moving from:

class _FalseColorSwitch extends StatelessWidget {

  @override
  Widget build(BuildContext context) {

to

class _FalseColorSwitch extends StatelessWidget {
  const _FalseColorSwitch(); // adding a const constructor that does nothing

  @override
  Widget build(BuildContext context) {

give me so many linter optimization suggestions:

Use 'const' with the constructor to improve performance.
Try adding the 'const' keyword to the constructor invocation.

I think no constructor should give me the same recommendation as a const constructor that does nothing. is there an actual difference?

@srawlins srawlins transferred this issue from dart-archive/linter Jul 6, 2024
@srawlins
Copy link
Member

srawlins commented Jul 6, 2024

Moving over to the language repo for the language request. I think this has been asked and answered before ("const by default"), but someone here can point to the past response.

@mateusfccp
Copy link
Contributor

Refer to dart-lang/language#1410. I don't know if it's the exact same case, but it has a related discussion explaining how some things work.

@lrhn
Copy link
Member

lrhn commented Jul 7, 2024

The reason you don't get const without asking for it, is that you might get const without wanting it.

A constructor being const is a promise to always allow const construction of the class. Changing that is a breaking change.
That kind of promise is not something the system should make for you, it's something you have to explicitly state.

If not, you could accidentally have a class that supports const construction in one version of a package, and not in the next one, because you added a mutable cache for some variable, and never even considered that the class could be const, because it was never intended as such. (So it's not tested.)
And that could break someone else, who relied on the implicit const-ness of the former version.

(I have no opinion on what lints should suggest here, but the default constructor you get if you write nothing is not a const constructor, whether it could be or not.)

@stephane-archer
Copy link
Author

stephane-archer commented Jul 8, 2024

@lrhn I understand now why the constructor is not const by default if it can.
I think a linter suggestion to add a const constructor would be nice.
That way people don't miss the opportunity for extra optimization and it's explicit that they have a const constructor, so the breaking change will be clear to anyone (especially removing it).

@lrhn
Copy link
Member

lrhn commented Jul 8, 2024

Ack. That's why I consider this an analyzer or linter issue, not a language issue.

@stephane-archer
Copy link
Author

@lrhn I agree, that is why it was originally in dart-lang/linter but @srawlins transferred it here.
Could you please transfer it back?

@bwilkerson bwilkerson transferred this issue from dart-lang/language Jul 8, 2024
@srawlins
Copy link
Member

srawlins commented Jul 8, 2024

Thanks for the clarification 👍

@srawlins
Copy link
Member

srawlins commented Jul 8, 2024

Looks like a duplicate of #59001 and #59273.

@stephane-archer
Copy link
Author

I think it's a duplicate too, thanks for pointing out existing issues.

@stephane-archer stephane-archer closed this as not planned Won't fix, can't repro, duplicate, stale Jul 8, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 15, 2024
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 15, 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.
Projects
None yet
Development

No branches or pull requests

5 participants