Analysis request `getReachableSources` (#24893).

Analysis request `getReachableSources` (#24893).

Implements a new request to get reachable sources.

This solves a specific issue for flutter (e.g., inferring execution type by checking the transitive closure of reachabe sources for `dart:flutter`) but can be more generallly useful for other client-side smarts (e.g., "is this a web entry point?" or "is this a test?").

More context here: https://github.com/dart-lang/sdk/issues/24893

R=brianwilkerson@google.com, devoncarew@google.com

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

[pquitslund, 2015-12-02 00:23:32]

Description was changed from

==========
Analysis request `getReachableSources` (#24893).

Implements a new request to get reachable sources.

This solves a specific issue for flutter (e.g., inferring execution type by
checking the transitive closure of reachabe sources for `dart:flutter`) but can
be more generallly useful for other client-side smarts (e.g., "is this a web
entry point?" or "is this a test?").

More context here: https://github.com/dart-lang/sdk/issues/24893

BUG=24893
==========

to

==========
Analysis request `getReachableSources` (#24893).

Implements a new request to get reachable sources.

This solves a specific issue for flutter (e.g., inferring execution type by
checking the transitive closure of reachabe sources for `dart:flutter`) but can
be more generallly useful for other client-side smarts (e.g., "is this a web
entry point?" or "is this a test?").

More context here: https://github.com/dart-lang/sdk/issues/24893
==========

[pquitslund, 2015-12-02 00:23:48]

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

[Brian Wilkerson, 2015-12-02 01:56:27]

LGTM

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/dependencies/reachable_source_collector.dart
(right):

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/dependencies/reachable_source_collector.dart:32:
String sourceUri = source?.uri?.toString();
nit: We don't need the null-aware operators if we test 'source' in
'collectSources'. Not sure whether there's enough of a performance penalty to
care, but it would make the code a little cleaner.

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/dependencies/reachable_source_collector.dart:35:
if (sourceUri == null) {
Similarly, we don't need this test.

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/test/do...
File pkg/analysis_server/test/domain_analysis_test.dart (right):

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/test/do...
pkg/analysis_server/test/domain_analysis_test.dart:58:
test('getReachableSources', () async {
Maybe put this in a group?

Also test what happens if the file passed to the request is not being analyzed
(is not in an analysis root).

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/test/do...
pkg/analysis_server/test/domain_analysis_test.dart:76:
unorderedEquals(['dart:core', 'file:///project/b.dart']));
Perhaps verify that b.dart is a key in the map?

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/tool/sp...
File pkg/analysis_server/tool/spec/spec_input.html (right):

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/tool/sp...
pkg/analysis_server/tool/spec/spec_input.html:400: Return the transitive closure
of reachable sources for a given file.
This should return an error response if the file is not currently being
analyzed. (I know there's wording in other requests that you could borrow.)

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/tool/sp...
pkg/analysis_server/tool/spec/spec_input.html:419: { "file:///foo.dart" :
["file:///bar.dart"] }.  If "bar.dart" has further imports
The "If" implies to me that 'bar.dart' might not be a key in the mapping, but I
think it always will be a key (because everything implicitly imports 'dart:core'
even if it doesn't explicitly import anything. Not sure how best to re-word it.

[Brian Wilkerson, 2015-12-02 01:57:01]

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

[pquitslund, 2015-12-02 16:40:23]

Thanks Brian!

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/dependencies/reachable_source_collector.dart
(right):

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/dependencies/reachable_source_collector.dart:32:
String sourceUri = source?.uri?.toString();
On 2015/12/02 01:56:27, Brian Wilkerson wrote:
> nit: We don't need the null-aware operators if we test 'source' in
> 'collectSources'. Not sure whether there's enough of a performance penalty to
> care, but it would make the code a little cleaner.

Except that this is called recursively...  My thinking is that a source (or it's
URI) could be null anywhere in the chain of dependencies.

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/dependencies/reachable_source_collector.dart:35:
if (sourceUri == null) {
On 2015/12/02 01:56:27, Brian Wilkerson wrote:
> Similarly, we don't need this test.

See above.

FWIW: my rationale is based on doing a bunch of scurrying around to harden lint
rules that fell down in the face of unexpected null values.

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/test/do...
File pkg/analysis_server/test/domain_analysis_test.dart (right):

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/test/do...
pkg/analysis_server/test/domain_analysis_test.dart:58:
test('getReachableSources', () async {
On 2015/12/02 01:56:27, Brian Wilkerson wrote:
> Maybe put this in a group?
> 
> Also test what happens if the file passed to the request is not being analyzed
> (is not in an analysis root).

Done.

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/test/do...
pkg/analysis_server/test/domain_analysis_test.dart:76:
unorderedEquals(['dart:core', 'file:///project/b.dart']));
On 2015/12/02 01:56:27, Brian Wilkerson wrote:
> Perhaps verify that b.dart is a key in the map?

Done.

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/tool/sp...
File pkg/analysis_server/tool/spec/spec_input.html (right):

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/tool/sp...
pkg/analysis_server/tool/spec/spec_input.html:400: Return the transitive closure
of reachable sources for a given file.
On 2015/12/02 01:56:27, Brian Wilkerson wrote:
> This should return an error response if the file is not currently being
> analyzed. (I know there's wording in other requests that you could borrow.)

Ah, yes.  Good.

Updated!

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/tool/sp...
pkg/analysis_server/tool/spec/spec_input.html:419: { "file:///foo.dart" :
["file:///bar.dart"] }.  If "bar.dart" has further imports
On 2015/12/02 01:56:27, Brian Wilkerson wrote:
> The "If" implies to me that 'bar.dart' might not be a key in the mapping, but
I
> think it always will be a key (because everything implicitly imports
'dart:core'
> even if it doesn't explicitly import anything. Not sure how best to re-word
it.

Acknowledged.

[devoncarew, 2015-12-02 17:50:37]

lgtm

Thanks! Perhaps bump the api version as well, since this is an addition to the
API?

[Brian Wilkerson, 2015-12-02 18:04:53]

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/dependencies/reachable_source_collector.dart
(right):

https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/dependencies/reachable_source_collector.dart:32:
String sourceUri = source?.uri?.toString();
That shouldn't be the case. IMPORTED_LIBRARIES and EXPORTED_LIBRARIES should
never contain nulls.

[pquitslund, 2015-12-02 18:27:40]

On 2015/12/02 17:50:37, devoncarew wrote:
> lgtm
> 
> Thanks! Perhaps bump the api version as well, since this is an addition to the
> API?

Right!

[pquitslund, 2015-12-02 19:15:24]

On 2015/12/02 18:04:53, Brian Wilkerson wrote:
>
https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/lib/src...
> File
>
pkg/analysis_server/lib/src/services/dependencies/reachable_source_collector.dart
> (right):
> 
>
https://codereview.chromium.org/1491013002/diff/1/pkg/analysis_server/lib/src...
>
pkg/analysis_server/lib/src/services/dependencies/reachable_source_collector.dart:32:
> String sourceUri = source?.uri?.toString();
> That shouldn't be the case. IMPORTED_LIBRARIES and EXPORTED_LIBRARIES should
> never contain nulls.

Got it.  I was playing it a little too safe here.

Fixed.

[pquitslund, 2015-12-02 21:26:42]

Description was changed from

==========
Analysis request `getReachableSources` (#24893).

Implements a new request to get reachable sources.

This solves a specific issue for flutter (e.g., inferring execution type by
checking the transitive closure of reachabe sources for `dart:flutter`) but can
be more generallly useful for other client-side smarts (e.g., "is this a web
entry point?" or "is this a test?").

More context here: https://github.com/dart-lang/sdk/issues/24893
==========

to

==========
Analysis request `getReachableSources` (#24893).

Implements a new request to get reachable sources.

This solves a specific issue for flutter (e.g., inferring execution type by
checking the transitive closure of reachabe sources for `dart:flutter`) but can
be more generallly useful for other client-side smarts (e.g., "is this a web
entry point?" or "is this a test?").

More context here: https://github.com/dart-lang/sdk/issues/24893

R=brianwilkerson@google.com, devoncarew@google.com

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

[pquitslund, 2015-12-02 21:26:44]

Committed patchset #3 (id:40001) manually as
43f027fc05f25a40049dee0a71418570f6f20d10 (presubmit successful).