fix #25573, add option to disable implicit dynamic

pkg/analyzer/test/src/task/strong/checker_test.dart

Index: pkg/analyzer/test/src/task/strong/checker_test.dart
diff --git a/pkg/analyzer/test/src/task/strong/checker_test.dart b/pkg/analyzer/test/src/task/strong/checker_test.dart
index 3286d2131411599d24520ff3373b5a8790753544..4a32cae58b667d2ea88aef360f814f3dea2db97a 100644
--- a/pkg/analyzer/test/src/task/strong/checker_test.dart
+++ b/pkg/analyzer/test/src/task/strong/checker_test.dart
@@ -315,6 +315,16 @@ test() {
 ''');
   }
 
+  void test_constructorInvalid() {
+    // Regression test for https://github.com/dart-lang/sdk/issues/26695
+    checkFile('''
+class A {
+  B({ /*error:FIELD_INITIALIZER_OUTSIDE_CONSTRUCTOR*/this.test: 1.0 }) {}
+  final double test = 0.0;
+}
+''');
+  }
+
   void test_constructors() {
     checkFile('''
 const num z = 25;
@@ -349,16 +359,6 @@ void main() {
 ''');
   }
 
-  void test_constructorInvalid() {
-    // Regression test for https://github.com/dart-lang/sdk/issues/26695
-    checkFile('''
-class A {
-  B({ /*error:FIELD_INITIALIZER_OUTSIDE_CONSTRUCTOR*/this.test: 1.0 }) {}
-  final double test = 0.0;
-}
-''');
-  }
-
   void test_conversionAndDynamicInvoke() {
     addFile(
         '''
@@ -1911,11 +1911,237 @@ main() {
     addFile('num n; int i = /*info:ASSIGNMENT_CAST*/n;');
     check();
     // TODO(jmesserly): should not be emitting the hint as well as the error.
+    // It is a "strong mode hint" however, so it will not be user visible.
     addFile(
         'num n; int i = /*info:ASSIGNMENT_CAST,error:INVALID_ASSIGNMENT*/n;');
     check(implicitCasts: false);
   }
 
+  void test_implicitDynamic_field() {
+    addFile(r'''
+class C {
+  var /*error:IMPLICIT_DYNAMIC_FIELD*/x0;
+  var /*error:IMPLICIT_DYNAMIC_FIELD*/x1 = (<dynamic>[])[0];

[Bob Nystrom, 2016/06/23 23:25:52]

This one is a little surprising to me. My intuition is that a value resulting
from an invocation on an explicit dynamic is also an explicit dynamic and would
be OK, but I don't have a strong opinion. I can see the value in flagging this.

I wonder what other people would think about this.

[Jennifer Messerly, 2016/06/24 01:15:56]

Yeah, I added it for illustration. The current technique lacks power to
implement different behavior. Think of it as a simple prototype -- Dart 2 may
want something more sophisticated.

The current model can be thought of as:
* find a place where type was omitted,
* see if dynamic was the conclusion,
* if so -> error

There are certainly other approaches. For example, we could internally track
"implicit dynamic" vs "explicit dynamic" as different types in Analyzer. That
seems tricky.

Another approach is to integrate this with inference. If we infer a type at all
(even if "dynamic"), then don't report an error. But we don't track inference
locations precisely enough to do that by the time we arrive in ErrorVerifier.

Perhaps there's a way to move the implementation into ResolverVisitor itself.
ResolverVisitor is a lot more hairy, which is why I implemented this in
ErrorVerifier, but I think that would get what you want without needing to
introduce any new types or inference tracking.

I'll reflect on it though.

+  var /*error:IMPLICIT_DYNAMIC_FIELD*/x2,
+      x3 = 42,
+      /*error:IMPLICIT_DYNAMIC_FIELD*/x4;
+  dynamic y0;
+  dynamic y1 = (<dynamic>[])[0];
+}
+    ''');
+    check(implicitDynamic: false);
+  }
+
+  void test_implicitDynamic_function() {
+    addFile(r'''
+/*=T*/ a/*<T>*/(/*=T*/ t) => t;
+/*=T*/ b/*<T>*/() => null;
+
+void main/*<S>*/() {
+  dynamic d;
+  int i;
+  /*error:IMPLICIT_DYNAMIC_FUNCTION*/a(d);
+  a(42);
+  /*error:IMPLICIT_DYNAMIC_FUNCTION*/b();
+  d = /*error:IMPLICIT_DYNAMIC_FUNCTION*/b();
+  i = b();
+
+  void f/*<T>*/(/*=T*/ t) {};
+  /*=T*/ g/*<T>*/() => null;
+
+  /*error:IMPLICIT_DYNAMIC_FUNCTION*/f(d);
+  f(42);
+  /*error:IMPLICIT_DYNAMIC_FUNCTION*/g();
+  d = /*error:IMPLICIT_DYNAMIC_FUNCTION*/g();
+  i = g();
+
+  /*error:IMPLICIT_DYNAMIC_INVOKE*/(/*<T>*/(/*=T*/ t) => t)(d);
+  (/*<T>*/(/*=T*/ t) => t)(42);
+  (/*<T>*/() => null as dynamic/*=T*/)/*<int>*/();
+}

[Bob Nystrom, 2016/06/23 23:25:52]

These test cases are fantastic. So thorough!

[Jennifer Messerly, 2016/06/24 01:15:56]

Thanks. Wanted to illustrate what it does, especially because I'm not sure we
have it right yet. :)

[Leaf, 2016/06/24 21:40:26]

+1!

+    ''');
+    check(implicitDynamic: false);
+  }
+  void test_implicitDynamic_listLiteral() {
+    addFile(r'''
+
+var l0 = /*error:IMPLICIT_DYNAMIC_LIST_LITERAL*/[];
+List l1 = /*error:IMPLICIT_DYNAMIC_LIST_LITERAL*/[];
+List<dynamic> l2 = /*error:IMPLICIT_DYNAMIC_LIST_LITERAL*/[];

[Bob Nystrom, 2016/06/23 23:25:52]

This case is interesting too. This might be one to noodle on. I'm not sure if I
consider downward inference "explicit" or not.

[Jennifer Messerly, 2016/06/24 01:15:55]

Yeah this dovetails into my other comment. We would really need to integrate
this into inference, and make sure inference doesn't push down a type in this
case:

    List l2 = ...;

but does in this case:

    List<dynamic> l2 = ...;

I don't think we currently have a distinction between those at all during
resolution/inference.

+dynamic d = 42;
+var l3 = /*error:IMPLICIT_DYNAMIC_LIST_LITERAL*/[d, d];

[Bob Nystrom, 2016/06/23 23:25:52]

I *think* I would consider this to be explicit. One way to look at it is like
this:

makePair<T>(T obj) => [obj, obj];

We wouldn't report an error on this method definition, which means there's no
error if you do:

dynamic d = ...;
makePair(d);

So that makes me feel like this is pretty much explicit too:

makePair(dynamic obj) => [obj, obj];

What do you think?

[Jennifer Messerly, 2016/06/24 01:15:55]

That is an error in the current impl. makePair is implicitly instantiated as
`makePair<dynamic>(d)`.
Well, this fits with your other comments :). What I am hearing is you want it to
treat inferred dynamic as explicit.

That is definitely sensible. After all, if we infer dynamic from something,
either:
A. it was explicit somewhere else, or
B. we issued an error somewhere else.

So either way, we aren't adding anything by issuing another error.

As I said somewhere further up let me think on this. Perhaps I can move the
implementation to ResolverVisitor without too much trouble. It makes me nervous
but given how simple the ErrorVerifier impl is, maybe it won't be too bad.

+
+var l4 = <dynamic>[];
+var l5 = <int>[];
+List<int> l6 = /*info:INFERRED_TYPE_LITERAL*/[];
+var l7 = /*info:INFERRED_TYPE_LITERAL*/[42];
+    ''');
+    check(implicitDynamic: false);
+  }
+
+  void test_implicitDynamic_mapLiteral() {
+    addFile(r'''
+var m0 = /*error:IMPLICIT_DYNAMIC_MAP_LITERAL*/{};
+Map m1 = /*error:IMPLICIT_DYNAMIC_MAP_LITERAL*/{};
+Map<dynamic, dynamic> m2 = /*error:IMPLICIT_DYNAMIC_MAP_LITERAL*/{};
+dynamic d = 42;
+var m3 = /*error:IMPLICIT_DYNAMIC_MAP_LITERAL*/{d: d};
+var m4 = /*info:INFERRED_TYPE_LITERAL,error:IMPLICIT_DYNAMIC_MAP_LITERAL*/{'x': d, 'y': d};
+var m5 = /*info:INFERRED_TYPE_LITERAL,error:IMPLICIT_DYNAMIC_MAP_LITERAL*/{d: 'x'};
+
+var m6 = <dynamic, dynamic>{};
+var m7 = <String, String>{};
+Map<String, String> m8 = /*info:INFERRED_TYPE_LITERAL*/{};
+var m9 = /*info:INFERRED_TYPE_LITERAL*/{'hi': 'there'};
+    ''');
+    check(implicitDynamic: false);
+  }
+
+  void test_implicitDynamic_method() {
+    addFile(r'''
+class C {
+  /*=T*/ m/*<T>*/(/*=T*/ s) => s;
+  /*=T*/ n/*<T>*/() => null;
+}
+class D<E> {
+  /*=T*/ m/*<T>*/(/*=T*/ s) => s;
+  /*=T*/ n/*<T>*/() => null;
+}
+void f() {
+  dynamic d;
+  int i;
+  new C()./*error:IMPLICIT_DYNAMIC_METHOD*/m(d);
+  new C().m(42);
+  new C()./*error:IMPLICIT_DYNAMIC_METHOD*/n();
+  d = new C()./*error:IMPLICIT_DYNAMIC_METHOD*/n();
+  i = new C().n();
+
+  new D<int>()./*error:IMPLICIT_DYNAMIC_METHOD*/m(d);

[Bob Nystrom, 2016/06/23 23:25:52]

This feels like an explicit use of dynamic to me since the type argument is
inferred from something explicitly dynamic. What do you think?

[Jennifer Messerly, 2016/06/24 01:15:56]

yes, same as your other examples. They will either all work or all be errors,
depending on our treatment of inference-from-dynamic :)

+  new D<int>().m(42);
+  new D<int>()./*error:IMPLICIT_DYNAMIC_METHOD*/n();
+  d = new D<int>()./*error:IMPLICIT_DYNAMIC_METHOD*/n();
+  i = new D<int>().n();
+}
+    ''');
+    check(implicitDynamic: false);
+  }
+
+  void test_implicitDynamic_parameter() {
+    addFile(r'''
+const dynamic DYNAMIC_VALUE = 42;
+
+// simple formal
+void f0(/*error:IMPLICIT_DYNAMIC_PARAMETER*/x) {}
+void f1(dynamic x) {}
+
+// default formal
+void df0([/*error:IMPLICIT_DYNAMIC_PARAMETER*/x = DYNAMIC_VALUE]) {}
+void df1([dynamic x = DYNAMIC_VALUE]) {}
+
+// BUG: this should work, see https://github.com/dart-lang/sdk/issues/25794
+void df2([/*error:IMPLICIT_DYNAMIC_PARAMETER*/x = 42]) {}
+
+// default formal (named)
+void nf0({/*error:IMPLICIT_DYNAMIC_PARAMETER*/x: DYNAMIC_VALUE}) {}
+void nf1({dynamic x: DYNAMIC_VALUE}) {}
+
+// BUG: this should work, see https://github.com/dart-lang/sdk/issues/25794
+void nf2({/*error:IMPLICIT_DYNAMIC_PARAMETER*/x: 42}) {}
+
+// field formal
+class C {
+  var /*error:IMPLICIT_DYNAMIC_FIELD*/x;
+  C(this.x);
+}
+
+// function typed formal
+void ftf0(void x(/*error:IMPLICIT_DYNAMIC_PARAMETER*/y)) {}
+void ftf1(void x(int y)) {}
+    ''');
+    check(implicitDynamic: false);
+  }
+
+  void test_implicitDynamic_return() {
+    addFile(r'''
+// function
+/*error:IMPLICIT_DYNAMIC_RETURN*/f0() {}
+dynamic f1() { return 42; }
+
+// nested function
+void main() {
+  /*error:IMPLICIT_DYNAMIC_RETURN*/g0() {}
+  dynamic g1() { return 42; }
+}
+
+// methods
+class B {
+  int m1() => 42;
+}
+class C extends B {
+  /*error:IMPLICIT_DYNAMIC_RETURN*/m0() => 123;
+  m1() => 123;
+  dynamic m2() => 'hi';
+}
+
+// accessors
+set x(int value) {}
+/*error:IMPLICIT_DYNAMIC_RETURN*/get y0 => 42;
+dynamic get y1 => 42;
+
+// function typed formals
+void ftf0(/*error:IMPLICIT_DYNAMIC_RETURN*/f(int x)) {}
+void ftf1(dynamic f(int x)) {}
+
+// function expressions
+var fe0 = /*error:IMPLICIT_DYNAMIC_RETURN*/(int x) => x as dynamic;
+var fe1 = (int x) => x;
+    ''');
+    check(implicitDynamic: false);
+  }
+
+  void test_implicitDynamic_type() {
+    addFile(r'''
+class C<T> {}
+class M1<T extends /*error:IMPLICIT_DYNAMIC_TYPE*/List> {}
+class M2<T> {}
+class I<T> {}
+class D<T, S> extends /*error:IMPLICIT_DYNAMIC_TYPE*/C
+    with M1, /*error:IMPLICIT_DYNAMIC_TYPE*/M2
+    implements /*error:IMPLICIT_DYNAMIC_TYPE*/I {}
+
+C f(D d) {
+  D x = new /*error:IMPLICIT_DYNAMIC_TYPE*/D();
+  D<int, dynamic> y = /*info:INFERRED_TYPE_ALLOCATION*/new /*error:IMPLICIT_DYNAMIC_TYPE*/D();
+  D<dynamic, int> z = /*info:INFERRED_TYPE_ALLOCATION*/new /*error:IMPLICIT_DYNAMIC_TYPE*/D();
+  return new /*error:IMPLICIT_DYNAMIC_TYPE*/C();
+}
+
+class A<T extends num> {}
+class N1<T extends List<int>> {}
+class N2<T extends Object> {}
+class J<T extends Object> {}
+class B<T extends Object> extends A with N1, N2 implements J {}
+A g(B b) {
+  B y = /*info:INFERRED_TYPE_ALLOCATION*/new B();
+  return /*info:INFERRED_TYPE_ALLOCATION*/new A();
+}
+    ''');
+    check(implicitDynamic: false);
+  }
+
+  void test_implicitDynamic_variable() {
+    addFile(r'''
+var /*error:IMPLICIT_DYNAMIC_VARIABLE*/x0;
+var /*error:IMPLICIT_DYNAMIC_VARIABLE*/x1 = (<dynamic>[])[0];
+var /*error:IMPLICIT_DYNAMIC_VARIABLE*/x2,
+    x3 = 42,
+    /*error:IMPLICIT_DYNAMIC_VARIABLE*/x4;
+dynamic y0;
+dynamic y1 = (<dynamic>[])[0];
+    ''');
+    check(implicitDynamic: false);
+  }
+
   void test_invalidOverrides_baseClassOverrideToChildInterface() {
     checkFile('''
 class A {}