dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Simplify testing analysis rule by registering in AnalysisRuleTest

Browse Source 
This changes the API of writing an analysis rule test. Instead of
overriding the `String get analysisRule` property, and registering the
rule in `setUp` (which requires importing a private type), the
developer can now just set the `AbstractAnalysisRule rule` field in
`setUp`. This both removes the requirement to manually register the
rule, and reduces the risk of typos, by removing the String API.

Fixes https://github.com/dart-lang/sdk/issues/61793

Change-Id: Ic7753d3157e06906ba5ccbccfea67aaa3179dcc8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/459340
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Sam Rawlins
2025-11-03 10:20:00 -08:00
committed by Commit Queue
parent 78f5cb305e
commit c0bd240db8
6 changed files with 68 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/analysis_server_plugin/doc/testing_rules.md
+9 -16
View File
@@ -1,21 +1,19 @@
# Testing rules
<!-- TODO(srawlins): Link to analyzer_testing, when published. -->
The `analyzer_testing` package provides an API for testing analysis rules. Tests
can be written concisely, encouraging the plugin author to write test cases with
good coverage of possible Dart syntax, and the analysis rules themselves.
The [`analyzer_testing`][] package provides an API for testing analysis rules.
Tests can be written concisely, encouraging the plugin author to write test
cases with good coverage of possible Dart syntax, and the analysis rules
themselves.
## The test class
Analysis rule tests that are written with the `analyzer_testing` package's
Analysis rule tests that are written with the [`analyzer_testing`][] package's
support use a class hierarchy to specify shared variables, helper methods, and
set-up and tear-down code. This is all based on the [`test_reflective_loader`][]
package. Here is the basic structure:
```dart
import 'package:analyzer/src/lint/registry.dart';
import 'package:analyzer_testing/analysis_rule/analysis_rule.dart';
import 'package:my_rule/src/rules/my_rule.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -24,13 +22,10 @@ import 'package:test_reflective_loader/test_reflective_loader.dart';
class MyRuleTest extends AnalysisRuleTest {
@override
void setUp() {
Registry.ruleRegistry.registerLintRule(MyRule());
rule = MyRule();
super.setUp();
}
@override
String get analysisRule => 'my_rule';
// Test cases go here.
}
```
@@ -52,11 +47,8 @@ components of the `MyRuleTest` class.
`newFile`.
* `void setUp` - Override this method to provide some set-up code that is
executed before each test. This method must call `super.setUp()`. This method
is where we register the analysis rule that we are testing:
`Registry.ruleRegistry.registerLintRule(MyRule());`.
* `String get analysisRule` - This getter must be implemented, returning the
analysis rule name, so that the test knows what analysis rule to expect. This
is the name that the rule class passes up to the super-constructor.
is where we instantiate the analysis rule that we are testing:
`rule = MyRule();`.
## The test cases
@@ -154,5 +146,6 @@ With this `main` function, tests can be run in the same way as class `test`
package tests. They can be run in the usual ways, such as using the IDE, or by
running `dart test` or `dart --enable-asserts test/my_rule_test.dart`.
[`analyzer_testing`]: https://pub.dev/packages/analyzer_testing
[writing rules]: https://github.com/dart-lang/sdk/blob/main/pkg/analysis_server_plugin/doc/writing_rules.md
[`test_reflective_loader`]: https://pub.dev/packages/test_reflective_loader
pkg/analyzer_testing/CHANGELOG.md
+7
View File
@@ -1,3 +1,10 @@
## 0.1.7
- Deprecate `AnalysisRuleTest.analysisRule`; instead of implementing this
getter, set the `rule` field in the `setUp` method, before calling
`super.setUp`. For example, when testing an analysis rule, `MyRule`, call
`rule = MyRule()` in `setUp`.
## 0.1.6
- Require version `9.0.0` of the `analyzer` package.
pkg/analyzer_testing/api.txt
+5 -1
View File
@@ -3,7 +3,9 @@ package:analyzer_testing/analysis_rule/analysis_rule.dart:
error (function: ExpectedDiagnostic Function(DiagnosticCode, int, int, {List<ExpectedContextMessage>? contextMessages, Pattern? correctionContains, deprecated Pattern? messageContains, List<Pattern> messageContainsAll}))
AnalysisRuleTest (class extends PubPackageResolutionTest):
new (constructor: AnalysisRuleTest Function())
analysisRule (getter: String)
analysisRule (getter: String, deprecated)
rule (getter: AbstractAnalysisRule)
rule= (setter: AbstractAnalysisRule)
assertNoPubspecDiagnostics (method: Future<void> Function(String))
assertPubspecDiagnostics (method: Future<void> Function(String, List<ExpectedDiagnostic>))
correctionMessage (method: String Function(List<Diagnostic>))
@@ -84,6 +86,8 @@ dart:core:
package:_fe_analyzer_shared/src/base/errors.dart:
Diagnostic (referenced)
DiagnosticCode (referenced)
package:analyzer/analysis_rule/analysis_rule.dart:
AbstractAnalysisRule (referenced)
package:analyzer/file_system/file_system.dart:
File (referenced)
Folder (referenced)
pkg/analyzer_testing/lib/analysis_rule/analysis_rule.dart
+42 -9
View File
@@ -73,8 +73,20 @@ ExpectedDiagnostic error(
/// A base class for analysis rule tests that use test_reflective_loader.
abstract class AnalysisRuleTest extends PubPackageResolutionTest {
/// The [AbstractAnalysisRule] under test.
///
/// In a test class that extends [AnalysisRuleTest], this field must be set
/// from within [setUp], before calling `super.setUp`.
AbstractAnalysisRule rule = _SentinelRule();
/// The name of the analysis rule which this test is concerned with.
String get analysisRule;
late String _analysisRule;
/// The name of the analysis rule which this test is concerned with.
// TODO(srawlins): In a major release, remove this getter, and implement
// `_analysisRule` as `=> rule.name;`.
@Deprecated("Set 'rule' in 'setUp' instead")
String get analysisRule => 'sentinel';
/// Asserts that no diagnostics are reported when resolving [content].
///
@@ -112,7 +124,7 @@ abstract class AnalysisRuleTest extends PubPackageResolutionTest {
buffer.write(' error(${actual.diagnosticCode}, ');
}
buffer.write('${actual.offset}, ${actual.length}');
if (actual.diagnosticCode.name != analysisRule) {
if (actual.diagnosticCode.name != _analysisRule) {
buffer.write(", name: '${actual.diagnosticCode.name}'");
}
buffer.writeln('),');
@@ -121,7 +133,7 @@ abstract class AnalysisRuleTest extends PubPackageResolutionTest {
return buffer.toString();
}
/// Returns an "expected diagnostic" for [analysisRule] (or [name], if given)
/// Returns an "expected diagnostic" for [rule] (or [name], if given)
/// at [offset] and [length].
///
/// If given, [messageContains] is used to match against a diagnostic's
@@ -144,7 +156,7 @@ abstract class AnalysisRuleTest extends PubPackageResolutionTest {
messageContainsAll = [messageContains];
}
return ExpectedLint(
name ?? analysisRule,
name ?? _analysisRule,
offset,
length,
messageContainsAll: messageContainsAll,
@@ -156,13 +168,24 @@ abstract class AnalysisRuleTest extends PubPackageResolutionTest {
@mustCallSuper
@override
void setUp() {
if (!Registry.ruleRegistry.any((r) => r.name == analysisRule)) {
throw Exception("Unrecognized rule: '$analysisRule'");
// TODO(srawlins): In a major release, change this logic to just ensure that
// `rule` has been set to an analysis rule instance other than
// `_SentinelRule`.
if (rule is _SentinelRule) {
if (analysisRule == 'sentinel') {
throw StateError('The `rule` field must be set in the `setUp` method.');
}
// The developer is using the deprecated `analysisRule` to set the
// rule-under-test.
_analysisRule = analysisRule;
} else {
_analysisRule = rule.name;
Registry.ruleRegistry.registerLintRule(rule);
}
super.setUp();
newAnalysisOptionsYamlFile(
testPackageRootPath,
analysisOptionsContent(experiments: experiments, rules: [analysisRule]),
analysisOptionsContent(experiments: experiments, rules: [_analysisRule]),
);
}
@@ -174,7 +197,7 @@ abstract class AnalysisRuleTest extends PubPackageResolutionTest {
}
buffer.writeln('Found but did not expect:');
for (var actual in unmatchedActual) {
buffer.write(' $analysisRule.${actual.diagnosticCode.name} [');
buffer.write(' $_analysisRule.${actual.diagnosticCode.name} [');
buffer.write('${actual.offset}, ${actual.length}, ${actual.message}');
if (actual.correctionMessage case Pattern correctionMessage) {
buffer.write(', ');
@@ -188,7 +211,7 @@ abstract class AnalysisRuleTest extends PubPackageResolutionTest {
Future<List<Diagnostic>> _analyzePubspecFile(String content) async {
var path = convertPath(testPackagePubspecPath);
var pubspecRules = <AbstractAnalysisRule, PubspecVisitor<Object?>>{};
var rules = Registry.ruleRegistry.where((r) => analysisRule == r.name);
var rules = Registry.ruleRegistry.where((r) => _analysisRule == r.name);
for (var rule in rules) {
var visitor = rule.pubspecVisitor;
if (visitor != null) {
@@ -219,3 +242,13 @@ abstract class AnalysisRuleTest extends PubPackageResolutionTest {
return [...listener.diagnostics];
}
}
/// A sentinel [AnalysisRule] for use while [AnalysisRuleTest.analysisRule] is
/// still available but deprecated, and using it's replacement,
/// [AnalysisRuleTest.rule], is not yet mandatory.
final class _SentinelRule extends AnalysisRule {
_SentinelRule() : super(name: 'sentinel', description: 'sentinel');
@override
dynamic noSuchMethod(invocation) => super.noSuchMethod(invocation);
}
pkg/analyzer_testing/pubspec.yaml
+1 -1
View File
@@ -1,6 +1,6 @@
name: analyzer_testing
description: Testing utilities related to the analyzer and analysis_server_plugin packages.
version: 0.1.6
version: 0.1.7
repository: https://github.com/dart-lang/sdk/tree/main/pkg/analyzer_testing
environment:
pkg/linter/test/rule_test_support.dart
+4
View File
@@ -2,6 +2,7 @@
// 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:analyzer/src/lint/registry.dart';
import 'package:analyzer_testing/analysis_rule/analysis_rule.dart';
import 'package:analyzer_testing/src/analysis_rule/pub_package_resolution.dart';
import 'package:linter/src/rules.dart';
@@ -33,6 +34,9 @@ abstract class LintRuleTest extends AnalysisRuleTest {
registerLintRules();
_lintRulesAreRegistered = true;
}
rule = Registry.ruleRegistry.rules.firstWhere(
(r) => r.name == analysisRule,
);
super.setUp();
}
}