Get rid of a backlog of potentially suboptimal coding practices in dartdoc (#2830)

* switch to lints/core.yaml

* recommended

* presubmit cleanup

* oops, recommended on presubmit too

Author: jcollins-g

analysis_options.yaml

@@ -1,6 +1,6 @@
 # Change analysis_options.yaml and analysis_options_presubmit.yaml
 # together.
-include: package:pedantic/analysis_options.1.11.0.yaml
+include: package:lints/recommended.yaml
 
 analyzer:
   errors:
@@ -19,45 +19,19 @@ analyzer:
     - 'testing/test_package_export_error/**'
 linter:
   rules:
-    - always_declare_return_types
-    - avoid_dynamic_calls
-    - avoid_single_cascade_in_expression_statements
-    - avoid_unused_constructor_parameters
-    - annotate_overrides
-    - avoid_init_to_null
-    - directives_ordering
-    - no_adjacent_strings_in_list
-    - package_api_docs
-    - prefer_final_fields
-    - prefer_initializing_formals
-    - prefer_void_to_null
-    - slash_for_doc_comments
-    - type_annotate_public_apis
-    #    - unnecessary_brace_in_string_interps
+    always_declare_return_types: true
+    annotate_overrides: true
+    avoid_dynamic_calls: true
+    avoid_single_cascade_in_expression_statements: true
+    avoid_unused_constructor_parameters: true
+    avoid_init_to_null: true
+    directives_ordering: true
+    no_adjacent_strings_in_list: true
+    package_api_docs: true
+    prefer_final_fields: true
+    prefer_initializing_formals: true
+    prefer_void_to_null: true
+    slash_for_doc_comments: true
+    type_annotate_public_apis: true
     # Work in progress canonical score lints
-    - avoid_empty_else
-    - avoid_relative_lib_imports
-    - avoid_shadowing_type_parameters
-    - await_only_futures
-    - camel_case_extensions
-    - camel_case_types
-    - curly_braces_in_flow_control_structures
-    - empty_catches
-    - file_names
-    - hash_and_equals
-    - iterable_contains_unrelated_type
-    - list_remove_unrelated_type
-    - no_duplicate_case_values
-    #    - non_constant_identifier_names
-    - package_prefixed_library_names
-    - prefer_generic_function_type_aliases
-    - prefer_is_empty
-    - prefer_is_not_empty
-    - prefer_iterable_whereType
-    - prefer_typing_uninitialized_variables
-    - provide_deprecation_message
-    - unawaited_futures
-    - unnecessary_overrides
-    - unrelated_type_equality_checks
-    - valid_regexps
-    - void_checks
+    unawaited_futures: true

analysis_options_presubmit.yaml

@@ -1,6 +1,6 @@
 # Change analysis_options.yaml and analysis_options_presubmit.yaml
 # together.
-include: package:pedantic/analysis_options.1.11.0.yaml
+include: package:lints/recommended.yaml
 
 analyzer:
   errors:
@@ -22,45 +22,19 @@ analyzer:
     - 'testing/test_package_export_error/**'
 linter:
   rules:
-    - always_declare_return_types
-    - avoid_dynamic_calls
-    - avoid_single_cascade_in_expression_statements
-    - avoid_unused_constructor_parameters
-    - annotate_overrides
-    - avoid_init_to_null
-    - directives_ordering
-    - no_adjacent_strings_in_list
-    - package_api_docs
-    - prefer_final_fields
-    - prefer_initializing_formals
-    - prefer_void_to_null
-    - slash_for_doc_comments
-    - type_annotate_public_apis
-    #    - unnecessary_brace_in_string_interps
+    always_declare_return_types: true
+    annotate_overrides: true
+    avoid_dynamic_calls: true
+    avoid_single_cascade_in_expression_statements: true
+    avoid_unused_constructor_parameters: true
+    avoid_init_to_null: true
+    directives_ordering: true
+    no_adjacent_strings_in_list: true
+    package_api_docs: true
+    prefer_final_fields: true
+    prefer_initializing_formals: true
+    prefer_void_to_null: true
+    slash_for_doc_comments: true
+    type_annotate_public_apis: true
     # Work in progress canonical score lints
-    - avoid_empty_else
-    - avoid_relative_lib_imports
-    - avoid_shadowing_type_parameters
-    - await_only_futures
-    - camel_case_extensions
-    - camel_case_types
-    - curly_braces_in_flow_control_structures
-    - empty_catches
-    - file_names
-    - hash_and_equals
-    - iterable_contains_unrelated_type
-    - list_remove_unrelated_type
-    - no_duplicate_case_values
-    #    - non_constant_identifier_names
-    - package_prefixed_library_names
-    - prefer_generic_function_type_aliases
-    - prefer_is_empty
-    - prefer_is_not_empty
-    - prefer_iterable_whereType
-    - prefer_typing_uninitialized_variables
-    - provide_deprecation_message
-    - unawaited_futures
-    - unnecessary_overrides
-    - unrelated_type_equality_checks
-    - valid_regexps
-    - void_checks
+    unawaited_futures: true

lib/dartdoc.dart

@@ -370,7 +370,7 @@ class Dartdoc {
     var indexPath = path.joinAll([origin, 'index.html']);
     var file = config.resourceProvider.getFile(fullPath);
     if (!file.exists) {
-      return null;
+      return;
     }
     var decoder = JsonDecoder();
     List<Object> jsonData = decoder.convert(file.readAsStringSync());
@@ -415,7 +415,7 @@ class Dartdoc {
       // Remove so that we properly count that the file doesn't exist for
       // the orphan check.
       visited.remove(fullPath);
-      return null;
+      return;
     }
     visited.add(fullPath);
     var stringLinks = stringLinksAndHref.item1;

lib/src/comment_references/parser.dart

@@ -131,13 +131,12 @@ class CommentReferenceParser {
         var typeVariablesResult = _parseTypeVariables();
         if (typeVariablesResult.type == _TypeVariablesResultType.endOfFile) {
           break;
-        } else if (typeVariablesResult.type ==
-            _TypeVariablesResultType.notTypeVariables) {
-          // Do nothing, _index has not moved.
-          ;
         } else if (typeVariablesResult.type ==
             _TypeVariablesResultType.parsedTypeVariables) {
           children.add(typeVariablesResult.node);
+        } else {
+          assert(typeVariablesResult.type ==
+              _TypeVariablesResultType.notTypeVariables);
         }
       }
       if (_atEnd || _thisChar != $dot) {

lib/src/dartdoc_options.dart

@@ -556,7 +556,9 @@ abstract class DartdocOption<T extends Object> {
   /// Apply the function [visit] to [this] and all children.
   void traverse(void Function(DartdocOption option) visit) {
     visit(this);
-    _children.values.forEach((d) => d.traverse(visit));
+    for (var value in _children.values) {
+      value.traverse(visit);
+    }
   }
 }
 
@@ -731,7 +733,7 @@ class DartdocOptionSet extends DartdocOption<void> {
 
   /// [DartdocOptionSet] always has the null value.
   @override
-  void valueAt(Folder dir) => null;
+  void valueAt(Folder dir) {}
 
   /// Since we have no value, [_onMissing] does nothing.
   @override
@@ -742,7 +744,9 @@ class DartdocOptionSet extends DartdocOption<void> {
   /// configuration object.
   @override
   void traverse(void Function(DartdocOption option) visitor) {
-    _children.values.forEach((d) => d.traverse(visitor));
+    for (var value in _children.values) {
+      value.traverse(visitor);
+    }
   }
 }
 
@@ -1033,7 +1037,9 @@ abstract class _DartdocFileOption<T> implements DartdocOption<T> {
         }
       }
     }
-    canonicalPaths.forEach((p) => _yamlAtCanonicalPathCache[p] = yamlData);
+    for (var canonicalPath in canonicalPaths) {
+      _yamlAtCanonicalPathCache[canonicalPath] = yamlData;
+    }
     return yamlData;
   }
 }
