Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Misleading dartanalyzer hint. #20144

Closed
mkustermann opened this issue Jul 22, 2014 · 6 comments
Closed

Misleading dartanalyzer hint. #20144

mkustermann opened this issue Jul 22, 2014 · 6 comments
Assignees
@scheglov
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures
Milestone
1.10

Comments

@mkustermann
Copy link
Member

The following program produces a hint that 'replaceAll' is not defined on class 'Map'.

$ cat main.dart
Map foo() => null;

main() {
  var x = foo();
  if (x != null) x = '$x';
  if (x != null) {
    x.replaceAll('x', 'y');
  }
}
$ dartanalyzer main.dart
Analyzing [main.dart]...
[hint] The method 'replaceAll' is not defined for the class 'Map' (/.../main.dart, line 8, col 7)
No issues found

This hint is misleading, since
 - the static type of [x] is dynamic
 - the concrete type at runtime at the point of 'x.replaceAll()' is 'String' (which has a 'replaceAll' method).

This hint seems to be not coming from the actual static type checker, but rather from the type inferrer.
I'm not sure how it's implemented, but if dartanalyzer infers a type for [x], the inferred type should be the union of the types of all assignments to [x] -- which would in this case be Union[Map,String] and that type can clearly have a 'replaceAll' method.

Example where we use this is:

dart/pkg/oauth2/lib/src/handle_access_token_response.dart:
  var contentType = response.headers['content-type'];
  if (contentType != null) contentType = new MediaType.parse(contentType);
  validate(contentType != null &&
      (contentType.mimeType == "application/json" ||
       contentType.mimeType == "text/javascript"),
      'content-type was "$contentType", expected "application/json"');

Here the hint is that [mimeType] is not a member of 'String'.
@DartBot
Copy link

DartBot commented Jul 22, 2014

This comment was originally written by @mezoni

var z = null;
var x = foo();
// Maybe x assigned to '$x', if x is null
if (x != z) x = '$x';
if (x != z) {
  x.replaceAll('x', 'y');
}

var z = {};
var x = foo();
// Maybe x assigned to '$x', if x is Map
// if x null
if (x != z) x = '$x';
if (x != z) {
  x.replaceAll('x', 'y');
}

var z = [];
var x = foo();
// x never assigned to '$x', becuase Map != List
if (x != z) x = '$x';
if (x != z) {
  x.replaceAll('x', 'y');
}

var x = foo();
// x is Map or Null, i.e x is Map
if (x != null) x = '$x';
// x is Map or Null or Srting, i.e x is dynamic
if (x != null) {
  // x is Map or Null or Srting, i.e x is dynamic
  x.replaceAll('x', 'y');
}

Dart analyzer never emulate all variable flow with all possible values.
This may take a lot of time.

After this statement with the assignment expression the analyzer should assume that value stored in a variable can have more then one value (except null value).
This means that becomes to dynamic or multi-type (variant) variable.

// After body of this statement x can be Map or String.
if (x != z) x = '$x';
 

@bwilkerson
Copy link
Member

It is clear that we need to clear the propagated type of a variable when it might have been assigned a value of a different type.

Set owner to collinsn@google.com.
Removed Priority-Unassigned label.
Added Priority-Medium label.

@DartBot
Copy link

DartBot commented Sep 26, 2014

This comment was originally written by collinsn@google.com

My internship is ending today.

Set owner to @bwilkerson.

@bwilkerson
Copy link
Member

Added this to the 1.10 milestone.
Removed Priority-Medium label.
Added Priority-High label.

@scheglov
Copy link
Contributor

https://codereview.chromium.org/1036233006

Set owner to @scheglov.
Added Started label.

@scheglov
Copy link
Contributor

https://code.google.com/p/dart/source/detail?r=44754

Added Fixed label.

@mkustermann mkustermann added Type-Defect P1 A high priority bug; for example, a single project is unusable or has many test failures area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Mar 27, 2015
@mkustermann mkustermann added this to the 1.10 milestone Mar 27, 2015
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@scheglov scheglov

Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Milestone
1.10
Development

No branches or pull requests
4 participants
@scheglov @mkustermann @bwilkerson @DartBot