Fixes in smoke, which in turn should fix todomvc.

Fixes in smoke, which in turn should fix todomvc.

R=jmesserly@google.com

Committed: https://code.google.com/p/dart/source/detail?r=33047

[Siggi Cherem (dart-lang), 2014-02-25 23:23:22]

https://codereview.chromium.org/180273004/diff/20001/pkg/observe/lib/src/path...
File pkg/observe/lib/src/path_observer.dart (left):

https://codereview.chromium.org/180273004/diff/20001/pkg/observe/lib/src/path...
pkg/observe/lib/src/path_observer.dart:568: int _minArgumentCount(fn) {
I forgot to delete this last time (now that smoke has it there too)

https://codereview.chromium.org/180273004/diff/20001/pkg/polymer/lib/polymer....
File pkg/polymer/lib/polymer.dart (right):

https://codereview.chromium.org/180273004/diff/20001/pkg/polymer/lib/polymer....
pkg/polymer/lib/polymer.dart:49: targets: const [PublishedProperty],
needed to check that this extends from ObservableProperty in the observe
package.

https://codereview.chromium.org/180273004/diff/20001/pkg/polymer/lib/src/decl...
File pkg/polymer/lib/src/declaration.dart (right):

https://codereview.chromium.org/180273004/diff/20001/pkg/polymer/lib/src/decl...
pkg/polymer/lib/src/declaration.dart:459: includeUpTo: HtmlElement);
this API change was not needed to fix things, but as I was debugging it was
useful to implement it. This was anyways how it used to be.

Later on we could add other filters. For example, stop whenever the base class
belongs to the core-libraries (this would help in the case of the observable
code)

https://codereview.chromium.org/180273004/diff/20001/pkg/smoke/lib/mirrors.dart
File pkg/smoke/lib/mirrors.dart (right):

https://codereview.chromium.org/180273004/diff/20001/pkg/smoke/lib/mirrors.da...
pkg/smoke/lib/mirrors.dart:154: cls.superclass !=
reflectClass(options.includeUpTo);
note that for the same reasons than below (classes with type-args fail), we
don't do this instead:
  _toType(cls.superclass) != options.includeUpTo

[Jennifer Messerly, 2014-02-25 23:50:09]

LGTM

https://codereview.chromium.org/180273004/diff/20001/pkg/polymer/lib/polymer....
File pkg/polymer/lib/polymer.dart (right):

https://codereview.chromium.org/180273004/diff/20001/pkg/polymer/lib/polymer....
pkg/polymer/lib/polymer.dart:49: targets: const [PublishedProperty],
On 2014/02/25 23:23:23, Siggi Cherem (dart-lang) wrote:
> needed to check that this extends from ObservableProperty in the observe
> package.

yeah, seems fine, but shouldn't it be a metatarget? :)
I guess we've been getting lucky and people are only using @published inside of
@CustomTag, therefore we haven't hit on the problem yet ...

https://codereview.chromium.org/180273004/diff/20001/pkg/polymer/lib/src/decl...
File pkg/polymer/lib/src/declaration.dart (right):

https://codereview.chromium.org/180273004/diff/20001/pkg/polymer/lib/src/decl...
pkg/polymer/lib/src/declaration.dart:459: includeUpTo: HtmlElement);
On 2014/02/25 23:23:23, Siggi Cherem (dart-lang) wrote:
> this API change was not needed to fix things, but as I was debugging it was
> useful to implement it. This was anyways how it used to be.
> 
> Later on we could add other filters. For example, stop whenever the base class
> belongs to the core-libraries (this would help in the case of the observable
> code)

Very nice!

https://codereview.chromium.org/180273004/diff/20001/pkg/smoke/lib/mirrors.dart
File pkg/smoke/lib/mirrors.dart (right):

https://codereview.chromium.org/180273004/diff/20001/pkg/smoke/lib/mirrors.da...
pkg/smoke/lib/mirrors.dart:154: cls.superclass !=
reflectClass(options.includeUpTo);
On 2014/02/25 23:23:23, Siggi Cherem (dart-lang) wrote:
> note that for the same reasons than below (classes with type-args fail), we
> don't do this instead:
>   _toType(cls.superclass) != options.includeUpTo
> 

might be worth adding this comment (it can refer to the one below), so we know
to clean it up when 16925 is fixed :)

[Siggi Cherem (dart-lang), 2014-02-26 00:09:50]

https://codereview.chromium.org/180273004/diff/20001/pkg/polymer/lib/polymer....
File pkg/polymer/lib/polymer.dart (right):

https://codereview.chromium.org/180273004/diff/20001/pkg/polymer/lib/polymer....
pkg/polymer/lib/polymer.dart:49: targets: const [PublishedProperty],
On 2014/02/25 23:50:09, John Messerly wrote:
> On 2014/02/25 23:23:23, Siggi Cherem (dart-lang) wrote:
> > needed to check that this extends from ObservableProperty in the observe
> > package.
> 
> yeah, seems fine, but shouldn't it be a metatarget? :)
> I guess we've been getting lucky and people are only using @published inside
of
> @CustomTag, therefore we haven't hit on the problem yet ...

good point. However, I had to include it in both. `metaTargets` preserves what
the annotation applies to, but not the annotation itself. So, to actually check
isSuperclassOf, I also need it in targets.

https://codereview.chromium.org/180273004/diff/20001/pkg/smoke/lib/mirrors.dart
File pkg/smoke/lib/mirrors.dart (right):

https://codereview.chromium.org/180273004/diff/20001/pkg/smoke/lib/mirrors.da...
pkg/smoke/lib/mirrors.dart:154: cls.superclass !=
reflectClass(options.includeUpTo);
On 2014/02/25 23:50:09, John Messerly wrote:
> On 2014/02/25 23:23:23, Siggi Cherem (dart-lang) wrote:
> > note that for the same reasons than below (classes with type-args fail), we
> > don't do this instead:
> >   _toType(cls.superclass) != options.includeUpTo
> > 
> 
> might be worth adding this comment (it can refer to the one below), so we know
> to clean it up when 16925 is fixed :)

Done.

[Siggi Cherem (dart-lang), 2014-02-26 00:27:14]

Committed patchset #4 manually as r33047 (presubmit successful).