Skip to content

missing strong_mode_invalid_cast_function for instance function (but shows up fine with top-level function) #35797

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
kevmoo opened this issue Jan 29, 2019 · 5 comments
Labels
devexp-server Issues related to some aspect of the analysis server legacy-area-analyzer Use area-devexp instead.

Comments

@kevmoo
Copy link
Member

kevmoo commented Jan 29, 2019

Dart VM version: 2.1.1-dev.3.2

Smells almost the same as #31858

Getting an error for a top-level function, but not an instance function – even though they have the same signature.

void main() {
  var bar = Map<String, int>.fromIterable([1, 2, 3],
      key: (k) => k.toString(), 
      value: valueConvert);
      // error: The function 'valueConvert' has type '(int) → int' that isn't of expected type '(dynamic) → int'. This means its parameter or return type does not match what is expected. (strong_mode_invalid_cast_function)
      // seems fair - crashes at runtime

  print(bar);

  var converter = Converter();
  var foo = Map<String, int>.fromIterable([1, 2, 3],
      key: (k) => k.toString(), 
      value: converter.valueConvert);
      // NO error! Also crashes at runtime!

  print(foo);
}

int valueConvert(int a) => a * 2;

class Converter {
  int valueConvert(int a) => a * 2;
}
@kevmoo kevmoo added legacy-area-analyzer Use area-devexp instead. devexp-server Issues related to some aspect of the analysis server labels Jan 29, 2019
@leafpetersen
Copy link
Member

This is WAI.

There's an implicit downcast in both cases. For literals, we can guarantee that the downcast will fail, so we make it an error. For the tear-off, it's possible that the downcast could succeed (since you could have overridden valueConvert with something of a more general type, so we can't issue the exact type error.

@kevmoo
Copy link
Member Author

kevmoo commented Jan 29, 2019

So we have a unnecessary_lambdas lint that says I can get rid of the lambda and do a tear off. I do – then I get this at runtime. Which is a REALLY crappy user experience.

Could/should the lint be updated to not make a suggestion in this case?

@kevmoo
Copy link
Member Author

kevmoo commented Jan 29, 2019

CC @pq RE unnecessary_lambdas lint

@leafpetersen
Copy link
Member

More than anything, I think this is (yet another) place where getting rid of implicit casts makes things much more straightforward. I assume that the lambda in question was because you had something like value : (x) => valueConvert(x), and the lint prodded you to change this to value : valueConvert?

Without implicit casts, the original code would look like: value : (x) => valueConvert(x as int) which makes it clear that the lambda is actually doing something.

It would be good if the lint could take the fact that an implicit cast is happening there into account - not sure if that information is available to it or not though.

@kevmoo
Copy link
Member Author

kevmoo commented Jan 29, 2019

value : (x) => valueConvert(x as int) worked

CC @munificent – I chatted w/ him about this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-server Issues related to some aspect of the analysis server legacy-area-analyzer Use area-devexp instead.
Projects
None yet
Development

No branches or pull requests

2 participants