diff --git a/pkg/analysis_server_plugin/doc/testing_rules.md b/pkg/analysis_server_plugin/doc/testing_rules.md index 65d181de29e..ba9b0b5b5ab 100644 --- a/pkg/analysis_server_plugin/doc/testing_rules.md +++ b/pkg/analysis_server_plugin/doc/testing_rules.md @@ -1,21 +1,19 @@ # Testing rules - - -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 \ No newline at end of file diff --git a/pkg/analyzer_testing/CHANGELOG.md b/pkg/analyzer_testing/CHANGELOG.md index b6992f859a5..ed5b8b6da19 100644 --- a/pkg/analyzer_testing/CHANGELOG.md +++ b/pkg/analyzer_testing/CHANGELOG.md @@ -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. diff --git a/pkg/analyzer_testing/api.txt b/pkg/analyzer_testing/api.txt index 7e1bf761e74..65bae4fb5d6 100644 --- a/pkg/analyzer_testing/api.txt +++ b/pkg/analyzer_testing/api.txt @@ -3,7 +3,9 @@ package:analyzer_testing/analysis_rule/analysis_rule.dart: error (function: ExpectedDiagnostic Function(DiagnosticCode, int, int, {List? contextMessages, Pattern? correctionContains, deprecated Pattern? messageContains, List 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 Function(String)) assertPubspecDiagnostics (method: Future Function(String, List)) correctionMessage (method: String Function(List)) @@ -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) diff --git a/pkg/analyzer_testing/lib/analysis_rule/analysis_rule.dart b/pkg/analyzer_testing/lib/analysis_rule/analysis_rule.dart index b5efe71e16a..b049f535c1c 100644 --- a/pkg/analyzer_testing/lib/analysis_rule/analysis_rule.dart +++ b/pkg/analyzer_testing/lib/analysis_rule/analysis_rule.dart @@ -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> _analyzePubspecFile(String content) async { var path = convertPath(testPackagePubspecPath); var pubspecRules = >{}; - 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); +} diff --git a/pkg/analyzer_testing/pubspec.yaml b/pkg/analyzer_testing/pubspec.yaml index b51dd4e1b0a..9e201920f86 100644 --- a/pkg/analyzer_testing/pubspec.yaml +++ b/pkg/analyzer_testing/pubspec.yaml @@ -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: diff --git a/pkg/linter/test/rule_test_support.dart b/pkg/linter/test/rule_test_support.dart index becf3028dac..5cc3e11fe27 100644 --- a/pkg/linter/test/rule_test_support.dart +++ b/pkg/linter/test/rule_test_support.dart @@ -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(); } }