Suppress follow-on errors when a file is imported with either a prefix or a show clause

Suppress follow-on errors when a file is imported with either a prefix or a show clause

R=scheglov@google.com

Committed: https://github.com/dart-lang/sdk/commit/b29c6bf33000ca129987edffe9e5df0fde793f3b

[Brian Wilkerson, 2016-08-08 20:50:37]

brianwilkerson@google.com changed reviewers:
+ scheglov@google.com

[Brian Wilkerson, 2016-08-08 20:50:38]

https://codereview.chromium.org/2226613004/diff/1/pkg/analyzer/lib/src/dart/e...
File pkg/analyzer/lib/src/dart/element/builder.dart (right):

https://codereview.chromium.org/2226613004/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/builder.dart:216: // The imported source will
be null if the URI in the import
I moved the test for a missing source to line 249 (new file) so that we will now
create elements even for missing files. One side effect is that we will also
create PrefixElement's for such an import, so we can now recognize more uses of
prefixes.

[scheglov, 2016-08-08 21:10:20]

I'm concerned that we use AnalysisContext.exists().
This might make analysis of internal projects slower.

https://codereview.chromium.org/2226613004/diff/1/pkg/analyzer/lib/src/dart/r...
File pkg/analyzer/lib/src/dart/resolver/scope.dart (right):

https://codereview.chromium.org/2226613004/diff/1/pkg/analyzer/lib/src/dart/r...
pkg/analyzer/lib/src/dart/resolver/scope.dart:445:
importElement.context.exists(importElement.importedLibrary.source);
AnalysisContext.exists() is an expensive operation, especially on NFS.
Maybe we could remember whether the import directive resolves to an existing
source in the element.

https://codereview.chromium.org/2226613004/diff/1/pkg/analyzer/lib/src/dart/r...
pkg/analyzer/lib/src/dart/resolver/scope.dart:457: if
(importElement.prefix?.name == prefix && !exists(importElement)) {
What if there is an import with the same prefix name, but with an existing
source?

[Brian Wilkerson, 2016-08-08 21:19:30]

https://codereview.chromium.org/2226613004/diff/1/pkg/analyzer/lib/src/dart/r...
File pkg/analyzer/lib/src/dart/resolver/scope.dart (right):

https://codereview.chromium.org/2226613004/diff/1/pkg/analyzer/lib/src/dart/r...
pkg/analyzer/lib/src/dart/resolver/scope.dart:445:
importElement.context.exists(importElement.importedLibrary.source);
> AnalysisContext.exists() is an expensive operation, especially on NFS.
> Maybe we could remember whether the import directive resolves to an existing
> source in the element.

Yeah, I considered that possibility, but forgot to go back and implement it.

https://codereview.chromium.org/2226613004/diff/1/pkg/analyzer/lib/src/dart/r...
pkg/analyzer/lib/src/dart/resolver/scope.dart:457: if
(importElement.prefix?.name == prefix && !exists(importElement)) {
> What if there is an import with the same prefix name, but with an existing
> source?

Then presumably we would have found the corresponding element when we looked the
name up and we would never invoke this method. The intent is that this method is
only invoked when we find a name that couldn't be resolved in order to determine
whether to report the fact.

[Brian Wilkerson, 2016-08-09 17:24:15]

PTAL

I have re-worked the existence tests so that they are now cached in a modifier
in the import/export element.

[scheglov, 2016-08-09 17:45:04]

LGTM

https://codereview.chromium.org/2226613004/diff/20001/pkg/analyzer/lib/src/da...
File pkg/analyzer/lib/src/dart/element/builder.dart (right):

https://codereview.chromium.org/2226613004/diff/20001/pkg/analyzer/lib/src/da...
pkg/analyzer/lib/src/dart/element/builder.dart:187: exportElement.uriExists =
exportedTime >= 0;
We have already checked "exportedTime != -1" above.

Just a note - it seems that we have an asymmetry now - we add unresolved
imports, but don't add unresolved exports.

[Brian Wilkerson, 2016-08-09 18:11:25]

https://codereview.chromium.org/2226613004/diff/20001/pkg/analyzer/lib/src/da...
File pkg/analyzer/lib/src/dart/element/builder.dart (right):

https://codereview.chromium.org/2226613004/diff/20001/pkg/analyzer/lib/src/da...
pkg/analyzer/lib/src/dart/element/builder.dart:187: exportElement.uriExists =
exportedTime >= 0;
Good catch! The asymmetry was unintentional, so I updated this method to
correspond to visitImportDirective. Nothing else needed to be changed.

[Brian Wilkerson, 2016-08-09 18:11:43]

Description was changed from

==========
Suppress follow-on errors when a file is imported with either a prefix or a show
clause
==========

to

==========
Suppress follow-on errors when a file is imported with either a prefix or a show
clause

R=scheglov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/b29c6bf33000ca129987edffe9e5df0fde793f3b
==========

[Brian Wilkerson, 2016-08-09 18:11:44]

Committed patchset #2 (id:20001) manually as
b29c6bf33000ca129987edffe9e5df0fde793f3b (presubmit successful).