Skip to content

Commit 95d5e62

Browse files
srawlinsCommit Queue
srawlins
authored and
Commit Queue
committed
Tidy LintConfig and RuleConfig
* Each was unnecessarily split into a public abstract class and a private impl class. Remove that separation. * Many functions on `_LintConfig` like `addAsListOrString` could be both static and private. Do that. * Improve the return type of `parseArgs` from `Map<String, dynamic>?` to `Map<String, bool>`. * Tighten many parameter types from `Object` to `YamlNode`. * Make all of `RuleConfig`'s fields final and pass them all as parameters into the constructor. Change-Id: Ia097e8c615f2c2d2e55b280e57dc30185403a9e9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/388960 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent c45243c commit 95d5e62

File tree

1 file changed

+103
-110
lines changed

1 file changed

+103
-110
lines changed

‎pkg/analyzer/lib/src/lint/config.dart‎

Lines changed: 103 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -31,39 +31,51 @@ LintConfig? processAnalysisOptionsFile(String fileContents, {String? fileUrl}) {
3131
}
3232

3333
/// The configuration of lint rules within an analysis options file.
34-
abstract class LintConfig {
35-
factory LintConfig.parse(String source, {String? sourceUrl}) =>
36-
_LintConfig().._parse(source, sourceUrl: sourceUrl);
34+
class LintConfig {
35+
final List<String> fileIncludes;
3736

38-
factory LintConfig.parseMap(YamlMap map) => _LintConfig().._parseYaml(map);
37+
final List<String> fileExcludes;
3938

40-
List<String> get fileExcludes;
41-
List<String> get fileIncludes;
42-
List<RuleConfig> get ruleConfigs;
43-
}
39+
final List<RuleConfig> ruleConfigs;
4440

45-
/// The configuration of a single lint rule within an analysis options file.
46-
abstract class RuleConfig {
47-
Map<String, dynamic> args = <String, dynamic>{};
48-
String? get group;
49-
String? get name;
41+
LintConfig(this.fileIncludes, this.fileExcludes, this.ruleConfigs);
5042

51-
// Provisional
52-
bool disables(String ruleName) =>
53-
ruleName == name && args['enabled'] == false;
43+
factory LintConfig.parse(String source, {String? sourceUrl}) {
44+
var yaml = loadYamlNode(source,
45+
sourceUrl: sourceUrl != null ? Uri.parse(sourceUrl) : null);
46+
if (yaml is! YamlMap) {
47+
throw StateError("Expected YAML at '$source' to be a Map, but is "
48+
'${yaml.runtimeType}.');
49+
}
5450

55-
bool enables(String ruleName) => ruleName == name && args['enabled'] == true;
56-
}
51+
return LintConfig.parseMap(yaml);
52+
}
53+
54+
factory LintConfig.parseMap(YamlMap yaml) {
55+
var fileIncludes = <String>[];
56+
var fileExcludes = <String>[];
57+
var ruleConfigs = <RuleConfig>[];
58+
59+
yaml.nodes.forEach((key, value) {
60+
if (key is! YamlScalar) {
61+
return;
62+
}
63+
switch (key.toString()) {
64+
case 'files':
65+
if (value is YamlMap) {
66+
_addAsListOrString(value['include'], fileIncludes);
67+
_addAsListOrString(value['exclude'], fileExcludes);
68+
}
69+
70+
case 'rules':
71+
ruleConfigs.addAll(_ruleConfigs(value));
72+
}
73+
});
5774

58-
class _LintConfig implements LintConfig {
59-
@override
60-
final fileIncludes = <String>[];
61-
@override
62-
final fileExcludes = <String>[];
63-
@override
64-
final ruleConfigs = <RuleConfig>[];
75+
return LintConfig(fileIncludes, fileExcludes, ruleConfigs);
76+
}
6577

66-
void addAsListOrString(Object? value, List<String> list) {
78+
static void _addAsListOrString(Object? value, List<String> list) {
6779
if (value is List) {
6880
for (var entry in value) {
6981
list.add(entry as String);
@@ -73,105 +85,86 @@ class _LintConfig implements LintConfig {
7385
}
7486
}
7587

76-
bool? asBool(Object scalar) {
88+
static bool? _asBool(YamlNode scalar) {
7789
var value = scalar is YamlScalar ? scalar.valueOrThrow : scalar;
78-
if (value is bool) {
79-
return value;
80-
}
81-
if (value is String) {
82-
if (value == 'true') {
83-
return true;
84-
}
85-
if (value == 'false') {
86-
return false;
87-
}
88-
}
89-
return null;
90+
return switch (value) {
91+
bool() => value,
92+
'true' => true,
93+
'false' => false,
94+
_ => null,
95+
};
9096
}
9197

92-
String? asString(Object scalar) {
98+
static String? _asString(Object scalar) {
9399
var value = scalar is YamlScalar ? scalar.value : scalar;
94-
if (value is String) {
95-
return value;
96-
}
97-
return null;
100+
return value is String ? value : null;
98101
}
99102

100-
Map<String, dynamic>? parseArgs(Object args) {
101-
var enabled = asBool(args);
103+
static Map<String, bool> _parseArgs(YamlNode args) {
104+
var enabled = _asBool(args);
102105
if (enabled != null) {
103106
return {'enabled': enabled};
104107
}
105-
return null;
108+
return {};
106109
}
107110

108-
void _parse(String src, {String? sourceUrl}) {
109-
var yaml = loadYamlNode(src,
110-
sourceUrl: sourceUrl != null ? Uri.parse(sourceUrl) : null);
111-
if (yaml is YamlMap) {
112-
_parseYaml(yaml);
111+
static List<RuleConfig> _ruleConfigs(YamlNode value) {
112+
// For example:
113+
//
114+
// ```yaml
115+
// - unnecessary_getters
116+
// - camel_case_types
117+
// ```
118+
if (value is YamlList) {
119+
return [
120+
for (var rule in value.nodes)
121+
RuleConfig(name: _asString(rule), args: {'enabled': true}),
122+
];
113123
}
114-
}
115-
116-
void _parseYaml(YamlMap yaml) {
117-
yaml.nodes.forEach((k, v) {
118-
if (k is! YamlScalar) {
119-
return;
120-
}
121-
YamlScalar key = k;
122-
switch (key.toString()) {
123-
case 'files':
124-
if (v is YamlMap) {
125-
addAsListOrString(v['include'], fileIncludes);
126-
addAsListOrString(v['exclude'], fileExcludes);
127-
}
128-
129-
case 'rules':
130124

131-
// - unnecessary_getters
132-
// - camel_case_types
133-
if (v is YamlList) {
134-
for (var rule in v.nodes) {
135-
var config = _RuleConfig();
136-
config.name = asString(rule);
137-
config.args = {'enabled': true};
138-
ruleConfigs.add(config);
139-
}
140-
}
125+
// style_guide: {unnecessary_getters: false, camel_case_types: true}
126+
if (value is YamlMap) {
127+
var ruleConfigs = <RuleConfig>[];
128+
value.nodes.cast<Object, YamlNode>().forEach((key, value) {
129+
// For example: `{unnecessary_getters: false}`.
130+
var valueAsBool = _asBool(value);
131+
if (valueAsBool != null) {
132+
ruleConfigs.add(RuleConfig(
133+
name: _asString(key),
134+
args: {'enabled': valueAsBool},
135+
));
136+
}
137+
138+
// style_guide: {unnecessary_getters: false, camel_case_types: true}
139+
if (value is YamlMap) {
140+
value.nodes.cast<Object, YamlNode>().forEach((rule, args) {
141+
// TODO(brianwilkerson): verify format.
142+
// For example: `unnecessary_getters: false`.
143+
ruleConfigs.add(RuleConfig(
144+
name: _asString(rule),
145+
args: _parseArgs(args),
146+
group: _asString(key),
147+
));
148+
});
149+
}
150+
});
151+
return ruleConfigs;
152+
}
141153

142-
// style_guide: {unnecessary_getters: false, camel_case_types: true}
143-
if (v is YamlMap) {
144-
v.nodes.cast<Object, YamlNode>().forEach((key, value) {
145-
//{unnecessary_getters: false}
146-
if (asBool(value) != null) {
147-
var config = _RuleConfig();
148-
config.name = asString(key);
149-
config.args = {'enabled': asBool(value)};
150-
ruleConfigs.add(config);
151-
}
152-
153-
// style_guide: {unnecessary_getters: false, camel_case_types: true}
154-
if (value is YamlMap) {
155-
value.nodes.cast<Object, YamlNode>().forEach((rule, args) {
156-
// TODO(brianwilkerson): verify format
157-
// unnecessary_getters: false
158-
var config = _RuleConfig();
159-
config.group = asString(key);
160-
config.name = asString(rule);
161-
config.args = parseArgs(args) ?? {};
162-
ruleConfigs.add(config);
163-
});
164-
}
165-
});
166-
}
167-
}
168-
});
154+
return const [];
169155
}
170156
}
171157

172-
class _RuleConfig extends RuleConfig {
173-
@override
174-
String? group;
175-
@override
176-
String? name;
158+
/// The configuration of a single lint rule within an analysis options file.
159+
class RuleConfig {
160+
final Map<String, bool> args;
161+
final String? group;
162+
final String? name;
163+
164+
RuleConfig({required this.name, required this.args, this.group});
165+
166+
bool disables(String ruleName) =>
167+
ruleName == name && args['enabled'] == false;
168+
169+
bool enables(String ruleName) => ruleName == name && args['enabled'] == true;
177170
}

0 commit comments

Comments
 (0)