@@ -1137,7 +1143,7 @@ abstract class _DartdocArgOption<T> implements DartdocOption<T> {
   /// 'something-bar-over-the-hill' (with default skip).
   /// This allows argument names to reflect nested structure.
   static String _keysToArgName(Iterable<String> keys, [int skip = 1]) {
-    var argName = "${keys.skip(skip).join('-')}";
+    var argName = keys.skip(skip).join('-');
     argName = argName.replaceAll('_', '-');
     // Do not consume the lowercase character after the uppercase one, to handle
     // two character words.

lib/src/experiment_options.dart

@@ -9,6 +9,7 @@
 library dartdoc.experiment_options;
 
 import 'package:analyzer/file_system/file_system.dart';
+// ignore: implementation_imports
 import 'package:analyzer/src/dart/analysis/experiments.dart';
 import 'package:dartdoc/src/dartdoc_options.dart';
 

lib/src/generator/empty_generator.dart

@@ -11,10 +11,10 @@ import 'package:dartdoc/src/model_utils.dart';
 /// it were.
 class EmptyGenerator extends Generator {
   @override
-  Future<void> generate(PackageGraph _packageGraph, FileWriter writer) {
-    logProgress(_packageGraph.defaultPackage.documentationAsHtml);
-    for (var package in {_packageGraph.defaultPackage}
-      ..addAll(_packageGraph.localPackages)) {
+  Future<void> generate(PackageGraph packageGraph, FileWriter writer) {
+    logProgress(packageGraph.defaultPackage.documentationAsHtml);
+    for (var package in {packageGraph.defaultPackage}
+      ..addAll(packageGraph.localPackages)) {
       for (var category in filterNonDocumented(package.categories)) {
         logProgress(category.documentationAsHtml);
       }

lib/src/generator/html_generator.dart

@@ -59,7 +59,7 @@ class HtmlGeneratorBackend extends DartdocGeneratorBackend {
   }
 
   Future<void> _copyResources(FileWriter writer) async {
-    for (var resourcePath in resources.resource_names) {
+    for (var resourcePath in resources.resourceNames) {
       if (!resourcePath.startsWith(_dartdocResourcePrefix)) {
         throw StateError('Resource paths must start with '
             '$_dartdocResourcePrefix, encountered $resourcePath');

lib/src/generator/html_resources.g.dart

@@ -1,6 +1,6 @@
-// WARNING: This file is auto-generated. Do not taunt.
+// WARNING: This file is auto-generated. Do not edit.
 
-const List<String> resource_names = [
+const List<String> resourceNames = [
   'package:dartdoc/resources/favicon.png',
   'package:dartdoc/resources/github.css',
   'package:dartdoc/resources/highlight.pack.js',

lib/src/generator/templates.aot_renderers_for_html.dart

@@ -7,6 +7,7 @@
 // the variable is not used; generally when the section is checking if a
 // non-bool, non-Iterable field is non-null.
 // ignore_for_file: unused_local_variable
+// ignore_for_file: non_constant_identifier_names, unnecessary_string_escapes
 
 import 'dart:convert' as _i19;
 

lib/src/generator/templates.aot_renderers_for_md.dart

@@ -7,6 +7,7 @@
 // the variable is not used; generally when the section is checking if a
 // non-bool, non-Iterable field is non-null.
 // ignore_for_file: unused_local_variable
+// ignore_for_file: non_constant_identifier_names, unnecessary_string_escapes
 
 import 'dart:convert' as _i19;
 

lib/src/generator/templates.runtime_renderers.dart

@@ -4,6 +4,7 @@
 // files in the tool/mustachio/ directory.
 
 // ignore_for_file: camel_case_types, deprecated_member_use_from_same_package
+// ignore_for_file: non_constant_identifier_names, unnecessary_string_escapes
 // ignore_for_file: unused_import
 import 'package:dartdoc/src/element_type.dart';
 import 'package:dartdoc/src/generator/template_data.dart';

lib/src/io_utils.dart

@@ -12,8 +12,10 @@ import 'dart:io' as io;
 
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/file_system/physical_file_system.dart';
-import 'package:analyzer/src/generated/sdk.dart';
-import 'package:analyzer/src/test_utilities/mock_sdk.dart';
+// ignore: implementation_imports
+import 'package:analyzer/src/generated/sdk.dart' show SdkLibrary;
+// ignore: implementation_imports
+import 'package:analyzer/src/test_utilities/mock_sdk.dart' show MockSdkLibrary;
 import 'package:path/path.dart' as path show Context;
 
 Encoding utf8AllowMalformed = Utf8Codec(allowMalformed: true);

lib/src/logging.dart

@@ -117,7 +117,7 @@ void startLogging(LoggingContext config) {
           // the backspace to occur for stderr as well.
           stderr.write('${ansi.backspace} ${ansi.backspace}');
         }
-        stderr.writeln('$message');
+        stderr.writeln(message);
       }
       writingProgress = false;
     });

lib/src/markdown_processor.dart

@@ -284,9 +284,9 @@ void showWarningsForGenericsOutsideSquareBracketsBlocks(
       PackageWarningMode.ignore) {
     for (var position in findFreeHangingGenericsPositions(text)) {
       var priorContext =
-          '${text.substring(max(position - maxPriorContext, 0), position)}';
+          text.substring(max(position - maxPriorContext, 0), position);
       var postContext =
-          '${text.substring(position, min(position + maxPostContext, text.length))}';
+          text.substring(position, min(position + maxPostContext, text.length));
       priorContext = priorContext.replaceAll(allBeforeFirstNewline, '');
       postContext = postContext.replaceAll(allAfterLastNewline, '');
       var errorMessage = '$priorContext$postContext';

lib/src/model/accessor.dart

@@ -4,6 +4,7 @@
 
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/source/line_info.dart';
+// ignore: implementation_imports
 import 'package:analyzer/src/dart/element/member.dart' show ExecutableMember;
 import 'package:dartdoc/src/element_type.dart';
 import 'package:dartdoc/src/model/comment_referable.dart';

lib/src/model/comment_referable.dart

@@ -70,10 +70,11 @@ extension CommentReferableEntryGenerators on Iterable<CommentReferable> {
 extension CommentReferableEntryBuilder on Map<String, CommentReferable> {
   /// Like [Map.putIfAbsent] except works on an iterable of entries.
   void addEntriesIfAbsent(
-          Iterable<MapEntry<String, CommentReferable>> entries) =>
-      entries.forEach((e) {
-        if (!containsKey(e.key)) this[e.key] = e.value;
-      });
+      Iterable<MapEntry<String, CommentReferable>> entries) {
+    for (var entry in entries) {
+      if (!containsKey(entry.key)) this[entry.key] = entry.value;
+    }
+  }
 }
 
 /// Support comment reference lookups on a Nameable object.

lib/src/model/documentation_comment.dart

@@ -393,7 +393,7 @@ mixin DocumentationComment
 
   /// Matches YouTube IDs from supported YouTube URLs.
   static final _validYouTubeUrlPattern =
-      RegExp('https://www\.youtube\.com/watch\\?v=([^&]+)\$');
+      RegExp(r'https://www\.youtube\.com/watch\?v=([^&]+)$');
 
   /// An argument parser used in [_injectYouTube] to parse a `{@youtube}`
   /// directive.
@@ -736,14 +736,14 @@ mixin DocumentationComment
         firstOfPair.add(results[i]);
       }
     }
-    firstOfPair.forEach((element) {
+    for (var element in firstOfPair) {
       final result = element.group(2).trim();
       if (result.isEmpty) {
         warn(PackageWarning.missingCodeBlockLanguage,
             message:
                 'A fenced code block in Markdown should have a language specified');
       }
-    });
+    }
   }
 
   /// Returns the documentation for this literal element unless

lib/src/model/field.dart

@@ -3,7 +3,6 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analyzer/dart/element/element.dart';
-import 'package:analyzer/src/dart/element/element.dart';
 import 'package:dartdoc/src/model/feature.dart';
 import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/render/source_code_renderer.dart';
@@ -82,8 +81,7 @@ class Field extends ModelElement
   /// Returns true if the FieldElement is covariant, or if the first parameter
   /// for the setter is covariant.
   @override
-  bool get isCovariant =>
-      setter?.isCovariant == true || (field as FieldElementImpl).isCovariant;
+  bool get isCovariant => setter?.isCovariant == true || field.isCovariant;
 
   @override
   bool get isFinal {