Enable recommended lints when verifying diagnostic documentation

This enables the remainder of the recommended lints when verifying
diagnostic documentation. As before, in most cases this amounted to
ignoring additional diagnostics and documenting why I thought that was
a reasonable choice.

At some point someone should create issues for all of the places where
a diagnostic is being reported where it shouldn't be and add the URL
of the issue to these files.

Change-Id: Iadcad9f13334e3f1702a32273e190a04017796aa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/532000
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Auto-Submit: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>

Author: bwilkerson

pkg/analyzer/messages.yaml

@@ -1683,6 +1683,9 @@ CompileTimeErrorCode:
     parameters: none
     problemMessage: "Functions can't be assigned a value."
     hasPublishedDocs: true
+    # In the documentation, the `prefer_function_declarations_over_variables`
+    # diagnostic is ignored because fixing it would require adding code that
+    # isn't relevant to the point of the fix.
     documentation: |-
       #### Description
 
@@ -1708,6 +1711,7 @@ CompileTimeErrorCode:
       local variable, then change the left-hand side:
 
       ```dart
+      %ignore=prefer_function_declarations_over_variables
       void f() {}
 
       void g() {
@@ -4585,6 +4589,8 @@ CompileTimeErrorCode:
     correctionMessage: Try correcting the name to match an existing class.
     isUnresolvedIdentifier: true
     hasPublishedDocs: true
+    # In the documentation, the `unnecessary_new` diagnostic is ignored because
+    # the diagnostic being documented requires using `new`.
     documentation: |-
       #### Description
 
@@ -4597,6 +4603,7 @@ CompileTimeErrorCode:
       rather than a class:
 
       ```dart
