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.21k stars 1.57k forks source link

Suppress `unnecessary_cast` to `Function` when the cast is allowing violation of `avoid_dynamic_calls` #56939

Open natebosch opened 2 hours ago

natebosch commented 2 hours ago

The pattern for allowing a dynamic call when the avoid_dynamic_calls is enabled is to explicitly cast to dynamic or Function in the expression that receives the dynamic call. Such a cast should suppress the unnecessary_cast diagnostic. When we cast to Function to allow the dynamic call, we end up with an unnecessary cast.

void foo(Function callback) {
  argument.something(); // avoid_dynamic_calls - this is a correct diagnostic, we want the dynamic function call to be obvious
  (argument as Function).something(); // unnecessary_cast - this should be suppressed
}

Idiomatic usage of the lint is to keep references as Object? throughout as much of the code as possible, and introduce the dynamic only within the expression where a dynamic call is made. We assume backends will optimize a cast from Object? to dynamic. Neither dynamic or Object? is more specific than the other so we lose nothing when we change all dynamic to Object?. Function is more specific than Object?, so we would lose information changing all Function to Object or Object? just to be allowed to cast.

I think we should allow specifically casting a Function to Function when avoid_dynamic_calls is enabled.

dart-github-bot commented 2 hours ago

Summary: The unnecessary_cast diagnostic incorrectly flags casts to Function when used to allow dynamic calls while avoid_dynamic_calls is enabled. This is because the cast is necessary to explicitly allow the dynamic call, even though it appears redundant.