Fix regression for import suggestions

Fix regression for import suggestions

Imports weren't being suggested for implicit source files,
for example files from other analysis roots.

BUG=
R=brianwilkerson@google.com, scheglov@google.com

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

[skybrian, 2016-03-03 22:26:32]

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

[Brian Wilkerson, 2016-03-03 22:33:28]

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

[Brian Wilkerson, 2016-03-03 22:33:28]

LGTM, but Dan knows the completion code better than I do, and I'd like his
opinion on changing the version of the library element that we're getting.

[danrubel, 2016-03-08 23:06:44]

On 2016/03/03 22:33:28, Brian Wilkerson wrote:
> LGTM, but Dan knows the completion code better than I do, and I'd like his
> opinion on changing the version of the library element that we're getting.

This is not completion code, but quick fix code.
I recommend that Konstantin look it over.

[danrubel, 2016-03-08 23:06:59]

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

[scheglov, 2016-03-09 19:05:32]

lgtm

https://codereview.chromium.org/1759333002/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/services/correction/fix_internal.dart (right):

https://codereview.chromium.org/1759333002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/correction/fix_internal.dart:1519:
context.getResult(librarySource, LIBRARY_ELEMENT4);
Do you need LIBRARY_ELEMENT4 or LIBRARY_ELEMENT1 would be enough?

[skybrian, 2016-03-09 19:15:05]

https://codereview.chromium.org/1759333002/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/services/correction/fix_internal.dart (right):

https://codereview.chromium.org/1759333002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/correction/fix_internal.dart:1519:
context.getResult(librarySource, LIBRARY_ELEMENT4);
On 2016/03/09 19:05:32, scheglov wrote:
> Do you need LIBRARY_ELEMENT4 or LIBRARY_ELEMENT1 would be enough?

I don't know; LIBRARY_ELEMENT4 was based on your suggestion.

There is a test now, so if we stop building LIBRARY_ELEMENT4 in this situation,
it will fail and we can choose something else.

[scheglov, 2016-03-09 19:18:21]

https://codereview.chromium.org/1759333002/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/services/correction/fix_internal.dart (right):

https://codereview.chromium.org/1759333002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/correction/fix_internal.dart:1519:
context.getResult(librarySource, LIBRARY_ELEMENT4);
On 2016/03/09 19:15:05, skybrian wrote:
> On 2016/03/09 19:05:32, scheglov wrote:
> > Do you need LIBRARY_ELEMENT4 or LIBRARY_ELEMENT1 would be enough?
> 
> I don't know; LIBRARY_ELEMENT4 was based on your suggestion.
> 
> There is a test now, so if we stop building LIBRARY_ELEMENT4 in this
situation,
> it will fail and we can choose something else.

Well, ideally we need to use the lowest level.

[skybrian, 2016-03-09 19:33:47]

https://codereview.chromium.org/1759333002/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/services/correction/fix_internal.dart (right):

https://codereview.chromium.org/1759333002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/correction/fix_internal.dart:1519:
context.getResult(librarySource, LIBRARY_ELEMENT4);
On 2016/03/09 19:18:21, scheglov wrote:
> On 2016/03/09 19:15:05, skybrian wrote:
> > On 2016/03/09 19:05:32, scheglov wrote:
> > > Do you need LIBRARY_ELEMENT4 or LIBRARY_ELEMENT1 would be enough?
> > 
> > I don't know; LIBRARY_ELEMENT4 was based on your suggestion.
> > 
> > There is a test now, so if we stop building LIBRARY_ELEMENT4 in this
> situation,
> > it will fail and we can choose something else.
> 
> Well, ideally we need to use the lowest level.

This fix uses getExportNamespaceForLibrary() on the LibraryElement. According to
the comment on LIBRARY_ELEMENT4, it sets exportNamespace. So it looks like
LIBRARY_ELEMENT4 is correct.

[scheglov, 2016-03-09 19:36:07]

LGTM

https://codereview.chromium.org/1759333002/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/services/correction/fix_internal.dart (right):

https://codereview.chromium.org/1759333002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/correction/fix_internal.dart:1519:
context.getResult(librarySource, LIBRARY_ELEMENT4);
On 2016/03/09 19:33:47, skybrian wrote:
> On 2016/03/09 19:18:21, scheglov wrote:
> > On 2016/03/09 19:15:05, skybrian wrote:
> > > On 2016/03/09 19:05:32, scheglov wrote:
> > > > Do you need LIBRARY_ELEMENT4 or LIBRARY_ELEMENT1 would be enough?
> > > 
> > > I don't know; LIBRARY_ELEMENT4 was based on your suggestion.
> > > 
> > > There is a test now, so if we stop building LIBRARY_ELEMENT4 in this
> > situation,
> > > it will fail and we can choose something else.
> > 
> > Well, ideally we need to use the lowest level.
> 
> This fix uses getExportNamespaceForLibrary() on the LibraryElement. According
to
> the comment on LIBRARY_ELEMENT4, it sets exportNamespace. So it looks like
> LIBRARY_ELEMENT4 is correct.
> 

OK, thank you for checking this.

[skybrian, 2016-03-09 20:59:31]

Description was changed from

==========
Fix regression for import suggestions

Imports weren't being suggested for implicit source files,
for example files from other analysis roots.

BUG=
==========

to

==========
Fix regression for import suggestions

Imports weren't being suggested for implicit source files,
for example files from other analysis roots.

BUG=
R=brianwilkerson@google.com, scheglov@google.com

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

[skybrian, 2016-03-09 20:59:32]

Committed patchset #1 (id:1) manually as
ae009e0870773989b311a34659e2b836c6d7e2c3 (presubmit successful).

[skybrian, 2016-03-09 21:10:09]

It seems suspicious that just above this change, the code to resolve an SDK
library uses LIBRARY_ELEMENT1.

But maybe it doesn't matter? The next lines are:

        Element element = getExportedElement(libraryElement, name);
        if (element == null) {
          continue;
        }

So even if getExportedElement() really requires LIBRARY_ELEMENT4, if this method
returns null when its requirements aren't met, it has the same effect and nobody
would notice.