Change i13n output files to live in a directory structure; fixes #38503

Bug: https://github.com/dart-lang/sdk/issues/38503
Change-Id: I6838a9b07c6a4acdf49839a3992254ec3c9a1ffd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/119120
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>

pkg/analysis_server/lib/src/edit/fix/non_nullable_fix.dart

@@ -217,18 +217,24 @@ analyzer:
     List<LibraryInfo> libraryInfos =
         await InfoBuilder(instrumentationListener.data, listener)
             .explainMigration();
-    listener.addDetail('libraryInfos has ${libraryInfos.length} libs');
+    var pathContext = provider.pathContext;
+    MigrationInfo migrationInfo =
+        MigrationInfo(libraryInfos, pathContext, includedRoot);
     for (LibraryInfo info in libraryInfos) {
-      var pathContext = provider.pathContext;
-      var libraryPath =
+      assert(info.units.isNotEmpty);
+      String libraryPath =
           pathContext.setExtension(info.units.first.path, '.html');
-      // TODO(srawlins): Choose a better scheme than the double underscores,
-      // likely with actual directories, which need to be individually created.
-      var relativePath = pathContext
-          .relative(libraryPath, from: includedRoot)
-          .replaceAll(pathContext.separator, '__');
-      File output = folder.getChildAssumingFile(relativePath);
-      String rendered = InstrumentationRenderer(info).render();
+      String relativePath =
+          pathContext.relative(libraryPath, from: includedRoot);
+      List<String> directories =
+          pathContext.split(pathContext.dirname(relativePath));
+      for (int i = 0; i < directories.length; i++) {
+        String directory = pathContext.joinAll(directories.sublist(0, i + 1));
+        folder.getChildAssumingFolder(directory).create();
+      }
+      File output =
+          provider.getFile(pathContext.join(folder.path, relativePath));
+      String rendered = InstrumentationRenderer(info, migrationInfo).render();
       output.writeAsStringSync(rendered);
     }
   }

pkg/analysis_server/lib/src/edit/nnbd_migration/instrumentation_renderer.dart

@@ -1,19 +1,31 @@
 import 'package:analysis_server/src/edit/nnbd_migration/migration_info.dart';
+import 'package:meta/meta.dart';
 import 'package:mustache/mustache.dart' as mustache;
+import 'package:path/path.dart' as path;
 
