support analysis options include declaration

work in progress - support analysis options include declaration

R=brianwilkerson@google.com

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

[danrubel, 2016-11-11 18:59:55]

This enhances AnalysisOptionsProvider to handle include declarations in the
analysis options file.

This functionality will be hooked into the context builder in a subsequent CL.

[Brian Wilkerson, 2016-11-11 19:37:47]

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

[Brian Wilkerson, 2016-11-11 19:37:47]

lgtm

https://codereview.chromium.org/2489973002/diff/1/pkg/analyzer/lib/source/ana...
File pkg/analyzer/lib/source/analysis_options_provider.dart (right):

https://codereview.chromium.org/2489973002/diff/1/pkg/analyzer/lib/source/ana...
pkg/analyzer/lib/source/analysis_options_provider.dart:55: YamlNode node =
options.remove('include');
Given that "participants" are able to look at the options in order to do their
own processing, it isn't clear to me that we should be removing the includes
key. Not sure it matters in practice, though.

https://codereview.chromium.org/2489973002/diff/1/pkg/analyzer/lib/source/ana...
pkg/analyzer/lib/source/analysis_options_provider.dart:142: return null;
There is linter support for linting options files. We should add a github issue
to add a lint for the cases where the include contains an invalid uri.

https://codereview.chromium.org/2489973002/diff/1/pkg/analyzer/test/source/an...
File pkg/analyzer/test/source/analysis_options_provider_test.dart (right):

https://codereview.chromium.org/2489973002/diff/1/pkg/analyzer/test/source/an...
pkg/analyzer/test/source/analysis_options_provider_test.dart:195: void
test_getOptions_include() {
Should we have a test for package: URIs, or is it too early?

[danrubel, 2016-11-14 20:12:09]

Description was changed from

==========
work in progress - support analysis options include declaration
==========

to

==========
work in progress - support analysis options include declaration

R=brianwilkerson@google.com

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

[danrubel, 2016-11-14 20:12:13]

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

[danrubel, 2016-11-14 20:39:33]

https://codereview.chromium.org/2489973002/diff/1/pkg/analyzer/lib/source/ana...
File pkg/analyzer/lib/source/analysis_options_provider.dart (right):

https://codereview.chromium.org/2489973002/diff/1/pkg/analyzer/lib/source/ana...
pkg/analyzer/lib/source/analysis_options_provider.dart:55: YamlNode node =
options.remove('include');
On 2016/11/11 19:37:47, Brian Wilkerson wrote:
> Given that "participants" are able to look at the options in order to do their
> own processing, it isn't clear to me that we should be removing the includes
> key. Not sure it matters in practice, though.

I don't think the "include" directive makes sense as part of the options once
options are merged. We can always add something like a
"getPreMergeOptionsFromSource" method if needed which will not merge and will
not remove any include directive.

That said, I can keep the "include" directive in the resulting yaml map if you'd
prefer.

https://codereview.chromium.org/2489973002/diff/1/pkg/analyzer/lib/source/ana...
pkg/analyzer/lib/source/analysis_options_provider.dart:142: return null;
On 2016/11/11 19:37:47, Brian Wilkerson wrote:
> There is linter support for linting options files. We should add a github
issue
> to add a lint for the cases where the include contains an invalid uri.

Good idea. https://github.com/dart-lang/sdk/issues/27817