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>
This commit is contained in:
committed by
Commit Queue
parent
dfac967abf
commit
312b46b444
@@ -283,6 +283,49 @@ 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 = [
|
||||
"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 @@ def _CommonChecks(input_api, output_api):
|
||||
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
|
||||
|
||||
|
||||
|
||||
@@ -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 @@ void main() {
|
||||
|
||||
@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);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
var errors = verifyErrorFixStatus();
|
||||
if (errors != null) {
|
||||
fail(errors);
|
||||
}
|
||||
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));
|
||||
}
|
||||
}
|
||||
|
||||
/// 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;
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
Reference in New Issue
Block a user