search

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.19k stars 1.56k forks source link

Give warning for probable wrong inclusion of `=>` in block lambdas: `() => { foo(); }` #56121

Open lukehutch opened 3 months ago

lukehutch commented 3 months ago

It is easy to accidentally include => in a lambda definition that is supposed to be of the block form () {...}, especially if you're converting an expression lambda into a block lambda: () => { foo(); } is wrong, and should be () { foo(); }.

The error given here is on the semicolon: Expected to find '}'. This is a red herring (the compiler assumes the user is trying to return a Set from the lambda). There is no indication at all that the => is the problem.

Really if a semicolon is detected in a curly brace (Set) block that is being returned from an expression lambda, the user should be given an error message that the => is probably erroneous -- not that the ; is erroneous.

This is probably the most common problem I have in using Dart (I make this mistake all the time). Sometimes it takes me a while to figure out the problem, despite having run into this numerous times.

(Sorry if this is a dup -- I think I mentioned this in another issue somewhere, but I can't find that issue right now.)

srawlins commented 3 months ago

Ugh we still don't have this. But I think you're looking for https://github.com/dart-lang/linter/issues/2780.

lrhn commented 3 months ago

Moved to be an analyzer issue. Could be a general front-end issue, since it's about error reporting for invalid input.

The syntax with the semicolon is not valid Dart. The syntax without semicolon is valid Dart and has a meaning, so we can't change that. If you write var f = () => {g()}; as a good JavaScript programmer, then we can't help you from the language's side, because it's valid.

The analyzer might want to warn if g returns void, since you probably didn't mean to use the value as a set element then.

bwilkerson commented 3 months ago

Could be a general front-end issue, since it's about error reporting for invalid input.

Yes, I think the best way to resolve this would be for the parser to recognize that set literals can't contain a semicolon (outside of closures). Unfortunately, that would require a change to the parser that would violate our current design pattern.

The parser is designed to assume that the code is valid and to not do any recovery until that assumption is proven wrong. (That is, it doesn't spend time looking ahead in order to detect problems, it only does look ahead when required to disambiguate between two possible valid interpretations.) Fixing this problem in the parser would require looking ahead (after finding the => {) to determine whether the left brace is the start of a set literal or the start of a block.

Without making parser changes I don't see a good path forward on this issue, and I'm not convinced that we want to make that kind of a change to the parser.

The analyzer might want to warn if g returns void, since you probably didn't mean to use the value as a set element then.

That's what dart-lang/linter#2780 (referenced above) is about.

johnniwinther commented 3 months ago

cc @jensjoha