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

Analyzer should reject prefix shadowed by parameter #42620

Closed
stereotype441 opened this issue Jul 7, 2020 · 5 comments
Closed

Analyzer should reject prefix shadowed by parameter #42620

stereotype441 opened this issue Jul 7, 2020 · 5 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion.

Comments

@stereotype441
Copy link
Member

Consider the following code:

import 'dart:async' as a;

g(int x) async => x;
f(int a) {
  a.Future<int> x = g(a);
  return x;
}
main() async {
  print(await f(0));
}

The front end rejects this code with the following error:

../../tmp/test.dart:5:3: Error: 'a.Future' can't be used as a type because 'a' doesn't refer to an import prefix.
  a.Future<int> x = g(a);
  ^^^^^^^^

The analyzer accepts it, offering only this hint:

  hint • Unused import: 'dart:async'. • /usr/local/google/home/paulberry/tmp/test.dart:1:8 • unused_import

It seems that for type analysis, the analyzer is considering the a in a.Future to refer to the import of dart:async, but for determining whether the import is used, it's considering it to refer to the a parameter of f.

The spec seems pretty clearly in agreement with the front end. In the section "Static Types", it says:

A type T is malformed iff:

  • T has the form id or the form prefix.id, and in the enclosing lexical scope, the name id (respectively prefix.id) does not denote a type.

Note that although the example above is contrived, this bug did arise in a real-world circumstance for me; I was attempting to write a method taking an argument called path, in a file that contained import 'package:path/path.dart' as path, and inside that method I attempted to declare a variable of type path.Context. The analyzer did not flag an error (nor did I realize my own mistake); I didn't see an error until I tried to run the program.

@stereotype441 stereotype441 added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Jul 7, 2020
@bwilkerson
Copy link
Member

@eernstg Will this continue to be the semantics after the spec changes related to prefixes that you mentioned recently in a different context?

@eernstg
Copy link
Member

eernstg commented Jul 8, 2020

Yes, there are no plans to change the lexical lookup rules. So when a denotes a formal parameter of type int in the body of f according to the scope rules, a.Future.... is an attempt to access a member named Future on the value of that parameter. The import prefix named a is shadowed by the parameter and hence it is never the result of a lexical lookup in this scope. So a "prefix object" is not a Dart object, but lookups of the form id1.id2 do follow the normal scope rules also when id1 denotes a prefix object.

@stereotype441 stereotype441 self-assigned this Jul 8, 2020
@scheglov
Copy link
Contributor

scheglov commented Jul 9, 2020

See also #42493, which also would fix the issue.

dart-bot pushed a commit that referenced this issue Jul 9, 2020
…able.

See #42620.

Change-Id: Ifd18c939082935390df0c2aa0bd724cf6f76c27c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153705
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
@stereotype441
Copy link
Member Author

Turning over to Konstantin to improve on my hacky fix

dart-bot pushed a commit that referenced this issue Jul 21, 2020
No much changes to resolution itself, only to the way how we create
scopes. But we still use `lookupIdentifier` that is basically the old
approach to resolution.

This also moves the fix of #42620
to TypeNameResolver.

I will continue improving in following CLs.

Change-Id: I89bae5afe0a7978aba9fe9bf7c8bf08fb59b1440
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/154920
Commit-Queue: Konstantin Shcheglov <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
@scheglov
Copy link
Contributor

scheglov commented Aug 1, 2020

Reporting CompileTimeErrorCode.PREFIX_SHADOWED_BY_LOCAL_DECLARATION was moved into TypeNameResolver in 86ec742.

@scheglov scheglov closed this as completed Aug 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion.
Projects
None yet
Development

No branches or pull requests

4 participants