Skip to content

Adding a lint: Extension methods crashed when called on a dynamic object #58289

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

Open
franklinyow opened this issue Dec 16, 2020 · 6 comments
Open
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-request P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@franklinyow
Copy link
Contributor

@chickenblood commented on Fri Nov 20 2020

Happens on MacOSX / Linux and reproducible in DartPad.

Dart version: Based on Flutter 1.23.0-18.1.pre Dart SDK 2.10.4

Consider the following code:

class MyClass {
  void foo() => print('foo');
}

extension Bar on MyClass {
  void bar() => print('bar');
}

void main() {
  dynamic val = MyClass();
  val.foo();
  val.bar(); // crashes
}

This compiles fine, but crashes at runtime because the extension method bar() cannot be found. The compiler should either fail with an error, or better still valid code should be generated that invokes the method at runtime.


@dnfield commented on Fri Nov 20 2020

This is by design: https://dart.dev/guides/language/extension-methods#static-types-and-dynamic

I have no idea if or how this could be turned into a compile time error - someone from the language team could better answer that.


@dnfield commented on Fri Nov 20 2020

It could probably at least be a lint.


@dnfield commented on Fri Nov 20 2020

@leafpetersen would know the langauge question, @pq the lint


@leafpetersen commented on Fri Nov 20 2020

For better or worse the contract with dynamic is that the compiler just believes that you know what you're doing. I think this would be a fine candidate for an opt in lint though.


@pq commented on Fri Nov 20 2020

Yeah. This seems to fit in a general category of advice that recommends avoiding dynamic almost altogether.

@chickenblood: can you provide some context? Why would you type val dynamic in practice?


@chickenblood commented on Fri Nov 20 2020

I try to avoid dynamic altogether. In this case I was consuming a Flutter API that returns a dynamic type.

https://api.flutter.dev/flutter/widgets/DragTargetMove.html

I did not realize this since most of the callbacks for this class are typed

I plan to file a bug on that API. However, I would still hope that the linter could help us in cases like this. It is often not apparent that an implicit conversion is taking place.


@vsmenon commented on Fri Nov 20 2020

There are a couple lints around implicit dynamics. I'm not sure they capture this case. @chickenblood - do you have an example of where the dynamic call is coming from? The dynamic here is the type parameter, so not obvious.


@chickenblood commented on Mon Nov 23 2020

Sure. I provided a callback for DragTarget.onMove and in there called details.data.doThing() (doThing() being the extension method).

DragTarget(
    …
    onMove: (details) {
       details.data.doThing();
    }
),

Because details.data is dynamic, the failure occurs.

DragTarget.onAccept and DragTarget.onAcceptWithDetails do not have this problem because the argument is typed.

The lint sounds like a very good idea here (as well as fixing the DragTarget API).


@devoncarew commented on Wed Dec 16 2020

@franklinyow - it sounds like the resolution for this issue is for a lint request? If that's the case, I'd update the title to reflect that, and move the issue to the dart-lang/linter repo.

@natebosch
Copy link
Member

@franklinyow - the native github "transfer issue" functionality works much better than the zenhub transfer.

@natebosch
Copy link
Member

I do think it would be nice to have a lint to disallow any dynamic calls. implicit-dynamic: false is intended around dynamic values, but I'm not aware of a lint to disallow dynamic calls.

@bwilkerson
Copy link
Member

@srawlins

@srawlins
Copy link
Member

That would be #57703, "I wish we had --no-dynamic-calls as a lint"

I'm sure this would be the easiest thing to write :P

See also #58082 "Request for lint: unnecessary_dynamic"

@leafpetersen
Copy link
Member

leafpetersen commented Dec 17, 2020

@srawlins I've actually been thinking that this might be a good addition to our strict checking proposal. That is, if an extension method would apply at type Object? but does not apply because the type is dynamic, then make it a strict error. WDYT?

@srawlins
Copy link
Member

Interesting idea. When you say "if an extension method would apply at type Object?, do that only include methods declared in extension E on Object? {}, instead of on MyClass, as above? For example, such a check would catch both lines below if it considered the m extension method on D, even though the first is not a runtime error:

class C {
  String m() => 'C.m';
}

class D {}

extension E on D {
  String m() => 'D extension.m';
}

void main() {
  dynamic c = C();
  print(c.m());
  
  dynamic d = D();
  print(d.m());
}

It would catch neither line of it only considered methods from extension E on Object?, but it would catch the correct one if E were declared on Object? rather than on D.

@pq pq added type-enhancement A request for a change that isn't a bug linter-lint-request labels Dec 23, 2020
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
@pq pq added the P3 A lower priority bug or feature request label Nov 20, 2024
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-request 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

7 participants