Check for missing elements (issue 26987)

Check for missing elements (issue 26987)

R=pquitslund@google.com

Committed: https://github.com/dart-lang/sdk/commit/51bda5ac481750ee8986ae0363a0385d988ebeb1

[Brian Wilkerson, 2016-07-29 17:09:06]

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

[Brian Wilkerson, 2016-07-29 17:09:06]

https://codereview.chromium.org/2196483004/diff/1/pkg/analyzer/test/generated...
File pkg/analyzer/test/generated/hint_code_test.dart (right):

https://codereview.chromium.org/2196483004/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/hint_code_test.dart:1781: void
test_missingJsLibAnnotation_variable() {
Should this test pass, or is the test invalid?

[pquitslund, 2016-07-29 18:05:26]

LGTM % questions.

https://codereview.chromium.org/2196483004/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/error_verifier.dart (right):

https://codereview.chromium.org/2196483004/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/error_verifier.dart:4397: if (element != null &&
element.library?.isJS != true) {
Should this be triggered if the library is null?

If not, maybe this would this be a better test?

if (element?.library?.isJS == false) { ... }

https://codereview.chromium.org/2196483004/diff/1/pkg/analyzer/test/generated...
File pkg/analyzer/test/generated/hint_code_test.dart (right):

https://codereview.chromium.org/2196483004/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/hint_code_test.dart:1781: void
test_missingJsLibAnnotation_variable() {
On 2016/07/29 17:09:06, Brian Wilkerson wrote:
> Should this test pass, or is the test invalid?

Hmmmm.  It seems to me like it *should* pass.

[Brian Wilkerson, 2016-07-29 18:56:22]

PTAL

https://codereview.chromium.org/2196483004/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/error_verifier.dart (right):

https://codereview.chromium.org/2196483004/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/error_verifier.dart:4397: if (element != null &&
element.library?.isJS != true) {
> Should this be triggered if the library is null?

I have no idea. The library should never be null; if it is it means there's a
bug in analyzer.

That said, in the process of fixing the other failing test I realized that this
is more complex than it needs to be, so the question is now moot.

https://codereview.chromium.org/2196483004/diff/1/pkg/analyzer/test/generated...
File pkg/analyzer/test/generated/hint_code_test.dart (right):

https://codereview.chromium.org/2196483004/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/hint_code_test.dart:1781: void
test_missingJsLibAnnotation_variable() {
Ok, it passes now.

[pquitslund, 2016-07-29 20:14:30]

LGTM (assuming my question is moot which I'm thinking it may well be!)

https://codereview.chromium.org/2196483004/diff/20001/pkg/analyzer/lib/src/ge...
File pkg/analyzer/lib/src/generated/error_verifier.dart (right):

https://codereview.chromium.org/2196483004/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/error_verifier.dart:4396: if
(_currentLibrary.isJS != true) {
Is `_currentLibrary` certain to be non-null?  Or did you mean
`_currentLibrary?.isJS != true`?

[Brian Wilkerson, 2016-07-29 20:19:37]

https://codereview.chromium.org/2196483004/diff/20001/pkg/analyzer/lib/src/ge...
File pkg/analyzer/lib/src/generated/error_verifier.dart (right):

https://codereview.chromium.org/2196483004/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/error_verifier.dart:4396: if
(_currentLibrary.isJS != true) {
> Is `_currentLibrary` certain to be non-null?

Yes. (It's a final field initialized in the constructor and de-referenced in the
constructor, so if `null` were ever passed in the constructor would throw an
exception.)

[Brian Wilkerson, 2016-07-29 20:19:58]

Description was changed from

==========
Check for missing elements (issue 26987)
==========

to

==========
Check for missing elements (issue 26987)

R=pquitslund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/51bda5ac481750ee8986ae0363a0385d988ebeb1
==========

[Brian Wilkerson, 2016-07-29 20:19:59]

Committed patchset #2 (id:20001) manually as
51bda5ac481750ee8986ae0363a0385d988ebeb1 (presubmit successful).