[linter] Avoid one-off classes in flutter-related box rules
Change-Id: Ie0d8d2f66eb2c5ab5a4664eeaa588c623a395a65
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/371786
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Sam Rawlins <srawlins@google.com>
Auto-Submit: Parker Lougheed <parlough@gmail.com>
diff --git a/pkg/linter/lib/src/rules/sized_box_for_whitespace.dart b/pkg/linter/lib/src/rules/sized_box_for_whitespace.dart
index 23bcbf2..9c3265d 100644
--- a/pkg/linter/lib/src/rules/sized_box_for_whitespace.dart
+++ b/pkg/linter/lib/src/rules/sized_box_for_whitespace.dart
@@ -72,35 +72,6 @@
}
}
-class _ArgumentData {
- var incompatibleParamsFound = false;
-
- var positionalArgumentFound = false;
- var seenWidth = false;
- var seenHeight = false;
- var seenChild = false;
- _ArgumentData(ArgumentList node) {
- for (var argument in node.arguments) {
- if (argument is! NamedExpression) {
- positionalArgumentFound = true;
- return;
- }
- var label = argument.name.label;
- if (label.name == 'width') {
- seenWidth = true;
- } else if (label.name == 'height') {
- seenHeight = true;
- } else if (label.name == 'child') {
- seenChild = true;
- } else if (label.name == 'key') {
- // key doesn't matter (both SizedBox and Container have it)
- } else {
- incompatibleParamsFound = true;
- }
- }
- }
-}
-
class _Visitor extends SimpleAstVisitor {
final LintRule rule;
@@ -112,14 +83,39 @@
return;
}
- var data = _ArgumentData(node.argumentList);
-
- if (data.incompatibleParamsFound || data.positionalArgumentFound) {
- return;
- }
- if (data.seenChild && (data.seenWidth || data.seenHeight) ||
- data.seenWidth && data.seenHeight) {
+ if (_shouldReportForArguments(node.argumentList)) {
rule.reportLint(node.constructorName);
}
}
+
+ /// Determine if the lint [rule] should be reported for
+ /// the specified [argumentList].
+ static bool _shouldReportForArguments(ArgumentList argumentList) {
+ var hasChild = false;
+ var hasHeight = false;
+ var hasWidth = false;
+
+ for (var argument in argumentList.arguments) {
+ if (argument is! NamedExpression) {
+ // Positional arguments are not supported.
+ return false;
+ }
+ switch (argument.name.label.name) {
+ case 'child':
+ hasChild = true;
+ case 'height':
+ hasHeight = true;
+ case 'width':
+ hasWidth = true;
+ case 'key':
+ // Ignore 'key' as both SizedBox and Container have it.
+ break;
+ case _:
+ // Other named arguments are not supported.
+ return false;
+ }
+ }
+
+ return hasChild && (hasWidth || hasHeight) || hasWidth && hasHeight;
+ }
}
diff --git a/pkg/linter/lib/src/rules/sized_box_shrink_expand.dart b/pkg/linter/lib/src/rules/sized_box_shrink_expand.dart
index b0b22ac..6588d07 100644
--- a/pkg/linter/lib/src/rules/sized_box_shrink_expand.dart
+++ b/pkg/linter/lib/src/rules/sized_box_shrink_expand.dart
@@ -85,29 +85,6 @@
}
}
-class _ArgumentData {
- var positionalArgumentFound = false;
-
- double? width;
-
- double? height;
-
- _ArgumentData(ArgumentList node) {
- for (var argument in node.arguments) {
- if (argument is! NamedExpression) {
- positionalArgumentFound = true;
- return;
- }
- var label = argument.name.label;
- if (label.name == 'width') {
- width = argument.expression.argumentValue;
- } else if (label.name == 'height') {
- height = argument.expression.argumentValue;
- }
- }
- }
-}
-
class _Visitor extends SimpleAstVisitor {
final SizedBoxShrinkExpand rule;
@@ -121,10 +98,11 @@
return;
}
- var data = _ArgumentData(node.argumentList);
- if (data.positionalArgumentFound) {
+ var data = _analyzeArguments(node.argumentList);
+ if (data == null) {
return;
}
+
if (data.width == 0 && data.height == 0) {
rule.reportLint(node.constructorName, arguments: ['shrink']);
} else if (data.width == double.infinity &&
@@ -132,6 +110,30 @@
rule.reportLint(node.constructorName, arguments: ['expand']);
}
}
+
+ /// Determine the value of the arguments specified in the [argumentList],
+ /// and return `null` if there are unsupported arguments.
+ static ({double? height, double? width})? _analyzeArguments(
+ ArgumentList argumentList) {
+ double? height;
+ double? width;
+
+ for (var argument in argumentList.arguments) {
+ if (argument is! NamedExpression) {
+ // Positional arguments are not supported.
+ return null;
+ }
+
+ switch (argument.name.label.name) {
+ case 'width':
+ width = argument.expression.argumentValue;
+ case 'height':
+ height = argument.expression.argumentValue;
+ }
+ }
+
+ return (height: height, width: width);
+ }
}
extension on Expression {
diff --git a/pkg/linter/lib/src/rules/use_colored_box.dart b/pkg/linter/lib/src/rules/use_colored_box.dart
index 77849f4..9550deb 100644
--- a/pkg/linter/lib/src/rules/use_colored_box.dart
+++ b/pkg/linter/lib/src/rules/use_colored_box.dart
@@ -62,34 +62,6 @@
}
}
-class _ArgumentData {
- var positionalArgumentsFound = false;
- var additionalArgumentsFound = false;
- var hasColor = false;
- var hasChild = false;
-
- _ArgumentData(ArgumentList node) {
- for (var argument in node.arguments) {
- if (argument is! NamedExpression) {
- positionalArgumentsFound = true;
- return;
- }
- var label = argument.name.label;
- if (label.name == 'color' &&
- argument.staticType?.nullabilitySuffix !=
- NullabilitySuffix.question) {
- hasColor = true;
- } else if (label.name == 'child') {
- hasChild = true;
- } else if (label.name == 'key') {
- // Ignore key
- } else {
- additionalArgumentsFound = true;
- }
- }
- }
-}
-
class _Visitor extends SimpleAstVisitor {
final LintRule rule;
@@ -101,14 +73,38 @@
return;
}
- var data = _ArgumentData(node.argumentList);
-
- if (data.additionalArgumentsFound || data.positionalArgumentsFound) {
- return;
- }
-
- if (data.hasChild && data.hasColor) {
+ if (_shouldReportForArguments(node.argumentList)) {
rule.reportLint(node.constructorName);
}
}
+
+ /// Determine if the lint [rule] should be reported for
+ /// the specified [argumentList].
+ static bool _shouldReportForArguments(ArgumentList argumentList) {
+ var hasChild = false;
+ var hasColor = false;
+
+ for (var argument in argumentList.arguments) {
+ if (argument is! NamedExpression) {
+ // Positional arguments are not supported.
+ return false;
+ }
+ switch (argument.name.label.name) {
+ case 'child':
+ hasChild = true;
+ case 'color'
+ when argument.staticType?.nullabilitySuffix !=
+ NullabilitySuffix.question:
+ hasColor = true;
+ case 'key':
+ // Ignore 'key' as both ColoredBox and Container have it.
+ break;
+ case _:
+ // Other named arguments are not supported.
+ return false;
+ }
+ }
+
+ return hasChild && hasColor;
+ }
}
diff --git a/pkg/linter/lib/src/rules/use_decorated_box.dart b/pkg/linter/lib/src/rules/use_decorated_box.dart
index dce1bd9..5676524 100644
--- a/pkg/linter/lib/src/rules/use_decorated_box.dart
+++ b/pkg/linter/lib/src/rules/use_decorated_box.dart
@@ -72,32 +72,6 @@
}
}
-class _ArgumentData {
- var positionalArgumentsFound = false;
- var additionalArgumentsFound = false;
- var hasDecoration = false;
- var hasChild = false;
-
- _ArgumentData(ArgumentList node) {
- for (var argument in node.arguments) {
- if (argument is! NamedExpression) {
- positionalArgumentsFound = true;
- return;
- }
- var label = argument.name.label;
- if (label.name == 'decoration') {
- hasDecoration = true;
- } else if (label.name == 'child') {
- hasChild = true;
- } else if (label.name == 'key') {
- // Ignore key
- } else {
- additionalArgumentsFound = true;
- }
- }
- }
-}
-
class _Visitor extends SimpleAstVisitor {
final LintRule rule;
@@ -109,14 +83,36 @@
return;
}
- var data = _ArgumentData(node.argumentList);
-
- if (data.additionalArgumentsFound || data.positionalArgumentsFound) {
- return;
- }
-
- if (data.hasChild && data.hasDecoration) {
+ if (_shouldReportForArguments(node.argumentList)) {
rule.reportLint(node.constructorName);
}
}
+
+ /// Determine if the lint [rule] should be reported for
+ /// the specified [argumentList].
+ static bool _shouldReportForArguments(ArgumentList argumentList) {
+ var hasChild = false;
+ var hasDecoration = false;
+
+ for (var argument in argumentList.arguments) {
+ if (argument is! NamedExpression) {
+ // Positional arguments are not supported.
+ return false;
+ }
+ switch (argument.name.label.name) {
+ case 'child':
+ hasChild = true;
+ case 'decoration':
+ hasDecoration = true;
+ case 'key':
+ // Ignore 'key' as both DecoratedBox and Container have it.
+ break;
+ case _:
+ // Other named arguments are not supported.
+ return false;
+ }
+ }
+
+ return hasChild && hasDecoration;
+ }
}
