Use import URIs when invalidating files

Fixes flutter/flutter#16368
Note that this is mostly a copy of
https://dart-review.googlesource.com/c/sdk/+/48180

Change-Id: Ibdf7719d8b362a4c179fd7a4a6ab5ae7028a5429
Reviewed-on: https://dart-review.googlesource.com/50420
Commit-Queue: Jens Johansen <[email protected]>
Reviewed-by: Peter von der Ahé <[email protected]>

Author: jensjoha

pkg/front_end/lib/src/fasta/incremental_compiler.dart

@@ -101,7 +101,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
         userBuilders = <Uri, LibraryBuilder>{};
         platformBuilders = <LibraryBuilder>[];
         dillLoadedData.loader.builders.forEach((uri, builder) {
-          if (builder.fileUri.scheme == "dart") {
+          if (builder.uri.scheme == "dart") {
             platformBuilders.add(builder);
           } else {
             userBuilders[uri] = builder;
@@ -110,7 +110,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
         if (userBuilders.isEmpty) userBuilders = null;
       }
 
-      List<Uri> invalidatedUris = this.invalidatedUris.toList();
+      Set<Uri> invalidatedUris = this.invalidatedUris.toSet();
       this.invalidatedUris.clear();
       if (fullComponent) {
         invalidatedUris.add(entryPoint);
@@ -156,7 +156,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
         List<Library> librariesWithSdk = userCode.component.libraries;
         List<Library> libraries = <Library>[];
         for (Library lib in librariesWithSdk) {
-          if (lib.fileUri.scheme == "dart") continue;
+          if (lib.importUri.scheme == "dart") continue;
           libraries.add(lib);
           break;
         }
@@ -199,20 +199,20 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
       IncrementalCompilerData data) {
     Map<Uri, Library> libraryMap = <Uri, Library>{};
     for (Library library in libraries) {
-      libraryMap[library.fileUri] = library;
+      libraryMap[library.importUri] = library;
     }
     List<Uri> worklist = new List<Uri>.from(libraryMap.keys);
-    worklist.add(mainMethod?.enclosingLibrary?.fileUri);
+    worklist.add(mainMethod?.enclosingLibrary?.importUri);
     if (entry != null) {
       worklist.add(entry);
     }
 
     Map<Uri, Library> potentiallyReferencedLibraries = <Uri, Library>{};
     for (LibraryBuilder library in reusedLibraries) {
-      if (library.fileUri.scheme == "dart") continue;
+      if (library.uri.scheme == "dart") continue;
       Library lib = library.target;
-      potentiallyReferencedLibraries[library.fileUri] = lib;
-      libraryMap[library.fileUri] = lib;
+      potentiallyReferencedLibraries[library.uri] = lib;
+      libraryMap[library.uri] = lib;
     }
 
     LibraryGraph graph = new LibraryGraph(libraryMap);
@@ -325,45 +325,52 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
   }
 
   List<LibraryBuilder> computeReusedLibraries(
-      Iterable<Uri> invalidatedUris, UriTranslator uriTranslator) {
+      Set<Uri> invalidatedUris, UriTranslator uriTranslator) {
     if (userCode == null && userBuilders == null) {
       return <LibraryBuilder>[];
     }
 
-    // [invalidatedUris] converted to a set.
-    Set<Uri> invalidatedFileUris = invalidatedUris.toSet();
-
     // Maps all non-platform LibraryBuilders from their import URI.
     Map<Uri, LibraryBuilder> builders = <Uri, LibraryBuilder>{};
 
     // Invalidated URIs translated back to their import URI (package:, dart:,
     // etc.).
     List<Uri> invalidatedImportUris = <Uri>[];
 
+    bool isInvalidated(Uri importUri, Uri fileUri) {
+      if (invalidatedUris.contains(importUri) ||
+          (importUri != fileUri && invalidatedUris.contains(fileUri))) {
+        return true;
+      }
+      if (importUri.scheme == "package" &&
+          uriTranslator.translate(importUri, false) != fileUri) {
+        return true;
+      }
+      return false;
+    }
+
     // Compute [builders] and [invalidatedImportUris].
     addBuilderAndInvalidateUris(Uri uri, LibraryBuilder library,
         [bool recursive = true]) {
       builders[uri] = library;
-      if (invalidatedFileUris.contains(uri) ||
-          (uri != library.fileUri &&
-              invalidatedFileUris.contains(library.fileUri)) ||
-          (library is DillLibraryBuilder &&
-              uri != library.library.fileUri &&
-              invalidatedFileUris.contains(library.library.fileUri)) ||
-          (library.uri.scheme == "package" &&
-              uriTranslator.translate(library.uri, false) !=
-                  library.target.fileUri)) {
+      if (isInvalidated(uri, library.target.fileUri)) {
         invalidatedImportUris.add(uri);
       }
-      if (!recursive) return;
       if (library is SourceLibraryBuilder) {
         for (LibraryBuilder part in library.parts) {
-          addBuilderAndInvalidateUris(part.uri, part, false);
+          if (isInvalidated(part.uri, part.fileUri)) {
+            invalidatedImportUris.add(part.uri);
+            builders[part.uri] = part;
+          }
         }
       } else if (library is DillLibraryBuilder) {
-        for (LibraryPart part in library.library.parts) {
+        for (LibraryPart part in library.target.parts) {
           Uri partUri = library.uri.resolve(part.partUri);
-          addBuilderAndInvalidateUris(partUri, library, false);
+          Uri fileUri = library.library.fileUri.resolve(part.partUri);
+          if (isInvalidated(partUri, fileUri)) {
+            invalidatedImportUris.add(partUri);
+            builders[partUri] = library;
+          }
         }
       }
     }
@@ -417,7 +424,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
     List<LibraryBuilder> result = <LibraryBuilder>[];
     for (LibraryBuilder builder in builders.values) {
       if (builder.isPart) continue;
-      if (!seenUris.add(builder.fileUri)) continue;
+      // TODO(jensj/ahe): This line can probably go away once
+      // https://dart-review.googlesource.com/47442 lands.
+      if (builder.isPatch) continue;
+      if (!seenUris.add(builder.uri)) continue;
       result.add(builder);
     }
     return result;

pkg/front_end/test/incremental_load_from_dill_test.dart

@@ -117,21 +117,30 @@ class RunCompilations extends Step<TestData, TestData, Context> {
 
 void basicTest(Map<String, String> sourceFiles, String entryPoint, bool strong,
     List<String> invalidate, Directory outDir) async {
-  Uri entryPointUri;
+  Uri entryPointUri = outDir.uri.resolve(entryPoint);
   Set<String> invalidateFilenames = invalidate?.toSet() ?? new Set<String>();
   List<Uri> invalidateUris = <Uri>[];
   Uri packagesUri;
   for (String filename in sourceFiles.keys) {
     Uri uri = outDir.uri.resolve(filename);
-    if (filename == entryPoint) entryPointUri = uri;
-    if (invalidateFilenames.contains(filename)) invalidateUris.add(uri);
+    if (invalidateFilenames.contains(filename)) {
+      invalidateUris.add(uri);
+      invalidateFilenames.remove(filename);
+    }
     String source = sourceFiles[filename];
     if (filename == ".packages") {
       source = substituteVariables(source, outDir.uri);
       packagesUri = uri;
     }
     new File.fromUri(uri).writeAsStringSync(source);
   }
+  for (String invalidateFilename in invalidateFilenames) {
+    if (invalidateFilename.startsWith('package:')) {
+      invalidateUris.add(Uri.parse(invalidateFilename));
+    } else {
+      throw "Error in test yaml: $invalidateFilename was not recognized.";
+    }
+  }
 
   Uri output = outDir.uri.resolve("full.dill");
   Uri initializedOutput = outDir.uri.resolve("full_from_initialized.dill");
@@ -145,9 +154,13 @@ void basicTest(Map<String, String> sourceFiles, String entryPoint, bool strong,
   print("Normal compile took ${stopwatch.elapsedMilliseconds} ms");
 
   stopwatch.reset();
+  options = getOptions(strong);
+  if (packagesUri != null) {
+    options.packagesFileUri = packagesUri;
+  }
   bool initializedResult = await initializedCompile(
       entryPointUri, initializedOutput, output, invalidateUris,
-      options: getOptions(strong));
+      options: options);
   print("Initialized compile(s) from ${output.pathSegments.last} "
       "took ${stopwatch.elapsedMilliseconds} ms");
   Expect.isTrue(initializedResult);
@@ -249,7 +262,8 @@ void newWorldTest(bool strong, List worlds) async {
     }
 
     Stopwatch stopwatch = new Stopwatch()..start();
-    Component component = await compiler.computeDelta(fullComponent: true);
+    Component component = await compiler.computeDelta(
+        fullComponent: brandNewWorld ? false : true);
     if (world["errors"] == true && !gotError) {
       throw "Expected error, but didn't get any.";
     } else if (world["errors"] != true && gotError) {
@@ -278,7 +292,7 @@ void newWorldTest(bool strong, List worlds) async {
     if (world["checkInvalidatedFiles"] != false) {
       if (world["invalidate"] != null) {
         Expect.equals(world["invalidate"].length,
-            compiler.invalidatedImportUrisForTesting.length);
+            compiler.invalidatedImportUrisForTesting?.length ?? 0);
         List expectedInvalidatedUri = world["expectedInvalidatedUri"];
         if (expectedInvalidatedUri != null) {
           Expect.setEquals(
@@ -357,6 +371,12 @@ Future<bool> initializedCompile(
   throwOnEmptyMixinBodies(initializedComponent);
   bool result = compiler.initializedFromDill;
   await writeComponentToFile(initializedComponent, output);
+  int actuallyInvalidatedCount =
+      compiler.invalidatedImportUrisForTesting?.length ?? 0;
+  if (result && actuallyInvalidatedCount < invalidateUris.length) {
+    Expect.fail("Expected at least ${invalidateUris.length} invalidated uris, "
+        "got $actuallyInvalidatedCount");
+  }
 
   var initializedComponent2 = await compiler.computeDelta(fullComponent: true);
   throwOnEmptyMixinBodies(initializedComponent2);
@@ -371,6 +391,13 @@ Future<bool> initializedCompile(
 
   var partialComponent = await compiler.computeDelta();
   throwOnEmptyMixinBodies(partialComponent);
+  actuallyInvalidatedCount =
+      (compiler.invalidatedImportUrisForTesting?.length ?? 0);
+  if (actuallyInvalidatedCount < invalidateUris.length) {
+    Expect.fail("Expected at least ${invalidateUris.length} invalidated uris, "
+        "got $actuallyInvalidatedCount");
+  }
+
   var emptyComponent = await compiler.computeDelta();
   throwOnEmptyMixinBodies(emptyComponent);
 
@@ -384,6 +411,7 @@ Future<bool> initializedCompile(
   Expect.isTrue(fullLibUris.length > partialLibUris.length ||
       partialLibUris.length == invalidateUris.length);
   Expect.isTrue(partialLibUris.isNotEmpty || invalidateUris.isEmpty);
+
   Expect.isTrue(emptyLibUris.isEmpty);
 
   return result;
@@ -417,7 +445,7 @@ String substituteVariables(String source, Uri base) {
 }
 
 class TestIncrementalCompiler extends IncrementalCompiler {
-  List<Uri> invalidatedImportUrisForTesting;
+  Set<Uri> invalidatedImportUrisForTesting;
 
   TestIncrementalCompiler(CompilerOptions options, Uri entryPoint,
       [Uri initializeFrom])
@@ -428,6 +456,6 @@ class TestIncrementalCompiler extends IncrementalCompiler {
 
   @override
   void recordInvalidatedImportUrisForTesting(List<Uri> uris) {
-    invalidatedImportUrisForTesting = uris.isEmpty ? null : uris.toList();
+    invalidatedImportUrisForTesting = uris.isEmpty ? null : uris.toSet();
   }
 }

pkg/front_end/testcases/incremental_initialize_from_dill/invalidate_package_part_as_file.yaml

@@ -8,6 +8,7 @@ type: basic
 entry: "package:dummy/main.dart"
 strong: false
 invalidate:
+  - b.dart
 sources:
   main.dart: |
     part "b.dart";

pkg/front_end/testcases/incremental_initialize_from_dill/invalidate_package_part_as_package.yaml

@@ -2,12 +2,13 @@
 # for details. All rights reserved. Use of this source code is governed by a
 # BSD-style license that can be found in the LICENSE.md file.
 
-# Test that invalidating a part of a package works with file URI.
+# Test that invalidating a part of a package works with package URI.
 
 type: basic
 entry: "package:dummy/main.dart"
 strong: false
 invalidate:
+  - "package:dummy/b.dart"
 sources:
   main.dart: |
     part "b.dart";

pkg/front_end/testcases/incremental_initialize_from_dill/status.status

@@ -5,7 +5,4 @@
 # Status file for the test suite ../test/incremental_load_from_dill_yaml_test.dart.
 
 calculated_bounds_no_strongmode: Crash
-invalidate_package_part: Crash
-invalidate_package_part_as_file: Crash
-invalidate_package_part_as_package: Crash
 strongmode_mixins_2: Crash