Analyzer summaries missing generic types for DartObject

[matanlurey]

We have a class called MultiToken and another called ExistingProvider:

https://github.com/dart-lang/angular/blob/1abcb5c62fc8a8267f1abdb2081d07f40567c56f/angular/lib/src/core/di/opaque_token.dart#L62-L74

https://github.com/dart-lang/angular/blob/1abcb5c62fc8a8267f1abdb2081d07f40567c56f/angular/lib/src/di/providers.dart#L241-L265

This pattern was created in order to correctly "infer" what the type of a given DI provider should be, but also not require manual type annotations (use flowing type inference from OpaqueToken<T> -> ExistingProvider<T> -> Provider<T> (T):

const NG_VALUE_ACCESSOR = const MultiToken<ControlValueAccessor>(
  'NgValueAccessor',
);

const DEFAULT_VALUE_ACCESSOR = const ExistingProvider.forToken(
  NG_VALUE_ACCESSOR,
  DefaultValueAccessor,
);

When analyzing this during our compilation, T of DEFAULT_VALUE_ACCESSOR is dynamic internally (with bazel_codegen) and (correctly) ControlValueAccessor externally (with build_runner). This is causing our sync to fail on travis:

https://api.travis-ci.org/v3/job/342583283/log.txt

If I manually type (don't rely on inference), it works as intended in both places:

const DEFAULT_VALUE_ACCESSOR = const ExistingProvider<ControlValueAccessor>.forToken(
  NG_VALUE_ACCESSOR,
  DefaultValueAccessor,
);

It's also possible this is due to different analyzer versions/configuration? I know top-level inference is inconsistent without using Dart 2 semantics, but I'm very confused why this works properly externally but not internally.

/cc @natebosch @scheglov @alorenzen

[natebosch]

Both places are using an AnalysisContext. Both places set strongMode to true and no other analysis options. (bazel, build_runner)

In bazel we use summaries for files outside the current package, in build_runner we always read Dart sources, never summaries.

[matanlurey]

Interestingly enough, this worked internally a while back :-/

[natebosch]

Could be a regression in the AnalysisContext in that case - I think we're likely getting older versions of the analyzer package externally than what is rolled internally. If you can build a test case you might want to see if you can try against older versions of analyzer internally.

[matanlurey]

For folks reading this thread:

INTERNALLY = 0.31.2-alpha.0
EXTERNALLY = ^0.31.0+1

[MichaelRFairhurst]

My understanding was that top levels don't have inference, they get implicit dynamic. It seems like for const values we do some kind of inference, though...

We did just change const values around a bit in order to support asserts in initializers?

[matanlurey]

@MichaelRFairhurst:

My understanding was that top levels don't have inference, they get implicit dynamic. It seems like for const values we do some kind of inference, though...

Yeah, it works as of 0.31.0+1 for this limited case. If I'm relying on something "bad", let me know, but my assumption is that it should continue to work indefinitely.

If you need a better reproduction case, I'm happy to write one.

[MichaelRFairhurst]

Its my understanding that this does not have any inference...const ExistingProvider.forToken(...) but rather gets implicit dynamic...I think that's the problem

[matanlurey]

@MichaelRFairhurst:

Its a little hard to see here, but the failing travis sees it:

https://api.travis-ci.org/v3/job/342583283/log.txt

     Which: is different.
            Expected: ... ;\n  List<dynamic> _ ...
              Actual: ... ;\n  List<import15.C ...
                                    ^
             Differ at offset 8427
  ./build/test/_files/directives/directive_wrapper.template_debug.golden is out of date. See ./build/test/_files/directives/directive_wrapper.template_debug.check.

i.e. externally, we read this provider as T=ControlValueAccessor, but internally it is read as T=dynamic. We obviously want the former.

EDIT: I will add that one of these uses summaries of dependencies (internally, with Bazel) and one of these does not (externally, with Build Runner). That may or may not matter?

[matanlurey]

In order to avoid this blocking the sync, I'll add a work-around for now @alorenzen.