[Dot Shorthands] Bugs in static-access-shorthand tests

[kallentu]

I was going through a bunch of the co19 tests while doing CFE implementation and came across these.
I believe we need an update on these tests.

cc. @eernstg to confirm for some of the spec related changes. I might also be wrong, so it'd be nice to get a double-check.

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/type_inference_A01_t02.dart
Problem: This errors even without a shorthand:

tests/co19/src/LanguageFeatures/Static-access-shorthand/type_inference_A01_t02.dart:162:40: Error: The getter 'value' isn't defined for the class 'ET<dynamic>'.
Try correcting the name to the name of an existing getter, or defining a getter or field named 'value'.
  Expect.equals(3, (await (await et9)).value);

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/type_inference_A01_t01.dart
Problem: e1 doesn't exist for E.

tests/co19/src/LanguageFeatures/Static-access-shorthand/type_inference_A01_t01.dart:85:19: Error: Member not found: 'e1'.
  Expect.equals(E.e1, e1);

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/semantics_A05_t01.dart
Problem: The LHS context is now C for example and C doesn’t have a .new. I think only Object as the context would work here for constructor tearoffs.

tests/co19/src/LanguageFeatures/Static-access-shorthand/semantics_A05_t01.dart:44:10: Error: The static getter or field 'new' isn't defined for the type 'C<int>'.
 - 'C' is from 'tests/co19/src/LanguageFeatures/Static-access-shorthand/semantics_A05_t01.dart'.
Try correcting the name to the name of an existing static getter or field, or defining a getter or field named 'new'.
    o = .new;
         ^^^
tests/co19/src/LanguageFeatures/Static-access-shorthand/semantics_A05_t01.dart:51:10: Error: The static getter or field 'id' isn't defined for the type 'C<int>'.
 - 'C' is from 'tests/co19/src/LanguageFeatures/Static-access-shorthand/semantics_A05_t01.dart'.
Try correcting the name to the name of an existing static getter or field, or defining a getter or field named 'id'.
    o = .id;

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/patterns_A01_t03.dart
Problem: res is declared twice

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/non_ambiguity_A02_t02.dart
Problem: Typo on line 218 - testMetods -> testMethods.

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/grammar_A01_t05.dart
Problem: We seem to have this compile-time error even without shorthands.

tests/co19/src/LanguageFeatures/Static-access-shorthand/grammar_A01_t05.dart:74:26: Error: The operator '[]' isn't defined for the class 'FutureOr<List<C>>'.
 - 'List' is from 'dart:core'.
 - 'C' is from 'tests/co19/src/LanguageFeatures/Static-access-shorthand/grammar_A01_t05.dart'.
Try correcting the operator to an existing operator, or defining a '[]' operator.
  C c3 = await .instances[0];
                         ^
tests/co19/src/LanguageFeatures/Static-access-shorthand/grammar_A01_t05.dart:104:28: Error: The operator '[]' isn't defined for the class 'FutureOr<List<ET>>'.
 - 'List' is from 'dart:core'.
Try correcting the operator to an existing operator, or defining a '[]' operator.
  ET et3 = await .instances[0];
                           ^

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t09.dart
Problem: On line 64, if ((C() as Object) == .new()) {} actually works since Object has a .new() ctor so no compile-time error is expected there.

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t01.dart
https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t02.dart
https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t03.dart
https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t04.dart
Problem: if (C(0) == foo(.values[0])) {} and all other instances where we use a shorthand as a method argument, should work because the context is the parameter type. No error expected?

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A01_t02.dart
Problem: m3 == .instances[0] where m3 is MC -- MC doesn’t have the getter/field instances.

tests/co19/src/LanguageFeatures/Static-access-shorthand/equality_A01_t02.dart:61:24: Error: The static getter or field 'instances' isn't defined for the type 'MC<String>'.
 - 'MC' is from 'tests/co19/src/LanguageFeatures/Static-access-shorthand/equality_A01_t02.dart'.
Try correcting the name to the name of an existing static getter or field, or defining a getter or field named 'instances'.
  Expect.isTrue(m3 == .instances[0]);
                       ^^^^^^^^^
tests/co19/src/LanguageFeatures/Static-access-shorthand/equality_A01_t02.dart:63:25: Error: The static getter or field 'instances' isn't defined for the type 'MC<String>'.
 - 'MC' is from 'tests/co19/src/LanguageFeatures/Static-access-shorthand/equality_A01_t02.dart'.
Try correcting the name to the name of an existing static getter or field, or defining a getter or field named 'instances'.
  Expect.isFalse(m3 != .instances[0]);

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A10_t17.dart
Problem: On line 48 and 52, the expressions for M and E are evaluating to false and true, so we don’t evaluate the other branch in these examples and that's where the error is located, but is skipped over. Seems to be an implementation thing, but everything is working as expected, we just don't evaluate the false branch and therefore, no error is reported. This might actually just be a CFE/analyzer discrepancy. With the types added in, it seems like the analyzer reports errors, but the CFE doesn't for line 48 and 52.

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A10_t13.dart
https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A10_t15.dart
Problem: Missing error for const b2 = (MC(42) as M) == .answer; but I believe we're using the context type M and M has a const answer which makes it so that we don't produce any errors.

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A04_t01.dart
https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A03_t02.dart
https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A03_t01.dart
Problem: I need @eernstg's confirmation on this one too, but I believe we're grabbing the context of C1 for Expect.isTrue(C1() == .new); and the constructor tearoff doesn't work for this context.

[eernstg]

That's a lot of stuff! ;-D

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/type_inference_A01_t02.dart

