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

Proposal prefer_const_unmodified_static_declarations #5049

Open
FMorschel opened this issue Aug 7, 2024 · 5 comments
Open

Proposal prefer_const_unmodified_static_declarations #5049

FMorschel opened this issue Aug 7, 2024 · 5 comments
Labels
lint-proposal set-flutter Affects a rule in the recommended Flutter rule set status-pending type-enhancement A request for a change that isn't a bug

Comments

@FMorschel
Copy link
Contributor

FMorschel commented Aug 7, 2024

Describe the issue
With the folwing code:

class A {
  const A();

  static A a = A();
}

We get a warning for the const keyword at the constructor in line 4.

After placing it there, it doesn't warn about the static A a declaration being possibly constant.

Expected behavior
Warn at the static declaration that a variable can be const.

@FMorschel FMorschel changed the title tatic instances could also be transformed with prefer_const_constructors Static instances could also be transformed with prefer_const_constructors Aug 7, 2024
@github-actions github-actions bot added the set-flutter Affects a rule in the recommended Flutter rule set label Aug 7, 2024
@bwilkerson bwilkerson added lint-proposal status-pending and removed set-flutter Affects a rule in the recommended Flutter rule set labels Aug 7, 2024
@bwilkerson
Copy link
Member

I suspect that this would want to be a separate lint as it would probably want to flag any static final field whose initializer is a constant expression.

I included 'final' in the description above because changing a variable (especially a public variable) from static to static const is a significant change. If the field is already final then changing it to const is mostly just an optimization.

@github-actions github-actions bot added the set-flutter Affects a rule in the recommended Flutter rule set label Aug 7, 2024
@FMorschel
Copy link
Contributor Author

FMorschel commented Aug 7, 2024

I included 'final' in the description above because changing a variable (especially a public variable) from static to static const is a significant change.

So if the static variable is private or if the class itself is private the lint could show. I agree.

I'm not so good in naming things and I can't seem to find your issue on naming lints (for wording and stuff - saw it once when browsing here) so I'll let others suggest a name for it.

Edit

I suspect the name of the lint on the title won't let you remove the tag.

Edit 2

Found the docs for that https://github.com/dart-lang/linter/blob/main/doc/writing-lints.md#lint-properties. Will think of a name later, just wanted to place it here for reference.

@FMorschel
Copy link
Contributor Author

FMorschel commented Aug 12, 2024

prefer_const_unmodified_static_declarations

Description

Should use const for static variables that are declared as final but are initialized with a constant value.

Details

Promoting the practice of marking static variables as const instead of final when they are initialized with a constant value. Using const ensures that the variables are compile-time constants, which can improve performance and code clarity by avoiding unnecessary allocations and guaranteeing immutability.

Kind

This rule enforces style advice and promotes code optimization by encouraging the use of const for static variables that are unmodified.

Bad Examples

static final myList = const [1, 2, 3];
static final myMap = const {'key': 'value'};

Good Examples

static const myList = [1, 2, 3];
static const myMap = {'key': 'value'};

Discussion

This proposal addresses a gap in the current Dart lints by targeting static final variables that could be declared as const. By ensuring that these variables are declared as const where applicable, the lint helps prevent missed opportunities for optimization and promotes clearer, more maintainable code.

I believe this should be added to Effective Dart. Since it is an extension of prefer_const_declarations.

Discussion checklist

  • List any existing rules this proposal modifies, complements, overlaps or conflicts with.
  • List any relevant issues (reported here, the [SDK Tracker], or elsewhere).
  • If there's any prior art (e.g., in other linters), please add references here.
  • If this proposal corresponds to [Effective Dart] or [Flutter Style Guide] advice, please call it out. (If there isn't any corresponding advice, should there be?)
  • If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

@FMorschel FMorschel changed the title Static instances could also be transformed with prefer_const_constructors Proposal prefer_const_unmodified_static_declarations Aug 12, 2024
@bwilkerson
Copy link
Member

@munificent (For the suggestion of adding this to Effective Dart.)

@munificent
Copy link
Member

If the field is already final then changing it to const is mostly just an optimization.

Mostly, but it does mean that the author of that API is committing to that static field being usable in constant expressions and it becomes a breaking API change to remove the const.

@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lint-proposal set-flutter Affects a rule in the recommended Flutter rule set status-pending type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants