Skip to content

Commit 155a64c

Browse files
srawlinsCommit Queue
srawlins
authored and
Commit Queue
committed
DAS plugins: Allow plugin rules to be disabled by default
This bumps the Registry class to be able to take "rules that are enabled by default" and "rules that need to be explicitly enabled", "lint rules" and "warning rules". Next is to improve the classes. Add parsing for the top-level `plugins` section, and `PluginConfiguration` class to hold this data, and `get pluginConfigurations` on AnalysisOptions. Rename `parseLintRuleConfigs` to `parseLinterSection` to align better with the other functions in engine.dart. Rename `_ruleConfigs` function to `parseRulesSection`. Change-Id: Ib93b7548bfb13cc94381971a5a2780a5dc81e9f1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/392040 Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Phil Quitslund <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent 2202f10 commit 155a64c

File tree

23 files changed

+267
-131
lines changed

23 files changed

+267
-131
lines changed

‎pkg/analysis_server/test/lsp/code_actions_fixes_test.dart‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ MyCla^ss? a;
190190
var camelCaseTypes = Registry.ruleRegistry.getRule('camel_case_types')!;
191191

192192
// Overwrite it.
193-
Registry.ruleRegistry.register(DeprecatedCamelCaseTypes());
193+
Registry.ruleRegistry.registerLintRule(DeprecatedCamelCaseTypes());
194194

195195
// Now we can assume it will have an action associated...
196196

@@ -219,7 +219,7 @@ linter:
219219
);
220220
} finally {
221221
// Restore the "real" `camel_case_types`.
222-
Registry.ruleRegistry.register(camelCaseTypes);
222+
Registry.ruleRegistry.registerLintRule(camelCaseTypes);
223223
}
224224
}
225225

