Add UNDEFINED_HIDDEN_NAME, UNDEFINED_SHOWN_NAME

Add UNDEFINED_HIDDEN_NAME, UNDEFINED_SHOWN_NAME

BUG=https://github.com/dart-lang/sdk/issues/20363

Also https://github.com/dart-lang/sdk/issues/25154, https://github.com/dart-lang/sdk/issues/25672

R=brianwilkerson@google.com

Committed: https://github.com/dart-lang/sdk/commit/53bf36acdb4a6a12addf15eba06eacdab7552f22

[srawlins, 2016-04-18 22:39:01]

srawlins@google.com changed reviewers:
+ brianwilkerson@google.com, lrn@chromium.org

[srawlins, 2016-04-18 22:39:35]

Description was changed from

==========
Add UNDEFINED_HIDDEN_NAME, UNDEFINED_SHOWN_NAME

BUG=https://github.com/dart-lang/sdk/issues/20363
==========

to

==========
Add UNDEFINED_HIDDEN_NAME, UNDEFINED_SHOWN_NAME

BUG=https://github.com/dart-lang/sdk/issues/20363

Also https://github.com/dart-lang/sdk/issues/25154,
https://github.com/dart-lang/sdk/issues/25672
==========

[Lasse Reichstein Nielsen, 2016-04-19 12:36:31]

lrn@google.com changed reviewers:
+ lrn@google.com

[Lasse Reichstein Nielsen, 2016-04-19 12:36:32]

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

https://codereview.chromium.org/1901923002/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/error.dart:3696: "The hidden name '{0}' is
undefined.");
"is undefined" is a little vague. Maybe:
 The library <URI> doesn't export a member with the hidden name <name>
?

[Brian Wilkerson, 2016-04-19 14:07:32]

I'm a little concerned about making this a hint rather than a lint because hints
are opt-out while lints are opt-in. There's a reason this was not made a warning
in the specification.

On the other hand, we already have a lot of "dead code" related hints, so
there's precedent. What do you think?

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

https://codereview.chromium.org/1901923002/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/element_resolver.dart:2092: ErrorCode hintCode;
I would prefer to have the hint generation moved to the code that generates the
majority of the hints (can't remember the class name at the moment, but I can
look it up if you'd like). (I'd actually like to remove all hint and error
generation from this class for performance reasons.)

[Brian Wilkerson, 2016-04-20 18:09:57]

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

https://codereview.chromium.org/1901923002/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/element_resolver.dart:2092: ErrorCode hintCode;
To follow up: turns out I couldn't remember because it doesn't exist. Hints are
generated by several classes, but the most appropriate would be
DeadCodeVerifier.

[srawlins, 2016-04-20 20:34:01]

Thanks for the review! Everything addressed.

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

https://codereview.chromium.org/1901923002/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/element_resolver.dart:2092: ErrorCode hintCode;
On 2016/04/19 14:07:32, Brian Wilkerson wrote:
> I would prefer to have the hint generation moved to the code that generates
the
> majority of the hints (can't remember the class name at the moment, but I can
> look it up if you'd like). (I'd actually like to remove all hint and error
> generation from this class for performance reasons.)

Done.

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

https://codereview.chromium.org/1901923002/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/error.dart:3696: "The hidden name '{0}' is
undefined.");
On 2016/04/19 12:36:32, Lasse Reichstein Nielsen wrote:
> "is undefined" is a little vague. Maybe:
>  The library <URI> doesn't export a member with the hidden name <name>
> ?

Done.

[srawlins, 2016-04-20 20:35:41]

On 2016/04/19 14:07:32, Brian Wilkerson wrote:
> I'm a little concerned about making this a hint rather than a lint because
hints
> are opt-out while lints are opt-in. There's a reason this was not made a
warning
> in the specification.
> 
> On the other hand, we already have a lot of "dead code" related hints, so
> there's precedent. What do you think?
> 
>
https://codereview.chromium.org/1901923002/diff/1/pkg/analyzer/lib/src/genera...
> File pkg/analyzer/lib/src/generated/element_resolver.dart (right):
> 
>
https://codereview.chromium.org/1901923002/diff/1/pkg/analyzer/lib/src/genera...
> pkg/analyzer/lib/src/generated/element_resolver.dart:2092: ErrorCode hintCode;
> I would prefer to have the hint generation moved to the code that generates
the
> majority of the hints (can't remember the class name at the moment, but I can
> look it up if you'd like). (I'd actually like to remove all hint and error
> generation from this class for performance reasons.)

Hi Brian,

I'd like it to be a Hint because of the precedent reason (UNUSED_IMPORT, etc),
and because it shouldn't have false positives.

Do you know what the reason is, that "this was not made a warning in the
specification."?

[Brian Wilkerson, 2016-04-20 21:34:25]

lgtm

The spec says:

Note that no errors or warnings are given if one hides or shows a name that is
not in a namespace. This prevents situations where removing a name from a
library would cause breakage of a client library.

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

https://codereview.chromium.org/1901923002/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/resolver.dart:2100: if (combinator is
HideCombinator) {
Can we pass in the list of names (and the hint code) so that we can avoid the
'is' checks? They're really expensive.

[srawlins, 2016-04-20 21:52:39]

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

https://codereview.chromium.org/1901923002/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/resolver.dart:2100: if (combinator is
HideCombinator) {
On 2016/04/20 21:34:25, Brian Wilkerson wrote:
> Can we pass in the list of names (and the hint code) so that we can avoid the
> 'is' checks? They're really expensive.

I've uploaded another patch. Is this what you want? I don't see how to avoid
_any_ 'is' checks.

[Brian Wilkerson, 2016-04-20 22:49:32]

Still lgtm

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

https://codereview.chromium.org/1901923002/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/resolver.dart:2100: if (combinator is
HideCombinator) {
Close enough. I was thinking of

void _checkCombinator(LibraryElement library, Combinator combinator,
NodeList<SimpleIdentifier> names, ErrorCode hintCode)

and moving the type-specific code to the call sites, but this is better than
before.

[srawlins, 2016-04-20 22:51:54]

Description was changed from

==========
Add UNDEFINED_HIDDEN_NAME, UNDEFINED_SHOWN_NAME

BUG=https://github.com/dart-lang/sdk/issues/20363

Also https://github.com/dart-lang/sdk/issues/25154,
https://github.com/dart-lang/sdk/issues/25672
==========

to

==========
Add UNDEFINED_HIDDEN_NAME, UNDEFINED_SHOWN_NAME

BUG=https://github.com/dart-lang/sdk/issues/20363

Also https://github.com/dart-lang/sdk/issues/25154,
https://github.com/dart-lang/sdk/issues/25672

R=brianwilkerson@google.com

Committed:
https://github.com/dart-lang/sdk/commit/53bf36acdb4a6a12addf15eba06eacdab7552f22
==========

[srawlins, 2016-04-20 22:51:56]

Committed patchset #3 (id:40001) manually as
53bf36acdb4a6a12addf15eba06eacdab7552f22 (presubmit successful).