diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 3956ec720d9..c7f17e415d9 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -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 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 8b3cec00417..01f89839a00 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 @@ 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 _errorCodeNames() { - var codes = errorCodeValues; - var codeNames = {}; - 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 _lintRuleNames() { - registerLintRules(); - var ruleNames = {}; - 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 codesWithFixes = []; - final List codesWithNoEntry = []; - final List codesWithoutFixes = []; - final List 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 00000000000..c281c60be51 --- /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 codesWithFixes = []; + final List codesWithNoEntry = []; + final List codesWithoutFixes = []; + final List 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; +}