task model: inferring type of variable that is explicit dynamic

[jmesserly]

This is from 'conversion and dynamic invoke' checker_test.dart:

        dynamic toString = () => null;
        (/*info:DynamicInvoke*/toString());

[jmesserly]

I'm able to reproduce this in Analyzer with a unit test (added to same file/class as test_localVariableInference_declaredType_disabled):

  void test_localVariableInference_declaredType_disabled_for_functions() {
    String code = r'''
main() {
  dynamic v = () => null;
  v; // marker
}''';
    SimpleIdentifier identifier = _findMarkedIdentifier(code, "v = ");
    expect(identifier.staticType, typeProvider.dynamicType);
    identifier = _findMarkedIdentifier(code, "; // marker");
    expect(identifier.staticType, typeProvider.dynamicType);
  }

edit: hmm, maybe not... investigating further ... above test is passing. The "failure" was caused by a test typo on my part

[jmesserly]

Here's a test that reproduces:

  void test_localVariableInference_declaredType_disabled_for_toString() {
    String name = 'toString';
    String code = '''
main() {
  dynamic $name = () => null;
  $name(); // marker
}''';
    SimpleIdentifier identifier = _findMarkedIdentifier(code, "$name = ");
    expect(identifier.staticType, typeProvider.dynamicType);
    SimpleIdentifier call = _findMarkedIdentifier(code, "(); // marker");
    expect(call.staticType, typeProvider.dynamicType);
    expect((call.parent as Expression).staticType, typeProvider.dynamicType);
    // edit: removed FunctionExpressionInvocation assert
  }

what is super crazy: if you change the name from toString to something like toStrin it behaves differently. The AST structure has a MethodInvocation node, and the type is correctly dynamic. Whereas with toString the failure is:

FAIL: StrongModeTypePropagationTest | test_localVariableInference_declaredType_disabled_for_toString
  Expected: DynamicTypeImpl:<dynamic>
    Actual: FunctionTypeImpl:<() → String>

[leafpetersen]

That may either be an analyzer bug, or a bug in the code that I added to handle object methods. I suspect the former because I don't think that should be a method invocation node - it should be a FunctionExpressionInvocation node, shouldn't it?

[jmesserly]

yes ... I'm trying to figure out why it parses different ... that's just spooky.
It is a MethodInvocation node with a null target, so I think it's the same semantically, but still...

[leafpetersen]

My take would be that it should not be a MethodInvocation node, since it's not a method invocation. If it is valid to have a method invocation node, then the special case code for object methods in static_type_analyzer.dart (_inferObjectAccess and _inferMethodInvocationObject) needs to be fixed to handle this (as should probably the type propagation code in the analyzer proper in that file, which I believe makes the same assumptions). Possibly the _isObjectProperty and _isObjectMethod code in ddc's checker.dart should be fixed as well. @bwilkerson , is that a correct parse or is this a bug?

[jmesserly]

oh ... it's always a MethodInvocation ... one mystery solved at least. I was confused because it wasn't getting to my FunctionExpressionInvocation assert in the test. Whew!

[jmesserly]

(that is: MethodInvocation with a null target)

[jmesserly]

https://codereview.chromium.org/1368793004/ has the fix. Thanks Leaf!

[jmesserly]

landed. Thanks Brian for the review!

[jmesserly]

reopening ... the original issue is fixed, but a library prefix like helper.toString() still fails. Doh!

[jmesserly]

https://codereview.chromium.org/1374773003/

[jmesserly]

landed that one. here's the final fix for checker_test https://codereview.chromium.org/1374813002/