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
report a warning for returning a value within a forEach() callback #2138
Comments
This reminds me of https://code.google.com/p/dart/issues/detail?id=73 Regardless, I wonder if we can issue a hint here. Removed Type-Defect label. |
Added this to the Later milestone. |
Removed Priority-Unassigned label. |
I'm concerned about the number of false positives such a hint would produce, but it might make sense in some cases, so we might want to investigate. Removed Priority-Medium label. |
This comment was originally written by bslesinsky...@gmail.com But that's a silly use for forEach. Why doesn't it return void? This code is mistaken and should be fixed. If you really want to call a function that returns an int in a void context, the warning can easily be suppressed: void main() { This is similar to calling a function with a different number of parameters; you can't pass it directly, but must wrap it. Regarding casting an int function to a void function, warnings are typically reported by the Dart Editor (or compiler) using static analysis. It should use the static type, and therefore no warning should be reported. That's okay; the warning is to detect simpler mistakes by beginners, not every case. Dart's language spec has things to say about when warnings are reported, so perhaps this means changing the language spec. If it's a good idea then that should be okay. |
This comment was originally written by bsles...@gmail.com That makes things more complicated. However, I don't think we should let language-lawyering get in the way of helping users. It's better to use language-lawyering for good, that is by figuring out how to carve out an exception so that we can warn about mistaken usage. Otherwise there's no way to make progress by doing better static analysis. |
Removed this from the Later milestone. |
Removed Oldschool-Milestone-Later label. |
I think this is a great request for the linter. Accidental return for new users. |
This is actually a tricky case, because the inferred return type of a function literal will be based on what is actually returned in the case where this is a subtype of the return type which is required by the context: void main() {
void f(void Function() g) => print(g.runtimeType);
f(() => 42); // Prints '() => int'.
} So we won't force the function literal It is not an error to pass a value of type void h() {
return 42; // Error, returning an `int` with return type `void`.
} So this is actually an example of a case where we need "voidness preservation", which is the extension of the checks concerned with the type Voidness preservation as a lint is requested in #1124, and it has previously been discussed (e.g., dart-lang/sdk#30352, dart-lang/sdk#34455). So this issue provides an example which is highly motivating, but other than that it is actually a duplicate of #1124. I won't close it because it could be argued that this special case should be treated separately, but I think it's useful to at least be aware of this connection. |
This issue was originally filed by bslesinsk...@gmail.com
The following code is wrong, but looks right. No warning is reported at compile time and no error happens at runtime:
String findFirstError(Map m) {
m.forEach((k,v) {
if (!validate(k,v)) {
return "found a problem with ${k}";
}
});
return "ok";
}
By analogy with regular for loops this looks like it should return early when it finds the first error, but actually the return value from the callback is ignored, so it always returns "ok".
It seems like we should be able to infer that the callback's return type is void, and therefore any return statements within the callback should not return a value.
The text was updated successfully, but these errors were encountered: