Remove references to AnalysisDriver from the plugin mixin classes

Remove references to AnalysisDriver from the plugin mixin classes

R=scheglov@google.com

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

[Brian Wilkerson, 2017-07-05 20:53:13]

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

[scheglov, 2017-07-05 21:58:03]

LGTM

[mfairhurst, 2017-07-05 21:59:45]

Sorry to get back to you late on this.

What was the reason to remove this?

Since every request begins with finding the right driver, seems like it should
be part of the base class.

We also use that 'covariant' so that we can accept AngularDriver's rather than
AnalysisDriverGeneric's and then cast them.

[Brian Wilkerson, 2017-07-05 22:47:40]

> What was the reason to remove this?

Before we publish a non-alpha version of this package, I want to remove any
references to non-public classes from the public API. AnalysisDriver[Generic] is
not a part of the public API, but the mixin classes are. Hence, it is required
that we remove AnalysisDriverGeneric from the mixin classes.

> Since every request begins with finding the right driver, seems like it should
be part of the base class.

I don't think this significantly increases the amount of work a plugin author
needs to do. Nor does it make plugins less efficient. We've just slightly
altered where the work is being done.

> We also use that 'covariant' so that we can accept AngularDriver's rather than
AnalysisDriverGeneric's and then cast them.

True, but if you write something like

  AngularAnalysisDriver driver = driverForPath(path);

you'll get the same implicit cast that you get with a `covariant` parameter.

I'm happy to discuss other ways to remove AnalysisDriverGeneric from these APIs,
but I think it has to happen.

[mfairhurst, 2017-07-05 22:50:39]

On 2017/07/05 22:47:40, Brian Wilkerson wrote:
> > What was the reason to remove this?
> 
> Before we publish a non-alpha version of this package, I want to remove any
> references to non-public classes from the public API. AnalysisDriver[Generic]
is
> not a part of the public API, but the mixin classes are. Hence, it is required
> that we remove AnalysisDriverGeneric from the mixin classes.
> 
> > Since every request begins with finding the right driver, seems like it
should
> be part of the base class.
> 
> I don't think this significantly increases the amount of work a plugin author
> needs to do. Nor does it make plugins less efficient. We've just slightly
> altered where the work is being done.
> 
> > We also use that 'covariant' so that we can accept AngularDriver's rather
than
> AnalysisDriverGeneric's and then cast them.
> 
> True, but if you write something like
> 
>   AngularAnalysisDriver driver = driverForPath(path);
> 
> you'll get the same implicit cast that you get with a `covariant` parameter.
> 
> I'm happy to discuss other ways to remove AnalysisDriverGeneric from these
APIs,
> but I think it has to happen.

This makes sense, for sure.

Especially since AnalysisDriverGeneric was a class that I threw together with no
real objectives except make what I needed to happen happen.

OK, we can work around this, I don't see a better solution. But I will miss the
cleanliness of the covariant driver arguments in terms of cutting out
boilerplate in our own plugin implementation.

[mfairhurst, 2017-07-05 22:50:51]

https://codereview.chromium.org/2970203002/diff/1/pkg/analyzer_plugin/lib/plu...
File pkg/analyzer_plugin/lib/plugin/plugin.dart (right):

https://codereview.chromium.org/2970203002/diff/1/pkg/analyzer_plugin/lib/plu...
pkg/analyzer_plugin/lib/plugin/plugin.dart:196: AnalysisDriverGeneric
driverForPath(String path) {
Isn't this also a public usage of a private API?

[Brian Wilkerson, 2017-07-05 23:06:49]

https://codereview.chromium.org/2970203002/diff/1/pkg/analyzer_plugin/lib/plu...
File pkg/analyzer_plugin/lib/plugin/plugin.dart (right):

https://codereview.chromium.org/2970203002/diff/1/pkg/analyzer_plugin/lib/plu...
pkg/analyzer_plugin/lib/plugin/plugin.dart:196: AnalysisDriverGeneric
driverForPath(String path) {
On 2017/07/05 22:50:51, mfairhurst wrote:
> Isn't this also a public usage of a private API?

Yes, it is. But it seemed like an expedient step toward the goal to get all of
the references out of the mixins at the cost of adding one more to ServerPlugin.
I might regret it, but...

[Brian Wilkerson, 2017-07-06 16:07:13]

Description was changed from

==========
Remove references to AnalysisDriver from the plugin mixin classes
==========

to

==========
Remove references to AnalysisDriver from the plugin mixin classes

R=scheglov@google.com

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

[Brian Wilkerson, 2017-07-06 16:07:14]

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