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

[analyzer] suggest truncate-divide when double-from-divide is used #50942

Open
srawlins opened this issue Jan 8, 2023 · 2 comments
Open

[analyzer] suggest truncate-divide when double-from-divide is used #50942

srawlins opened this issue Jan 8, 2023 · 2 comments
Labels
analyzer-ux area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. 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

srawlins commented Jan 8, 2023

Forked from dart-lang/linter#3930 (comment)

@rakudrama suggests a new error message for code like this:

String suffix(String s) => s.substring(s.length / 2);

Currently this code produces an error, argument_type_not_assignable: "The argument type 'double' can't be assigned to the parameter type 'int'." But it could suggest using /~ instead of /. Then users would be less likely to write (s.length / 2).toInt(), and we could retire the division_optimization hint (which fires for code like (s.length / 2).toInt().

However, to make a sufficiently complete replacement, we'd have to catch more than argument_type_not_assignable. I can think of the following other cases:

// return_of_invalid_type
int f2(String s) => s.length / 2;

// invalid_assignment
int x = "hello".length / 2;

// list_element_type_not_assignable
List<int> f2(String s) => [7, s.length / 2];

I'm sure there are more.

Also, is it sufficient to catch x / y as int? Or would we want to go deeper and catch, for example, (x / y) + 1?

CC @rakudrama @bwilkerson

@srawlins srawlins added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request analyzer-ux type-enhancement A request for a change that isn't a bug labels Jan 8, 2023
@bwilkerson
Copy link
Member

I suspect that the rule we'd want here is to report this situation when an expression has type double when int is required and the expression is a binary expression using / (and the operands are a combination of double or int that would be valid for ~/).

It's an interesting question as to whether we'd want to catch cases where a subexpression of that form caused the problem. I'd want to understand better how complex the logic would be and compare that to the value to users (cost/benefit analysis).

The cast issue seems different in some ways. I just tried this:

void main() {
  f(12.0, 3);
}

void f(double x, int y) {
  print((x / y) as int);
}

which throws an exception. I guess I expected to be told that the cast would always fail, but I wasn't. Maybe that's a lint that I don't have enabled?

@rakudrama
Copy link
Member

rakudrama commented Jan 9, 2023

With web numbers, where all num are double and some double are int, (x / y) as int passes when y divides x exactly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-ux area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. 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

3 participants