Skip to content

Commit 312b46b

Browse files
srawlinsCommit Queue
srawlins
authored and
Commit Queue
committed
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 #53578 Change-Id: Ie3980e6194e46574a01ad3e0bd8e36f7ac248917 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329620 Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Sigmund Cherem <[email protected]> Reviewed-by: Phil Quitslund <[email protected]> Reviewed-by: Kallen Tu <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Jonas Termansen <[email protected]>
1 parent dfac967 commit 312b46b

File tree

3 files changed

+255
-193
lines changed

3 files changed

+255
-193
lines changed

‎PRESUBMIT.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,49 @@ def _CheckClangTidy(input_api, output_api):
283283
]
284284

285285

286+
def _CheckAnalyzerFiles(input_api, output_api):
287+
"""Run analyzer checks on source files."""
288+
289+
# The first (and so far, only) check, is to verify the "error fix status"
290+
# file.
291+
relevant_files = [
292+
"pkg/analyzer/lib/src/error/error_code_values.g.dart",
293+
"pkg/linter/lib/src/rules.dart",
294+
]
295+
296+
if any(f.LocalPath() in relevant_files for f in input_api.AffectedFiles()):
297+
args = [
298+
"tools/sdks/dart-sdk/bin/dart",
299+
"pkg/analysis_server/tool/presubmit/verify_error_fix_status.dart",
300+
]
301+
stdout = input_api.subprocess.check_output(args).strip()
302+
if not stdout:
303+
return []
304+
305+
return [
306+
output_api.PresubmitError(
307+
"The verify_error_fix_status Analyzer tool revealed issues:",
308+
long_text=stdout)
309+
]
310+
311+
# TODO(srawlins): Check more:
312+
# * "verify_sorted" for individual modified (not deleted) files in
313+
# Analyzer-team-owned directories.
314+
# * "verify_tests" for individual modified (not deleted) test files in
315+
# Analyzer-team-owned directories.
316+
# * Verify that `messages/generate.dart` does not produce different
317+
# content, when `pkg/analyzer/messages.yaml` is modified.
318+
# * Verify that `diagnostics/generate.dart` does not produce different
319+
# content, when `pkg/analyzer/messages.yaml` is modified.
320+
# * Verify that `machine.json` is not outdated, when any
321+
# `pkg/linter/lib/src/rules` file is modified.
322+
# * Maybe "verify_no_solo" for individual modified (not deleted test files
323+
# in Analyzer-team-owned directories.
324+
325+
# No files are relevant.
326+
return []
327+
328+
286329
def _CheckTestMatrixValid(input_api, output_api):
287330
"""Run script to check that the test matrix has no errors."""
288331

@@ -348,6 +391,7 @@ def _CommonChecks(input_api, output_api):
348391
results.extend(
349392
input_api.canned_checks.CheckPatchFormatted(input_api, output_api))
350393
results.extend(_CheckCopyrightYear(input_api, output_api))
394+
results.extend(_CheckAnalyzerFiles(input_api, output_api))
351395
return results
352396

353397

‎pkg/analysis_server/test/verify_error_fix_status_test.dart

Lines changed: 5 additions & 193 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,10 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
import 'package:analysis_server/src/services/correction/dart/data_driven.dart';
6-
import 'package:analysis_server/src/services/correction/fix_internal.dart';
7-
import 'package:analyzer/error/error.dart';
8-
import 'package:analyzer/file_system/physical_file_system.dart';
9-
import 'package:analyzer/src/lint/registry.dart';
10-
import 'package:analyzer_utilities/package_root.dart' as package_root;
11-
import 'package:linter/src/rules.dart';
125
import 'package:test/test.dart';
136
import 'package:test_reflective_loader/test_reflective_loader.dart';
14-
import 'package:yaml/yaml.dart';
7+
8+
import '../tool/presubmit/verify_error_fix_status.dart';
159

