Update nnbd branch for latest changes and new resources-dir option (#2867)

parlough · scheglov · srawlins · web-flow · commit 55ca10f9ca81 · 2021-12-30T09:14:39.000-07:00
* Stop using JavaFile. (#2856) * Add a resources-dir option (#2857) * Skip sdk-analyzer job (#2863) * Use 'index' to access the enum value index. (#2865) Co-authored-by: Sam Rawlins <srawlins@google.com> * Re-enable sdk-analyzer job, but make it not failing (#2864) * Update new resources-dir option for null safety * Fix html generator test merge Co-authored-by: Konstantin Scheglov <scheglov@google.com> Co-authored-by: Sam Rawlins <srawlins@google.com>
diff --git a/analysis_options.yaml b/analysis_options.yaml
@@ -20,7 +20,6 @@ analyzer:
 linter:
   rules:
     always_declare_return_types: true
-    annotate_overrides: true
     avoid_dynamic_calls: true
     avoid_single_cascade_in_expression_statements: true
     avoid_unused_constructor_parameters: true
diff --git a/analysis_options_presubmit.yaml b/analysis_options_presubmit.yaml
@@ -25,7 +25,6 @@ analyzer:
 linter:
   rules:
     always_declare_return_types: true
-    annotate_overrides: true
     avoid_dynamic_calls: true
     avoid_single_cascade_in_expression_statements: true
     avoid_unused_constructor_parameters: true
diff --git a/lib/options.dart b/lib/options.dart
@@ -41,12 +41,13 @@ class DartdocGeneratorOptionContext extends DartdocOptionContext {
   String get relCanonicalPrefix =>
       optionSet['relCanonicalPrefix'].valueAt(context);
 
-  /// The 'templatesDir' Dartdoc option if one was specified; otherwise `null`.
   String? get templatesDir => optionSet['templatesDir'].valueAt(context);
 
   // TODO(jdkoren): duplicated temporarily so that GeneratorContext is enough for configuration.
   @override
   bool get useBaseHref => optionSet['useBaseHref'].valueAt(context);
+
+  String? get resourcesDir => optionSet['resourcesDir'].valueAt(context);
 }
 
 class DartdocProgramOptionContext extends DartdocGeneratorOptionContext
diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart
@@ -1507,6 +1507,8 @@ Future<List<DartdocOption>> createDartdocOptions(
         help:
             'A list of package names to place first when grouping libraries in '
             'packages. Unmentioned packages are sorted after these.'),
+    DartdocOptionArgOnly<String?>('resourcesDir', null, resourceProvider,
+        help: "An absolute path to dartdoc's resources directory.", hide: true),
     DartdocOptionArgOnly<bool>('sdkDocs', false, resourceProvider,
         help: 'Generate ONLY the docs for the Dart SDK.'),
     DartdocOptionArgSynth<String?>('sdkDir',
diff --git a/lib/src/generator/dartdoc_generator_backend.dart b/lib/src/generator/dartdoc_generator_backend.dart
@@ -38,6 +38,8 @@ class DartdocGeneratorBackendOptions implements TemplateOptions {
   @override
   final String customInnerFooterText;
 
+  final String? resourcesDir;
+
   DartdocGeneratorBackendOptions.fromContext(
       DartdocGeneratorOptionContext context)
       : relCanonicalPrefix = context.relCanonicalPrefix,
@@ -47,7 +49,8 @@ class DartdocGeneratorBackendOptions implements TemplateOptions {
         useBaseHref = context.useBaseHref,
         customHeaderContent = context.header,
         customFooterContent = context.footer,
-        customInnerFooterText = context.footerText;
+        customInnerFooterText = context.footerText,
+        resourcesDir = context.resourcesDir;
 }
 
 class SidebarGenerator<T extends TemplateData> {
diff --git a/lib/src/generator/html_generator.dart b/lib/src/generator/html_generator.dart
@@ -52,26 +52,28 @@ class HtmlGeneratorBackend extends DartdocGeneratorBackend {
       // Allow overwrite of favicon.
       var bytes = resourceProvider.getFile(favicon).readAsBytesSync();
       writer.writeBytes(
-        resourceProvider.pathContext.join('static-assets', 'favicon.png'),
+        _pathJoin('static-assets', 'favicon.png'),
         bytes,
         allowOverwrite: true,
       );
     }
   }
 
   Future<void> _copyResources(FileWriter writer) async {
-    for (var resourcePath in resources.resourceNames) {
-      if (!resourcePath.startsWith(_dartdocResourcePrefix)) {
-        throw StateError('Resource paths must start with '
-            '$_dartdocResourcePrefix, encountered $resourcePath');
-      }
-      var destFileName = resourcePath.substring(_dartdocResourcePrefix.length);
-      var destFilePath =
-          resourceProvider.pathContext.join('static-assets', destFileName);
-      writer.writeBytes(destFilePath,
-          await resourceProvider.loadResourceAsBytes(resourcePath));
+    var resourcesDir = options.resourcesDir ??
+        (await resourceProvider.getResourceFolder(_dartdocResourcePrefix)).path;
+    for (var resourceFileName in resources.resourceNames) {
+      var destinationPath = _pathJoin('static-assets', resourceFileName);
+      var sourcePath = _pathJoin(resourcesDir, resourceFileName);
+      writer.writeBytes(
+        destinationPath,
+        resourceProvider.getFile(sourcePath).readAsBytesSync(),
+      );
     }
   }
 
-  static const _dartdocResourcePrefix = 'package:dartdoc/resources/';
+  String _pathJoin(String a, String b) =>
+      resourceProvider.pathContext.join(a, b);
+
+  static const _dartdocResourcePrefix = 'package:dartdoc/resources';
 }
diff --git a/lib/src/generator/html_resources.g.dart b/lib/src/generator/html_resources.g.dart
@@ -1,11 +1,11 @@
 // WARNING: This file is auto-generated. Do not edit.
 
 const List<String> resourceNames = [
-  'package:dartdoc/resources/favicon.png',
-  'package:dartdoc/resources/github.css',
-  'package:dartdoc/resources/highlight.pack.js',
-  'package:dartdoc/resources/play_button.svg',
-  'package:dartdoc/resources/readme.md',
-  'package:dartdoc/resources/script.js',
-  'package:dartdoc/resources/styles.css'
+  'favicon.png',
+  'github.css',
+  'highlight.pack.js',
+  'play_button.svg',
+  'readme.md',
+  'script.js',
+  'styles.css',
 ];
diff --git a/lib/src/model/model_element.dart b/lib/src/model/model_element.dart
@@ -177,7 +177,7 @@ abstract class ModelElement extends Canonicalization
     if (e is FieldElement) {
       if (enclosingContainer == null) {
         if (e.isEnumConstant) {
-          var index = e.computeConstantValue()!.getField(e.name)!.toIntValue();
+          var index = e.computeConstantValue()!.getField('index')!.toIntValue();
           newModelElement =
               EnumField.forConstant(index, e, library, packageGraph, getter);
         } else if (e.enclosingElement is ExtensionElement) {
diff --git a/lib/src/model/package_builder.dart b/lib/src/model/package_builder.dart
@@ -19,8 +19,6 @@ import 'package:analyzer/src/dart/sdk/sdk.dart'
 // ignore: implementation_imports
 import 'package:analyzer/src/generated/engine.dart' show AnalysisOptionsImpl;
 // ignore: implementation_imports
-import 'package:analyzer/src/generated/java_io.dart' show JavaFile;
-// ignore: implementation_imports
 import 'package:analyzer/src/generated/sdk.dart' show DartSdk;
 import 'package:collection/collection.dart' show IterableExtension;
 import 'package:dartdoc/src/dartdoc_options.dart';
@@ -144,16 +142,18 @@ class PubPackageBuilder implements PackageBuilder {
   /// If [filePath] is not a library, returns null.
   Future<DartDocResolvedLibrary?> processLibrary(String filePath) async {
     var name = filePath;
-    var directoryCurrentPath = resourceProvider.pathContext.current;
+    var pathContext = resourceProvider.pathContext;
+    var directoryCurrentPath = pathContext.current;
 
     if (name.startsWith(directoryCurrentPath)) {
       name = name.substring(directoryCurrentPath.length);
-      if (name.startsWith(resourceProvider.pathContext.separator)) {
+      if (name.startsWith(pathContext.separator)) {
         name = name.substring(1);
       }
     }
-    var javaFile = JavaFile(filePath).getAbsoluteFile();
-    filePath = javaFile.getPath();
+
+    // TODO(scheglov) Do we need this? Maybe the argument is already valid?
+    filePath = pathContext.normalize(pathContext.absolute(filePath));
 
     var analysisContext = contextCollection!.contextFor(config.inputDir);
     var session = analysisContext.currentSession;
diff --git a/test/html_generator_test.dart b/test/html_generator_test.dart
@@ -146,8 +146,7 @@ void main() {
           .getFolder(pathContext.join(outputPath, 'static-assets'));
       expect(output, doesExist);
 
-      for (var resource in resourceNames.map((r) =>
-          pathContext.relative(Uri.parse(r).path, from: 'dartdoc/resources'))) {
+      for (var resource in resourceNames) {
         expect(
             resourceProvider.getFile(pathContext.join(output.path, resource)),
             doesExist);
diff --git a/testing/test_package/lib/features/nullable_elements.dart b/testing/test_package/lib/features/nullable_elements.dart
@@ -15,6 +15,7 @@ void set nullableSetter(String? value) {
 }
 
 /// This should have return type of `Future?`.
+// ignore: unnecessary_question_mark
 dynamic? oddAsyncFunction() async {}
 
 /// This should also have return type of `Future?`.
diff --git a/testing/test_package/lib/features/opt_out_of_nnbd.dart b/testing/test_package/lib/features/opt_out_of_nnbd.dart
@@ -6,4 +6,5 @@
 
 library opt_out_of_nnbd;
 
+// ignore: dead_code
 String notOptedIn = false ? 'hi' : null;
diff --git a/testing/test_package/lib/src/somelib.dart b/testing/test_package/lib/src/somelib.dart
@@ -18,11 +18,13 @@ class ExtendedBaseReexported extends BaseReexported {}
 
 /// A private extension.
 extension _Unseen on Object {
+  // ignore: unused_element
   void doYouSeeMe() {}
 }
 
 /// An extension without a name
 extension on List {
+  // ignore: unused_element
   void somethingNew() {}
 }
 
diff --git a/tool/builder.dart b/tool/builder.dart
@@ -12,22 +12,25 @@ String _resourcesFile(Iterable<String> packagePaths) => '''
 // WARNING: This file is auto-generated. Do not edit.
 
 const List<String> resourceNames = [
-${packagePaths.map((p) => "  '$p'").join(',\n')}
+${packagePaths.map((p) => "  '$p',").join('\n')}
 ];
 ''';
 
 class ResourceBuilder implements Builder {
   final BuilderOptions builderOptions;
+
   ResourceBuilder(this.builderOptions);
 
-  static final _allResources = Glob('lib/resources/**');
+  static const _resourcesPath = 'lib/resources';
+
   @override
   Future<void> build(BuildStep buildStep) async {
-    var packagePaths = <String>[];
-    await for (AssetId asset in buildStep.findAssets(_allResources)) {
-      packagePaths.add(asset.uri.toString());
-    }
-    packagePaths.sort();
+    var resourceAssets =
+        await buildStep.findAssets(Glob('$_resourcesPath/**')).toList();
+    var packagePaths = [
+      for (var asset in resourceAssets)
+        path.url.relative(asset.path, from: _resourcesPath),
+    ]..sort();
     await buildStep.writeAsString(
         AssetId(buildStep.inputId.package,
             path.url.join('lib', 'src', 'generator', 'html_resources.g.dart')),
diff --git a/tool/grind.dart b/tool/grind.dart
@@ -260,8 +260,10 @@ void analyzeTestPackages() async {
       workingDirectory: testPackagePath,
     );
     await SubprocessLauncher('analyze-test-package').runStreamed(
-      sdkBin('dartanalyzer'),
-      ['.'],
+      sdkBin('dart'),
+      // TODO(srawlins): Analyze the whole directory by ignoring the pubspec
+      // reports.
+      ['analyze', 'lib'],
       workingDirectory: testPackagePath,
     );
   }
@@ -548,9 +550,15 @@ Future<void> testWithAnalyzerSdk() async {
   var sdkDartdoc = await createSdkDartdoc();
   var defaultGrindParameter =
       Platform.environment['DARTDOC_GRIND_STEP'] ?? 'test';
-  await launcher.runStreamed(
-      sdkBin('pub'), ['run', 'grinder', defaultGrindParameter],
-      workingDirectory: sdkDartdoc);
+  // TODO(srawlins): Re-enable sdk-analyzer when dart_style is published using
+  // analyzer 3.0.0.
+  try {
+    await launcher.runStreamed(
+        sdkBin('pub'), ['run', 'grinder', defaultGrindParameter],
+        workingDirectory: sdkDartdoc);
+  } catch (e, st) {
+    print('Warning: SDK analyzer job threw "$e":\n$st');
+  }
 }
 
 Future<Iterable<Map<String, Object>>> _buildSdkDocs(