+      %ignore=unnecessary_new
       int f() => 0;
 
       void g() {
@@ -4610,6 +4617,7 @@ CompileTimeErrorCode:
       of a valid class:
 
       ```dart
+      %ignore=unnecessary_new
       int f() => 0;
 
       void g() {
@@ -6633,6 +6641,10 @@ CompileTimeErrorCode:
     problemMessage: "Extension overrides have no value so they can't be used as the receiver of a cascade expression."
     correctionMessage: "Try using '.' instead of '..'."
     hasPublishedDocs: true
+    # In the documentation, the `avoid_single_cascade_in_expression_statements`
+    # diagnostic is being ignored because it shouldn't be produced here. They
+    # are both signalling the same problem, but the diagnostic being documented
+    # provides the better message.
     documentation: |-
       #### Description
 
@@ -6647,6 +6659,7 @@ CompileTimeErrorCode:
       expression:
 
       ```dart
+      %ignore=avoid_single_cascade_in_expression_statements
       extension E on int {
         void m() {}
       }
@@ -10167,6 +10180,10 @@ CompileTimeErrorCode:
     problemMessage: "Inline function types can't be used for parameters in a generic function type."
     correctionMessage: "Try using a generic function type (returnType 'Function(' parameters ')')."
     hasPublishedDocs: true
+    # In the documentation, the `use_function_type_syntax_for_parameters`
+    # diagnostic is being ignored because it shouldn't be produced here. Because
+    # it's an error to use a function typed parameter in a generic function
+    # type, there's no value in reporting both diagnostics.
     documentation: |-
       #### Description
 
@@ -10181,6 +10198,7 @@ CompileTimeErrorCode:
       type syntax:
 
       ```dart
+      %ignore=use_function_type_syntax_for_parameters
       typedef F = int Function(int f[!(!]String s));
       ```
 
@@ -10198,6 +10216,9 @@ CompileTimeErrorCode:
     problemMessage: "The modifier '#modifier' can't be applied to the body of a constructor."
     correctionMessage: Try removing the modifier.
     hasPublishedDocs: true
+    # In the documentation, the `empty_constructor_bodies` diagnostic is being
+    # ignored because it shouldn't be produced here. The constructor body can't
+    # be replaced by a semicolon because of the `async` keyword.
     documentation: |-
       #### Description
 
@@ -10211,6 +10232,7 @@ CompileTimeErrorCode:
       constructor for `C` is marked as being `async`:
 
       ```dart
+      %ignore=empty_constructor_bodies
       class C {
         C() [!async!] {}
       }
@@ -12094,38 +12116,39 @@ CompileTimeErrorCode:
     documentation: |-
       #### Description
 
-      The analyzer produces this diagnostic when a mixin class declares a non-trivial
-      generative constructor. Trivial constructors are allowed, but non-trivial ones
-      (those with parameters, bodies, or initializers) aren't.
+      The analyzer produces this diagnostic when a mixin class declares a
+      non-trivial generative constructor. Trivial constructors are allowed, but
+      non-trivial ones (those with parameters, bodies, or initializers) aren't.
 
       #### Example
 
       The following code produces this diagnostic because the `mixin class` `A`
-      defines a generative constructor with a body:
+      defines a generative constructor with an initializer:
 
       ```dart
       mixin class A {
-        [!A!]() {}
+        [!A!]() : assert(true);
       }
       ```
 
       #### Common fixes
 
-      If you can remove the body or parameters of the constructor to make it trivial,
-      then do so:
+      If you can remove the initialize, body, or parameters of the constructor
+      to make it trivial, then do so:
 
       ```dart
       mixin class A {
         A(); // Trivial constructor
       }
       ```
 
-      If the constructor can't be made trivial, then convert the `mixin class` to a
-      regular `class` and extend or implement it, or separate the mixin capability:
+      If the constructor can't be made trivial, then convert the `mixin class`
+      to a regular `class` and extend or implement it, or separate the mixin
+      capability:
 
       ```dart
       class A {
-        A();
+        A() : assert(true);
       }
 
       // A separate mixin for mixing in behavior
@@ -13850,6 +13873,10 @@ CompileTimeErrorCode:
     problemMessage: "The return type of the setter must be 'void' or absent."
     correctionMessage: Try removing the return type, or define a method rather than a setter.
     hasPublishedDocs: true
+    # In the documentation, the `avoid_return_types_on_setters` diagnostic is
+    # ignored because it shouldn't be reported here. If the return type is
+    # anything other than `void`, then both will be reported. The lint should
+    # only report for a return type of `void`.
     documentation: |-
       #### Description
 
@@ -13862,6 +13889,7 @@ CompileTimeErrorCode:
       return type of `int`:
 
       ```dart
+      %ignore=avoid_return_types_on_setters
       class C {
         [!int!] set p(int i) => 0;
       }
@@ -15281,6 +15309,9 @@ CompileTimeErrorCode:
     problemMessage: Positional super parameters can't be used when the super constructor invocation has a positional argument.
     correctionMessage: Try making all the positional parameters passed to the super constructor be either all super parameters or all normal parameters.
     hasPublishedDocs: true
+    # In the documentation, the `use_super_parameters` diagnostic is ignored
+    # because the only way to resolve the diagnostic would also resolve the
+    # diagnostic being documented.
     documentation: |-
       #### Description
 
@@ -15304,6 +15335,7 @@ CompileTimeErrorCode:
       other in the super constructor invocation:
 
       ```dart
+      %ignore=use_super_parameters
       class A {
         A(int x, int y);
       }
@@ -27997,6 +28029,9 @@ WarningCode:
       String name: the name of the member
     problemMessage: "The member '#name' can only be used within its package."
     hasPublishedDocs: true
+    # In the documentation, the `implementation_imports` diagnostic is being
+    # ignored because it's a necessary pre-condition to referencing an internal
+    # member.
     documentation: |-
       #### Description
 
@@ -28021,6 +28056,7 @@ WarningCode:
       class `C`, which isn't intended to be used outside the package `p`:
 
       ```dart
+      %ignore=implementation_imports
       import 'package:p/src/p.dart';
 
       void f([!C!] c) {}

pkg/analyzer/test/verify_diagnostics_test.dart

@@ -774,18 +774,18 @@ class _SnippetTest extends PubPackageResolutionTest {
 
     // The lints from the 'recommended' lint set.
     LintNames.annotate_overrides,
-    // LintNames.avoid_function_literals_in_foreach_calls,
+    LintNames.avoid_function_literals_in_foreach_calls,
     LintNames.avoid_init_to_null,
     LintNames.avoid_renaming_method_parameters,
-    // LintNames.avoid_return_types_on_setters,
+    LintNames.avoid_return_types_on_setters,
     LintNames.avoid_returning_null_for_void,
-    // LintNames.avoid_single_cascade_in_expression_statements,
+    LintNames.avoid_single_cascade_in_expression_statements,
     LintNames.constant_identifier_names,
     LintNames.control_flow_in_finally,
-    // LintNames.empty_constructor_bodies,
-    // LintNames.empty_statements,
+    LintNames.empty_constructor_bodies,
+    LintNames.empty_statements,
     LintNames.exhaustive_cases,
-    // LintNames.implementation_imports,
+    LintNames.implementation_imports,
     LintNames.invalid_runtime_check_with_js_interop_types,
     LintNames.library_prefixes,
     LintNames.library_private_types_in_public_api,
@@ -794,15 +794,15 @@ class _SnippetTest extends PubPackageResolutionTest {
     LintNames.null_closures,
     LintNames.overridden_fields,
     LintNames.package_names,
-    // LintNames.prefer_adjacent_string_concatenation,
+    LintNames.prefer_adjacent_string_concatenation,
     LintNames.prefer_collection_literals,
     LintNames.prefer_conditional_assignment,
     LintNames.prefer_contains,
     LintNames.prefer_final_fields,
     LintNames.prefer_for_elements_to_map_fromiterable,
-    // LintNames.prefer_function_declarations_over_variables,
+    LintNames.prefer_function_declarations_over_variables,
     LintNames.prefer_if_null_operators,
-    // LintNames.prefer_initializing_formals,
+    LintNames.prefer_initializing_formals,
     LintNames.prefer_inlined_adds,
     LintNames.prefer_interpolation_to_compose_strings,
     LintNames.prefer_is_not_operator,
@@ -816,8 +816,8 @@ class _SnippetTest extends PubPackageResolutionTest {
     LintNames.unnecessary_constructor_name,
     LintNames.unnecessary_getters_setters,
     LintNames.unnecessary_late,
-    // LintNames.unnecessary_library_name,
-    // LintNames.unnecessary_new,
+    LintNames.unnecessary_library_name,
+    LintNames.unnecessary_new,
     LintNames.unnecessary_null_aware_assignments,
     LintNames.unnecessary_null_in_if_null_operators,
     LintNames.unnecessary_nullable_for_final_variable_declarations,
@@ -826,10 +826,10 @@ class _SnippetTest extends PubPackageResolutionTest {
     LintNames.unnecessary_this,
     LintNames.unnecessary_to_list_in_spreads,
     LintNames.unnecessary_underscores,
-    // LintNames.use_function_type_syntax_for_parameters,
+    LintNames.use_function_type_syntax_for_parameters,
     LintNames.use_null_aware_elements,
     LintNames.use_rethrow_when_possible,
-    // LintNames.use_super_parameters,
+    LintNames.use_super_parameters,
 
     // The lints from the 'flutter' lint set.
     // LintNames.avoid_print,

pkg/linter/messages.yaml

@@ -1444,9 +1444,9 @@ LinterLintCode:
       stable: "2.0"
     categories: [brevity, errorProne]
     hasPublishedDocs: true
-    # In the documentation, the `curly_braces_in_flow_control_structures`
-    # diagnostic is being ignored because the lint shouldn't be reported when
-    # there's already an error being reported.
+    # In the documentation, the `curly_braces_in_flow_control_structures` and
+    # `empty_statements` diagnostics are being ignored because they can't be
+    # fixed without also fixing the diagnostic being documented.
     documentation: |-
       #### Description
 
@@ -1462,7 +1462,7 @@ LinterLintCode:
       following the `else` is an empty statement:
 
       ```dart
-      %ignore=curly_braces_in_flow_control_structures
+      %ignore=curly_braces_in_flow_control_structures,empty_statements
       void f(int x, int y) {
         if (x > y) {
           print('1');
@@ -7021,6 +7021,9 @@ LinterLintCode:
       stable: "2.0"
     categories: [style]
     hasPublishedDocs: true
+    # In the documentation, the `unnecessary_library_name` diagnostic is ignored
+    # because the diagnostic being documented requires the existance of a
+    # library name.
     documentation: |-
       #### Description
 
@@ -7033,6 +7036,7 @@ LinterLintCode:
       `libraryName` isn't a lower_case_with_underscores identifier:
 
       ```dart
+      %ignore=unnecessary_library_name
       library [!libraryName!];
       ```
 
@@ -7048,6 +7052,7 @@ LinterLintCode:
       lower_case_with_underscores naming convention:
 
       ```dart
+      %ignore=unnecessary_library_name
       library library_name;
       ```
     deprecatedDetails: |-
@@ -7664,6 +7669,10 @@ LinterLintCode:
       stable: "2.0"
     categories: [style]
     hasPublishedDocs: true
+    # In the documentation, the `prefer_adjacent_string_concatenation`
+    # diagnostic is ignored because even though the use of `+` isn't a
+    # recommended solution, it is a valid way to avoid this lint. We should
+    # consider allowing the use of `+` in list literals.
     documentation: |-
       #### Description
 
@@ -7704,6 +7713,7 @@ LinterLintCode:
       Or use the `+` operator to concatenate the strings:
 
       ```dart
+      %ignore=prefer_adjacent_string_concatenation
       List<String> list = ['a' + 'b', 'c'];
       ```
     deprecatedDetails: |-
@@ -15048,6 +15058,11 @@ LinterLintCode:
       stable: "2.0"
     categories: [style]
     hasPublishedDocs: true
+    # In the documentation, the `avoid_function_literals_in_foreach_calls`
+    # diagnostic is ignored because it's a bi-product of the choice of example.
+    # The documentation is more clear if we use a familiar method that taked a
+    # function as a parameter, and `forEach` is a reasonable choice. We could
+    # remove the ignore by choosing a different method.
     documentation: |-
       #### Description
 
@@ -15061,6 +15076,7 @@ LinterLintCode:
       parameter of the closure:
 
       ```dart
+      %ignore=avoid_function_literals_in_foreach_calls
       void f(List<String> strings) {
         strings.forEach([!(string) {
           print(string);
@@ -17197,6 +17213,9 @@ LinterLintCode:
       stable: "3.13"
     categories: [style]
     hasPublishedDocs: true
+    # In the documentation, the `prefer_initializing_formals` diagnostic is
+    # ignored because not using an initializing formal shows a more dramatic
+    # improvement in the code.
     documentation: |-
       #### Description
 
@@ -17210,6 +17229,7 @@ LinterLintCode:
       assigns the parameter `i` to the field `i`:
 
       ```dart
+      %ignore=prefer_initializing_formals
       class C(int [!i!]) {
         final int i;