Make Analyzer, VM and dart2js accept URI strings as part-of library identifier.

Make Analyzer, VM and dart2js accept URI strings as part-of library identifier.

R=brianwilkerson@google.com, floitsch@google.com, hausner@google.com, johnniwinther@google.com, sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/ef097edad6e5e4e82c3891ad2d8f9cde8fa8cfdc
Committed: https://github.com/dart-lang/sdk/commit/5ae186153c24db9df664b49e75388a4454e4be60

[Lasse Reichstein Nielsen, 2017-01-19 12:03:11]

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

[floitsch, 2017-01-19 13:48:09]

LGTM.

[floitsch, 2017-01-19 13:48:58]

Consider updating the Changelog.
Make sure the dartformatter deals with it correctly too.

[floitsch, 2017-01-19 13:49:08]

No tests?

[Lasse Reichstein Nielsen, 2017-01-23 10:09:30]

lrn@google.com changed reviewers:
+ brianwilkerson@google.com, hausner@google.com, johnniwinther@google.com,
siggis@gmail.com

[Lasse Reichstein Nielsen, 2017-01-23 10:09:31]

I have an implementation of this for VM, dart2js and analyzer (just enabling the
existing analyzer-flag by default).

The analyzer isn't perfect - I now allow libraries with no name to have parts,
but it should still warn if the URI isn't correct.

Spec is in CL: https://codereview.chromium.org/2495613004/
Aiming this at 1.23.

[Johnni Winther, 2017-01-23 12:05:13]

dart2js part lgtm

[Siggi Cherem (dart-lang), 2017-01-23 15:30:38]

sigmund@google.com changed reviewers:
+ sigmund@google.com
- siggis@gmail.com

[Siggi Cherem (dart-lang), 2017-01-23 16:07:24]

lgtm

[hausner, 2017-01-23 16:43:58]

VM change LGTM.

[Lasse Reichstein Nielsen, 2017-01-25 12:34:47]

Description was changed from

==========
Make VM and dart2js accept URI strings as part-of library identifier.
==========

to

==========
Make Analyzer, VM and dart2js accept URI strings as part-of library identifier.
==========

[Brian Wilkerson, 2017-01-25 17:05:14]

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/lib/src/ta...
File pkg/analyzer/lib/src/task/dart.dart (right):

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/lib/src/ta...
pkg/analyzer/lib/src/task/dart.dart:1622:
!context.analysisOptions.enableUriInPartOf) {
Will we issue a warning if a part file contains a part-of directive that uses a
library name when the library doesn't have a name? (I'm assuming that's still an
error.)

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/test/gener...
File pkg/analyzer/test/generated/parser_test.dart (right):

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/test/gener...
pkg/analyzer/test/generated/parser_test.dart:2321: "part of 'lib';", []);
We should still check that this error is emitted when the `of` is followed by an
invalid token. Perhaps change the test to "part of 3;".

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/test/src/c...
File pkg/analyzer/test/src/context/context_test.dart (right):

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/test/src/c...
pkg/analyzer/test/src/context/context_test.dart:770: // Should report that 'lib'
isn't the correct URI.
Could you make this (and similar comments in other files) TODO comments and open
an issue for the missed cases?

[Lasse Reichstein Nielsen, 2017-01-26 10:39:13]

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/lib/src/ta...
File pkg/analyzer/lib/src/task/dart.dart (right):

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/lib/src/ta...
pkg/analyzer/lib/src/task/dart.dart:1622:
!context.analysisOptions.enableUriInPartOf) {
It's still a warning, and I don't think there is a warning in that case.
I didn't find the place to change for that (I'm assuming there is a warning
somewhere if the names don't match, and this should just be the same thing - the
non-empty part-of name doesn't match the empty library name).

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/test/gener...
File pkg/analyzer/test/generated/parser_test.dart (right):

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/test/gener...
pkg/analyzer/test/generated/parser_test.dart:2321: "part of 'lib';", []);
On 2017/01/25 17:05:14, Brian Wilkerson wrote:
> We should still check that this error is emitted when the `of` is followed by
an
> invalid token. Perhaps change the test to "part of 3;".

Done.

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/test/src/c...
File pkg/analyzer/test/src/context/context_test.dart (right):

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/test/src/c...
pkg/analyzer/test/src/context/context_test.dart:770: // Should report that 'lib'
isn't the correct URI.
On 2017/01/25 17:05:14, Brian Wilkerson wrote:
> Could you make this (and similar comments in other files) TODO comments and
open
> an issue for the missed cases?

Done.

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/test/src/t...
File pkg/analyzer/test/src/task/dart_test.dart (right):

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/test/src/t...
pkg/analyzer/test/src/task/dart_test.dart:874: // Should report that names are
wrong.
TODO added here.

https://codereview.chromium.org/2640853005/diff/20001/pkg/analyzer/test/src/t...
pkg/analyzer/test/src/task/dart_test.dart:897: // Should report that names are
wrong.
TODO added here.

[Lasse Reichstein Nielsen, 2017-02-03 15:23:45]

@Brian Wilkerson
Ping

[Brian Wilkerson, 2017-02-27 14:48:35]

lgtm

[Lasse Reichstein Nielsen, 2017-03-01 11:15:17]

Description was changed from

==========
Make Analyzer, VM and dart2js accept URI strings as part-of library identifier.
==========

to

==========
Make Analyzer, VM and dart2js accept URI strings as part-of library identifier.

R=brianwilkerson@google.com, floitsch@google.com, hausner@google.com,
johnniwinther@google.com, sigmund@google.com

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

[Lasse Reichstein Nielsen, 2017-03-01 11:15:19]

Committed patchset #5 (id:80001) manually as
ef097edad6e5e4e82c3891ad2d8f9cde8fa8cfdc (presubmit successful).

[Lasse Reichstein Nielsen, 2017-03-13 07:31:45]

Description was changed from

==========
Make Analyzer, VM and dart2js accept URI strings as part-of library identifier.

R=brianwilkerson@google.com, floitsch@google.com, hausner@google.com,
johnniwinther@google.com, sigmund@google.com

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

to

==========
Make Analyzer, VM and dart2js accept URI strings as part-of library identifier.

R=brianwilkerson@google.com, floitsch@google.com, hausner@google.com,
johnniwinther@google.com, sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/ef097edad6e5e4e82c3891ad2d8f9cde8fa8cfdc
Committed:
https://github.com/dart-lang/sdk/commit/5ae186153c24db9df664b49e75388a4454e4be60
==========

[Lasse Reichstein Nielsen, 2017-03-13 07:31:47]

Committed patchset #6 (id:100001) manually as
5ae186153c24db9df664b49e75388a4454e4be60 (presubmit successful).