From 1aa0575ef1665812066bc1693ddecce65bee3262 Mon Sep 17 00:00:00 2001 From: Kallen Tu Date: Thu, 1 Feb 2024 20:04:59 +0000 Subject: [PATCH] [analyzer] Add doc imports to UnlinkedLibraryDirective and report errors. This CL adds doc import information to UnlinkedLibraryDirective and reports any static errors such as `URI_DOES_NOT_EXIST` from the doc imports. The doc imports should be treated similarly to actual imports so it uses a lot of the same logic. Based largely on Sam's WIP work in this CL: https://dart-review.googlesource.com/c/sdk/+/344340 Bug: https://github.com/dart-lang/sdk/issues/50702 Change-Id: Ic001fd6d4077eea04905288be0424e7b11b2b56c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345361 Reviewed-by: Samuel Rawlins Reviewed-by: Konstantin Shcheglov Commit-Queue: Kallen Tu --- .../lib/src/dart/analysis/file_state.dart | 102 +++++++++----- .../src/dart/analysis/library_analyzer.dart | 128 ++++++++++++------ .../lib/src/dart/analysis/unlinked_data.dart | 10 ++ .../diagnostics/uri_does_not_exist_test.dart | 37 ++++- .../uri_with_interpolation_test.dart | 15 +- .../use_of_native_extension_test.dart | 9 ++ 6 files changed, 219 insertions(+), 82 deletions(-) diff --git a/pkg/analyzer/lib/src/dart/analysis/file_state.dart b/pkg/analyzer/lib/src/dart/analysis/file_state.dart index c279d0645ddf..14ad5c016c81 100644 --- a/pkg/analyzer/lib/src/dart/analysis/file_state.dart +++ b/pkg/analyzer/lib/src/dart/analysis/file_state.dart @@ -869,6 +869,7 @@ class FileState { UnlinkedPartOfNameDirective? partOfNameDirective; UnlinkedPartOfUriDirective? partOfUriDirective; var augmentations = []; + var docImports = []; var exports = []; var imports = []; var parts = []; @@ -904,8 +905,17 @@ class FileState { length: uri.length, ), ); + // TODO(srawlins): Add doc imports. } else if (directive is LibraryDirective) { + var libraryDocComment = directive.documentationComment; + if (libraryDocComment != null) { + for (var docImport in libraryDocComment.docImports) { + var builder = _serializeImport(docImport.import); + docImports.add(builder); + } + } libraryDirective = UnlinkedLibraryDirective( + docImports: docImports.toFixedList(), name: directive.name2?.name, ); } else if (directive is PartDirective) { @@ -2073,6 +2083,7 @@ final class LibraryImportWithUriStr abstract class LibraryOrAugmentationFileKind extends FileKind { List? _augmentationImports; + List? _libraryDocImports; List? _libraryExports; List? _libraryImports; @@ -2110,6 +2121,18 @@ abstract class LibraryOrAugmentationFileKind extends FileKind { }).toFixedList(); } + /// The import states of each `@docImport` on the library directive. + List get libraryDocImports { + var existingDocImports = _libraryDocImports; + if (existingDocImports != null) return existingDocImports; + + var docImports = file.unlinked2.libraryDirective?.docImports + .map(_createLibraryImportState) + .toFixedList(); + if (docImports == null) return _libraryDocImports = []; + return _libraryDocImports = docImports; + } + List get libraryExports { return _libraryExports ??= file.unlinked2.exports.map((unlinked) { @@ -2153,43 +2176,7 @@ abstract class LibraryOrAugmentationFileKind extends FileKind { List get libraryImports { return _libraryImports ??= - file.unlinked2.imports.map((unlinked) { - final uris = file._buildNamespaceDirectiveUris(unlinked); - final selectedUri = uris.selected; - switch (selectedUri) { - case DirectiveUriWithFile(): - return LibraryImportWithFile( - container: this, - unlinked: unlinked, - selectedUri: selectedUri, - uris: uris, - ); - case DirectiveUriWithInSummarySource(): - return LibraryImportWithInSummarySource( - unlinked: unlinked, - selectedUri: selectedUri, - uris: uris, - ); - case DirectiveUriWithUri(): - return LibraryImportWithUri( - unlinked: unlinked, - selectedUri: selectedUri, - uris: uris, - ); - case DirectiveUriWithString(): - return LibraryImportWithUriStr( - unlinked: unlinked, - selectedUri: selectedUri, - uris: uris, - ); - case DirectiveUriWithoutString(): - return LibraryImportState( - unlinked: unlinked, - selectedUri: selectedUri, - uris: uris, - ); - } - }).toFixedList(); + file.unlinked2.imports.map(_createLibraryImportState).toFixedList(); } /// Collect files that are transitively referenced by this library. @@ -2233,6 +2220,7 @@ abstract class LibraryOrAugmentationFileKind extends FileKind { @override void dispose() { _augmentationImports?.disposeAll(); + _libraryDocImports?.disposeAll(); _libraryExports?.disposeAll(); _libraryImports?.disposeAll(); super.dispose(); @@ -2261,6 +2249,46 @@ abstract class LibraryOrAugmentationFileKind extends FileKind { /// Invalidates the containing [LibraryFileKind] cycle. void invalidateLibraryCycle() {} + + /// Creates a [LibraryImportState] with the given unlinked [directive]. + LibraryImportState _createLibraryImportState( + UnlinkedLibraryImportDirective directive) { + final uris = file._buildNamespaceDirectiveUris(directive); + final selectedUri = uris.selected; + switch (selectedUri) { + case DirectiveUriWithFile(): + return LibraryImportWithFile( + container: this, + unlinked: directive, + selectedUri: selectedUri, + uris: uris, + ); + case DirectiveUriWithInSummarySource(): + return LibraryImportWithInSummarySource( + unlinked: directive, + selectedUri: selectedUri, + uris: uris, + ); + case DirectiveUriWithUri(): + return LibraryImportWithUri( + unlinked: directive, + selectedUri: selectedUri, + uris: uris, + ); + case DirectiveUriWithString(): + return LibraryImportWithUriStr( + unlinked: directive, + selectedUri: selectedUri, + uris: uris, + ); + case DirectiveUriWithoutString(): + return LibraryImportState( + unlinked: directive, + selectedUri: selectedUri, + uris: uris, + ); + } + } } class NamespaceDirectiveUris { diff --git a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart index eca39aa144aa..2c1f9c409c1f 100644 --- a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart +++ b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart @@ -590,6 +590,55 @@ class LibraryAnalyzer { _computeConstants(_libraryUnits.values); } + /// Reports URI-related import directive errors to the [errorReporter]. + void _reportImportDirectiveErrors({ + required ImportDirectiveImpl directive, + required LibraryImportState state, + required ErrorReporter errorReporter, + }) { + if (state is LibraryImportWithUri) { + final selectedUriStr = state.selectedUri.relativeUriStr; + if (selectedUriStr.startsWith('dart-ext:')) { + errorReporter.reportErrorForNode( + CompileTimeErrorCode.USE_OF_NATIVE_EXTENSION, + directive.uri, + ); + } else if (state.importedSource == null) { + errorReporter.reportErrorForNode( + CompileTimeErrorCode.URI_DOES_NOT_EXIST, + directive.uri, + [selectedUriStr], + ); + } else if (state is LibraryImportWithFile && !state.importedFile.exists) { + final errorCode = isGeneratedSource(state.importedSource) + ? CompileTimeErrorCode.URI_HAS_NOT_BEEN_GENERATED + : CompileTimeErrorCode.URI_DOES_NOT_EXIST; + errorReporter.reportErrorForNode( + errorCode, + directive.uri, + [selectedUriStr], + ); + } else if (state.importedLibrarySource == null) { + errorReporter.reportErrorForNode( + CompileTimeErrorCode.IMPORT_OF_NON_LIBRARY, + directive.uri, + [selectedUriStr], + ); + } + } else if (state is LibraryImportWithUriStr) { + errorReporter.reportErrorForNode( + CompileTimeErrorCode.INVALID_URI, + directive.uri, + [state.selectedUri.relativeUriStr], + ); + } else { + errorReporter.reportErrorForNode( + CompileTimeErrorCode.URI_WITH_INTERPOLATION, + directive.uri, + ); + } + } + void _resolveAugmentationImportDirective({ required AugmentationImportDirectiveImpl? directive, required AugmentationImportElementImpl element, @@ -757,6 +806,21 @@ class LibraryAnalyzer { ); } } + + final docImports = containerUnit.directives + .whereType() + .firstOrNull + ?.documentationComment + ?.docImports; + if (docImports != null) { + for (var i = 0; i < docImports.length; i++) { + _resolveLibraryDocImportDirective( + directive: docImports[i].import as ImportDirectiveImpl, + state: containerKind.libraryDocImports[i], + errorReporter: containerErrorReporter, + ); + } + } } void _resolveFile(FileState file, CompilationUnitImpl unit) { @@ -850,6 +914,24 @@ class LibraryAnalyzer { ); } + /// Resolves the `@docImport` directive URI and reports any import errors of + /// the [directive] to the [errorReporter]. + void _resolveLibraryDocImportDirective({ + required ImportDirectiveImpl directive, + required LibraryImportState state, + required ErrorReporter errorReporter, + }) { + _resolveNamespaceDirective( + configurationNodes: directive.configurations, + configurationUris: state.uris.configurations, + ); + _reportImportDirectiveErrors( + directive: directive, + state: state, + errorReporter: errorReporter, + ); + } + void _resolveLibraryExportDirective({ required ExportDirectiveImpl directive, required LibraryExportElement element, @@ -916,47 +998,11 @@ class LibraryAnalyzer { configurationNodes: directive.configurations, configurationUris: state.uris.configurations, ); - if (state is LibraryImportWithUri) { - final selectedUriStr = state.selectedUri.relativeUriStr; - if (selectedUriStr.startsWith('dart-ext:')) { - errorReporter.reportErrorForNode( - CompileTimeErrorCode.USE_OF_NATIVE_EXTENSION, - directive.uri, - ); - } else if (state.importedSource == null) { - errorReporter.reportErrorForNode( - CompileTimeErrorCode.URI_DOES_NOT_EXIST, - directive.uri, - [selectedUriStr], - ); - } else if (state is LibraryImportWithFile && !state.importedFile.exists) { - final errorCode = isGeneratedSource(state.importedSource) - ? CompileTimeErrorCode.URI_HAS_NOT_BEEN_GENERATED - : CompileTimeErrorCode.URI_DOES_NOT_EXIST; - errorReporter.reportErrorForNode( - errorCode, - directive.uri, - [selectedUriStr], - ); - } else if (state.importedLibrarySource == null) { - errorReporter.reportErrorForNode( - CompileTimeErrorCode.IMPORT_OF_NON_LIBRARY, - directive.uri, - [selectedUriStr], - ); - } - } else if (state is LibraryImportWithUriStr) { - errorReporter.reportErrorForNode( - CompileTimeErrorCode.INVALID_URI, - directive.uri, - [state.selectedUri.relativeUriStr], - ); - } else { - errorReporter.reportErrorForNode( - CompileTimeErrorCode.URI_WITH_INTERPOLATION, - directive.uri, - ); - } + _reportImportDirectiveErrors( + directive: directive, + state: state, + errorReporter: errorReporter, + ); } void _resolveNamespaceDirective({ diff --git a/pkg/analyzer/lib/src/dart/analysis/unlinked_data.dart b/pkg/analyzer/lib/src/dart/analysis/unlinked_data.dart index 012eb27fdc56..914d56d19226 100644 --- a/pkg/analyzer/lib/src/dart/analysis/unlinked_data.dart +++ b/pkg/analyzer/lib/src/dart/analysis/unlinked_data.dart @@ -181,9 +181,13 @@ class UnlinkedLibraryAugmentationDirective { } class UnlinkedLibraryDirective { + /// `@docImport` directives in a library doc comment. + final List docImports; + final String? name; UnlinkedLibraryDirective({ + required this.docImports, required this.name, }); @@ -191,11 +195,17 @@ class UnlinkedLibraryDirective { SummaryDataReader reader, ) { return UnlinkedLibraryDirective( + docImports: reader.readTypedList( + () => UnlinkedLibraryImportDirective.read(reader), + ), name: reader.readOptionalStringUtf8(), ); } void write(BufferedSink sink) { + sink.writeList(docImports, (docImport) { + docImport.write(sink); + }); sink.writeOptionalStringUtf8(name); } } diff --git a/pkg/analyzer/test/src/diagnostics/uri_does_not_exist_test.dart b/pkg/analyzer/test/src/diagnostics/uri_does_not_exist_test.dart index 6467d9d5f12f..abdd1b7cb514 100644 --- a/pkg/analyzer/test/src/diagnostics/uri_does_not_exist_test.dart +++ b/pkg/analyzer/test/src/diagnostics/uri_does_not_exist_test.dart @@ -37,6 +37,41 @@ void f() { ]); } + test_libraryDocImport_cannotResolve_dart() async { + await assertErrorsInCode(r''' +/// @docImport 'dart:foo'; +library; +''', [ + error(CompileTimeErrorCode.URI_DOES_NOT_EXIST, 15, 10), + ]); + } + + test_libraryDocImport_cannotResolve_file() async { + await assertErrorsInCode(r''' +/// @docImport 'foo.dart'; +library; +''', [ + error(CompileTimeErrorCode.URI_DOES_NOT_EXIST, 15, 10), + ]); + } + + test_libraryDocImport_canResolve_dart() async { + await assertNoErrorsInCode(r''' +/// @docImport 'dart:math'; +library; +'''); + } + + test_libraryDocImport_canResolve_file() async { + newFile('$testPackageLibPath/foo.dart', r''' +class A {} +'''); + await assertNoErrorsInCode(r''' +/// @docImport 'foo.dart'; +library; +'''); + } + test_libraryExport() async { await assertErrorsInCode(''' export 'unknown.dart'; @@ -69,7 +104,7 @@ import 'unknown.dart'; ]); } - test_libraryImport_appears_after_deleting_target() async { + test_libraryImport_appearsAfterDeletingTarget() async { String filePath = newFile('$testPackageLibPath/target.dart', '').path; await assertErrorsInCode(''' diff --git a/pkg/analyzer/test/src/diagnostics/uri_with_interpolation_test.dart b/pkg/analyzer/test/src/diagnostics/uri_with_interpolation_test.dart index 7272aac50750..12fc51e0ab8b 100644 --- a/pkg/analyzer/test/src/diagnostics/uri_with_interpolation_test.dart +++ b/pkg/analyzer/test/src/diagnostics/uri_with_interpolation_test.dart @@ -15,7 +15,7 @@ main() { @reflectiveTest class UriWithInterpolationTest extends PubPackageResolutionTest { - test_augmentationImport() async { + test_augmentation_import() async { await assertErrorsInCode(r''' import augment '${'foo'}.dart'; ''', [ @@ -23,7 +23,16 @@ import augment '${'foo'}.dart'; ]); } - test_libraryExport() async { + test_library_docImport() async { + await assertErrorsInCode(r''' +/// @docImport '${'foo'}.dart'; +library; +''', [ + error(CompileTimeErrorCode.URI_WITH_INTERPOLATION, 15, 15), + ]); + } + + test_library_export() async { await assertErrorsInCode(r''' export '${'foo'}.dart'; ''', [ @@ -31,7 +40,7 @@ export '${'foo'}.dart'; ]); } - test_libraryImport() async { + test_library_import() async { await assertErrorsInCode(r''' import '${'foo'}.dart'; ''', [ diff --git a/pkg/analyzer/test/src/diagnostics/use_of_native_extension_test.dart b/pkg/analyzer/test/src/diagnostics/use_of_native_extension_test.dart index 8a89fab4a5ad..1b554ddeeec7 100644 --- a/pkg/analyzer/test/src/diagnostics/use_of_native_extension_test.dart +++ b/pkg/analyzer/test/src/diagnostics/use_of_native_extension_test.dart @@ -15,6 +15,15 @@ main() { @reflectiveTest class UseOfNativeExtensionTest extends PubPackageResolutionTest { + test_docImport() async { + await assertErrorsInCode(r''' +/// @docImport 'dart-ext:x'; +library; +''', [ + error(CompileTimeErrorCode.USE_OF_NATIVE_EXTENSION, 15, 12), + ]); + } + test_export() async { await assertErrorsInCode(r''' export 'dart-ext:x';