Spurious compiler output when running Flutter tests

[Hixie]

IMHO https://dart-review.googlesource.com/c/sdk/+/66160 should be reverted. It's causing spurious warnings when running Flutter's tests:

compiler message: widgets/editable_text_test.dart:1341:23: Warning: This expression has type 'void' and can't be used.
compiler message:       verify(controls.handleCopy(any)).called(1);
compiler message:                       ^
compiler message: widgets/editable_text_test.dart:1344:23: Warning: This expression has type 'void' and can't be used.
compiler message:       verify(controls.handleCut(any)).called(1);
compiler message:                       ^
compiler message: widgets/routes_test.dart:435:30: Warning: This expression has type 'void' and can't be used.
compiler message:       verify(pageRouteAware1.didPush()).called(1);
compiler message:                              ^
compiler message: widgets/routes_test.dart:440:30: Warning: This expression has type 'void' and can't be used.
compiler message:       verify(pageRouteAware1.didPushNext()).called(1);
compiler message:                              ^
compiler message: widgets/routes_test.dart:443:30: Warning: This expression has type 'void' and can't be used.
compiler message:       verify(pageRouteAware2.didPush()).called(1);
compiler message:                              ^
compiler message: widgets/routes_test.dart:446:30: Warning: This expression has type 'void' and can't be used.
compiler message:       verify(pageRouteAware2.didPop()).called(1);
compiler message:                              ^
compiler message: widgets/routes_test.dart:447:30: Warning: This expression has type 'void' and can't be used.
compiler message:       verify(pageRouteAware1.didPopNext()).called(1);
...

By "spurious" I mean that they are messages that the analyzer is not reporting, and that the code works fine.

IMHO the compiler should never output any messages except if it has a fatal error. The analyzer is the tool whose role it is to report problems, not the compiler.

cc @aam @peter-ahe-google

[aam]

I imagine that flutter-specific compiler output filtering should be done in flutter engine frontend server, so dart commit would stay, but frontend server is extended with "quiet" mode.

[tvolkert]

This can be seen in the following reduced case:

import 'package:mockito/mockito.dart';
import 'package:test/test.dart';

abstract class Foo {
  void bar(int value);
}

class MockFoo extends Mock implements Foo {}

void main() {
  test('wat?', () {
    Foo foo = new MockFoo();
    foo.bar(2);
    var value = verify(foo.bar(captureAny)).captured.single;
    expect(value, 2);
  });
}

This produces the following output:

$ dart lib/main.dart 
lib/main.dart:14:28: Warning: This expression has type 'void' and can't be used.
    var value = verify(foo.bar(captureAny)).captured.single;
                           ^
00:00 +0: wat?
00:00 +1: All tests passed!

This is showing up in Fuchsia builds as well.

[Hixie]

That code is indeed pretty dubious (you're not supposed to be able to pass a void as an argument), but I guess that means the real question is why does the analyzer not report it?

[Hixie]

cc @devoncarew

[devoncarew]

but I guess that means the real question is why does the analyzer not report it

@MichaelRFairhurst, is this working as intended, or an analyzer bug?

[tvolkert]

you're not supposed to be able to pass a void as an argument

What about:

verify<void>(foo.bar(captureAny))...

😉

[Hixie]

That's pointless code. The argument can't be used.

[MichaelRFairhurst]

That code is indeed pretty dubious (you're not supposed to be able to pass a void as an argument),

That's not true. A void value cannot be used, so the argument cannot be used in the function.

  f(void arg) {
    print(arg); // error, cannot use arg
  }

Void values are values with a bit saying "do not use." Its perfectly safe to provide any value to f(), including a void value. As opposed to Object, which cannot accept a void value but can accept everything else:

  acceptVoid(void x) {}
  acceptObject(Object x) {}

  acceptVoid(123); // valid
  acceptObject(123); // valid
  acceptVoid(print('')); // valid
  acceptObject(print('')); // invalid

We considered making the third option illegal, but there was no evidence it offered anything of value (no theoretical basis nor data suggesting so) and it would have been a change to mockito's API that would have had a less readable end result.

This was reported in the original flutter-dev post about void: https://groups.google.com/forum/#!searchin/flutter-dev/void$20mockito%7Csort:date/flutter-dev/scnh7SfYa_o/Mcqv70L6AAAJ

*Why ever pass void into a function?( It may seem strange, but it is useful for sequencing. Consider Mockito's nicely readable API: verify(mock.voidMethod()).called(1). This is only allowed where the parameter's type is void, either by definition or through inference on a type parameter.

I am working on a blog post that explains all this stuff better in one clear concise place.

but I guess that means the real question is why does the analyzer not report it?

This is a CFE bug, the analyzer is working correctly.

#33934

[MichaelRFairhurst]

I agree with Ian's original opinion it could be reverted. It was reverted originally, (I reverted it), due to known issues such as this one. It was then relanded as warnings with the intention that the known issues would be fixed. However they are still not fixed so that indicates to me that this should not have been rolled forward, it disincentivizes the fix. The issues here are part of a really complex topic and we risk causing a lot of confusion/alienation here. Making users "fix" code we intentionally wanted them to keep. Also there is no obvious workaround for this mockito case. I wish it hadn't been part of Dart 2 Stable but it should definitely be fixed by 2.1.

…

On Sat, Aug 11, 2018, 11:52 PM Ian Hickson ***@***.***> wrote: That's pointless code. The argument can't be used. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#34031 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABjWe_fiiABs6WoU3yuNq7qJ9xp7NqY5ks5uP9DAgaJpZM4VnZNT> .

[mit-mit]

cc @kmillikin

[kmillikin]

This is due to incorrect handling of void types. Closing in favor of #30470 which has some more discussion.

[kmillikin]

These warnings should be gone with 9bdf248. How can we verify that?

[aam]

Once current dart->engine->flutter roll lands, we could confirm that flutter test run doesn't produce that diagnostic.