Presumably value should have been v in line 162

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/type_inference_A01_t01.dart

E.e1 --> E.e0 in line 85, probably.

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/semantics_A05_t01.dart

This one looks like it should work. In line 43 at o is C<int> I'd expect o to be promoted to have type C<int> which would presumably make .new a shorthand for C.new (a constructor tear-off), subject to type inference in the context C<int>. This makes no difference because C<S>.new isn't assignable to C<int> for any S, but this just means that C.new is inferred as C<Object?>.new. As a result, the assignment to o will demote o to have type Object?.

Line 51 is similar.

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/grammar_A01_t05.dart

(await C.instances)[0] should work, but in order to use a dot shorthand we can't afford to drop the context type (and with (e)[0], e has context type _ if and until we generalize inference to pass a context type to member access receivers). So we can't use (await .instances)[0].

We could presumably use await .instances[0] in the case where C.instances is modified to have type List<FutureOr<C>>. This should make line 104 work as well.

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t01.dart https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t02.dart https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t03.dart https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t04.dart

.. where we use a shorthand as a method argument .. should work ..

I can't spot an issue that matches this description in A02_t01. The context type for the right hand operand of == is generally Object (and that's also the parameter type of C<_>.==), and this causes terms like .id(1) in [.id(1)] to have a context type which is Object? because that's the inferred actual type argument of the list literal, so we're inspecting Object and it doesn't have a static id or a constructor named Object.id. Similarly for A02_t0{2,3,4}, I can't see any cases where the error should not be reported. What did I overlook?

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A01_t02.dart

Perhaps line 60-64, including both, should just be deleted: m3 should have type M rather than MC because MC has no static members, but this is already what lines 54-58 are doing.

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A10_t17.dart

A constant expression of the form b ? e1 : e2 is indeed specified with no reference to e2 at all when b evaluates to true, and with no reference to e1 at all when b evaluates to false. It is still expected that the ignored branch is type checked as an expression, but there is no requirement that it must be a constant expression.

We have a really weird combination here, because we can have an expression which is constant, but not potentially constant (like 2 > 1 ? myConstantVariable : myNonConstantVariable), and I don't think that was supposed to be possible. Anyway, that's good to know. ;-)

I think the expectations should simply be adjusted to expect success in the two cases line 48 and 52, preferably with a comment that explains why there is no error.

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A04_t01.dart https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A03_t02.dart https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A03_t01.dart

I don't see a problem with C1() == .new. It is true that C1 is used as the namespace where .new is looked up, and hence this means C1() == C1.new. This is false, but not a problem (the operand of operator == needs to be assignable to Object, but that's true for C1.new).

[sgrekhov]

@eernstg thank you for your analysis. A couple of notes.

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t01.dart https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t02.dart https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t03.dart https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t04.dart

What did I overlook?

C<int> foo(C<int> c) => c;
...
if (C(0) == foo(.values[0])) {}

Here foo() provides a context type for .values[0] to be C.values[0]. Updated to expect no error here.

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A10_t17.dart

A constant expression of the form b ? e1 : e2 is indeed specified with no reference to e2 at all when b evaluates to true, and with no reference to e1 at all when b evaluates to false. It is still expected that the ignored branch is type checked as an expression, but there is no requirement that it must be a constant expression.

We have a really weird combination here, because we can have an expression which is constant, but not potentially constant (like 2 > 1 ? myConstantVariable : myNonConstantVariable), and I don't think that was supposed to be possible. Anyway, that's good to know. ;-)

I think the expectations should simply be adjusted to expect success in the two cases line 48 and 52, preferably with a comment that explains why there is no error.

Let's hold on with this test until dart-lang/language#4311

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A01_t02.dart

Perhaps line 60-64, including both, should just be deleted: m3 should have type M rather than MC because MC has no static members, but this is already what lines 54-58 are doing.

Just a typo. Replaced by M.

So, finally.

• https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/type_inference_A01_t02.dart - fixed
• https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/type_inference_A01_t01.dart - fixed
• https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/semantics_A05_t01.dart - the test is Ok. Not changed
• https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/patterns_A01_t03.dart - fixed
• https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/non_ambiguity_A02_t02.dart - fixed
• https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/grammar_A01_t05.dart - fixed
• https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t09.dart - fixed
• https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A02_t01-4.dart - fixed
• https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/equality_A01_t02.dart - fixed
• https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A10_t17.dart - let's wait for Should constant expressions be potentially constant? language#4311
• https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A10_t13/15.dart - fixed
• https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/constant_expression_A03_t01/A03_t02/A04_t01.dart - tests are Ok. Not changed.

[eernstg]

What did I overlook?

OK, turns out that I completely missed the last line in main. ;-)

[kallentu]

Thank you @eernstg for checking it over. I'll fix the implementation bugs on the tests that are fine.
And thank you @sgrekhov for fixing the other tests so quickly!

[kallentu]

Oh there's another test that may need updating: https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/type_inference_A07_t01.dart. I think we're expecting the wrong value.

// ...
  C<int> c2 = .id("String").toInt(2);
  Expect.equals(1, c2.value); // Should expect 2?

[sgrekhov]

Oh there's another test that may need updating: https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Static-access-shorthand/type_inference_A07_t01.dart. I think we're expecting the wrong value.

// ...
C c2 = .id("String").toInt(2);
Expect.equals(1, c2.value); // Should expect 2?

Ah, yes. #3125

[sgrekhov]

Changed status to blocked. Work on constant_expression_A10_t17.dart is blocked by dart-lang/language#4311