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

omit_local_variable_types is not safe with inferred type from generic method #57967

Closed
xvrh opened this issue May 27, 2019 · 4 comments
Closed
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.

Comments

@xvrh
Copy link
Contributor

xvrh commented May 27, 2019

This code with the rule omit_local_variable_types

main() {
  List<int> list1 = createList(); // currently: LINT
  var list2 = createList();

  print(list1.runtimeType); // List<int>
  print(list2.runtimeType); // List<dynamic>

  Future<int> future1 = createFuture(); // currently: LINT
  var future2 = createFuture();

  print(future1.runtimeType); // Future<int>
  print(future2.runtimeType); // Future<dynamic>
}

List<T> createList<T>() => [];
Future<T> createFuture<T>() async => null;

The linter emits 2 warnings to remove the local type for list1 and future1 (Omit type annotations for local variables).

However applying the change is not safe because it changes the inferred type and the runtime type.

I think the linter should not report a lint in this case.

@eernstg
Copy link
Member

eernstg commented May 27, 2019

There is a specific thing that you can do here (which eliminates the need to change the linter): Add explicit type arguments to the generic function calls.

main() {
  var list2 = createList<int>();
  print(list2.runtimeType); // List<int>

  var future2 = createFuture<int>();
  print(future2.runtimeType); // Future<dynamic>
}

List<T> createList<T>() => [];
Future<T> createFuture<T>() async => null;

Those generic functions would typically depend on their context to obtain type arguments, because there are no parameters (and the situation would be similar if some parameter type annotations exist, but they do not mention some of the type parameters). There's "nothing wrong" in giving explicit type arguments to createList and createFuture, because they are inherently dependent on getting that information from somewhere else than the (pure value part of the) invocation.

So if the applicable coding standards specify that local variables shouldn't have type annotations then we need to provide this information in some other way. Explicit type arguments on the call is one way to do it.

@xvrh
Copy link
Contributor Author

xvrh commented May 27, 2019

Ok, thanks.
You are correct. And now I realized that this is even the case for simpler things like:

List<int> list = [];

We would lose type information if converted mechanically.

That just make me realise that the lint is a bit dangerous and should be enabled cautiously.

@xvrh xvrh closed this as completed May 27, 2019
@Blasanka
Copy link

Blasanka commented Jan 23, 2020

What is the solution for below case:

      // ignore: omit_local_variable_types
      int years = (dif.inDays / 365).floor();

@eernstg
Copy link
Member

eernstg commented Jan 23, 2020

That case should not create any problems: The return type of floor is int, so there is no loss of typing precision if the declaration is changed to var years = (dif.inDays / 365).floor();.

If the type annotation int is intended to document the developer's intentions and serve as a check on the sanity of the initializing expression then removing it will eliminate that sanity check, so you could say that this is a loss of information at the software engineering level, but that's a different discussion. Cf. googlearchive/pedantic#45.

@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 18, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 18, 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.
Projects
None yet
Development

No branches or pull requests

4 participants