Skip to content

Lint when local variables could be final #57195

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
eernstg opened this issue Mar 11, 2015 · 9 comments
Closed

Lint when local variables could be final #57195

eernstg opened this issue Mar 11, 2015 · 9 comments
Assignees
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-lint-request type-enhancement A request for a change that isn't a bug

Comments

@eernstg
Copy link
Member

eernstg commented Mar 11, 2015

The style guide says 'PREFER using var without a type annotation for local variables' (https://www.dartlang.org/articles/style-guide/#prefer-using-var-without-a-type-annotation-for-local-variables), which is sufficiently flexible to allow for an additional (per project or similar) rule along the lines of 'PREFER using final for local variables when this does not conflict with its usage'.

The rationale for this is that the explicit final annotation helps programmers thinking clearly about the distinction between variables that simply stand for a value (final ones), and variables whose usage includes mutation (the remaining ones). For a mutated variable, the interplay between the control flow and the assignments to this variable must be taken into account when understanding what it does, or how to correctly modify the explicit occurrences of this variable, or how to correctly modify the control flow; hence, every final in the code simplifies comprehension.

In order to help programmers maintain this discipline, it would be useful to have a lint diagnostic on cases where a variable is never mutated, but still it is not marked as final.

A similar argument could be made for formal parameters in functions/methods, but presumably there is a culture around the treatment of such formals which is so strict that it is acceptable to outlaw mutation at all (and a lint diagnostic is given for all mutations, as proposed in #57194).

@eernstg eernstg added type-enhancement A request for a change that isn't a bug linter-lint-request labels Mar 11, 2015
@zoechi
Copy link
Contributor

zoechi commented Mar 11, 2015

+1 I use final everywhere and only change it to var or some concrete type when it should be mutable because of the reason mentioned by @eernstg.
I think to remember that this also helps dart2js to generate more efficient code.

@polux
Copy link
Contributor

polux commented Mar 11, 2015

Not sure votes matter here, but in case they do: +1 :)

@seaneagan
Copy link

'PREFER using var ...' and 'PREFER using final ...' are in conflict. Maybe there can be separate rules for avoiding type annotations 'AVOID type annotating local variables' and for using var: 'PREFER using var for local variables'. Then projects that prefer final when applicable can just disable the latter rule. If there is a desire to change the rule in general for all projects, I think that should be hashed out in the style guide directly, not by creating a conflicting rule for the linter.

@eernstg
Copy link
Member Author

eernstg commented Mar 11, 2015

The point made on another thread was that 'PREFER' is inherently flexible, which means that there can be good reasons to make another choice (like final x.. rather than var x). Whether it's a conflict to have another rule saying 'PREFER' something else --- I'm not sure, but I didn't intend to take that discussion here. So please just read it as if I had said "it does not violate the style guide to use final with local variables". I'd love to have the style guide adjusted to say 'PREFER final or var .. with local variables', but that discussion is infinite. ;-)

@seaneagan
Copy link

The point made on another thread was that 'PREFER' is inherently flexible, which means that there can be good reasons to make another choice (like final x.. rather than var x).

Right, I'm just saying that the way to make a different choice is to disable the rule for your project, not to create a separate conflicting rule.

Whether it's a conflict to have another rule saying 'PREFER' something else --- I'm not sure, but I didn't intend to take that discussion here.

One can't use var and final simultaneously, so I'd say it's a conflict ;)

I'd love to have the style guide adjusted to say 'PREFER final or var .. with local variables', but that discussion is infinite. ;-)

I think when one disagrees with a style guide rule, they have two options:

  • Get the style guide changed
  • Disable the rule for their projects via the linter config

Separate rules for not type annotating and var vs. final would at least allow one to disable only the part the part they disagree with. There are probably a lot of style guide rules that could be split up for better configurability.

@eernstg
Copy link
Member Author

eernstg commented Mar 11, 2015

On Wed, Mar 11, 2015 at 5:38 PM, Sean Eagan [email protected]
wrote:

The point made on another thread was that 'PREFER' is inherently flexible,
which means that there can be good reasons to make another choice (like
final x.. rather than var x).

Right, I'm just saying that the way to make a different choice is to
disable the rule for your project, not to create a separate conflicting
rule.

Whether it's a conflict to have another rule saying 'PREFER' something
else --- I'm not sure, but I didn't intend to take that discussion here.

One can't use var and final simultaneously, so I'd say it's a conflict ;)

Sure, there is a conflict, but it wasn't my intention to discuss the style
guide rules, I just wanted to give a hint that the style guide discussion
already showed the consensus that using final on locals would not be a
violation.

I'd love to have the style guide adjusted to say 'PREFER final or var ..
with local variables', but that discussion is infinite. ;-)

I think when one disagrees with a style guide rule, they have two options:

  • Get the style guide changed
  • Disable the rule for their projects via the linter config

Separate rules for not type annotating and var vs. final would at least
allow one to disable only the part the part they disagree with. There are
probably a lot of style guide rules that could be split up for better
configurability.

Indeed!

best regards,

Erik Ernst - Google Danmark ApS
Skt Petri Passage 5, 2 sal, 1165 København K, Denmark
CVR no. 28866984

@srawlins
Copy link
Member

How could you tell if a field is not used, if it is public? This would yield more false positives than real lint, for most code, I imagine.

@zoechi
Copy link
Contributor

zoechi commented Nov 22, 2016

I think this is fixed

@alexeieleusis
Copy link
Contributor

http://dart-lang.github.io/linter/lints/parameter_assignments.html taked care of the last part regarding parameters reassignments.

@alexeieleusis alexeieleusis self-assigned this Nov 22, 2016
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 18, 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-lint-request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants