Merge branch 'modelType-refactor' into modelType-refactor+type-alias-referencing

jcollins-g · jcollins-g · commit 0b2f932d4a82 · 2021-03-18T10:25:00.000-07:00
diff --git a/README.md b/README.md
@@ -289,6 +289,7 @@ dartdoc:
       command: ["bin/drill.dart"]
       setup_command: ["bin/setup.dart"]
       description: "Puts holes in things."
+      compile_args: ["--no-sound-null-safety"]
     echo:
       macos: ['/bin/sh', '-c', 'echo']
       setup_macos: ['/bin/sh', '-c', 'setup.sh']
@@ -328,6 +329,9 @@ The `description` is just a short description of the tool for use as help text.
 Only tools which are configured in the `dartdoc_options.yaml` file are able to
 be invoked.
 
+The `compile_args` tag is used to pass options to the dart compiler when the
+first run of the tool is being snapshotted.
+
 To use the tools in comment documentation, use the `{@tool <name> [<options>
 ...] [$INPUT]}` directive to invoke the tool:
 
diff --git a/analysis_options.yaml b/analysis_options.yaml
@@ -18,6 +18,7 @@ analyzer:
 linter:
   rules:
     - always_declare_return_types
+    - avoid_dynamic_calls
     - avoid_single_cascade_in_expression_statements
     - avoid_unused_constructor_parameters
     - annotate_overrides
diff --git a/analysis_options_presubmit.yaml b/analysis_options_presubmit.yaml
@@ -21,6 +21,7 @@ analyzer:
 linter:
   rules:
     - always_declare_return_types
+    - avoid_dynamic_calls
     - avoid_single_cascade_in_expression_statements
     - avoid_unused_constructor_parameters
     - annotate_overrides
diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart
@@ -39,6 +39,8 @@ const int _kIntVal = 0;
 const double _kDoubleVal = 0.0;
 const bool _kBoolVal = true;
 
+const String _kCompileArgsTagName = 'compile_args';
+
 int get _usageLineLength => stdout.hasTerminal ? stdout.terminalColumns : null;
 
 typedef ConvertYamlToType<T> = T Function(YamlMap, String, ResourceProvider);
@@ -147,14 +149,22 @@ class ToolDefinition {
       List<String> command,
       List<String> setupCommand,
       String description,
-      ResourceProvider resourceProvider) {
+      ResourceProvider resourceProvider,
+      {List<String> compileArgs}) {
     assert(command != null);
     assert(command.isNotEmpty);
     assert(description != null);
     if (isDartExecutable(command[0])) {
       return DartToolDefinition(
-          command, setupCommand, description, resourceProvider);
+          command, setupCommand, description, resourceProvider,
+          compileArgs: compileArgs ?? const []);
     } else {
+      if (compileArgs != null && compileArgs.isNotEmpty) {
+        throw DartdocOptionError(
+            'Compile arguments may only be specified for Dart tools, but '
+            '$_kCompileArgsTagName of $compileArgs were specified for '
+            '$command.');
+      }
       return ToolDefinition(command, setupCommand, description);
     }
   }
@@ -274,6 +284,9 @@ class SnapshotCache {
 class DartToolDefinition extends ToolDefinition {
   final ResourceProvider _resourceProvider;
 
+  /// A list of arguments to add to the snapshot compilation arguments.
+  final List<String> compileArgs;
+
   /// Takes a list of args to modify, and returns the name of the executable
   /// to run. If no snapshot file existed, then create one and modify the args
   /// so that if they are executed with dart, will result in the snapshot being
@@ -295,7 +308,8 @@ class DartToolDefinition extends ToolDefinition {
         '--ignore-unrecognized-flags',
         '--verbosity=error',
         '--snapshot=${_resourceProvider.pathContext.absolute(snapshotFile.path)}',
-        '--snapshot_kind=app-jit'
+        '--snapshot_kind=app-jit',
+        ...compileArgs,
       ]);
     } else {
       await snapshot.snapshotValid();
@@ -307,8 +321,10 @@ class DartToolDefinition extends ToolDefinition {
   }
 
   DartToolDefinition(List<String> command, List<String> setupCommand,
-      String description, this._resourceProvider)
-      : super(command, setupCommand, description);
+      String description, this._resourceProvider,
+      {this.compileArgs = const []})
+      : assert(compileArgs != null),
+        super(command, setupCommand, description);
 }
 
 /// A configuration class that can interpret [ToolDefinition]s from a YAML map.
@@ -335,6 +351,7 @@ class ToolConfiguration {
     for (var entry in yamlMap.entries) {
       var name = entry.key.toString();
       var toolMap = entry.value;
+      List<String> compileArgs;
       String description;
       List<String> command;
       List<String> setupCommand;
@@ -343,39 +360,60 @@ class ToolConfiguration {
         List<String> findCommand([String prefix = '']) {
           List<String> command;
           // If the command key is given, then it applies to all platforms.
-          var commandFrom = toolMap.containsKey('${prefix}command')
+          var commandFromKey = toolMap.containsKey('${prefix}command')
               ? '${prefix}command'
               : '$prefix${Platform.operatingSystem}';
-          if (toolMap.containsKey(commandFrom)) {
-            if (toolMap[commandFrom].value is String) {
-              command = [toolMap[commandFrom].toString()];
+          if (toolMap.containsKey(commandFromKey)) {
+            var commandFrom = toolMap[commandFromKey] as YamlNode;
+            if (commandFrom.value is String) {
+              command = [commandFrom.toString()];
               if (command[0].isEmpty) {
                 throw DartdocOptionError(
                     'Tool commands must not be empty. Tool $name command entry '
-                    '"$commandFrom" must contain at least one path.');
+                    '"$commandFromKey" must contain at least one path.');
               }
-            } else if (toolMap[commandFrom] is YamlList) {
-              command = (toolMap[commandFrom] as YamlList)
-                  .map<String>((node) => node.toString())
-                  .toList();
+            } else if (commandFrom is YamlList) {
+              command =
+                  commandFrom.map<String>((node) => node.toString()).toList();
               if (command.isEmpty) {
                 throw DartdocOptionError(
                     'Tool commands must not be empty. Tool $name command entry '
-                    '"$commandFrom" must contain at least one path.');
+                    '"$commandFromKey" must contain at least one path.');
               }
             } else {
               throw DartdocOptionError(
                   'Tool commands must be a path to an executable, or a list of '
                   'strings that starts with a path to an executable. '
-                  'The tool $name has a $commandFrom entry that is a '
-                  '${toolMap[commandFrom].runtimeType}');
+                  'The tool $name has a $commandFromKey entry that is a '
+                  '${commandFrom.runtimeType}');
             }
           }
           return command;
         }
 
         command = findCommand();
         setupCommand = findCommand('setup_');
+
+        List<String> findArgs() {
+          List<String> args;
+          if (toolMap.containsKey(_kCompileArgsTagName)) {
+            var compileArgs = toolMap[_kCompileArgsTagName];
+            if (compileArgs is String) {
+              args = [toolMap[_kCompileArgsTagName].toString()];
+            } else if (compileArgs is YamlList) {
+              args =
+                  compileArgs.map<String>((node) => node.toString()).toList();
+            } else {
+              throw DartdocOptionError(
+                  'Tool compile arguments must be a list of strings. The tool '
+                  '$name has a $_kCompileArgsTagName entry that is a '
+                  '${compileArgs.runtimeType}');
+            }
+          }
+          return args;
+        }
+
+        compileArgs = findArgs();
       } else {
         throw DartdocOptionError(
             'Tools must be defined as a map of tool names to definitions. Tool '
@@ -421,7 +459,8 @@ class ToolConfiguration {
             setupCommand;
       }
       newToolDefinitions[name] = ToolDefinition.fromCommand(
-          [executable] + command, setupCommand, description, resourceProvider);
+          [executable] + command, setupCommand, description, resourceProvider,
+          compileArgs: compileArgs ?? const []);
     }
     return ToolConfiguration._(newToolDefinitions, resourceProvider);
   }
@@ -598,16 +637,17 @@ abstract class DartdocOption<T> {
       _OptionValueWithContext<Object> valueWithContext, String missingFilename);
 
   /// Call [_onMissing] for every path that does not exist.
-  void _validatePaths(_OptionValueWithContext<dynamic> valueWithContext) {
+  void _validatePaths(_OptionValueWithContext<Object> valueWithContext) {
     if (!mustExist) return;
     assert(isDir || isFile);
     List<String> resolvedPaths;
-    if (valueWithContext.value is String) {
+    var value = valueWithContext.value;
+    if (value is String) {
       resolvedPaths = [valueWithContext.resolvedValue];
-    } else if (valueWithContext.value is List<String>) {
-      resolvedPaths = valueWithContext.resolvedValue.toList();
-    } else if (valueWithContext.value is Map<String, String>) {
-      resolvedPaths = valueWithContext.resolvedValue.values.toList();
+    } else if (value is List<String>) {
+      resolvedPaths = valueWithContext.resolvedValue as List;
+    } else if (value is Map<String, String>) {
+      resolvedPaths = (valueWithContext.resolvedValue as Map).values.toList();
     } else {
       assert(
           false,
@@ -1120,19 +1160,18 @@ abstract class _DartdocFileOption<T> implements DartdocOption<T> {
   _OptionValueWithContext<Object> _valueAtFromFile(Folder dir) {
     var yamlFileData = _yamlAtDirectory(dir);
     var contextPath = yamlFileData.canonicalDirectoryPath;
-    dynamic yamlData = yamlFileData.data ?? {};
+    Object yamlData = yamlFileData.data ?? {};
     for (var key in keys) {
-      if (!yamlData.containsKey(key)) return null;
-      yamlData = yamlData[key] ?? {};
+      if (yamlData is Map && !yamlData.containsKey(key)) return null;
+      yamlData = (yamlData as Map)[key] ?? {};
     }
 
-    dynamic returnData;
+    Object returnData;
     if (_isListString) {
       if (yamlData is YamlList) {
-        returnData = <String>[];
-        for (var item in yamlData) {
-          returnData.add(item.toString());
-        }
+        returnData = [
+          for (var item in yamlData) item.toString(),
+        ];
       }
     } else if (yamlData is YamlMap) {
       // TODO(jcollins-g): This special casing is unfortunate.  Consider
@@ -1700,7 +1739,7 @@ Future<List<DartdocOption<Object>>> createDartdocOptions(
         help: 'Generate ONLY the docs for the Dart SDK.'),
     DartdocOptionArgSynth<String>('sdkDir',
         (DartdocSyntheticOption<String> option, Folder dir) {
-      if (!option.parent['sdkDocs'].valueAt(dir) &&
+      if (!(option.parent['sdkDocs'].valueAt(dir) as bool) &&
           (option.root['topLevelPackageMeta'].valueAt(dir) as PackageMeta)
               .requiresFlutter) {
         String flutterRoot = option.root['flutterRoot'].valueAt(dir);
diff --git a/lib/src/model/accessor.dart b/lib/src/model/accessor.dart
@@ -26,17 +26,8 @@ class Accessor extends ModelElement implements EnclosedElement {
   ExecutableMember get originalMember => super.originalMember;
 
   CallableElementTypeMixin _modelType;
-  CallableElementTypeMixin get modelType {
-    if (_modelType == null) {
-      if (originalMember != null) {
-        _modelType =
-            ElementType.from(originalMember.type, library, packageGraph);
-      } else {
-        _modelType = ElementType.from(element.type, library, packageGraph);
-      }
-    }
-    return _modelType;
-  }
+  CallableElementTypeMixin get modelType => _modelType ??=
+      ElementType.from((originalMember ?? element).type, library, packageGraph);
 
   bool get isSynthetic => element.isSynthetic;
 
diff --git a/lib/src/model/documentation_comment.dart b/lib/src/model/documentation_comment.dart
@@ -191,11 +191,14 @@ mixin DocumentationComment
   /// ```yaml
   /// dartdoc:
   ///   tools:
-  ///     # Prefixes the given input with "## "
-  ///     # Path is relative to project root.
-  ///     prefix: "bin/prefix.dart"
-  ///     # Prints the date
-  ///     date: "/bin/date"
+  ///     prefix:
+  ///       # Path is relative to project root.
+  ///       command: ["bin/prefix.dart"]
+  ///       description: "Prefixes the given input with '##'."
+  ///       compile_args: ["--no-sound-null-safety"]
+  ///     date:
+  ///       command: ["/bin/date"]
+  ///       description: "Prints the date"
   /// ```
   ///
   /// In code:
diff --git a/lib/src/model/method.dart b/lib/src/model/method.dart
@@ -97,17 +97,8 @@ class Method extends ModelElement
   ExecutableMember get originalMember => super.originalMember;
 
   CallableElementTypeMixin _modelType;
-  CallableElementTypeMixin get modelType {
-    if (_modelType == null) {
-      if (originalMember != null) {
-        _modelType =
-            ElementType.from(originalMember.type, library, packageGraph);
-      } else {
-        _modelType = ElementType.from(element.type, library, packageGraph);
-      }
-    }
-    return _modelType;
-  }
+  CallableElementTypeMixin get modelType => _modelType ??=
+      ElementType.from((originalMember ?? element).type, library, packageGraph);
 
   @override
   Method get overriddenElement {
diff --git a/lib/src/model/model_element.dart b/lib/src/model/model_element.dart
@@ -1056,7 +1056,7 @@ abstract class ModelElement extends Canonicalization
         }
         if (params == null && element is FunctionTypedElement) {
           if (_originalMember != null) {
-            params = (_originalMember as dynamic).parameters;
+            params = (_originalMember as FunctionTypedElement).parameters;
           } else {
             params = (element as FunctionTypedElement).parameters;
           }
diff --git a/lib/src/package_meta.dart b/lib/src/package_meta.dart
@@ -379,8 +379,12 @@ class _FilePackageMeta extends PubPackageMeta {
 
   @override
   bool get requiresFlutter =>
-      _pubspec['environment']?.containsKey('flutter') == true ||
-      _pubspec['dependencies']?.containsKey('flutter') == true;
+      _environment?.containsKey('flutter') == true ||
+      _dependencies?.containsKey('flutter') == true;
+
+  YamlMap /*?*/ get _environment => _pubspec['environment'];
+
+  YamlMap /*?*/ get _dependencies => _pubspec['environment'];
 
   @override
   File getReadmeContents() =>
diff --git a/test/dartdoc_options_test.dart b/test/dartdoc_options_test.dart
@@ -63,7 +63,7 @@ void main() {
     dartdocOptionSetSynthetic.add(
         DartdocOptionSyntheticOnly<List<String>>('vegetableLoader',
             (DartdocSyntheticOption<List<String>> option, Folder dir) {
-      if (option.root['mySpecialInteger'].valueAt(dir) > 20) {
+      if ((option.root['mySpecialInteger'].valueAt(dir) as num) > 20) {
         return <String>['existing.dart'];
       } else {
         return <String>['not_existing.dart'];
@@ -81,7 +81,7 @@ void main() {
     dartdocOptionSetSynthetic.add(
         DartdocOptionArgSynth<String>('nonCriticalFileOption',
             (DartdocSyntheticOption<String> option, Folder dir) {
-      return option.root['vegetableLoader'].valueAt(dir).first;
+      return (option.root['vegetableLoader'].valueAt(dir) as List).first;
     }, resourceProvider, optionIs: OptionKind.file));
 
     dartdocOptionSetFiles = DartdocOptionSet('dartdoc', resourceProvider);
@@ -637,13 +637,13 @@ dartdoc:
           equals(
               'Field dartdoc.fileOptionList from ${path.canonicalize(dartdocOptionsTwo.path)}, set to [existing.dart, thing/that/does/not/exist], resolves to missing path: '
               '"${path.joinAll([
-            path.canonicalize(secondDir.path),
-            'thing',
-            'that',
-            'does',
-            'not',
-            'exist'
-          ])}"'));
+                path.canonicalize(secondDir.path),
+                'thing',
+                'that',
+                'does',
+                'not',
+                'exist'
+              ])}"'));
       // It doesn't matter that this fails.
       expect(dartdocOptionSetFiles['nonCriticalFileOption'].valueAt(firstDir),
           equals(path.joinAll([path.canonicalize(firstDir.path), 'whatever'])));
@@ -663,9 +663,9 @@ dartdoc:
           equals(
               'Field dartdoc.fileOption from ${path.canonicalize(dartdocOptionsTwo.path)}, set to not existing, resolves to missing path: '
               '"${path.joinAll([
-            path.canonicalize(secondDir.path),
-            "not existing"
-          ])}"'));
+                path.canonicalize(secondDir.path),
+                "not existing"
+              ])}"'));
     });
 
     test('DartdocOptionSetFile works for directory options', () {
diff --git a/test/end2end/dartdoc_integration_test.dart b/test/end2end/dartdoc_integration_test.dart
@@ -107,7 +107,7 @@ void main() {
     });
 
     test('missing a required file path prints a fatal-error', () async {
-      var outputLines = [];
+      var outputLines = <String>[];
       var impossiblePath = path.join(dartdocPath, 'impossible');
       await expectLater(
           () => subprocessLauncher.runStreamed(
diff --git a/test/tool_runner_test.dart b/test/tool_runner_test.dart
diff --git a/tool/doc_packages.dart b/tool/doc_packages.dart
diff --git a/tool/grind.dart b/tool/grind.dart
diff --git a/tool/subprocess_launcher.dart b/tool/subprocess_launcher.dart

Original file line number	Diff line number	Diff line change
`@@ -1056,7 +1056,7 @@ abstract class ModelElement extends Canonicalization`
`1056`	`1056`	`}`
`1057`	`1057`	`if (params == null && element is FunctionTypedElement) {`
`1058`	`1058`	`if (_originalMember != null) {`
`1059`		`- params = (_originalMember as dynamic).parameters;`
	`1059`	`+ params = (_originalMember as FunctionTypedElement).parameters;`
`1060`	`1060`	`} else {`
`1061`	`1061`	`params = (element as FunctionTypedElement).parameters;`
`1062`	`1062`	`}`