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: type_local_variable_instead_of_casting #58903

Open
5 tasks
srawlins opened this issue Oct 25, 2022 · 2 comments
Open
5 tasks

proposal: type_local_variable_instead_of_casting #58903

srawlins opened this issue Oct 25, 2022 · 2 comments
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@srawlins
Copy link
Member

type_local_variable_instead_of_casting

Please respond with a better name if you have one. prefer_type_annotation_over_cast?

Description

This rule encourages one to "cast" (generally upcast) an expression in a new local variable via the variable's declaration rather than an as-expression. I hope that's crystal clear :)

Details

Sometimes an expression is upcast via as, such as:

var bar = foo.map((e) => e.toString()).toList() as Iterable<String>;
if (iterable.isEmpty) bar = someIterable;

// or
var x = [1, 2, 3] as List<num>;
x.add(0.5);

Errors would arise if either cast were removed. However, we could make the code more idiomatic by casting with the variable's declaration (side-stepping inference):

Iterable<String> bar = foo.map((e) => e.toString()).toList();
if (iterable.isEmpty) bar = someIterable;

// or
List<num> x = [1, 2, 3];
x.add(0.5);

This arises from lots of discussion in #48984, and examples in #44411, #49269, and #49550.

Kind

Style.

Discussion

See discussion at #48984.

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.
@eernstg
Copy link
Member

eernstg commented Oct 25, 2022

I like the

(generally upcast)

part, because this presumably means that we would lint the following bug as well:

var bar = foo.map((e) => e.toString()).toList() as Iterable<int>; // Oops, will throw.

because we're linting the syntactic pattern var v = e as T;, not just the situation where e as T is an upcast.

@lrhn
Copy link
Member

lrhn commented Oct 25, 2022

Naming: prefer_variable_type_for_upcast.

It won't work for a downcast anyway, except from dynamic, and then it doesn't matter which version you use, it's either an explicit or implicit as cast. (I guess dart2js omits the implicit cast, but I wouldn't push the user in either direction then, since they might want to control whether there is a runtime check or not.)

@pq pq added the P3 A lower priority bug or feature request label May 22, 2023
copybara-service bot referenced this issue Oct 24, 2023
… type

Fixes #44411
Fixes #48984
Fixes #49269
Fixes #49550
Fixes #52086

The "prefer type annotations on local variables over upcasting" that this used to cover can be covered with a new lint rule: https://github.com/dart-lang/linter/issues/3786

Change-Id: I6a112732269e918a7faacc399bccebe13de8462d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/265420
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Mar 27, 2024
@devoncarew devoncarew added analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants