Introduce first presubmit check for analyzer team.
See the issue below, and the code comments, for the further plans.
I plan on adding whichever ones we find useful, as long as I can
keep them performant. This one is performant, and we can discuss
possible problematic checks on the issue.
Work towards https://github.com/dart-lang/sdk/issues/53578
Change-Id: Ie3980e6194e46574a01ad3e0bd8e36f7ac248917
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329620
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Kallen Tu <kallentu@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Jonas Termansen <sortie@google.com>
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 3956ec7..c7f17e4 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -283,6 +283,49 @@
]
+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 = [
+ "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()):
+ args = [
+ "tools/sdks/dart-sdk/bin/dart",
+ "pkg/analysis_server/tool/presubmit/verify_error_fix_status.dart",
+ ]
+ stdout = input_api.subprocess.check_output(args).strip()
+ if not stdout:
+ return []
+
+ return [
+ output_api.PresubmitError(
+ "The verify_error_fix_status Analyzer tool revealed issues:",
+ long_text=stdout)
+ ]
+
+ # TODO(srawlins): Check more:
+ # * "verify_sorted" for individual modified (not deleted) files in
+ # Analyzer-team-owned directories.
+ # * "verify_tests" for individual modified (not deleted) test files in
+ # Analyzer-team-owned directories.
+ # * Verify that `messages/generate.dart` does not produce different
+ # content, when `pkg/analyzer/messages.yaml` is modified.
+ # * Verify that `diagnostics/generate.dart` does not produce different
+ # content, when `pkg/analyzer/messages.yaml` is modified.
+ # * Verify that `machine.json` is not outdated, when any
+ # `pkg/linter/lib/src/rules` file is modified.
+ # * Maybe "verify_no_solo" for individual modified (not deleted test files
+ # in Analyzer-team-owned directories.
+
+ # No files are relevant.
+ return []
+
+
def _CheckTestMatrixValid(input_api, output_api):
"""Run script to check that the test matrix has no errors."""
@@ -348,6 +391,7 @@
results.extend(
input_api.canned_checks.CheckPatchFormatted(input_api, output_api))
results.extend(_CheckCopyrightYear(input_api, output_api))
+ results.extend(_CheckAnalyzerFiles(input_api, output_api))
return results
diff --git a/pkg/analysis_server/test/verify_error_fix_status_test.dart b/pkg/analysis_server/test/verify_error_fix_status_test.dart
index 8b3cec0..01f8983 100644
--- a/pkg/analysis_server/test/verify_error_fix_status_test.dart
+++ b/pkg/analysis_server/test/verify_error_fix_status_test.dart
@@ -2,16 +2,10 @@
// 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 'package:analysis_server/src/services/correction/dart/data_driven.dart';
-import 'package:analysis_server/src/services/correction/fix_internal.dart';
-import 'package:analyzer/error/error.dart';
-import 'package:analyzer/file_system/physical_file_system.dart';
-import 'package:analyzer/src/lint/registry.dart';
-import 'package:analyzer_utilities/package_root.dart' as package_root;
-import 'package:linter/src/rules.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
-import 'package:yaml/yaml.dart';
+
+import '../tool/presubmit/verify_error_fix_status.dart';
void main() {
defineReflectiveSuite(() {
@@ -21,192 +15,10 @@
@reflectiveTest
class VerifyErrorFixStatusTest {
- PhysicalResourceProvider resourceProvider = PhysicalResourceProvider.INSTANCE;
-
void test_statusFile() {
- var statusInfo = _statusInfo();
- var errorCodeNames = _errorCodeNames();
- var lintRuleNames = _lintRuleNames();
-
- var errorData = _ErrorData();
- for (var code in errorCodeValues) {
- var name = code.uniqueName;
- if (name.startsWith('TodoCode.')) {
- // To-do codes are ignored.
- } else {
- var info = statusInfo.nodes[name];
- if (info == null) {
- errorData.codesWithNoEntry.add(name);
- } else if (info is YamlMap) {
- var markedAsHavingFix = info['status'] == 'hasFix';
- var hasFix = _hasCodeFix(code);
- if (hasFix) {
- if (!markedAsHavingFix) {
- errorData.codesWithFixes.add(name);
- }
- } else {
- if (markedAsHavingFix) {
- errorData.codesWithoutFixes.add(name);
- }
- }
- }
- }
- }
- for (var name in lintRuleNames) {
- var info = statusInfo.nodes[name];
- if (info == null) {
- errorData.codesWithNoEntry.add(name);
- } else if (info is YamlMap) {
- var markedAsHavingFix = info['status'] == 'hasFix';
- var hasFix = _hasLintFix(name);
- if (hasFix) {
- if (!markedAsHavingFix) {
- errorData.codesWithFixes.add(name);
- }
- } else {
- if (markedAsHavingFix) {
- errorData.codesWithoutFixes.add(name);
- }
- }
- }
- }
-
- for (var key in statusInfo.keys) {
- if (key is String) {
- if (!errorCodeNames.contains(key) && !lintRuleNames.contains(key)) {
- errorData.entriesWithNoCode.add(key);
- }
- }
- }
-
- if (errorData.isNotEmpty) {
- fail(_failureMessage(errorData));
+ var errors = verifyErrorFixStatus();
+ if (errors != null) {
+ fail(errors);
}
}
-
- /// Return the unique names of the error codes.
- Set<String> _errorCodeNames() {
- var codes = errorCodeValues;
- var codeNames = <String>{};
- for (var code in codes) {
- codeNames.add(code.uniqueName);
- }
- return codeNames;
- }
-
- /// Return a failure message composed from the given lists.
- String _failureMessage(_ErrorData errorData) {
- var buffer = StringBuffer('In ${_statusFilePath()}:\n');
- var needsBlankLine = false;
- if (errorData.codesWithNoEntry.isNotEmpty) {
- buffer.writeln('Add the following entries:');
- buffer.writeln();
- for (var code in errorData.codesWithNoEntry) {
- buffer.writeln('$code:');
- buffer.writeln(' status: needsEvaluation');
- }
- needsBlankLine = true;
- }
- if (errorData.entriesWithNoCode.isNotEmpty) {
- if (needsBlankLine) {
- buffer.writeln();
- }
- buffer.writeln('Remove the following entries:');
- for (var code in errorData.entriesWithNoCode) {
- buffer.writeln('- $code');
- }
- needsBlankLine = true;
- }
- if (errorData.codesWithFixes.isNotEmpty) {
- if (needsBlankLine) {
- buffer.writeln();
- }
- buffer.writeln('Mark the following entries as having fixes:');
- for (var code in errorData.codesWithFixes) {
- buffer.writeln('- $code');
- }
- needsBlankLine = true;
- }
- if (errorData.codesWithoutFixes.isNotEmpty) {
- if (needsBlankLine) {
- buffer.writeln();
- }
- buffer.writeln('Mark the following entries as not having fixes:');
- for (var code in errorData.codesWithoutFixes) {
- buffer.writeln('- $code');
- }
- needsBlankLine = true;
- }
- return buffer.toString();
- }
-
- /// Return `true` if the given error [code] has a fix associated with it.
- bool _hasCodeFix(ErrorCode code) {
- var producers = FixProcessor.nonLintProducerMap[code];
- if (producers != null) {
- return true;
- }
- var multiProducers = FixProcessor.nonLintMultiProducerMap[code];
- if (multiProducers != null) {
- for (var producer in multiProducers) {
- if (producer is! DataDriven) {
- return true;
- }
- }
- }
- return false;
- }
-
- /// Return `true` if the lint with the given name has a fix associated with
- /// it.
- bool _hasLintFix(String codeName) {
- var name = codeName.substring('LintCode.'.length);
- var producers = FixProcessor.lintProducerMap[name];
- return producers != null;
- }
-
- /// Return the unique names of the lint rules.
- Set<String> _lintRuleNames() {
- registerLintRules();
- var ruleNames = <String>{};
- for (var rule in Registry.ruleRegistry.rules) {
- for (var code in rule.lintCodes) {
- ruleNames.add(code.uniqueName);
- }
- }
- return ruleNames;
- }
-
- /// Return the path to the file containing the status information.
- String _statusFilePath() {
- var pathContext = resourceProvider.pathContext;
- var packageRoot = pathContext.normalize(package_root.packageRoot);
- return pathContext.join(packageRoot, 'analysis_server', 'lib', 'src',
- 'services', 'correction', 'error_fix_status.yaml');
- }
-
- /// Return the content of the file containing the status information, parsed
- /// as a YAML map.
- YamlMap _statusInfo() {
- var statusFile = resourceProvider.getFile(_statusFilePath());
- var document = loadYamlDocument(statusFile.readAsStringSync());
- var statusInfo = document.contents;
- if (statusInfo is! YamlMap) {
- fail('Expected a YamlMap, found ${statusInfo.runtimeType}');
- }
- return statusInfo;
- }
-}
-
-class _ErrorData {
- final List<String> codesWithFixes = [];
- final List<String> codesWithNoEntry = [];
- final List<String> codesWithoutFixes = [];
- final List<String> entriesWithNoCode = [];
-
- bool get isNotEmpty =>
- codesWithFixes.isNotEmpty ||
- codesWithNoEntry.isNotEmpty ||
- codesWithoutFixes.isNotEmpty ||
- entriesWithNoCode.isNotEmpty;
}
diff --git a/pkg/analysis_server/tool/presubmit/verify_error_fix_status.dart b/pkg/analysis_server/tool/presubmit/verify_error_fix_status.dart
new file mode 100644
index 0000000..c281c60
--- /dev/null
+++ b/pkg/analysis_server/tool/presubmit/verify_error_fix_status.dart
@@ -0,0 +1,206 @@
+// 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:analysis_server/src/services/correction/dart/data_driven.dart';
+import 'package:analysis_server/src/services/correction/fix_internal.dart';
+import 'package:analyzer/error/error.dart';
+import 'package:analyzer/file_system/physical_file_system.dart';
+import 'package:analyzer/src/lint/registry.dart';
+import 'package:analyzer_utilities/package_root.dart' as package_root;
+import 'package:linter/src/rules.dart';
+import 'package:yaml/yaml.dart';
+
+void main() {
+ var errors = verifyErrorFixStatus();
+ if (errors != null) {
+ print(errors);
+ exitCode = 1;
+ }
+}
+
+PhysicalResourceProvider resourceProvider = PhysicalResourceProvider.INSTANCE;
+
+/// Returns whether the given error [code] has a fix associated with it.
+bool hasCodeFix(ErrorCode code) {
+ var producers = FixProcessor.nonLintProducerMap[code];
+ if (producers != null) {
+ return true;
+ }
+ var multiProducers = FixProcessor.nonLintMultiProducerMap[code];
+ if (multiProducers != null) {
+ for (var producer in multiProducers) {
+ if (producer is! DataDriven) {
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
+/// Returns `true` if the lint rule with the given name has a fix associated
+/// with it.
+bool hasLintFix(String codeName) {
+ var name = codeName.substring('LintCode.'.length);
+ var producers = FixProcessor.lintProducerMap[name];
+ return producers != null;
+}
+
+/// Returns the path to the file containing the status information.
+String statusFilePath() {
+ var pathContext = resourceProvider.pathContext;
+ var packageRoot = pathContext.normalize(package_root.packageRoot);
+ return pathContext.join(packageRoot, 'analysis_server', 'lib', 'src',
+ 'services', 'correction', 'error_fix_status.yaml');
+}
+
+/// Verifies that the "error fix status" file is up-to-date and returns an error
+/// string describing how the file is out-of-date, or `null` if the file is
+/// up-to-date.
+String? verifyErrorFixStatus() {
+ var (error, statusInfo) = _statusInfo();
+ if (error != null) {
+ return error;
+ }
+ statusInfo!; // This is non-null when `error` is `null`.
+ registerLintRules();
+ var errorCodeNames = {
+ for (var code in errorCodeValues) code.uniqueName,
+ };
+ var lintRuleNames = {
+ for (var rule in Registry.ruleRegistry.rules)
+ ...rule.lintCodes.map((e) => e.uniqueName),
+ };
+
+ var errorData = ErrorData();
+ for (var code in errorCodeValues) {
+ var name = code.uniqueName;
+ if (name.startsWith('TodoCode.')) {
+ // To-do codes are ignored.
+ continue;
+ }
+
+ var info = statusInfo.nodes[name];
+ if (info == null) {
+ errorData.codesWithNoEntry.add(name);
+ } else if (info is YamlMap) {
+ var markedAsHavingFix = info['status'] == 'hasFix';
+ var hasFix = hasCodeFix(code);
+ if (hasFix) {
+ if (!markedAsHavingFix) {
+ errorData.codesWithFixes.add(name);
+ }
+ } else {
+ if (markedAsHavingFix) {
+ errorData.codesWithoutFixes.add(name);
+ }
+ }
+ }
+ }
+ for (var name in lintRuleNames) {
+ var info = statusInfo.nodes[name];
+ if (info == null) {
+ errorData.codesWithNoEntry.add(name);
+ } else if (info is YamlMap) {
+ var markedAsHavingFix = info['status'] == 'hasFix';
+ var hasFix = hasLintFix(name);
+ if (hasFix) {
+ if (!markedAsHavingFix) {
+ errorData.codesWithFixes.add(name);
+ }
+ } else {
+ if (markedAsHavingFix) {
+ errorData.codesWithoutFixes.add(name);
+ }
+ }
+ }
+ }
+
+ for (var key in statusInfo.keys) {
+ if (key is String) {
+ if (!errorCodeNames.contains(key) && !lintRuleNames.contains(key)) {
+ errorData.entriesWithNoCode.add(key);
+ }
+ }
+ }
+
+ if (errorData.isNotEmpty) {
+ return errorData.failureMessage;
+ }
+
+ // No errors.
+ return null;
+}
+
+/// Returns the content of the file containing the status information, parsed
+/// as a YAML map.
+(String? error, YamlMap? info) _statusInfo() {
+ var statusFile = resourceProvider.getFile(statusFilePath());
+ var document = loadYamlDocument(statusFile.readAsStringSync());
+ var statusInfo = document.contents;
+ if (statusInfo is! YamlMap) {
+ return ('Expected a YamlMap, found ${statusInfo.runtimeType}', null);
+ }
+ return (null, statusInfo);
+}
+
+class ErrorData {
+ final List<String> codesWithFixes = [];
+ final List<String> codesWithNoEntry = [];
+ final List<String> codesWithoutFixes = [];
+ final List<String> entriesWithNoCode = [];
+
+ /// A failure message composed from the various lists.
+ String get failureMessage {
+ var buffer = StringBuffer('In ${statusFilePath()}:\n');
+ var needsBlankLine = false;
+ if (codesWithNoEntry.isNotEmpty) {
+ buffer.writeln('Add the following entries:');
+ buffer.writeln();
+ for (var code in codesWithNoEntry) {
+ buffer.writeln('$code:');
+ buffer.writeln(' status: needsEvaluation');
+ }
+ needsBlankLine = true;
+ }
+ if (entriesWithNoCode.isNotEmpty) {
+ if (needsBlankLine) {
+ buffer.writeln();
+ }
+ buffer.writeln('Remove the following entries:');
+ for (var code in entriesWithNoCode) {
+ buffer.writeln('- $code');
+ }
+ needsBlankLine = true;
+ }
+ if (codesWithFixes.isNotEmpty) {
+ if (needsBlankLine) {
+ buffer.writeln();
+ }
+ buffer.writeln('Mark the following entries as having fixes:');
+ for (var code in codesWithFixes) {
+ buffer.writeln('- $code');
+ }
+ needsBlankLine = true;
+ }
+ if (codesWithoutFixes.isNotEmpty) {
+ if (needsBlankLine) {
+ buffer.writeln();
+ }
+ buffer.writeln('Mark the following entries as not having fixes:');
+ for (var code in codesWithoutFixes) {
+ buffer.writeln('- $code');
+ }
+ needsBlankLine = true;
+ }
+ return buffer.toString();
+ }
+
+ bool get isNotEmpty =>
+ codesWithFixes.isNotEmpty ||
+ codesWithNoEntry.isNotEmpty ||
+ codesWithoutFixes.isNotEmpty ||
+ entriesWithNoCode.isNotEmpty;
+}