‎pkg/analysis_server/test/src/services/completion/yaml/analysis_options_generator_test.dart‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class AnalysisOptionsGeneratorTest extends YamlGeneratorTest {
3333

3434
void registerRule(LintRule rule) {
3535
addedRules.add(rule);
36-
Registry.ruleRegistry.register(rule);
36+
Registry.ruleRegistry.registerLintRule(rule);
3737
}
3838

3939
void setUp() {
@@ -42,7 +42,7 @@ class AnalysisOptionsGeneratorTest extends YamlGeneratorTest {
4242

4343
void tearDown() {
4444
for (var rule in addedRules) {
45-
Registry.ruleRegistry.unregister(rule);
45+
Registry.ruleRegistry.unregisterLintRule(rule);
4646
}
4747
}
4848

‎pkg/analysis_server/test/src/services/correction/fix/analysis_options/remove_lint_test.dart‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,13 @@ class RemoveLintTest extends AnalysisOptionsFixTest {
5050

5151
void setUp() {
5252
registerLintRules();
53-
Registry.ruleRegistry.register(deprecatedRule);
54-
Registry.ruleRegistry.register(removedRule);
53+
Registry.ruleRegistry.registerLintRule(deprecatedRule);
54+
Registry.ruleRegistry.registerLintRule(removedRule);
5555
}
5656

5757
void tearDown() {
58-
Registry.ruleRegistry.unregister(deprecatedRule);
59-
Registry.ruleRegistry.unregister(removedRule);
58+
Registry.ruleRegistry.unregisterLintRule(deprecatedRule);
59+
Registry.ruleRegistry.unregisterLintRule(removedRule);
6060
}
6161

6262
Future<void> test_deprecated() async {

‎pkg/analysis_server_plugin/lib/registry.dart‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ import 'package:analyzer/src/lint/linter.dart';
99
abstract class PluginRegistry {
1010
void registerFixForRule(LintCode code, ProducerGenerator generator);
1111

12-
/// Register this [lint] with the analyzer's rule registry.
13-
void registerRule(LintRule lint);
12+
/// Register this [rule] with the analyzer's rule registry.
13+
void registerLintRule(LintRule rule);
14+
15+
/// Register this [rule] with the analyzer's rule registry.
16+
void registerWarningRule(LintRule rule);
1417
}

‎pkg/analysis_server_plugin/lib/src/plugin_server.dart‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import 'package:analyzer/src/generated/engine.dart' show AnalysisOptionsImpl;
2929
import 'package:analyzer/src/lint/lint_rule_timers.dart';
3030
import 'package:analyzer/src/lint/linter.dart';
3131
import 'package:analyzer/src/lint/linter_visitor.dart';
32+
import 'package:analyzer/src/lint/registry.dart';
3233
import 'package:analyzer_plugin/channel/channel.dart';
3334
import 'package:analyzer_plugin/protocol/protocol.dart';
3435
import 'package:analyzer_plugin/protocol/protocol_common.dart' as protocol;
@@ -253,13 +254,16 @@ class PluginServer {
253254
null,
254255
);
255256

256-
// TODO(srawlins): Distinguish between registered rules and enabled rules.
257-
for (var rule in _registry.registeredRules) {
258-
rule.reporter = errorReporter;
259-
var timer = enableTiming ? lintRuleTimers.getTimer(rule) : null;
260-
timer?.start();
261-
rule.registerNodeProcessors(nodeRegistry, context);
262-
timer?.stop();
257+
for (var configuration in analysisOptions.pluginConfigurations) {
258+
if (!configuration.isEnabled) continue;
259+
var rules = Registry.ruleRegistry.enabled(configuration.ruleConfigs);
260+
for (var rule in rules) {
261+
rule.reporter = errorReporter;
262+
var timer = enableTiming ? lintRuleTimers.getTimer(rule) : null;
263+
timer?.start();
264+
rule.registerNodeProcessors(nodeRegistry, context);
265+
timer?.stop();
266+
}
263267
}
264268

265269
currentUnit.unit.accept(LinterVisitor(nodeRegistry));

‎pkg/analysis_server_plugin/lib/src/registry.dart‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,14 @@ final class PluginRegistryImpl implements PluginRegistry {
1818
}
1919

2020
@override
21-
void registerRule(LintRule lint) {
22-
Registry.ruleRegistry.register(lint);
21+
void registerLintRule(LintRule rule) {
22+
Registry.ruleRegistry.registerLintRule(rule);
2323
}
24+
25+
@override
26+
void registerWarningRule(LintRule rule) {
27+
Registry.ruleRegistry.registerWarningRule(rule);
28+
}
29+
30+
LintRule? ruleNamed(String name) => Registry.ruleRegistry[name];
2431
}

‎pkg/analysis_server_plugin/test/src/plugin_server_error_test.dart‎

Lines changed: 39 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import 'package:analyzer/dart/ast/ast.dart';
1212
import 'package:analyzer/dart/ast/visitor.dart';
1313
import 'package:analyzer/error/error.dart';
1414
import 'package:analyzer/src/lint/linter.dart';
15-
import 'package:analyzer_plugin/protocol/protocol_common.dart' as protocol;
1615
import 'package:analyzer_plugin/protocol/protocol_generated.dart' as protocol;
1716
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
1817
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
@@ -29,28 +28,45 @@ void main() async {
2928

3029
@reflectiveTest
3130
class PluginServerErrorTest extends PluginServerTestBase {
31+
String get packagePath => convertPath('/package1');
32+
33+
@override
34+
Future<void> setUp() async {
35+
await super.setUp();
36+
newAnalysisOptionsYamlFile(packagePath, '''
37+
plugins:
38+
no_bools:
39+
rules:
40+
- no_bools
41+
''');
42+
}
43+
3244
Future<void> test_handleAnalysisSetContextRoots_throwingAsyncError() async {
3345
pluginServer = PluginServer(
3446
resourceProvider: resourceProvider,
3547
plugins: [_RuleThrowsAsyncErrorPlugin()],
3648
);
3749
await startPlugin();
3850

39-
var packagePath = convertPath('/package1');
4051
var filePath = join(packagePath, 'lib', 'test.dart');
4152
newFile(filePath, 'bool b = false;');
4253
var contextRoot = protocol.ContextRoot(packagePath, []);
4354

4455
await channel
4556
.sendRequest(protocol.AnalysisSetContextRootsParams([contextRoot]));
4657

47-
var notifications = StreamQueue(channel.notifications);
48-
var analysisErrorsParams = protocol.AnalysisErrorsParams.fromNotification(
49-
await notifications.next);
58+
// Create a broadcast Stream of notifications, so that we can have multiple
59+
// StreamQueues listening.
60+
var notifications = channel.notifications.asBroadcastStream();
61+
var analysisErrorsParamsQueue = StreamQueue(notifications
62+
.map((n) => protocol.AnalysisErrorsParams.fromNotification(n))
63+
.where((p) => p.file == filePath));
64+
var analysisErrorsParams = await analysisErrorsParamsQueue.next;
5065
expect(analysisErrorsParams.errors, isEmpty);
5166

52-
var pluginErrorParams =
53-
protocol.PluginErrorParams.fromNotification(await notifications.next);
67+
var pluginErrorParamsQueue = StreamQueue(notifications
68+
.map((n) => protocol.PluginErrorParams.fromNotification(n)));
69+
var pluginErrorParams = await pluginErrorParamsQueue.next;
5470
expect(pluginErrorParams.isFatal, false);
5571
expect(pluginErrorParams.message, 'Bad state: A message.');
5672
// TODO(srawlins): Does `StackTrace.toString()` not do what I think?
@@ -64,7 +80,6 @@ class PluginServerErrorTest extends PluginServerTestBase {
6480
);
6581
await startPlugin();
6682

67-
var packagePath = convertPath('/package1');
6883
var filePath = join(packagePath, 'lib', 'test.dart');
6984
newFile(filePath, 'bool b = false;');
7085
var contextRoot = protocol.ContextRoot(packagePath, []);
@@ -87,7 +102,6 @@ class PluginServerErrorTest extends PluginServerTestBase {
87102
);
88103
await startPlugin();
89104

90-
var packagePath = convertPath('/package1');
91105
var filePath = join(packagePath, 'lib', 'test.dart');
92106
newFile(filePath, 'bool b = false;');
93107
var contextRoot = protocol.ContextRoot(packagePath, []);
@@ -97,13 +111,18 @@ class PluginServerErrorTest extends PluginServerTestBase {
97111
await channel
98112
.sendRequest(protocol.EditGetFixesParams(filePath, 'bool b = '.length));
99113

100-
var notifications = StreamQueue(channel.notifications);
101-
var analysisErrorsParams = protocol.AnalysisErrorsParams.fromNotification(
102-
await notifications.next);
103-
expect(analysisErrorsParams.errors.single, isA<protocol.AnalysisError>());
104-
105-
var pluginErrorParams =
106-
protocol.PluginErrorParams.fromNotification(await notifications.next);
114+
// Create a broadcast Stream of notifications, so that we can have multiple
115+
// StreamQueues listening.
116+
var notifications = channel.notifications.asBroadcastStream();
117+
var analysisErrorsParamsQueue = StreamQueue(notifications
118+
.map((n) => protocol.AnalysisErrorsParams.fromNotification(n))
119+
.where((p) => p.file == filePath));
120+
var analysisErrorsParams = await analysisErrorsParamsQueue.next;
121+
expect(analysisErrorsParams.errors.single, isNotNull);
122+
123+
var pluginErrorParamsQueue = StreamQueue(notifications
124+
.map((n) => protocol.PluginErrorParams.fromNotification(n)));
125+
var pluginErrorParams = await pluginErrorParamsQueue.next;
107126
expect(pluginErrorParams.isFatal, false);
108127
expect(pluginErrorParams.message, 'Bad state: A message.');
109128
// TODO(srawlins): Does `StackTrace.toString()` not do what I think?
@@ -117,7 +136,6 @@ class PluginServerErrorTest extends PluginServerTestBase {
117136
);
118137
await startPlugin();
119138

120-
var packagePath = convertPath('/package1');
121139
var filePath = join(packagePath, 'lib', 'test.dart');
122140
newFile(filePath, 'bool b = false;');
123141
var contextRoot = protocol.ContextRoot(packagePath, []);
@@ -139,30 +157,30 @@ class PluginServerErrorTest extends PluginServerTestBase {
139157
class _FixThrowsAsyncErrorPlugin extends Plugin {
140158
@override
141159
void register(PluginRegistry registry) {
142-
registry.registerRule(NoBoolsRule());
160+
registry.registerWarningRule(NoBoolsRule());
143161
registry.registerFixForRule(NoBoolsRule.code, _ThrowsAsyncErrorFix.new);
144162
}
145163
}
146164

147165
class _FixThrowsSyncErrorPlugin extends Plugin {
148166
@override
149167
void register(PluginRegistry registry) {
150-
registry.registerRule(NoBoolsRule());
168+
registry.registerWarningRule(NoBoolsRule());
151169
registry.registerFixForRule(NoBoolsRule.code, _ThrowsSyncErrorFix.new);
152170
}
153171
}
154172

155173
class _RuleThrowsAsyncErrorPlugin extends Plugin {
156174
@override
157175
void register(PluginRegistry registry) {
158-
registry.registerRule(_ThrowsAsyncErrorRule());
176+
registry.registerWarningRule(_ThrowsAsyncErrorRule());
159177
}
160178
}
161179

162180
class _RuleThrowsSyncErrorPlugin extends Plugin {
163181
@override
164182
void register(PluginRegistry registry) {
165-
registry.registerRule(_ThrowsSyncErrorRule());
183+
registry.registerWarningRule(_ThrowsSyncErrorRule());
166184
}
167185
}
168186

‎pkg/analysis_server_plugin/test/src/plugin_server_test.dart‎

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ class PluginServerTest extends PluginServerTestBase {
3434
@override
3535
Future<void> setUp() async {
3636
await super.setUp();
37+
newAnalysisOptionsYamlFile(packagePath, '''
38+
plugins:
39+
no_bools:
40+
rules:
41+
- no_bools
42+
''');
3743

3844
pluginServer = PluginServer(
3945
resourceProvider: resourceProvider, plugins: [_NoBoolsPlugin()]);
@@ -44,9 +50,10 @@ class PluginServerTest extends PluginServerTestBase {
4450
newFile(filePath, 'bool b = false;');
4551
await channel
4652
.sendRequest(protocol.AnalysisSetContextRootsParams([contextRoot]));
47-
var notification = await channel.notifications.first;
48-
var params = protocol.AnalysisErrorsParams.fromNotification(notification);
49-
expect(params.file, convertPath('/package1/lib/test.dart'));
53+
var paramsQueue = StreamQueue(channel.notifications
54+
.map((n) => protocol.AnalysisErrorsParams.fromNotification(n))
55+
.where((p) => p.file == filePath));
56+
var params = await paramsQueue.next;
5057
expect(params.errors, hasLength(1));
5158
_expectAnalysisError(params.errors.single, message: 'No bools message');
5259
}
@@ -71,18 +78,16 @@ class PluginServerTest extends PluginServerTestBase {
7178
await channel
7279
.sendRequest(protocol.AnalysisSetContextRootsParams([contextRoot]));
7380

74-
var notifications = StreamQueue(channel.notifications);
75-
var notification = await notifications.next;
76-
var params = protocol.AnalysisErrorsParams.fromNotification(notification);
77-
expect(params.file, convertPath('/package1/lib/test.dart'));
81+
var paramsQueue = StreamQueue(channel.notifications
82+
.map((n) => protocol.AnalysisErrorsParams.fromNotification(n))
83+
.where((p) => p.file == filePath));
84+
var params = await paramsQueue.next;
7885
expect(params.errors, isEmpty);
7986

8087
await channel.sendRequest(protocol.AnalysisUpdateContentParams(
8188
{filePath: protocol.AddContentOverlay('bool b = false;')}));
8289

83-
notification = await notifications.next;
84-
params = protocol.AnalysisErrorsParams.fromNotification(notification);
85-
expect(params.file, convertPath('/package1/lib/test.dart'));
90+
params = await paramsQueue.next;
8691
expect(params.errors, hasLength(1));
8792
_expectAnalysisError(params.errors.single, message: 'No bools message');
8893
}
@@ -92,28 +97,24 @@ class PluginServerTest extends PluginServerTestBase {
9297
await channel
9398
.sendRequest(protocol.AnalysisSetContextRootsParams([contextRoot]));
9499

95-
var notifications = StreamQueue(channel.notifications);
96-
var notification = await notifications.next;
97-
var params = protocol.AnalysisErrorsParams.fromNotification(notification);
98-
expect(params.file, convertPath('/package1/lib/test.dart'));
100+
var paramsQueue = StreamQueue(channel.notifications
101+
.map((n) => protocol.AnalysisErrorsParams.fromNotification(n))
102+
.where((p) => p.file == filePath));
103+
var params = await paramsQueue.next;
99104
expect(params.errors, isEmpty);
100105

101106
await channel.sendRequest(protocol.AnalysisUpdateContentParams(
102107
{filePath: protocol.AddContentOverlay('int b = 0;')}));
103108

104-
notification = await notifications.next;
105-
params = protocol.AnalysisErrorsParams.fromNotification(notification);
106-
expect(params.file, convertPath('/package1/lib/test.dart'));
109+
params = await paramsQueue.next;
107110
expect(params.errors, isEmpty);
108111

109112
await channel.sendRequest(protocol.AnalysisUpdateContentParams({
110113
filePath: protocol.ChangeContentOverlay(
111114
[protocol.SourceEdit(0, 9, 'bool b = false')])
112115
}));
113116

114-
notification = await notifications.next;
115-
params = protocol.AnalysisErrorsParams.fromNotification(notification);
116-
expect(params.file, convertPath('/package1/lib/test.dart'));
117+
params = await paramsQueue.next;
117118
expect(params.errors, hasLength(1));
118119
_expectAnalysisError(params.errors.single, message: 'No bools message');
119120
}
@@ -123,27 +124,23 @@ class PluginServerTest extends PluginServerTestBase {
123124
await channel
124125
.sendRequest(protocol.AnalysisSetContextRootsParams([contextRoot]));
125126

126-
var notifications = StreamQueue(channel.notifications);
127-
var notification = await notifications.next;
128-
var params = protocol.AnalysisErrorsParams.fromNotification(notification);
129-
expect(params.file, convertPath('/package1/lib/test.dart'));
127+
var paramsQueue = StreamQueue(channel.notifications
128+
.map((n) => protocol.AnalysisErrorsParams.fromNotification(n))
129+
.where((p) => p.file == filePath));
130+
var params = await paramsQueue.next;
130131
expect(params.errors, hasLength(1));
131132
_expectAnalysisError(params.errors.single, message: 'No bools message');
132133

133134
await channel.sendRequest(protocol.AnalysisUpdateContentParams(
134135
{filePath: protocol.AddContentOverlay('int b = 7;')}));
135136

136-
notification = await notifications.next;
137-
params = protocol.AnalysisErrorsParams.fromNotification(notification);
138-
expect(params.file, convertPath('/package1/lib/test.dart'));
137+
params = await paramsQueue.next;
139138
expect(params.errors, isEmpty);
140139

141140
await channel.sendRequest(protocol.AnalysisUpdateContentParams(
142141
{filePath: protocol.RemoveContentOverlay()}));
143142

144-
notification = await notifications.next;
145-
params = protocol.AnalysisErrorsParams.fromNotification(notification);
146-
expect(params.file, convertPath('/package1/lib/test.dart'));
143+
params = await paramsQueue.next;
147144
expect(params.errors, hasLength(1));
148145
_expectAnalysisError(params.errors.single, message: 'No bools message');
149146
}
@@ -165,7 +162,7 @@ class PluginServerTest extends PluginServerTestBase {
165162
class _NoBoolsPlugin extends Plugin {
166163
@override
167164
void register(PluginRegistry registry) {
168-
registry.registerRule(NoBoolsRule());
165+
registry.registerWarningRule(NoBoolsRule());
169166
registry.registerFixForRule(NoBoolsRule.code, _WrapInQuotes.new);
170167
}
171168
}

0 commit comments

Comments
 (0)