-/// Instrumentation display output for a library that was migrated to use non-nullable types.
+/// Instrumentation display output for a library that was migrated to use
+/// non-nullable types.
 class InstrumentationRenderer {
   /// Display information for a library.
-  final LibraryInfo info;
+  final LibraryInfo libraryInfo;
+
+  /// Information for a whole migration, so that libraries can reference each
+  /// other.
+  final MigrationInfo migrationInfo;
 
   /// Creates an output object for the given library info.
-  InstrumentationRenderer(this.info);
+  InstrumentationRenderer(this.libraryInfo, this.migrationInfo);
 
-  /// Builds an HTML view of the instrumentation information in [info].
+  /// Builds an HTML view of the instrumentation information in [libraryInfo].
   String render() {
     int previousIndex = 0;
-    Map<String, dynamic> mustacheContext = {'units': <Map<String, dynamic>>[]};
-    for (var compilationUnit in info.units) {
+    Map<String, dynamic> mustacheContext = {
+      'units': <Map<String, dynamic>>[],
+      'links': migrationInfo.libraryLinks(libraryInfo),
+      'highlightJsPath': migrationInfo.highlightJsPath(libraryInfo),
+      'highlightStylePath': migrationInfo.highlightStylePath(libraryInfo),
+    };
+    for (var compilationUnit in libraryInfo.units) {
       // List of Mustache context for both unmodified and modified regions:
       //
       // * 'modified': Whether this region represents modified source, or
@@ -55,13 +67,70 @@ class InstrumentationRenderer {
   }
 }
 
+/// A class storing rendering information for an entire migration report.
+///
+/// This generally provides one [InstrumentationRenderer] (for one library)
+/// with information about the rest of the libraries represented in the
+/// instrumentation output.
+class MigrationInfo {
+  /// The information about the libraries that are are migrated.
+  final List<LibraryInfo> libraries;
+
+  /// The resource provider's path context.
+  final path.Context pathContext;
+
+  /// The filesystem root used to create relative paths for each unit.
+  final String includedRoot;
+
+  MigrationInfo(this.libraries, this.pathContext, this.includedRoot);
+
+  /// Generate mustache context for library links, for navigation in the
+  /// instrumentation document for [thisLibrary].
+  List<Map<String, Object>> libraryLinks(LibraryInfo thisLibrary) {
+    return [
+      for (var library in libraries)
+        {
+          'name': _computeName(library),
+          'isLink': library != thisLibrary,
+          if (library != thisLibrary)
+            'href': _pathTo(library, source: thisLibrary)
+        }
+    ];
+  }
+
+  /// Return the path to [library] from [includedRoot], to be used as a display
+  /// name for a library.
+  String _computeName(LibraryInfo library) =>
+      pathContext.relative(library.units.first.path, from: includedRoot);
+
+  /// The path to [target], relative to [from].
+  String _pathTo(LibraryInfo target, {@required LibraryInfo source}) {
+    assert(target.units.isNotEmpty);
+    assert(source.units.isNotEmpty);
+    String targetPath =
+        pathContext.setExtension(target.units.first.path, '.html');
+    String sourceDir = pathContext.dirname(source.units.first.path);
+    return pathContext.relative(targetPath, from: sourceDir);
+  }
+
+  /// The path to the highlight.js script, relative to [libraryInfo].
+  String highlightJsPath(LibraryInfo libraryInfo) =>
+      pathContext.relative(pathContext.join(includedRoot, 'highlight.pack.js'),
+          from: pathContext.dirname(libraryInfo.units.first.path));
+
+  /// The path to the highlight.js stylesheet, relative to [libraryInfo].
+  String highlightStylePath(LibraryInfo libraryInfo) => pathContext.relative(
+      pathContext.join(includedRoot, 'styles', 'androidstudio.css'),
+      from: pathContext.dirname(libraryInfo.units.first.path));
+}
+
 /// A mustache template for one library's instrumentation output.
 mustache.Template _template = mustache.Template(r'''
 <html>
   <head>
     <title>Non-nullable fix instrumentation report</title>
-    <script src="highlight.pack.js"></script>
-    <link rel="stylesheet" href="styles/androidstudio.css">
+    <script src="{{ highlightJsPath }}"></script>
+    <link rel="stylesheet" href="{{ highlightStylePath }}">
     <style>
 body {
   font-family: sans-serif;
@@ -122,10 +191,17 @@ h2 {
   </head>
   <body>
     <h1>Non-nullable fix instrumentation report</h1>
-    <p><em>Well-written introduction to this report.</em></p>'''
-    '    {{# units }}'
-    '      <h2>{{{ path }}}</h2>'
-    '      <div class="content highlighting">'
+    <p><em>Well-written introduction to this report.</em></p>
+    <div class="navigation">
+      {{# links }}
+        {{# isLink }}<a href="{{ href }}">{{ name }}</a>{{/ isLink }}
+        {{^ isLink }}{{ name }}{{/ isLink }}
+        <br />
+      {{/ links }}
+    </div>
+    {{# units }}'''
+    '  <h2>{{{ path }}}</h2>'
+    '  <div class="content highlighting">'
     '{{! These regions are written out, unmodified, as they need to be found }}'
     '{{! in one simple text string for highlight.js to hightlight them. }}'
     '{{# regions }}'
@@ -140,13 +216,14 @@ h2 {
     '<span class="tooltip">{{explanation}}</span></span>{{/ modified }}'
     '{{/ regions }}'
     '</div></div>'
-    '    {{/ units }}'
-    '    <script lang="javascript">'
-    'document.addEventListener("DOMContentLoaded", (event) => {'
-    '  document.querySelectorAll(".highlighting").forEach((block) => {'
-    '    hljs.highlightBlock(block);'
-    '  });'
-    '});'
-    '    </script>'
-    '  </body>'
-    '</html>');
+    r'''
+    {{/ units }}
+    <script lang="javascript">
+document.addEventListener("DOMContentLoaded", (event) => {
+  document.querySelectorAll(".highlighting").forEach((block) => {
+    hljs.highlightBlock(block);
+  });
+});
+    </script>
+  </body>
+</html>''');

pkg/analysis_server/test/edit/nnbd_migration/instrumentation_output_test.dart

@@ -17,6 +17,16 @@ main() {
 
 @reflectiveTest
 class InstrumentationRendererTest extends AbstractAnalysisTest {
+  /// Render [libraryInfo], using a [MigrationInfo] which knows only about this
+  /// library.
+  // TODO(srawlins): Add tests for navigation links, which use multiple
+  // libraries.
+  String renderLibrary(LibraryInfo libraryInfo) {
+    MigrationInfo migrationInfo =
+        MigrationInfo([libraryInfo], resourceProvider.pathContext, '/project');
+    return InstrumentationRenderer(libraryInfo, migrationInfo).render();
+  }
+
   test_outputContainsEachPath() async {
     LibraryInfo info = LibraryInfo([
       unit('/lib/a.dart', 'int? a = null;',
@@ -26,7 +36,7 @@ class InstrumentationRendererTest extends AbstractAnalysisTest {
       unit('/lib/part2.dart', 'int? c = null;',
           regions: [RegionInfo(3, 1, 'null was assigned', [])]),
     ]);
-    String output = InstrumentationRenderer(info).render();
+    String output = renderLibrary(info);
     expect(output, contains('<h2>/lib/a.dart</h2>'));
     expect(output, contains('<h2>/lib/part1.dart</h2>'));
     expect(output, contains('<h2>/lib/part2.dart</h2>'));
@@ -37,7 +47,7 @@ class InstrumentationRendererTest extends AbstractAnalysisTest {
       unit('/lib/a.dart', 'List<String>? a = null;',
           regions: [RegionInfo(12, 1, 'null was assigned', [])]),
     ]);
-    String output = InstrumentationRenderer(info).render();
+    String output = renderLibrary(info);
     expect(
         output,
         contains('List&lt;String&gt;<span class="region">?'
@@ -48,7 +58,7 @@ class InstrumentationRendererTest extends AbstractAnalysisTest {
     LibraryInfo info = LibraryInfo([
       unit('/lib/a.dart', 'bool a = true && false;', regions: []),
     ]);
-    String output = InstrumentationRenderer(info).render();
+    String output = renderLibrary(info);
     expect(output, contains('bool a = true &amp;&amp; false;'));
   }
 
@@ -57,7 +67,7 @@ class InstrumentationRendererTest extends AbstractAnalysisTest {
       unit('/lib/a.dart', 'int? a = null;',
           regions: [RegionInfo(3, 1, 'null was assigned', [])]),
     ]);
-    String output = InstrumentationRenderer(info).render();
+    String output = renderLibrary(info);
     expect(
         output,
         contains('int<span class="region">?'

pkg/analysis_server/test/src/edit/fix/non_nullable_fix_test.dart

@@ -136,12 +136,11 @@ class NonNullableFixTest extends AbstractAnalysisTest {
     Folder outputDir = getFolder('/outputDir');
     await performFix(included: [projectPath], outputDir: outputDir.path);
     expect(outputDir.exists, true);
-    expect(outputDir.getChildAssumingFile('bin__bin.html').exists, isTrue);
-    expect(outputDir.getChildAssumingFile('lib__lib1.html').exists, isTrue);
-    expect(outputDir.getChildAssumingFile('lib__lib2.html').exists, isTrue);
-    expect(
-        outputDir.getChildAssumingFile('lib__src__lib3.html').exists, isTrue);
-    expect(outputDir.getChildAssumingFile('test__test.html').exists, isTrue);
+    expect(getFile('/outputDir/bin/bin.html').exists, isTrue);
+    expect(getFile('/outputDir/lib/lib1.html').exists, isTrue);
+    expect(getFile('/outputDir/lib/lib2.html').exists, isTrue);
+    expect(getFile('/outputDir/lib/src/lib3.html').exists, isTrue);
+    expect(getFile('/outputDir/test/test.html').exists, isTrue);
   }
 
   test_outputDirContainsFilesRootedInASubdirectory() async {
@@ -151,9 +150,9 @@ class NonNullableFixTest extends AbstractAnalysisTest {
         included: [context.join(projectPath, 'lib')],
         outputDir: outputDir.path);
     expect(outputDir.exists, true);
-    expect(outputDir.getChildAssumingFile('lib1.html').exists, isTrue);
-    expect(outputDir.getChildAssumingFile('lib2.html').exists, isTrue);
-    expect(outputDir.getChildAssumingFile('src__lib3.html').exists, isTrue);
+    expect(getFile('/outputDir/lib1.html').exists, isTrue);
+    expect(getFile('/outputDir/lib2.html').exists, isTrue);
+    expect(getFile('/outputDir/src/lib3.html').exists, isTrue);
   }
 
   test_outputDirContainsFilesRootedInParentOfSingleFile() async {