Add validation of private symbols to the dart2js mirrors.

Add validation of private symbols to the dart2js mirrors.

The mirrors.getSymbol function works for non-private symbols, and
validates private symbols (but still fail to create a Symbol object).

R=ahe@google.com

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

[ahe, 2014-02-24 10:29:03]

+Karl

[ahe, 2014-02-24 10:43:22]

Awesome, LGTM!

https://codereview.chromium.org/177483002/diff/1/sdk/lib/_internal/lib/mirror...
File sdk/lib/_internal/lib/mirrors_patch.dart (right):

https://codereview.chromium.org/177483002/diff/1/sdk/lib/_internal/lib/mirror...
sdk/lib/_internal/lib/mirrors_patch.dart:13: patch static Symbol
getSymbol(String name, [LibraryMirror library]) {
Could you move this code to a top-level in _js_mirrors? Generally, I think that
the less code you have in patch files, the easier it is to understand what is
going on. Also, you reduce the number of imports of dart:_internal.

I failed to point this out when reflectClass and reflectType were changed.

https://codereview.chromium.org/177483002/diff/1/sdk/lib/internal/symbol.dart
File sdk/lib/internal/symbol.dart (right):

https://codereview.chromium.org/177483002/diff/1/sdk/lib/internal/symbol.dart...
sdk/lib/internal/symbol.dart:18: // Reserved words are not allowed as
identifiers.
How about turning these comments into proper documentation comments?

https://codereview.chromium.org/177483002/diff/1/sdk/lib/internal/symbol.dart...
sdk/lib/internal/symbol.dart:27: // Matches a public or private identifier.
I suggest:

/// Matches an identifier (public or private).

https://codereview.chromium.org/177483002/diff/1/sdk/lib/internal/symbol.dart...
sdk/lib/internal/symbol.dart:60: static final RegExp privateValidationPattern =
new RegExp(
This name is slightly confusing: this doesn't validate that something is
private.

How about this rename:

validationPattern -> nonPrivateValidationPattern.
privateValidationPattern -> validationPattern.

[Lasse Reichstein Nielsen, 2014-03-03 12:58:44]

https://codereview.chromium.org/177483002/diff/1/sdk/lib/_internal/lib/mirror...
File sdk/lib/_internal/lib/mirrors_patch.dart (right):

https://codereview.chromium.org/177483002/diff/1/sdk/lib/_internal/lib/mirror...
sdk/lib/_internal/lib/mirrors_patch.dart:13: patch static Symbol
getSymbol(String name, [LibraryMirror library]) {
On 2014/02/24 10:43:23, ahe wrote:
> Could you move this code to a top-level in _js_mirrors? Generally, I think
that
> the less code you have in patch files, the easier it is to understand what is
> going on. Also, you reduce the number of imports of dart:_internal.
> 
> I failed to point this out when reflectClass and reflectType were changed.

Done.

https://codereview.chromium.org/177483002/diff/1/sdk/lib/internal/symbol.dart
File sdk/lib/internal/symbol.dart (right):

https://codereview.chromium.org/177483002/diff/1/sdk/lib/internal/symbol.dart...
sdk/lib/internal/symbol.dart:18: // Reserved words are not allowed as
identifiers.
On 2014/02/24 10:43:23, ahe wrote:
> How about turning these comments into proper documentation comments?

Done.

https://codereview.chromium.org/177483002/diff/1/sdk/lib/internal/symbol.dart...
sdk/lib/internal/symbol.dart:27: // Matches a public or private identifier.
rewritten.

https://codereview.chromium.org/177483002/diff/1/sdk/lib/internal/symbol.dart...
sdk/lib/internal/symbol.dart:60: static final RegExp privateValidationPattern =
new RegExp(
Changed to publicSymbolPattern and symbolPattern.

[Lasse Reichstein Nielsen, 2014-03-03 12:59:02]

Committed patchset #2 manually as r33212 (presubmit successful).