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.28k stars 1.59k forks source link

proposal: `anonymous_functions_tojs` #59561

Open jezell opened 1 month ago

jezell commented 1 month ago

anonymous_functions_tojs

Description

Warns about the usage of await and untyped arguments and how they interact with .toJS

Details

Anonymous functions can cause confusing compile time errors when passed to toJS. Avoid the use of await and specify types when passing anonymous functions directly to toJS. Future and dynamic are not allowed in functions passed to toJS but this doesn't show up until compile time.

Kind

Guards against compile errors

Bad Examples

These examples taken from dart-webrtc issue investigating changes in compiler behavior with flutter beta:

https://github.com/dart-lang/sdk/issues/56950

This fails to compile because Future<None> is not valid for toJS

  web.window.addEventListener(
      'message',
      (web.MessageEvent event) async {
        onMessage?.call(event);
      }.toJS,
      false.toJS);

This fails to compile because dynamic event is not valid for toJS

  web.window.addEventListener(
      'message',
      (event) {
        onMessage?.call(event);
      }.toJS,
      false.toJS);

Good Examples

Add a few examples that demonstrate a “good” adoption of this lint’s principle.

  web.window.addEventListener(
      'message',
      (web.MessageEvent event) {
        onMessage?.call(event);
      }.toJS,
      false.toJS);

Discussion

At the moment the analyzer doesn't surface these compiler errors, but even if it did, the errors themselves can be confusing because dynamic and Future don't explicitly appear in these signatures.

bwilkerson commented 1 month ago

@dcharkes Are these issues that the analyzer should be reporting? If so, should they be lints that are conditionally enabled or should they be warnings that are enabled by default?

jezell commented 1 month ago

As a side note, this was especially confusing because the compiler behavior changed in the current flutter beta. If a linter rule made it into the stable release, it would probably save a lot of people head scratching.

mraleph commented 1 month ago

@bwilkerson I think you are meaning to ask @srujzs / @biggs0125. See also discussion on https://github.com/dart-lang/sdk/issues/54366

srujzs commented 1 month ago

I think this can be added under the umbrella of https://github.com/dart-lang/sdk/issues/54366. I don't see a reason this should ever be a lint you can opt-out of as it's a compile-time error already. We may also find it easier to do some of the error reporting here with the analyzer so it's more obvious what's wrong with the user code.