Use implementation (full content) based signatures for results that might be affected by macros.
Change-Id: I5106930ac904a99e674cba7e3ed1ecd29b104d1d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/241861
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analyzer/lib/src/dart/analysis/file_state.dart b/pkg/analyzer/lib/src/dart/analysis/file_state.dart
index e2a6383..618627c 100644
--- a/pkg/analyzer/lib/src/dart/analysis/file_state.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/file_state.dart
@@ -331,7 +331,7 @@
String get transitiveSignature {
var librarySignatureBuilder = ApiSignature()
..addString(uriStr)
- ..addString(libraryCycle.transitiveSignature);
+ ..addString(libraryCycle.apiSignature);
return librarySignatureBuilder.toHex();
}
diff --git a/pkg/analyzer/lib/src/dart/analysis/library_context.dart b/pkg/analyzer/lib/src/dart/analysis/library_context.dart
index f19c23c..3d44aa2 100644
--- a/pkg/analyzer/lib/src/dart/analysis/library_context.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/library_context.dart
@@ -160,7 +160,7 @@
}
}
- var resolutionKey = '${cycle.transitiveSignature}.linked_bundle';
+ var resolutionKey = '${cycle.apiSignature}.linked_bundle';
var resolutionBytes = byteStore.get(resolutionKey);
if (resolutionBytes == null) {
@@ -244,7 +244,7 @@
final macroKernelBuilder = this.macroKernelBuilder;
if (macroKernelBuilder != null && macroLibraries.isNotEmpty) {
- var macroKernelKey = '${cycle.transitiveSignature}.macro_kernel';
+ var macroKernelKey = '${cycle.implSignature}.macro_kernel';
var macroKernelBytes = byteStore.get(macroKernelKey);
if (macroKernelBytes == null) {
macroKernelBytes = await macroKernelBuilder.build(
diff --git a/pkg/analyzer/lib/src/dart/analysis/library_graph.dart b/pkg/analyzer/lib/src/dart/analysis/library_graph.dart
index 1a4ed51..1457c6b 100644
--- a/pkg/analyzer/lib/src/dart/analysis/library_graph.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/library_graph.dart
@@ -28,18 +28,46 @@
/// The cycles that use this cycle, used to [invalidate] transitively.
final List<LibraryCycle> _directUsers = [];
- /// The transitive signature of this cycle.
+ /// The transitive API signature of this cycle.
///
/// It is based on the API signatures of all files of the [libraries], and
- /// transitive signatures of the cycles that the [libraries] reference
- /// directly. So, indirectly it is based on the transitive closure of all
- /// files that [libraries] reference (but we don't compute these files).
- String transitiveSignature;
+ /// API signatures of the cycles that the [libraries] reference directly.
+ /// So, indirectly it is based on API signatures of the transitive closure
+ /// of all files that [libraries] reference.
+ String apiSignature;
+
+ /// The transitive implementation signature of this cycle.
+ ///
+ /// It is based on the full code signatures of all files of the [libraries],
+ /// and full code signatures of the cycles that the [libraries] reference
+ /// directly. So, indirectly it is based on full code signatures of the
+ /// transitive closure of all files that [libraries] reference.
+ ///
+ /// Usually, when a library is imported we need its [apiSignature], because
+ /// its API is all we can see from outside. But if the library contains
+ /// a macro, and we use it, we run full code of the macro defining library,
+ /// potentially executing every method body of the transitive closure of
+ /// the libraries imported by the macro defining library. So, the resulting
+ /// library (that imports a macro defining library) API signature must
+ /// include [implSignature] of the macro defining library.
+ String implSignature;
+
+ late final bool hasMacroClass = () {
+ for (final library in libraries) {
+ for (final file in library.libraryFiles) {
+ if (file.unlinked2.macroClasses.isNotEmpty) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }();
LibraryCycle({
required this.libraries,
required this.directDependencies,
- required this.transitiveSignature,
+ required this.apiSignature,
+ required this.implSignature,
}) {
for (var directDependency in directDependencies) {
directDependency._directUsers.add(this);
@@ -99,8 +127,10 @@
@override
void evaluateScc(List<_LibraryNode> scc) {
- var signature = ApiSignature();
- signature.addUint32List(_salt);
+ var apiSignature = ApiSignature();
+ var implSignature = ApiSignature();
+ apiSignature.addUint32List(_salt);
+ implSignature.addUint32List(_salt);
// Sort libraries to produce stable signatures.
scc.sort((first, second) {
@@ -115,7 +145,8 @@
var file = node.file;
_appendDirectlyReferenced(
directDependencies,
- signature,
+ apiSignature,
+ implSignature,
file.directReferencedLibraries.whereNotNull().toList(),
);
}
@@ -125,13 +156,22 @@
for (var node in scc) {
libraries.add(node.file);
- signature.addLanguageVersion(node.file.packageLanguageVersion);
- signature.addString(node.file.uriStr);
+ apiSignature.addLanguageVersion(node.file.packageLanguageVersion);
+ apiSignature.addString(node.file.uriStr);
- signature.addInt(node.file.libraryFiles.length);
+ implSignature.addLanguageVersion(node.file.packageLanguageVersion);
+ implSignature.addString(node.file.uriStr);
+
+ apiSignature.addInt(node.file.libraryFiles.length);
for (var file in node.file.libraryFiles) {
- signature.addBool(file.exists);
- signature.addBytes(file.apiSignature);
+ apiSignature.addBool(file.exists);
+ apiSignature.addBytes(file.apiSignature);
+ }
+
+ implSignature.addInt(node.file.libraryFiles.length);
+ for (var file in node.file.libraryFiles) {
+ implSignature.addBool(file.exists);
+ implSignature.addString(file.contentHash);
}
}
@@ -139,7 +179,8 @@
var cycle = LibraryCycle(
libraries: libraries,
directDependencies: directDependencies,
- transitiveSignature: signature.toHex(),
+ apiSignature: apiSignature.toHex(),
+ implSignature: implSignature.toHex(),
);
// Set the instance into the libraries.
@@ -154,10 +195,12 @@
void _appendDirectlyReferenced(
Set<LibraryCycle> directDependencies,
- ApiSignature signature,
+ ApiSignature apiSignature,
+ ApiSignature implSignature,
List<FileState> directlyReferenced,
) {
- signature.addInt(directlyReferenced.length);
+ apiSignature.addInt(directlyReferenced.length);
+ implSignature.addInt(directlyReferenced.length);
for (var referencedLibrary in directlyReferenced) {
var referencedCycle = referencedLibrary.internal_libraryCycle;
@@ -165,7 +208,12 @@
if (referencedCycle == null) continue;
if (directDependencies.add(referencedCycle)) {
- signature.addString(referencedCycle.transitiveSignature);
+ apiSignature.addString(
+ referencedCycle.hasMacroClass
+ ? referencedCycle.implSignature
+ : referencedCycle.apiSignature,
+ );
+ implSignature.addString(referencedCycle.implSignature);
}
}
}
diff --git a/pkg/analyzer/test/src/dart/analysis/driver_caching_test.dart b/pkg/analyzer/test/src/dart/analysis/driver_caching_test.dart
index 28a1626..648a942 100644
--- a/pkg/analyzer/test/src/dart/analysis/driver_caching_test.dart
+++ b/pkg/analyzer/test/src/dart/analysis/driver_caching_test.dart
@@ -5,11 +5,15 @@
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/error/error.dart';
+import 'package:analyzer/file_system/file_system.dart';
import 'package:analyzer/src/dart/analysis/driver.dart';
import 'package:analyzer/src/error/codes.dart';
+import 'package:analyzer/src/summary2/kernel_compilation_service.dart';
+import 'package:analyzer/src/summary2/macro.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
+import '../../summary/macros_environment.dart';
import '../resolution/context_collection_resolution.dart';
main() {
@@ -20,6 +24,11 @@
@reflectiveTest
class AnalysisDriverCachingTest extends PubPackageResolutionTest {
+ @override
+ MacroKernelBuilder? get macroKernelBuilder {
+ return FrontEndServerMacroKernelBuilder();
+ }
+
String get testFilePathPlatform => convertPath(testFilePath);
List<Set<String>> get _linkedCycles {
@@ -27,6 +36,24 @@
return driver.test.libraryContextTestView.linkedCycles;
}
+ @override
+ void setUp() {
+ super.setUp();
+
+ writeTestPackageConfig(
+ PackageConfigFileBuilder(),
+ macrosEnvironment: MacrosEnvironment.instance,
+ );
+ }
+
+ @override
+ Future<void> tearDown() async {
+ await super.tearDown();
+ KernelCompilationService.disposeDelayed(
+ const Duration(milliseconds: 100),
+ );
+ }
+
test_analysisOptions_strictCasts() async {
useEmptyByteStore();
@@ -340,6 +367,180 @@
_assertNoLinkedCycles();
}
+ test_macro_libraryElement_changeMacroCode() async {
+ File newFileWithFixedNameMacro(String className) {
+ return newFile('$testPackageLibPath/my_macro.dart', '''
+import 'dart:async';
+import 'package:_fe_analyzer_shared/src/macros/api.dart';
+
+macro class MyMacro implements ClassTypesMacro {
+ const MyMacro();
+
+ FutureOr<void> buildTypesForClass(clazz, builder) {
+ builder.declareType(
+ '$className',
+ DeclarationCode.fromString('class $className {}'),
+ );
+ }
+}
+''');
+ }
+
+ final macroFile = newFileWithFixedNameMacro('MacroA');
+
+ var a = newFile('$testPackageLibPath/a.dart', r'''
+import 'my_macro.dart';
+
+@MyMacro()
+class A {}
+''');
+
+ var b = newFile('$testPackageLibPath/b.dart', r'''
+export 'a.dart';
+''');
+
+ final analysisContext = contextFor(macroFile.path);
+
+ Future<LibraryElement> getLibrary(String uriStr) async {
+ final result = await analysisContext.currentSession
+ .getLibraryByUri(uriStr) as LibraryElementResult;
+ return result.element;
+ }
+
+ // This macro generates `MacroA`, but not `MacroB`.
+ {
+ final libraryA = await getLibrary('package:test/a.dart');
+ expect(libraryA.getType('MacroA'), isNotNull);
+ expect(libraryA.getType('MacroB'), isNull);
+ // This propagates transitively.
+ final libraryB = await getLibrary('package:test/b.dart');
+ expect(libraryB.exportNamespace.get('MacroA'), isNotNull);
+ expect(libraryB.exportNamespace.get('MacroB'), isNull);
+ }
+
+ _assertContainsLinkedCycle({a.path});
+ _assertContainsLinkedCycle({b.path}, andClear: true);
+
+ // The macro will generate `MacroB`.
+ newFileWithFixedNameMacro('MacroB');
+
+ // Notify about changes.
+ analysisContext.changeFile(macroFile.path);
+ await analysisContext.applyPendingFileChanges();
+
+ // This macro generates `MacroB`, but not `MacroA`.
+ {
+ final libraryA = await getLibrary('package:test/a.dart');
+ expect(libraryA.getType('MacroA'), isNull);
+ expect(libraryA.getType('MacroB'), isNotNull);
+ // This propagates transitively.
+ final libraryB = await getLibrary('package:test/b.dart');
+ expect(libraryB.exportNamespace.get('MacroA'), isNull);
+ expect(libraryB.exportNamespace.get('MacroB'), isNotNull);
+ }
+
+ _assertContainsLinkedCycle({a.path});
+ _assertContainsLinkedCycle({b.path}, andClear: true);
+ }
+
+ test_macro_resolvedUnit_changeCodeUsedByMacro() async {
+ final a = newFile('$testPackageLibPath/a.dart', r'''
+String getClassName() => 'MacroA';
+''');
+
+ newFile('$testPackageLibPath/my_macro.dart', r'''
+import 'dart:async';
+import 'package:_fe_analyzer_shared/src/macros/api.dart';
+import 'a.dart';
+
+macro class MyMacro implements ClassTypesMacro {
+ const MyMacro();
+
+ FutureOr<void> buildTypesForClass(clazz, builder) {
+ final className = getClassName();
+ builder.declareType(
+ '$className',
+ DeclarationCode.fromString('class $className {}'),
+ );
+ }
+}
+''');
+
+ // The macro will generate `MacroA`, so no errors.
+ await assertNoErrorsInCode('''
+import 'my_macro.dart';
+
+@MyMacro()
+class A {}
+
+void f(MacroA a) {}
+''');
+
+ // The macro will generate `MacroB`.
+ newFile(a.path, r'''
+String getClassName() => 'MacroB';
+''');
+
+ // Notify about changes.
+ var analysisContext = contextFor(a.path);
+ analysisContext.changeFile(a.path);
+ await analysisContext.applyPendingFileChanges();
+
+ // Resolve the test file, it still references `MacroA`, but the macro
+ // generates `MacroB` now, so we have an error.
+ await resolveTestFile();
+ assertErrorsInResult([
+ error(CompileTimeErrorCode.UNDEFINED_CLASS, 55, 6),
+ ]);
+ }
+
+ test_macro_resolvedUnit_changeMacroCode() async {
+ File newFileWithFixedNameMacro(String className) {
+ return newFile('$testPackageLibPath/my_macro.dart', '''
+import 'dart:async';
+import 'package:_fe_analyzer_shared/src/macros/api.dart';
+
+macro class MyMacro implements ClassTypesMacro {
+ const MyMacro();
+
+ FutureOr<void> buildTypesForClass(clazz, builder) {
+ builder.declareType(
+ '$className',
+ DeclarationCode.fromString('class $className {}'),
+ );
+ }
+}
+''');
+ }
+
+ var macroFile = newFileWithFixedNameMacro('MacroA');
+
+ // The macro will generate `MacroA`, so no errors.
+ await assertNoErrorsInCode('''
+import 'my_macro.dart';
+
+@MyMacro()
+class A {}
+
+void f(MacroA a) {}
+''');
+
+ // The macro will generate `MacroB`.
+ newFileWithFixedNameMacro('MacroB');
+
+ // Notify about changes.
+ var analysisContext = contextFor(macroFile.path);
+ analysisContext.changeFile(macroFile.path);
+ await analysisContext.applyPendingFileChanges();
+
+ // Resolve the test file, it still references `MacroA`, but the updated
+ // macro generates `MacroB` now, so we have an error.
+ await resolveTestFile();
+ assertErrorsInResult([
+ error(CompileTimeErrorCode.UNDEFINED_CLASS, 55, 6),
+ ]);
+ }
+
void _assertContainsLinkedCycle(Set<String> expectedPosix,
{bool andClear = false}) {
var expected = expectedPosix.map(convertPath).toSet();
diff --git a/pkg/analyzer/test/src/dart/resolution/macro_test.dart b/pkg/analyzer/test/src/dart/resolution/macro_test.dart
index 0178ec0..64b9e01 100644
--- a/pkg/analyzer/test/src/dart/resolution/macro_test.dart
+++ b/pkg/analyzer/test/src/dart/resolution/macro_test.dart
@@ -33,9 +33,6 @@
void setUp() {
super.setUp();
- // TODO(scheglov) Dependency tracking for macros is not right yet.
- useEmptyByteStore();
-
writeTestPackageConfig(
PackageConfigFileBuilder(),
macrosEnvironment: MacrosEnvironment.instance,
