linter: Add a PRESUBMIT check for example/all.yaml

Work towards #53578

Change-Id: Ia07d999abc2fcf4b8195c9f7688799bc099a1d88
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341385
Reviewed-by: Phil Quitslund <[email protected]>
Reviewed-by: Alexander Thomas <[email protected]>
Reviewed-by: Jonas Termansen <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: srawlins

PRESUBMIT.py

@@ -298,14 +298,13 @@ def _CheckClangTidy(input_api, output_api):
 def _CheckAnalyzerFiles(input_api, output_api):
     """Run analyzer checks on source files."""
 
-    # The first (and so far, only) check, is to verify the "error fix status"
-    # file.
-    relevant_files = [
+    # Verify the "error fix status" file.
+    code_files = [
         "pkg/analyzer/lib/src/error/error_code_values.g.dart",
         "pkg/linter/lib/src/rules.dart",
     ]
 
-    if any(f.LocalPath() in relevant_files for f in input_api.AffectedFiles()):
+    if any(f.LocalPath() in code_files for f in input_api.AffectedFiles()):
         args = [
             "tools/sdks/dart-sdk/bin/dart",
             "pkg/analysis_server/tool/presubmit/verify_error_fix_status.dart",
@@ -320,6 +319,23 @@ def _CheckAnalyzerFiles(input_api, output_api):
                 long_text=stdout)
         ]
 
+    # Verify the linter's `example/all.yaml` file.
+    if any(f.LocalPath().startswith('pkg/linter/lib/src/rules')
+           for f in input_api.AffectedFiles()):
+        args = [
+            "tools/sdks/dart-sdk/bin/dart",
+            "pkg/analysis_server/tool/checks/check_all_yaml.dart",
+        ]
+        stdout = input_api.subprocess.check_output(args).strip()
+        if not stdout:
+            return []
+
+        return [
+            output_api.PresubmitError(
+                "The check_all_yaml linter tool revealed issues:",
+                long_text=stdout)
+        ]
+
     # TODO(srawlins): Check more:
     # * "verify_sorted" for individual modified (not deleted) files in
     #   Analyzer-team-owned directories.

pkg/linter/test/integration_test.dart

@@ -5,25 +5,13 @@
 import 'dart:io';
 
 import 'package:analyzer/src/lint/io.dart';
-import 'package:analyzer/src/lint/state.dart';
-import 'package:linter/src/analyzer.dart';
-import 'package:linter/src/rules.dart';
-import 'package:linter/src/utils.dart';
 import 'package:test/test.dart';
-import 'package:yaml/yaml.dart';
 
+import '../tool/checks/check_all_yaml.dart';
 import 'mocks.dart';