1610
void main() {
1711
defineReflectiveSuite(() {
@@ -21,192 +15,10 @@ void main() {
2115

2216
@reflectiveTest
2317
class VerifyErrorFixStatusTest {
24-
PhysicalResourceProvider resourceProvider = PhysicalResourceProvider.INSTANCE;
25-
2618
void test_statusFile() {
27-
var statusInfo = _statusInfo();
28-
var errorCodeNames = _errorCodeNames();
29-
var lintRuleNames = _lintRuleNames();
30-
31-
var errorData = _ErrorData();
32-
for (var code in errorCodeValues) {
33-
var name = code.uniqueName;
34-
if (name.startsWith('TodoCode.')) {
35-
// To-do codes are ignored.
36-
} else {
37-
var info = statusInfo.nodes[name];
38-
if (info == null) {
39-
errorData.codesWithNoEntry.add(name);
40-
} else if (info is YamlMap) {
41-
var markedAsHavingFix = info['status'] == 'hasFix';
42-
var hasFix = _hasCodeFix(code);
43-
if (hasFix) {
44-
if (!markedAsHavingFix) {
45-
errorData.codesWithFixes.add(name);
46-
}
47-
} else {
48-
if (markedAsHavingFix) {
49-
errorData.codesWithoutFixes.add(name);
50-
}
51-
}
52-
}
53-
}
54-
}
55-
for (var name in lintRuleNames) {
56-
var info = statusInfo.nodes[name];
57-
if (info == null) {
58-
errorData.codesWithNoEntry.add(name);
59-
} else if (info is YamlMap) {
60-
var markedAsHavingFix = info['status'] == 'hasFix';
61-
var hasFix = _hasLintFix(name);
62-
if (hasFix) {
63-
if (!markedAsHavingFix) {
64-
errorData.codesWithFixes.add(name);
65-
}
66-
} else {
67-
if (markedAsHavingFix) {
68-
errorData.codesWithoutFixes.add(name);
69-
}
70-
}
71-
}
72-
}
73-
74-
for (var key in statusInfo.keys) {
75-
if (key is String) {
76-
if (!errorCodeNames.contains(key) && !lintRuleNames.contains(key)) {
77-
errorData.entriesWithNoCode.add(key);
78-
}
79-
}
80-
}
81-
82-
if (errorData.isNotEmpty) {
83-
fail(_failureMessage(errorData));
84-
}
85-
}
86-
87-
/// Return the unique names of the error codes.
88-
Set<String> _errorCodeNames() {
89-
var codes = errorCodeValues;
90-
var codeNames = <String>{};
91-
for (var code in codes) {
92-
codeNames.add(code.uniqueName);
93-
}
94-
return codeNames;
95-
}
96-
97-
/// Return a failure message composed from the given lists.
98-
String _failureMessage(_ErrorData errorData) {
99-
var buffer = StringBuffer('In ${_statusFilePath()}:\n');
100-
var needsBlankLine = false;
101-
if (errorData.codesWithNoEntry.isNotEmpty) {
102-
buffer.writeln('Add the following entries:');
103-
buffer.writeln();
104-
for (var code in errorData.codesWithNoEntry) {
105-
buffer.writeln('$code:');
106-
buffer.writeln(' status: needsEvaluation');
107-
}
108-
needsBlankLine = true;
109-
}
110-
if (errorData.entriesWithNoCode.isNotEmpty) {
111-
if (needsBlankLine) {
112-
buffer.writeln();
113-
}
114-
buffer.writeln('Remove the following entries:');
115-
for (var code in errorData.entriesWithNoCode) {
116-
buffer.writeln('- $code');
117-
}
118-
needsBlankLine = true;
119-
}
120-
if (errorData.codesWithFixes.isNotEmpty) {
121-
if (needsBlankLine) {
122-
buffer.writeln();
123-
}
124-
buffer.writeln('Mark the following entries as having fixes:');
125-
for (var code in errorData.codesWithFixes) {
126-
buffer.writeln('- $code');
127-
}
128-
needsBlankLine = true;
129-
}
130-
if (errorData.codesWithoutFixes.isNotEmpty) {
131-
if (needsBlankLine) {
132-
buffer.writeln();
133-
}
134-
buffer.writeln('Mark the following entries as not having fixes:');
135-
for (var code in errorData.codesWithoutFixes) {
136-
buffer.writeln('- $code');
137-
}
138-
needsBlankLine = true;
19+
var errors = verifyErrorFixStatus();
20+
if (errors != null) {
21+
fail(errors);
13922
}
140-
return buffer.toString();
14123
}
142-
143-
/// Return `true` if the given error [code] has a fix associated with it.
144-
bool _hasCodeFix(ErrorCode code) {
145-
var producers = FixProcessor.nonLintProducerMap[code];
146-
if (producers != null) {
147-
return true;
148-
}
149-
var multiProducers = FixProcessor.nonLintMultiProducerMap[code];
150-
if (multiProducers != null) {
151-
for (var producer in multiProducers) {
152-
if (producer is! DataDriven) {
153-
return true;
154-
}
155-
}
156-
}
157-
return false;
158-
}
159-
160-
/// Return `true` if the lint with the given name has a fix associated with
161-
/// it.
162-
bool _hasLintFix(String codeName) {
163-
var name = codeName.substring('LintCode.'.length);
164-
var producers = FixProcessor.lintProducerMap[name];
165-
return producers != null;
166-
}
167-
168-
/// Return the unique names of the lint rules.
169-
Set<String> _lintRuleNames() {
170-
registerLintRules();
171-
var ruleNames = <String>{};
172-
for (var rule in Registry.ruleRegistry.rules) {
173-
for (var code in rule.lintCodes) {
174-
ruleNames.add(code.uniqueName);
175-
}
176-
}
177-
return ruleNames;
178-
}
179-
180-
/// Return the path to the file containing the status information.
181-
String _statusFilePath() {
182-
var pathContext = resourceProvider.pathContext;
183-
var packageRoot = pathContext.normalize(package_root.packageRoot);
184-
return pathContext.join(packageRoot, 'analysis_server', 'lib', 'src',
185-
'services', 'correction', 'error_fix_status.yaml');
186-
}
187-
188-
/// Return the content of the file containing the status information, parsed
189-
/// as a YAML map.
190-
YamlMap _statusInfo() {
191-
var statusFile = resourceProvider.getFile(_statusFilePath());
192-
var document = loadYamlDocument(statusFile.readAsStringSync());
193-
var statusInfo = document.contents;
194-
if (statusInfo is! YamlMap) {
195-
fail('Expected a YamlMap, found ${statusInfo.runtimeType}');
196-
}
197-
return statusInfo;
198-
}
199-
}
200-
201-
class _ErrorData {
202-
final List<String> codesWithFixes = [];
203-
final List<String> codesWithNoEntry = [];
204-
final List<String> codesWithoutFixes = [];
205-
final List<String> entriesWithNoCode = [];
206-
207-
bool get isNotEmpty =>
208-
codesWithFixes.isNotEmpty ||
209-
codesWithNoEntry.isNotEmpty ||
210-
codesWithoutFixes.isNotEmpty ||
211-
entriesWithNoCode.isNotEmpty;
21224
}

0 commit comments

Comments
 (0)