From dc26b30fddaf958895d6b20f8e3887dd11883167 Mon Sep 17 00:00:00 2001 From: Sigmund Cherem Date: Mon, 1 Apr 2024 19:53:50 +0000 Subject: [PATCH 1/3] [ddc] reduce flakiness of asset_file_system_test. In the refactor to speed up this test, we removed most retry logic (except for the "unreliable tests" that now apply retries in a predictable manner). The "noisy" tests have a similar behavior but for different reasons. These put a lot of timing presure by serving hundreds of very large files. The default response timeout of 5s trips this sometimes. We could try to make this more predictable by providing a larger timeout upfront, but I believe part of the intent with these tests was also to incorporate retries for reasons like response timeouts. So instead, I've added a retry, which will use by default a larger timeout on the second attempt. This will hopefully be sufficient to remove this source of flakiness. Change-Id: I8eb5e9dceed5e7af36f6db45147c2d961247f6e2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360503 Reviewed-by: Nicholas Shahan Commit-Queue: Sigmund Cherem --- .../test/expression_compiler/asset_file_system_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/dev_compiler/test/expression_compiler/asset_file_system_test.dart b/pkg/dev_compiler/test/expression_compiler/asset_file_system_test.dart index 82cb418d62ce..0280c9a3e087 100644 --- a/pkg/dev_compiler/test/expression_compiler/asset_file_system_test.dart +++ b/pkg/dev_compiler/test/expression_compiler/asset_file_system_test.dart @@ -155,7 +155,7 @@ void main() async { server = await HttpMultiServer.bind(hostname, port); fileSystem = AssetFileSystem.forTesting( StandardFileSystem.instance, hostname, '$port', - retries: 0); + retries: 1); serveRequests(server, noisyHandler); }); From 71e5a6b6d0084ff5aeca7c04a586d3027732e0ad Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Mon, 1 Apr 2024 20:17:05 +0000 Subject: [PATCH 2/3] Augment. Issue 55324. Recover from missing ';' in 'import augment'. Bug: https://github.com/dart-lang/sdk/issues/55324 Change-Id: I7365a45718455d69985a968c683bcf6dd74f9505 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360504 Commit-Queue: Konstantin Shcheglov Reviewed-by: Brian Wilkerson --- pkg/analyzer/lib/src/fasta/ast_builder.dart | 69 ++++++++------ .../lib/src/test_utilities/find_node.dart | 3 + .../augmentation_import_directive_test.dart | 92 +++++++++++++++++++ .../dart/parser/import_directive_test.dart | 88 ++++++++++++++++++ .../test/src/dart/parser/test_all.dart | 5 + 5 files changed, 229 insertions(+), 28 deletions(-) create mode 100644 pkg/analyzer/test/src/dart/parser/augmentation_import_directive_test.dart create mode 100644 pkg/analyzer/test/src/dart/parser/import_directive_test.dart diff --git a/pkg/analyzer/lib/src/fasta/ast_builder.dart b/pkg/analyzer/lib/src/fasta/ast_builder.dart index 50584499f303..bd53fb1283d8 100644 --- a/pkg/analyzer/lib/src/fasta/ast_builder.dart +++ b/pkg/analyzer/lib/src/fasta/ast_builder.dart @@ -5251,34 +5251,47 @@ class AstBuilder extends StackListener { var prefix = pop(NullValues.Prefix) as SimpleIdentifierImpl?; var configurations = pop() as List?; - final directive = directives.last as ImportDirectiveImpl; - - // TODO(scheglov): This code would be easier if we used one object. - var mergedAsKeyword = directive.asKeyword; - var mergedPrefix = directive.prefix; - if (directive.asKeyword == null && asKeyword != null) { - mergedAsKeyword = asKeyword; - mergedPrefix = prefix; - } - - directives.last = ImportDirectiveImpl( - comment: directive.documentationComment, - metadata: directive.metadata, - importKeyword: directive.importKeyword, - uri: directive.uri, - configurations: [ - ...directive.configurations, - ...?configurations, - ], - deferredKeyword: directive.deferredKeyword ?? deferredKeyword, - asKeyword: mergedAsKeyword, - prefix: mergedPrefix, - combinators: [ - ...directive.combinators, - ...?combinators, - ], - semicolon: semicolon ?? directive.semicolon, - ); + final directive = directives.last; + switch (directive) { + case AugmentationImportDirectiveImpl(): + directives.last = AugmentationImportDirectiveImpl( + comment: directive.documentationComment, + metadata: directive.metadata, + importKeyword: directive.importKeyword, + augmentKeyword: directive.augmentKeyword, + uri: directive.uri, + semicolon: semicolon ?? directive.semicolon, + ); + case ImportDirectiveImpl(): + // TODO(scheglov): This code would be easier if we used one object. + var mergedAsKeyword = directive.asKeyword; + var mergedPrefix = directive.prefix; + if (directive.asKeyword == null && asKeyword != null) { + mergedAsKeyword = asKeyword; + mergedPrefix = prefix; + } + + directives.last = ImportDirectiveImpl( + comment: directive.documentationComment, + metadata: directive.metadata, + importKeyword: directive.importKeyword, + uri: directive.uri, + configurations: [ + ...directive.configurations, + ...?configurations, + ], + deferredKeyword: directive.deferredKeyword ?? deferredKeyword, + asKeyword: mergedAsKeyword, + prefix: mergedPrefix, + combinators: [ + ...directive.combinators, + ...?combinators, + ], + semicolon: semicolon ?? directive.semicolon, + ); + default: + throw UnimplementedError('${directive.runtimeType}'); + } } @override diff --git a/pkg/analyzer/lib/src/test_utilities/find_node.dart b/pkg/analyzer/lib/src/test_utilities/find_node.dart index f30e24f4f738..787132e7c733 100644 --- a/pkg/analyzer/lib/src/test_utilities/find_node.dart +++ b/pkg/analyzer/lib/src/test_utilities/find_node.dart @@ -36,6 +36,9 @@ class FindNode { AssignmentExpression get singleAssignmentExpression => _single(); + AugmentationImportDirective get singleAugmentationImportDirective => + _single(); + AwaitExpression get singleAwaitExpression => _single(); BinaryExpression get singleBinaryExpression => _single(); diff --git a/pkg/analyzer/test/src/dart/parser/augmentation_import_directive_test.dart b/pkg/analyzer/test/src/dart/parser/augmentation_import_directive_test.dart new file mode 100644 index 000000000000..34616d66c465 --- /dev/null +++ b/pkg/analyzer/test/src/dart/parser/augmentation_import_directive_test.dart @@ -0,0 +1,92 @@ +// Copyright (c) 2024, 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. + +import 'package:analyzer/src/dart/error/syntactic_errors.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../../diagnostics/parser_diagnostics.dart'; + +main() { + defineReflectiveSuite(() { + defineReflectiveTests(AugmentationImportDirectiveParserTest); + }); +} + +@reflectiveTest +class AugmentationImportDirectiveParserTest extends ParserDiagnosticsTest { + test_it() { + final parseResult = parseStringWithErrors(r''' +import augment 'a.dart'; +'''); + parseResult.assertNoErrors(); + + final node = parseResult.findNode.singleAugmentationImportDirective; + assertParsedNodeText(node, r''' +AugmentationImportDirective + importKeyword: import + augmentKeyword: augment + uri: SimpleStringLiteral + literal: 'a.dart' + semicolon: ; +'''); + } + + test_noSemicolon() { + final parseResult = parseStringWithErrors(r''' +import augment 'a.dart' +'''); + parseResult.assertErrors([ + error(ParserErrorCode.EXPECTED_TOKEN, 15, 8), + ]); + + final node = parseResult.findNode.singleAugmentationImportDirective; + assertParsedNodeText(node, r''' +AugmentationImportDirective + importKeyword: import + augmentKeyword: augment + uri: SimpleStringLiteral + literal: 'a.dart' + semicolon: ; +'''); + } + + test_noUri_hasSemicolon() { + final parseResult = parseStringWithErrors(r''' +import augment ; +'''); + parseResult.assertErrors([ + error(ParserErrorCode.EXPECTED_STRING_LITERAL, 15, 1), + ]); + + final node = parseResult.findNode.singleAugmentationImportDirective; + assertParsedNodeText(node, r''' +AugmentationImportDirective + importKeyword: import + augmentKeyword: augment + uri: SimpleStringLiteral + literal: "" + semicolon: ; +'''); + } + + test_noUri_noSemicolon() { + final parseResult = parseStringWithErrors(r''' +import augment +'''); + parseResult.assertErrors([ + error(ParserErrorCode.EXPECTED_TOKEN, 7, 7), + error(ParserErrorCode.EXPECTED_STRING_LITERAL, 15, 0), + ]); + + final node = parseResult.findNode.singleAugmentationImportDirective; + assertParsedNodeText(node, r''' +AugmentationImportDirective + importKeyword: import + augmentKeyword: augment + uri: SimpleStringLiteral + literal: "" + semicolon: ; +'''); + } +} diff --git a/pkg/analyzer/test/src/dart/parser/import_directive_test.dart b/pkg/analyzer/test/src/dart/parser/import_directive_test.dart new file mode 100644 index 000000000000..133f77e995f9 --- /dev/null +++ b/pkg/analyzer/test/src/dart/parser/import_directive_test.dart @@ -0,0 +1,88 @@ +// Copyright (c) 2024, 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. + +import 'package:analyzer/src/dart/error/syntactic_errors.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../../diagnostics/parser_diagnostics.dart'; + +main() { + defineReflectiveSuite(() { + defineReflectiveTests(ImportDirectiveParserTest); + }); +} + +@reflectiveTest +class ImportDirectiveParserTest extends ParserDiagnosticsTest { + test_it() { + final parseResult = parseStringWithErrors(r''' +import 'a.dart'; +'''); + parseResult.assertNoErrors(); + + final node = parseResult.findNode.singleImportDirective; + assertParsedNodeText(node, r''' +ImportDirective + importKeyword: import + uri: SimpleStringLiteral + literal: 'a.dart' + semicolon: ; +'''); + } + + test_noSemicolon() { + final parseResult = parseStringWithErrors(r''' +import 'a.dart' +'''); + parseResult.assertErrors([ + error(ParserErrorCode.EXPECTED_TOKEN, 7, 8), + ]); + + final node = parseResult.findNode.singleImportDirective; + assertParsedNodeText(node, r''' +ImportDirective + importKeyword: import + uri: SimpleStringLiteral + literal: 'a.dart' + semicolon: ; +'''); + } + + test_noUri_hasSemicolon() { + final parseResult = parseStringWithErrors(r''' +import ; +'''); + parseResult.assertErrors([ + error(ParserErrorCode.EXPECTED_STRING_LITERAL, 7, 1), + ]); + + final node = parseResult.findNode.singleImportDirective; + assertParsedNodeText(node, r''' +ImportDirective + importKeyword: import + uri: SimpleStringLiteral + literal: "" + semicolon: ; +'''); + } + + test_noUri_noSemicolon() { + final parseResult = parseStringWithErrors(r''' +import +'''); + parseResult.assertErrors([ + error(ParserErrorCode.EXPECTED_TOKEN, 0, 6), + error(ParserErrorCode.EXPECTED_STRING_LITERAL, 7, 0), + ]); + + final node = parseResult.findNode.singleImportDirective; + assertParsedNodeText(node, r''' +ImportDirective + importKeyword: import + uri: SimpleStringLiteral + literal: "" + semicolon: ; +'''); + } +} diff --git a/pkg/analyzer/test/src/dart/parser/test_all.dart b/pkg/analyzer/test/src/dart/parser/test_all.dart index 1d80466da51c..f1748c8e9b9f 100644 --- a/pkg/analyzer/test/src/dart/parser/test_all.dart +++ b/pkg/analyzer/test/src/dart/parser/test_all.dart @@ -4,10 +4,13 @@ import 'package:test_reflective_loader/test_reflective_loader.dart'; +import 'augmentation_import_directive_test.dart' + as augmentation_import_directive; import 'class_test.dart' as class_; import 'doc_comment_test.dart' as doc_comment; import 'extension_test.dart' as extension_; import 'extension_type_test.dart' as extension_type; +import 'import_directive_test.dart' as import_directive; import 'mixin_test.dart' as mixin_; import 'top_level_function_test.dart' as top_level_function; import 'top_level_variable_test.dart' as top_level_variable; @@ -17,10 +20,12 @@ import 'variable_declaration_statement_test.dart' /// Utility for manually running all tests. main() { defineReflectiveSuite(() { + augmentation_import_directive.main(); class_.main(); doc_comment.main(); extension_.main(); extension_type.main(); + import_directive.main(); mixin_.main(); top_level_function.main(); top_level_variable.main(); From aced0523a4afbb0ef4fac7685cd8dcdeda72d0d3 Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Mon, 1 Apr 2024 23:06:19 +0000 Subject: [PATCH 3/3] Augment. Fixes for navigation for constructor annotations. ...and for import prefixes in augmentations. Change-Id: I4c2f358d5bd1819c8691990a0b86a37d04449213 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360505 Commit-Queue: Konstantin Shcheglov Reviewed-by: Brian Wilkerson --- .../notification_navigation_test.dart | 67 +++++++++++++++++++ .../lib/utilities/analyzer_converter.dart | 5 +- .../utilities/navigation/navigation_dart.dart | 3 + 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/pkg/analysis_server/test/analysis/notification_navigation_test.dart b/pkg/analysis_server/test/analysis/notification_navigation_test.dart index e53bedc75064..ac727cae0d40 100644 --- a/pkg/analysis_server/test/analysis/notification_navigation_test.dart +++ b/pkg/analysis_server/test/analysis/notification_navigation_test.dart @@ -463,6 +463,44 @@ void f() { assertHasTarget('foo', targetFile: aFile); } + Future + test_class_constructor_annotationConstructor_importPrefix() async { + final a = newFile('$testPackageLibPath/a.dart', r''' +class A { + const A(); + const A.named(); +} +'''); + + addTestFile(''' +library augment 'b.dart'; +import 'a.dart' as prefix; + +class B { + @prefix.A() + @prefix.A.named() + B().foo(); +} +'''); + + await prepareNavigation(); + + assertHasRegion('prefix.A()'); + assertHasTarget('prefix;'); + + assertHasRegion('A()'); + assertHasTarget('A();', targetFile: a); + + assertHasRegion('prefix.A.named()'); + assertHasTarget('prefix;'); + + assertHasRegion('A.named()'); + assertHasTarget('named()', targetFile: a); + + assertHasRegion('named()'); + assertHasTarget('named()', targetFile: a); + } + Future test_class_constructor_named() async { addTestFile(''' class A { @@ -1381,6 +1419,35 @@ library my.lib; assertHasTargetString('my.lib'); } + Future + test_libraryAugmentation_topLevelFunction_annotationConstructor_importPrefix() async { + final a = newFile('$testPackageLibPath/a.dart', r''' +class A { + const A(); +} +'''); + + newFile('$testPackageLibPath/b.dart', r''' +import augment 'test.dart'; +'''); + + addTestFile(''' +library augment 'b.dart'; +import 'a.dart' as prefix; + +@prefix.A() +external B().foo(); +'''); + + await prepareNavigation(); + + assertHasRegion('prefix.A()'); + assertHasTarget('prefix;'); + + assertHasRegion('A()'); + assertHasTarget('A();', targetFile: a); + } + Future test_multiplyDefinedElement() async { newFile('$testPackageLibPath/libA.dart', 'library A; int TEST = 1;'); newFile('$testPackageLibPath/libB.dart', 'library B; int TEST = 2;'); diff --git a/pkg/analyzer_plugin/lib/utilities/analyzer_converter.dart b/pkg/analyzer_plugin/lib/utilities/analyzer_converter.dart index a5f85959e39f..86e209ef2c2a 100644 --- a/pkg/analyzer_plugin/lib/utilities/analyzer_converter.dart +++ b/pkg/analyzer_plugin/lib/utilities/analyzer_converter.dart @@ -321,10 +321,11 @@ extension ElementExtensions on analyzer.Element? { if (currentElement is analyzer.CompilationUnitElement) { return currentElement; } - if (currentElement?.enclosingElement is analyzer.LibraryElement) { + if (currentElement?.enclosingElement + is analyzer.LibraryOrAugmentationElement) { currentElement = currentElement?.enclosingElement; } - if (currentElement is analyzer.LibraryElement) { + if (currentElement is analyzer.LibraryOrAugmentationElement) { return currentElement.definingCompilationUnit; } for (; diff --git a/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart b/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart index 425419c0779f..3f7f5585d872 100644 --- a/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart +++ b/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart @@ -293,6 +293,8 @@ class _DartNavigationComputerVisitor extends RecursiveAstVisitor { @override void visitConstructorDeclaration(ConstructorDeclaration node) { + node.metadata.accept(this); + // For a default constructor, override the class name to be the declaration // itself rather than linking to the class. var nameToken = node.name; @@ -302,6 +304,7 @@ class _DartNavigationComputerVisitor extends RecursiveAstVisitor { node.returnType.accept(this); computer._addRegionForToken(nameToken, node.declaredElement); } + node.parameters.accept(this); node.initializers.accept(this); node.redirectedConstructor?.accept(this);