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.1k stars 1.56k forks source link

Inference of type arguments to generic method calls. #37863

Open mkustermann opened 5 years ago

mkustermann commented 5 years ago

For the following example:

class Wrapper<T> {}

doSomething<T>(Wrapper<T> wrapper, T value) {}

main() {
  final wrapper = Wrapper<int>();
  doSomething(wrapper, 'this is not an int');
}

the CFE will infer the type argument to doSomething as Object:

static method main() → dynamic {
  final x::Wrapper<core::int*>* wrapper = new x::Wrapper::•<core::int*>();
  x::doSomething<core::Object*>(wrapper, "this is not an int");
}

Some users find this, understandably, a bit confusing and would prefer to get a compile-time error for such code (which one obviously gets by changing doSomething(wrapper, ...) to doSomething<int>(wrapper, ...))

The dart analyzer's linter does not seem to flag this, even if always_specify_types is turned on. @stereotype441 Is there an existing lint for enforcing explicit type arguments to generic method calls?

/cc @lrhn @leafpetersen

eernstg commented 5 years ago

I think we should consider variance control explicitly for this discussion: If Wrapper<T> is considered to be properly covariant in T then it's not a problem to pass a Wrapper<int> as an actual argument where a Wrapper<Object> is required. Similarly, the string argument is also good enough for a receiver that wants an Object.

So the reason why this could be considered wrong is that Wrapper<T> "should" have been contravariant (or even invariant) in T, but for the fact that we can't (yet) specify that.

But if we allow class type parameters to have explicit variance modifiers we could manage the issue like this:

class Wrapper<in X> {...} // That is, `Wrapper<X>` is contravariant in `X`.

doSomething<T>(Wrapper<T> wrapper, T value) {}

main() {
  final wrapper = Wrapper<int>();
  doSomething(wrapper, 'this is not an int'); // Compile-time error.
}

wrapper would get the inferred type Wrapper<int> as before, but that's just not a subtype of Wrapper<Object> any more. Using inout X and making X invariant would also have the same effect, and could be used if Wrapper uses X in all kinds of variance positions. It would also be possible to use use-site invariance to get a similar effect.

So I think we could say that this kind of issue can be solved in known ways, and we might get there in a reasonably near future.

On the other hand, I don't think we can expect a static check on the current (unsoundly covariant) type parameters to be compatible with all the existing code that uses covariance in ways that are actually working, and still be able to flag any problems with the situation that we're considering here.

stereotype441 commented 5 years ago

cc @pq regarding Martin's question about the linter

bwilkerson commented 5 years ago

I don't know of a lint for this case, but @srawlins has been doing some work related to this.

srawlins commented 5 years ago

This wouldn't be covered by any of the static analysis I have lined up, but @leafpetersen did mention this case. Basically when LUB results in Object, but none of the input types were Object, that should be a red flag. It could be covered by strict-inference if we so desired.

NikolayZherebtsovGoogle commented 5 years ago

Not necessarily Object, it could be another type (e.g. GenericMessage):

doSomething(Wrapper wrapper, T value) {}

class Type1 extends MyType {} class Type2 extends MyType {}

final wrapper = Wrapper(); doSomething(wrapper, Type2());

esDotDev commented 3 years ago

It could be covered by strict-inference if we so desired. - Please!

redcartel commented 3 years ago

Not sure if this is a different bug or is covered by this issue

typedef void Builder<C extends Controller>(C controller);

class View<C extends Controller> {
  final Builder<C> builder;
  final C controller;
  View({required this.controller, required this.builder});
}

class Controller {}

class MyController extends Controller {
  String title = 'Title';
}

class MyPage {
  // works:
  var v = View(
      controller: MyController(),
      builder: (controller) {
        print(controller.title);
      });

  // does not:
  build() {
    return View(
        controller: MyController(),
        builder: (controller) {
          print(controller.title);
        });
  }
}

void main() {}

The declaration of the field v does not produce any error, but the function build() contains the same error discussed above.

Even though the analyzer infers the type of the View constructor in build() as

View<MyController> View({required MyController controller, required void Function(MyController) builder})

Making the generic explicit resolves the issue>

// now it works:
  build() {
    return View<MyController>(
        controller: MyController(),
        builder: (controller) {
          print(controller.title);
        });
  }

Dart SDK version: 2.13.0-150.0.dev (dev) (Thu Mar 18 13:37:37 2021 -0700) on "macos_x64"