From ef097edad6e5e4e82c3891ad2d8f9cde8fa8cfdc Mon Sep 17 00:00:00 2001 From: "Lasse R.H. Nielsen" Date: Wed, 1 Mar 2017 12:15:11 +0100 Subject: [PATCH] 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 Review-Url: https://codereview.chromium.org/2640853005 . --- .../src/dart/analysis/library_analyzer.dart | 3 +- pkg/analyzer/lib/src/generated/engine.dart | 4 +-- pkg/analyzer/lib/src/generated/parser.dart | 2 +- .../lib/src/summary/summarize_ast.dart | 2 +- pkg/analyzer/lib/src/task/dart.dart | 3 +- pkg/analyzer/test/generated/parser_test.dart | 3 +- .../test/src/context/context_test.dart | 9 +++-- .../test/src/dart/analysis/driver_test.dart | 11 +++++-- pkg/analyzer/test/src/task/dart_test.dart | 33 +++++++++++++++++-- .../lib/src/diagnostics/messages.dart | 21 ++++++++++++ pkg/compiler/lib/src/elements/modelx.dart | 14 ++++++++ .../lib/src/fasta/parser/parser.dart | 9 +++-- runtime/vm/parser.cc | 16 +++++---- tests/language/part_of_uri2_part.dart | 9 +++++ tests/language/part_of_uri2_part2.dart | 9 +++++ tests/language/part_of_uri2_test.dart | 15 +++++++++ tests/language/part_of_uri_part.dart | 9 +++++ tests/language/part_of_uri_part2.dart | 9 +++++ tests/language/part_of_uri_test.dart | 14 ++++++++ 19 files changed, 173 insertions(+), 22 deletions(-) create mode 100644 tests/language/part_of_uri2_part.dart create mode 100644 tests/language/part_of_uri2_part2.dart create mode 100644 tests/language/part_of_uri2_test.dart create mode 100644 tests/language/part_of_uri_part.dart create mode 100644 tests/language/part_of_uri_part2.dart create mode 100644 tests/language/part_of_uri_test.dart diff --git a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart index 7ac3b30239b..71f1f2093d0 100644 --- a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart +++ b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart @@ -474,7 +474,8 @@ class LibraryAnalyzer { } } - if (hasPartDirective && libraryNameNode == null) { + if (hasPartDirective && libraryNameNode == null && + !_context.analysisOptions.enableUriInPartOf) { libraryErrorReporter.reportErrorForOffset( ResolverErrorCode.MISSING_LIBRARY_DIRECTIVE_WITH_PART, 0, 0); } diff --git a/pkg/analyzer/lib/src/generated/engine.dart b/pkg/analyzer/lib/src/generated/engine.dart index 97b72500d82..f96e9f674ec 100644 --- a/pkg/analyzer/lib/src/generated/engine.dart +++ b/pkg/analyzer/lib/src/generated/engine.dart @@ -1422,7 +1422,7 @@ class AnalysisOptionsImpl implements AnalysisOptions { List _excludePatterns; @override - bool enableUriInPartOf = false; + bool enableUriInPartOf = true; @override bool generateImplicitErrors = true; @@ -1686,7 +1686,7 @@ class AnalysisOptionsImpl implements AnalysisOptions { enableStrictCallChecks = false; enableSuperMixins = false; enableTiming = false; - enableUriInPartOf = false; + enableUriInPartOf = true; _errorProcessors = null; _excludePatterns = null; finerGrainedInvalidation = false; diff --git a/pkg/analyzer/lib/src/generated/parser.dart b/pkg/analyzer/lib/src/generated/parser.dart index e8bc7dd5d90..54e3e800651 100644 --- a/pkg/analyzer/lib/src/generated/parser.dart +++ b/pkg/analyzer/lib/src/generated/parser.dart @@ -208,7 +208,7 @@ class Parser { * A flag indicating whether the parser is to allow URI's in part-of * directives. */ - bool _enableUriInPartOf = false; + bool _enableUriInPartOf = true; /** * A flag indicating whether parser is to parse function bodies. diff --git a/pkg/analyzer/lib/src/summary/summarize_ast.dart b/pkg/analyzer/lib/src/summary/summarize_ast.dart index 240fa8000ca..ac2f3df3cf1 100644 --- a/pkg/analyzer/lib/src/summary/summarize_ast.dart +++ b/pkg/analyzer/lib/src/summary/summarize_ast.dart @@ -1368,7 +1368,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor { @override void visitPartOfDirective(PartOfDirective node) { - isCoreLibrary = node.libraryName.name == 'dart.core'; + isCoreLibrary = node.libraryName?.name == 'dart.core'; isPartOf = true; } diff --git a/pkg/analyzer/lib/src/task/dart.dart b/pkg/analyzer/lib/src/task/dart.dart index a8cd2e6afbb..daff329cb9e 100644 --- a/pkg/analyzer/lib/src/task/dart.dart +++ b/pkg/analyzer/lib/src/task/dart.dart @@ -1626,7 +1626,8 @@ class BuildLibraryElementTask extends SourceBasedAnalysisTask { } } } - if (hasPartDirective && libraryNameNode == null) { + if (hasPartDirective && libraryNameNode == null && + !context.analysisOptions.enableUriInPartOf) { errors.add(new AnalysisError(librarySource, 0, 0, ResolverErrorCode.MISSING_LIBRARY_DIRECTIVE_WITH_PART)); } diff --git a/pkg/analyzer/test/generated/parser_test.dart b/pkg/analyzer/test/generated/parser_test.dart index ae8b9c2dcea..e989e911258 100644 --- a/pkg/analyzer/test/generated/parser_test.dart +++ b/pkg/analyzer/test/generated/parser_test.dart @@ -3419,7 +3419,8 @@ class Foo { void test_nonIdentifierLibraryName_partOf() { CompilationUnit unit = parseCompilationUnit( - "part of 'lib';", [ParserErrorCode.NON_IDENTIFIER_LIBRARY_NAME]); + "part of 3;", [ParserErrorCode.MISSING_NAME_IN_PART_OF_DIRECTIVE, + ParserErrorCode.UNEXPECTED_TOKEN]); expect(unit, isNotNull); } diff --git a/pkg/analyzer/test/src/context/context_test.dart b/pkg/analyzer/test/src/context/context_test.dart index 659538731be..8374e0501d1 100644 --- a/pkg/analyzer/test/src/context/context_test.dart +++ b/pkg/analyzer/test/src/context/context_test.dart @@ -766,8 +766,13 @@ library lib; Source partSource = addSource("/part.dart", "part of 'lib';"); context.parseCompilationUnit(librarySource); List errors = context.computeErrors(partSource); - expect(errors, isNotNull); - expect(errors.length > 0, isTrue); + if (context.analysisOptions.enableUriInPartOf) { + // TODO(28522) + // Should report that 'lib' isn't the correct URI. + } else { + expect(errors, isNotNull); + expect(errors.length > 0, isTrue); + } } void test_computeErrors_dart_some() { diff --git a/pkg/analyzer/test/src/dart/analysis/driver_test.dart b/pkg/analyzer/test/src/dart/analysis/driver_test.dart index 79e551fb2d4..53c4702f348 100644 --- a/pkg/analyzer/test/src/dart/analysis/driver_test.dart +++ b/pkg/analyzer/test/src/dart/analysis/driver_test.dart @@ -414,9 +414,14 @@ part of lib; AnalysisResult libResult = await driver.getResult(lib); List errors = libResult.errors; - expect(errors, hasLength(1)); - expect(errors[0].errorCode, - ResolverErrorCode.MISSING_LIBRARY_DIRECTIVE_WITH_PART); + if (libResult.unit.element.context.analysisOptions.enableUriInPartOf) { + // TODO(28522): Should cause an error for wrong library name. + expect(errors, hasLength(0)); + } else { + expect(errors, hasLength(1)); + expect(errors[0].errorCode, + ResolverErrorCode.MISSING_LIBRARY_DIRECTIVE_WITH_PART); + } } test_analyze_resolveDirectives_error_partOfDifferentLibrary_byName() async { diff --git a/pkg/analyzer/test/src/task/dart_test.dart b/pkg/analyzer/test/src/task/dart_test.dart index d20b9b29d9e..f043d2a9047 100644 --- a/pkg/analyzer/test/src/task/dart_test.dart +++ b/pkg/analyzer/test/src/task/dart_test.dart @@ -870,8 +870,37 @@ part of my_lib; part of my_lib; ''' }); - _assertErrorsWithCodes( - [ResolverErrorCode.MISSING_LIBRARY_DIRECTIVE_WITH_PART]); + if (context.analysisOptions.enableUriInPartOf) { + // TODO(28522) + // Should report that names are wrong. + } else { + _assertErrorsWithCodes( + [ResolverErrorCode.MISSING_LIBRARY_DIRECTIVE_WITH_PART]); + AnalysisError error = errorListener.errors[0]; + } + } + + test_perform_error_missingLibraryDirectiveWithPart_noCommon() { + _performBuildTask({ + '/lib.dart': ''' +part 'partA.dart'; +part 'partB.dart'; +''', + '/partA.dart': ''' +part of libA; + ''', + '/partB.dart': ''' +part of libB; +''' + }); + if (context.analysisOptions.enableUriInPartOf) { + // TODO(28522) + // Should report that names are wrong. + } else { + _assertErrorsWithCodes( + [ResolverErrorCode.MISSING_LIBRARY_DIRECTIVE_WITH_PART]); + AnalysisError error = errorListener.errors[0]; + } } test_perform_error_partDoesNotExist() { diff --git a/pkg/compiler/lib/src/diagnostics/messages.dart b/pkg/compiler/lib/src/diagnostics/messages.dart index 751ffe73f3a..bdb8a2eacc6 100644 --- a/pkg/compiler/lib/src/diagnostics/messages.dart +++ b/pkg/compiler/lib/src/diagnostics/messages.dart @@ -292,6 +292,7 @@ enum MessageKind { LIBRARY_NOT_FOUND, LIBRARY_NOT_SUPPORTED, LIBRARY_TAG_MUST_BE_FIRST, + LIBRARY_URI_MISMATCH, MAIN_HAS_PART_OF, MAIN_NOT_A_FUNCTION, MAIN_WITH_EXTRA_PARAMETER, @@ -2073,6 +2074,26 @@ main() {} """, 'part.dart': """ part of lib.bar; +""" + } + ]), + + MessageKind.LIBRARY_URI_MISMATCH: const MessageTemplate( + MessageKind.LIBRARY_URI_MISMATCH, + "Expected URI of library '#{libraryUri}'.", + howToFix: "Try changing the directive to 'part of " + "\"#{libraryUri}\";'.", + examples: const [ + const { + 'main.dart': """ +library lib.foo; + +part 'part.dart'; + +main() {} +""", + 'part.dart': """ +part of "main.dart"; """ } ]), diff --git a/pkg/compiler/lib/src/elements/modelx.dart b/pkg/compiler/lib/src/elements/modelx.dart index bf589e1cbac..d07cd7eebf6 100644 --- a/pkg/compiler/lib/src/elements/modelx.dart +++ b/pkg/compiler/lib/src/elements/modelx.dart @@ -767,6 +767,20 @@ class CompilationUnitElementX extends ElementX } partTag = tag; LibraryName libraryTag = library.libraryTag; + + Expression libraryReference = tag.name; + if (libraryReference is LiteralString) { + // Name is a URI. Resolve and compare to library's URI. + String content = libraryReference.dartString.slowToString(); + Uri uri = this.script.readableUri.resolve(content); + Uri expectedUri = library.canonicalUri; + if (uri != expectedUri) { + // Consider finding a relative URI reference for the error message. + reporter.reportWarningMessage(tag.name, + MessageKind.LIBRARY_URI_MISMATCH, {'libraryUri': expectedUri}); + } + return; + } String actualName = tag.name.toString(); if (libraryTag != null) { String expectedName = libraryTag.name.toString(); diff --git a/pkg/front_end/lib/src/fasta/parser/parser.dart b/pkg/front_end/lib/src/fasta/parser/parser.dart index 1eda2a6d2be..186f8965fcc 100644 --- a/pkg/front_end/lib/src/fasta/parser/parser.dart +++ b/pkg/front_end/lib/src/fasta/parser/parser.dart @@ -344,8 +344,13 @@ class Parser { assert(optional('part', token)); assert(optional('of', token.next)); Token partKeyword = token; - token = parseQualified(token.next.next, IdentifierContext.partName, - IdentifierContext.partNameContinuation); + token = token.next.next; + if (token.isIdentifier()) { + token = parseQualified(token, IdentifierContext.partName, + IdentifierContext.partNameContinuation); + } else { + token = parseLiteralStringOrRecoverExpression(token); + } Token semicolon = token; token = expect(';', token); listener.endPartOf(partKeyword, semicolon); diff --git a/runtime/vm/parser.cc b/runtime/vm/parser.cc index f139e766cd6..6a43faee2d6 100644 --- a/runtime/vm/parser.cc +++ b/runtime/vm/parser.cc @@ -6386,12 +6386,16 @@ void Parser::ParsePartHeader() { ReportError("'part of' expected"); } ConsumeToken(); - // The VM is not required to check that the library name matches the - // name of the current library, so we ignore it. - ExpectIdentifier("library name expected"); - while (CurrentToken() == Token::kPERIOD) { - ConsumeToken(); - ExpectIdentifier("malformed library name"); + // The VM is not required to check that the library name or URI matches the + // name or URI of the current library, so we ignore them. + if (CurrentToken() == Token::kSTRING) { + ParseStringLiteral(false); + } else { + ExpectIdentifier("library name expected"); + while (CurrentToken() == Token::kPERIOD) { + ConsumeToken(); + ExpectIdentifier("malformed library name"); + } } ExpectSemicolon(); } diff --git a/tests/language/part_of_uri2_part.dart b/tests/language/part_of_uri2_part.dart new file mode 100644 index 00000000000..22c889a5fd7 --- /dev/null +++ b/tests/language/part_of_uri2_part.dart @@ -0,0 +1,9 @@ +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +part of "part_of_uri2_test.dart"; + +// Refer to declaration in library and other part. +const bar1 = "part1"; +const baz1 = "$foo${bar2}1"; diff --git a/tests/language/part_of_uri2_part2.dart b/tests/language/part_of_uri2_part2.dart new file mode 100644 index 00000000000..0b049fbea66 --- /dev/null +++ b/tests/language/part_of_uri2_part2.dart @@ -0,0 +1,9 @@ +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +part of part_of_uri2; + +// Refer to declaration in library and other part. +const bar2 = "part2"; +const baz2 = "$foo${bar1}2"; diff --git a/tests/language/part_of_uri2_test.dart b/tests/language/part_of_uri2_test.dart new file mode 100644 index 00000000000..220c7449776 --- /dev/null +++ b/tests/language/part_of_uri2_test.dart @@ -0,0 +1,15 @@ +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +library part_of_uri2; + +part "part_of_uri2_part.dart"; // declares bar1, baz1, uses URI. +part "part_of_uri2_part2.dart"; // declares bar2, baz2, uses id. + +const foo = 'foo'; +const qux = "$baz1$baz2"; + +main() { + if (!identical(qux, "foopart21foopart12")) throw "Fail: $qux"; +} diff --git a/tests/language/part_of_uri_part.dart b/tests/language/part_of_uri_part.dart new file mode 100644 index 00000000000..55edadf67a4 --- /dev/null +++ b/tests/language/part_of_uri_part.dart @@ -0,0 +1,9 @@ +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +part of "part_of_uri_test.dart"; + +// Refer to declaration in library and other part. +const bar1 = "part1"; +const baz1 = "$foo${bar2}1"; diff --git a/tests/language/part_of_uri_part2.dart b/tests/language/part_of_uri_part2.dart new file mode 100644 index 00000000000..5069b531381 --- /dev/null +++ b/tests/language/part_of_uri_part2.dart @@ -0,0 +1,9 @@ +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +part of "part_of_uri_test.dart"; + +// Refer to declaration in library and other part. +const bar2 = "part2"; +const baz2 = "$foo${bar1}2"; diff --git a/tests/language/part_of_uri_test.dart b/tests/language/part_of_uri_test.dart new file mode 100644 index 00000000000..4c313a0cccd --- /dev/null +++ b/tests/language/part_of_uri_test.dart @@ -0,0 +1,14 @@ +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// No library declaration +part "part_of_uri_part.dart"; // declares bar1, baz1 +part "part_of_uri_part2.dart"; // declares bar2, baz2 + +const foo = 'foo'; +const qux = "$baz1$baz2"; + +main() { + if (!identical(qux, "foopart21foopart12")) throw "Fail: $qux"; +}