Plugin manifest parsing.

Plugin manifest parsing.

R=brianwilkerson@google.com

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

[pquitslund, 2015-09-24 17:59:41]

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

[Brian Wilkerson, 2015-09-24 19:22:57]

https://chromiumcodereview.appspot.com/1366023002/diff/1/pkg/analyzer/lib/src...
File pkg/analyzer/lib/src/plugin/plugin_configuration.dart (right):

https://chromiumcodereview.appspot.com/1366023002/diff/1/pkg/analyzer/lib/src...
pkg/analyzer/lib/src/plugin/plugin_configuration.dart:18: PluginInfo
analyzerPlugin = _parsePlugin(yaml['analyzer-plugin']);
I'm confused. I thought we were going to (a) have a one-to-one relationship
between plugins and packages and (b) have a one-to-one relationship between
plugin manifest files and packages. If those are both true, then a single plugin
manifest file should describe a single plugin.

[pquitslund, 2015-09-24 19:32:36]

Right!  I guess I'm back-peddling... :)

https://chromiumcodereview.appspot.com/1366023002/diff/1/pkg/analyzer/lib/src...
File pkg/analyzer/lib/src/plugin/plugin_configuration.dart (right):

https://chromiumcodereview.appspot.com/1366023002/diff/1/pkg/analyzer/lib/src...
pkg/analyzer/lib/src/plugin/plugin_configuration.dart:18: PluginInfo
analyzerPlugin = _parsePlugin(yaml['analyzer-plugin']);
On 2015/09/24 19:22:57, Brian Wilkerson wrote:
> I'm confused. I thought we were going to (a) have a one-to-one relationship
> between plugins and packages and (b) have a one-to-one relationship between
> plugin manifest files and packages. If those are both true, then a single
plugin
> manifest file should describe a single plugin.

I guess I didn't want to completely rule out the possibility that we might allow
folks to declare an analyzer and server plugin *pair* in one package.  If that's
categorically inadvisable, maybe we shouldn't support it though?

Not a huge deal either way.

[Brian Wilkerson, 2015-09-24 19:39:37]

> I guess I didn't want to completely rule out the possibility that we might
allow
> folks to declare an analyzer and server plugin *pair* in one package.

If we want to support more than one we should support an arbitrary number. If
we're going to support an arbitrary number we should make the top-level key be
the name of the plugin (the name used wherever we're configuring which plugins
to include).

The advantage of the single-plugin-per-package model is that the configuration
specification can reference the package without additional information. If a
package can contain more than one plugin then the configuration specification
needs to reference both the package and plugin.

[pquitslund, 2015-09-24 21:15:41]

Updated as per our discussion.

Provisional manifest format now looks like:

     class_name: MyAnalyzerPlugin
     library_uri: 'my_plugin/my_analyzer_plugin.dart'
     contributes_to: analyzer

[Brian Wilkerson, 2015-09-24 21:40:00]

LGTM

https://codereview.chromium.org/1366023002/diff/40001/pkg/analyzer/test/src/p...
File pkg/analyzer/test/src/plugin/plugin_config_test.dart (right):

https://codereview.chromium.org/1366023002/diff/40001/pkg/analyzer/test/src/p...
pkg/analyzer/test/src/plugin/plugin_config_test.dart:55: library_uri:
myplugin/analyzer_plugin.dart
I suspect that in most cases the URI will be a "package:" URI. It might be nice
to test that case at least once.

[pquitslund, 2015-09-24 21:52:44]

Thanks!

https://codereview.chromium.org/1366023002/diff/40001/pkg/analyzer/test/src/p...
File pkg/analyzer/test/src/plugin/plugin_config_test.dart (right):

https://codereview.chromium.org/1366023002/diff/40001/pkg/analyzer/test/src/p...
pkg/analyzer/test/src/plugin/plugin_config_test.dart:55: library_uri:
myplugin/analyzer_plugin.dart
On 2015/09/24 21:40:00, Brian Wilkerson wrote:
> I suspect that in most cases the URI will be a "package:" URI. It might be
nice
> to test that case at least once.

Will do.  I want to add more validation for sure as we make this more
executable.

[pquitslund, 2015-09-24 21:53:21]

Committed patchset #3 (id:40001) manually as
5dd6a543e74b8fc183563394ef5507d7740ed5c5 (presubmit successful).