Fix handling of static names for JS interop and improve test coverage.

Fix handling of static names for JS interop and improve test coverage.

BUG=
R=jmesserly@google.com

Committed: https://github.com/dart-lang/sdk/commit/1370b6bb4613f64e1e2ee2cf0d23b0afdaee743e

[Jacob, 2016-11-29 17:16:13]

jacobr@google.com changed reviewers:
+ jmesserly@google.com

[Jennifer Messerly, 2016-11-29 20:54:53]

with two small changes, LGTM :)

https://codereview.chromium.org/2536823003/diff/20001/pkg/dev_compiler/lib/sr...
File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/2536823003/diff/20001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:376:
assert(!name.contains('.'));
This should be an error or a throw, rather than an assert. asserts should mean
invariants that (we believe) cannot be broken under any circumstances. This one
could trip because of user input.

https://codereview.chromium.org/2536823003/diff/20001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3461: if (enclosingElement
is ClassElement) {
the "is" check is unnecessary -- FieldElement is a ClassMemberElement, here is
how it is defined in Analyzer:

abstract class FieldElement
    implements ClassMemberElement ...

abstract class ClassMemberElement implements Element {
  @override
  ClassElement get enclosingElement;
  // ...
}

(I'm surprised this wasn't a hint since that evaluates to "true")

[Jacob, 2016-11-29 21:33:25]

Description was changed from

==========
Fix handling of static names for JS interop and improve test coverage.

BUG=
==========

to

==========
Fix handling of static names for JS interop and improve test coverage.

BUG=
R=jmesserly@google.com

Committed:
https://github.com/dart-lang/sdk/commit/1370b6bb4613f64e1e2ee2cf0d23b0afdaee743e
==========

[Jacob, 2016-11-29 21:33:26]

Committed patchset #3 (id:40001) manually as
1370b6bb4613f64e1e2ee2cf0d23b0afdaee743e (presubmit successful).

[Jacob, 2016-11-29 21:37:33]

https://codereview.chromium.org/2536823003/diff/20001/pkg/dev_compiler/lib/sr...
File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/2536823003/diff/20001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:376:
assert(!name.contains('.'));
On 2016/11/29 20:54:52, Jennifer Messerly wrote:
> This should be an error or a throw, rather than an assert. asserts should mean
> invariants that (we believe) cannot be broken under any circumstances. This
one
> could trip because of user input.

Done.

https://codereview.chromium.org/2536823003/diff/20001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3461: if (enclosingElement
is ClassElement) {
On 2016/11/29 20:54:52, Jennifer Messerly wrote:
> the "is" check is unnecessary -- FieldElement is a ClassMemberElement, here is
> how it is defined in Analyzer:
> 
> abstract class FieldElement
>     implements ClassMemberElement ...
> 
> abstract class ClassMemberElement implements Element {
>   @override
>   ClassElement get enclosingElement;
>   // ...
> }
> 
> (I'm surprised this wasn't a hint since that evaluates to "true")

Surprised I ever thought that is check was needed.