-import 'test_constants.dart';
 
 void main() {
-  // ignore: unnecessary_lambdas
   group('integration', () {
-    coreTests();
-  });
-}
-
-void coreTests() {
-  group('core', () {
     group('config', () {
       var currentOut = outSink;
       var collectingOut = CollectingSink();
@@ -40,61 +28,11 @@ void coreTests() {
 
     group('examples', () {
       test('all.yaml', () {
-        var src = readFile(pathRelativeToPackageRoot(['example', 'all.yaml']));
-
-        var options = _getOptionsFromString(src);
-        var configuredLints =
-            // ignore: cast_nullable_to_non_nullable
-            (options['linter'] as YamlMap)['rules'] as YamlList;
-
-        // rules are sorted
-        expect(
-            configuredLints, orderedEquals(configuredLints.toList()..sort()));
-
-        registerLintRules();
-
-        var registered = Analyzer.facade.registeredRules
-            .where((r) =>
-                !r.state.isDeprecated &&
-                !r.state.isInternal &&
-                !r.state.isRemoved)
-            .map((r) => r.name);
-
-        for (var l in configuredLints) {
-          if (!registered.contains(l)) {
-            printToConsole(l);
-          }
+        var errors = checkAllYaml();
+        if (errors != null) {
+          fail(errors);
         }
-
-        expect(configuredLints, unorderedEquals(registered));
       });
     });
   });
 }
-
-/// Provide the options found in [optionsSource].
-Map<String, YamlNode> _getOptionsFromString(String optionsSource) {
-  var options = <String, YamlNode>{};
-  var doc = loadYamlNode(optionsSource);
-
-  // Empty options.
-  if (doc is YamlScalar && doc.value == null) {
-    return options;
-  }
-  if (doc is! YamlMap) {
-    throw Exception(
-        'Bad options file format (expected map, got ${doc.runtimeType})');
-  }
-  doc.nodes.forEach((k, YamlNode v) {
-    Object? key;
-    if (k is YamlScalar) {
-      key = k.value;
-    }
-    if (key is! String) {
-      throw Exception('Bad options file format (expected String scope key, '
-          'got ${k.runtimeType})');
-    }
-    options[key] = v;
-  });
-  return options;
-}

pkg/linter/test/test_constants.dart

@@ -2,28 +2,11 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import 'dart:io';
-
-import 'package:path/path.dart' as path;
+import '../tool/util/path_utils.dart';
 
 final String integrationTestDir =
     pathRelativeToPackageRoot(['test_data', 'integration']);
 final String ruleTestDataDir =
     pathRelativeToPackageRoot(['test_data', 'rules']);
 final String ruleTestDir = pathRelativeToPackageRoot(['test', 'rules']);
 final String testConfigDir = pathRelativeToPackageRoot(['test', 'configs']);
-
-List<String> get _scriptPathParts =>
-    path.split(path.dirname(path.fromUri(Platform.script.path)));
-
-String pathRelativeToPackageRoot(Iterable<String> parts) {
-  var pathParts = _scriptPathParts;
-  pathParts.replaceRange(pathParts.length - 1, pathParts.length, parts);
-  return path.joinAll(pathParts);
-}
-
-String pathRelativeToPkgDir(Iterable<String> parts) {
-  var pathParts = _scriptPathParts;
-  pathParts.replaceRange(pathParts.length - 2, pathParts.length, parts);
-  return path.joinAll(pathParts);
-}

pkg/linter/tool/checks/check_all_yaml.dart

@@ -0,0 +1,126 @@
+// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:io';
+
+import 'package:analyzer/src/lint/io.dart';
+import 'package:analyzer/src/lint/state.dart';
+import 'package:linter/src/analyzer.dart';
+import 'package:linter/src/rules.dart';
+import 'package:yaml/yaml.dart';
+
+import '../util/path_utils.dart';
+
+/// Checks the 'example/all.yaml' file for correctness.
+///
+/// Prints any errors.
+void main() {
+  var errors = checkAllYaml();
+  if (errors != null) {
+    // ignore: avoid_print
+    print(errors);
+    exitCode = 1;
+  }
+}
+
+/// Checks the 'example/all.yaml' file for correctness, returning a String if
+/// there are errors, and `null` otherwise.
+String? checkAllYaml() {
+  var allYamlPath = pathRelativeToPackageRoot(['example', 'all.yaml']);
+  var src = readFile(allYamlPath);
+
+  var options = _getOptionsFromString(src);
+  var linterSection = options['linter'] as YamlMap?;
+  if (linterSection == null) {
+    return "Error: '$allYamlPath' does not have a 'linter' section.";
+  }
+
+  var configuredRules = (linterSection['rules'] as YamlList?)?.cast<String>();
+  if (configuredRules == null) {
+    return "Error: '$allYamlPath' does not have a 'rules' section.";
+  }
+
+  var sortedRules = configuredRules.toList()..sort();
+
+  for (var i = 0; i < configuredRules.length; i++) {
+    if (configuredRules[i] != sortedRules[i]) {
+      return "Error: Rules in '$allYamlPath' are not sorted alphabetically, "
+          "starting at '${configuredRules[i]}'.";
+    }
+  }
+
+  registerLintRules();
+
+  var registeredRules = Analyzer.facade.registeredRules
+      .where((r) =>
+          !r.state.isDeprecated && !r.state.isInternal && !r.state.isRemoved)
+      .map((r) => r.name);
+
+  var extraRules = <String>[];
+  var missingRules = <String>[];
+
+  for (var rule in configuredRules) {
+    if (!registeredRules.contains(rule)) {
+      extraRules.add(rule);
+    }
+  }
+  for (var rule in registeredRules) {
+    if (!configuredRules.contains(rule)) {
+      missingRules.add(rule);
+    }
+  }
+
+  if (extraRules.isEmpty && missingRules.isEmpty) {
+    return null;
+  }
+
+  var errors = StringBuffer();
+  if (extraRules.isNotEmpty) {
+    errors.writeln('Found unknown (or deprecated/removed) rules:');
+    for (var rule in extraRules) {
+      errors.writeln('- $rule');
+    }
+  }
+  if (missingRules.isNotEmpty) {
+    errors.writeln('Missing rules:');
+    for (var rule in missingRules) {
+      errors.writeln('- $rule');
+    }
+  }
+  return errors.toString();
+}
+
+/// Provides the options found in [optionsSource].
+Map<String, YamlNode> _getOptionsFromString(String optionsSource) {
+  var options = <String, YamlNode>{};
+  var doc = loadYamlNode(optionsSource);
+
+  // Empty options.
+  if (doc is YamlScalar && doc.value == null) {
+    return options;
+  }
+  if (doc is! YamlMap) {
+    throw Exception(
+        'Bad options file format (expected map, got ${doc.runtimeType})');
+  }
+  doc.nodes.forEach((k, YamlNode v) {
+    if (k is! YamlScalar) {
+      throw YamlException(
+        'Bad options file format (expected YamlScalar key, got '
+        "'${k.runtimeType}'",
+        v.span,
+      );
+    }
+    var key = k.value;
+    if (key is! String) {
+      throw YamlException(
+        'Bad options file format (expected String key, got '
+        "'${key.runtimeType})'",
+        v.span,
+      );
+    }
+    options[key] = v;
+  });
+  return options;
+}

pkg/linter/tool/machine.dart

@@ -13,7 +13,7 @@ import 'package:linter/src/rules.dart';
 import 'package:linter/src/utils.dart';
 import 'package:yaml/yaml.dart';
 
-import '../test/test_constants.dart';
+import '../tool/util/path_utils.dart';
 import 'since.dart';
 import 'util/score_utils.dart' as score_utils;
 

pkg/linter/tool/scorecard.dart

@@ -13,7 +13,7 @@ import 'package:linter/src/analyzer.dart';
 import 'package:linter/src/rules.dart';
 import 'package:linter/src/utils.dart';
 
-import '../test/test_constants.dart';
+import '../tool/util/path_utils.dart';
 import 'crawl.dart';
 import 'parse.dart';
 import 'since.dart';

pkg/linter/tool/since.dart

@@ -7,7 +7,7 @@ import 'dart:io';
 
 import 'package:yaml/yaml.dart';
 
-import '../test/test_constants.dart';
+import '../tool/util/path_utils.dart';
 import 'changelog.dart';
 import 'crawl.dart';
 

pkg/linter/tool/util/path_utils.dart

@@ -0,0 +1,28 @@
+// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:io';
+
+import 'package:path/path.dart' as path;
+
+List<String> get _packageRoot {
+  var parts = path.split(path.dirname(path.fromUri(Platform.script.path)));
+  while (parts.last != 'linter') {
+    parts.removeLast();
+    if (parts.isEmpty) {
+      throw StateError("Script is not located inside a 'linter' directory? "
+          "'${Platform.script.path}'");
+    }
+  }
+  return parts;
+}
+
+String pathRelativeToPackageRoot(Iterable<String> parts) =>
+    path.joinAll([..._packageRoot, ...parts]);
+
+String pathRelativeToPkgDir(Iterable<String> parts) {
+  var pathParts = _packageRoot;
+  pathParts.replaceRange(pathParts.length - 1, pathParts.length, parts);
+  return path.joinAll(pathParts);
+}