Skip to content
This repository was archived by the owner on Nov 20, 2024. It is now read-only.

Commit 2e1800e

Browse files
domesticmousedomesticmouse
authored
Lint for SizedBox.shrink(...) and SizedBox.expand(...) (#3039)
* Start of a lint for `SizedBox.shrink(...)` and `SizedBox.expand(...)` * Copyright year * Add test cases * double.infinity -> double.INFINITY * next step * next step * Add single backquotes to diagnostic messages * Add description * use static LintCodes * tidyup * Check for positional arguments * Define use case for SizedBox.shrink and SizedBox.expand * Drop the SimpleAstVisitor * Dart language tags and backticks * Add in PrefixedIdentifier for double * Move `// LINT` comment to the right line * double.INFINITY -> double.infinity * Updated to reflect reality * Introduce local var for argument.name.label
1 parent 7058a80 commit 2e1800e

File tree

7 files changed

+261
-22
lines changed

7 files changed

+261
-22
lines changed

‎example/all.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ linter:
147147
- recursive_getters
148148
- require_trailing_commas
149149
- sized_box_for_whitespace
150+
- sized_box_shrink_expand
150151
- slash_for_doc_comments
151152
- sort_child_properties_last
152153
- sort_constructors_first

‎lib/src/rules.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ import 'rules/public_member_api_docs.dart';
151151
import 'rules/recursive_getters.dart';
152152
import 'rules/require_trailing_commas.dart';
153153
import 'rules/sized_box_for_whitespace.dart';
154+
import 'rules/sized_box_shrink_expand.dart';
154155
import 'rules/slash_for_doc_comments.dart';
155156
import 'rules/sort_child_properties_last.dart';
156157
import 'rules/sort_constructors_first.dart';
@@ -350,6 +351,7 @@ void registerLintRules({bool inTestMode = false}) {
350351
..register(RecursiveGetters())
351352
..register(RequireTrailingCommas())
352353
..register(SizedBoxForWhitespace())
354+
..register(SizedBoxShrinkExpand())
353355
..register(SlashForDocComments())
354356
..register(SortChildPropertiesLast())
355357
..register(SortConstructorsFirst())

‎lib/src/rules/sized_box_for_whitespace.dart

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -74,40 +74,43 @@ class _Visitor extends SimpleAstVisitor {
7474
return;
7575
}
7676

77-
var visitor = _WidthOrHeightArgumentVisitor();
78-
node.visitChildren(visitor);
79-
if (visitor.seenIncompatibleParams) {
77+
var data = _ArgumentData(node.argumentList);
78+
79+
if (data.incompatibleParamsFound || data.positionalArgumentFound) {
8080
return;
8181
}
82-
if (visitor.seenChild && (visitor.seenWidth || visitor.seenHeight) ||
83-
visitor.seenWidth && visitor.seenHeight) {
82+
if (data.seenChild && (data.seenWidth || data.seenHeight) ||
83+
data.seenWidth && data.seenHeight) {
8484
rule.reportLint(node.constructorName);
8585
}
8686
}
8787
}
8888

89-
class _WidthOrHeightArgumentVisitor extends SimpleAstVisitor<void> {
90-
var seenWidth = false;
91-
var seenHeight = false;
92-
var seenChild = false;
93-
var seenIncompatibleParams = false;
94-
95-
@override
96-
void visitArgumentList(ArgumentList node) {
97-
for (var name in node.arguments
98-
.cast<NamedExpression>()
99-
.map((arg) => arg.name.label.name)) {
100-
if (name == 'width') {
89+
class _ArgumentData {
90+
_ArgumentData(ArgumentList node) {
91+
for (var argument in node.arguments) {
92+
if (argument is! NamedExpression) {
93+
positionalArgumentFound = true;
94+
return;
95+
}
96+
var label = argument.name.label;
97+
if (label.name == 'width') {
10198
seenWidth = true;
102-
} else if (name == 'height') {
99+
} else if (label.name == 'height') {
103100
seenHeight = true;
104-
} else if (name == 'child') {
101+
} else if (label.name == 'child') {
105102
seenChild = true;
106-
} else if (name == 'key') {
107-
// key doesn't matter (both SiezdBox and Container have it)
103+
} else if (label.name == 'key') {
104+
// key doesn't matter (both SizedBox and Container have it)
108105
} else {
109-
seenIncompatibleParams = true;
106+
incompatibleParamsFound = true;
110107
}
111108
}
112109
}
110+
111+
var incompatibleParamsFound = false;
112+
var positionalArgumentFound = false;
113+
var seenWidth = false;
114+
var seenHeight = false;
115+
var seenChild = false;
113116
}

‎lib/src/rules/sized_box_shrink_expand.dart

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:analyzer/dart/ast/ast.dart';
6+
import 'package:analyzer/dart/ast/visitor.dart';
7+
8+
import '../analyzer.dart';
9+
import '../util/flutter_utils.dart';
10+
11+
const _details =
12+
r'''Use `SizedBox.shrink(...)` and `SizedBox.expand(...)` constructors appropriately.
13+
14+
The `SizedBox.shrink(...)` and `SizedBox.expand(...)` constructors should be used
15+
instead of the more general `SizedBox(...)` constructor when the named constructors
16+
capture the intent of the code more succinctly.
17+
18+
**Examples**
19+
20+
**BAD:**
21+
```dart
22+
Widget buildLogo() {
23+
return SizedBox(
24+
height: 0,
25+
width: 0,
26+
child:const MyLogo(),
27+
);
28+
}
29+
```
30+
31+
```dart
32+
Widget buildLogo() {
33+
return SizedBox(
34+
height: double.infinity,
35+
width: double.infinity,
36+
child:const MyLogo(),
37+
);
38+
}
39+
```
40+
41+
**GOOD:**
42+
```dart
43+
Widget buildLogo() {
44+
return SizedBox.shrink(
45+
child:const MyLogo(),
46+
);
47+
}
48+
```
49+
50+
```dart
51+
Widget buildLogo() {
52+
return SizedBox.expand(
53+
child:const MyLogo(),
54+
);
55+
}
56+
```
57+
''';
58+
59+
class SizedBoxShrinkExpand extends LintRule {
60+
SizedBoxShrinkExpand()
61+
: super(
62+
name: 'sized_box_shrink_expand',
63+
description: 'Use SizedBox shrink and expand named constructors.',
64+
details: _details,
65+
group: Group.style);
66+
67+
@override
68+
void registerNodeProcessors(
69+
NodeLintRegistry registry, LinterContext context) {
70+
var visitor = _Visitor(this);
71+
72+
registry.addInstanceCreationExpression(this, visitor);
73+
}
74+
}
75+
76+
class _Visitor extends SimpleAstVisitor {
77+
final SizedBoxShrinkExpand rule;
78+
79+
_Visitor(this.rule);
80+
81+
static const LintCode useShrink = LintCode(
82+
'sized_box_shrink_expand', 'Use the `SizedBox.shrink` constructor.');
83+
static const LintCode useExpand = LintCode(
84+
'sized_box_shrink_expand', 'Use the `SizedBox.expand` constructor.');
85+
86+
@override
87+
void visitInstanceCreationExpression(InstanceCreationExpression node) {
88+
// Only interested in the default constructor for the SizedBox widget
89+
if (!isExactWidgetTypeSizedBox(node.staticType) ||
90+
node.constructorName.name != null) {
91+
return;
92+
}
93+
94+
var data = _ArgumentData(node.argumentList);
95+
if (data.positionalArgumentFound) {
96+
return;
97+
}
98+
if (data.width == 0 && data.height == 0) {
99+
rule.reportLint(node.constructorName, errorCode: useShrink);
100+
} else if (data.width == double.infinity &&
101+
data.height == double.infinity) {
102+
rule.reportLint(node.constructorName, errorCode: useExpand);
103+
}
104+
}
105+
}
106+
107+
class _ArgumentData {
108+
_ArgumentData(ArgumentList node) {
109+
for (var argument in node.arguments) {
110+
if (argument is! NamedExpression) {
111+
positionalArgumentFound = true;
112+
return;
113+
}
114+
var label = argument.name.label;
115+
if (label.name == 'width') {
116+
width = _argumentValue(argument.expression);
117+
} else if (label.name == 'height') {
118+
height = _argumentValue(argument.expression);
119+
}
120+
}
121+
}
122+
123+
double? _argumentValue(Expression argument) {
124+
if (argument is IntegerLiteral) {
125+
return argument.value?.toDouble();
126+
} else if (argument is DoubleLiteral) {
127+
return argument.value;
128+
} else if (argument is PrefixedIdentifier &&
129+
argument.identifier.name == 'infinity' &&
130+
argument.prefix.name == 'double') {
131+
return double.infinity;
132+
}
133+
return null;
134+
}
135+
136+
var positionalArgumentFound = false;
137+
double? width;
138+
double? height;
139+
}

‎lib/src/util/flutter_utils.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ bool isExactWidget(ClassElement element) => _flutter.isExactWidget(element);
3131
bool isExactWidgetTypeContainer(DartType? type) =>
3232
_flutter.isExactWidgetTypeContainer(type);
3333

34+
bool isExactWidgetTypeSizedBox(DartType? type) =>
35+
_flutter.isExactWidgetTypeSizedBox(type);
36+
3437
bool isKDebugMode(Element? element) => _flutter.isKDebugMode(element);
3538

3639
bool isStatefulWidget(ClassElement? element) =>
@@ -56,16 +59,19 @@ class _Flutter {
5659
static const _nameStatefulWidget = 'StatefulWidget';
5760
static const _nameWidget = 'Widget';
5861
static const _nameContainer = 'Container';
62+
static const _nameSizedBox = 'SizedBox';
5963

6064
final String packageName;
6165
final String widgetsUri;
6266

67+
final Uri _uriBasic;
6368
final Uri _uriContainer;
6469
final Uri _uriFramework;
6570
final Uri _uriFoundation;
6671

6772
_Flutter(this.packageName, String uriPrefix)
6873
: widgetsUri = '$uriPrefix/widgets.dart',
74+
_uriBasic = Uri.parse('$uriPrefix/src/widgets/basic.dart'),
6975
_uriContainer = Uri.parse('$uriPrefix/src/widgets/container.dart'),
7076
_uriFramework = Uri.parse('$uriPrefix/src/widgets/framework.dart'),
7177
_uriFoundation = Uri.parse('$uriPrefix/src/foundation/constants.dart');
@@ -99,6 +105,10 @@ class _Flutter {
99105
type is InterfaceType &&
100106
_isExactWidget(type.element, _nameContainer, _uriContainer);
101107

108+
bool isExactWidgetTypeSizedBox(DartType? type) =>
109+
type is InterfaceType &&
110+
_isExactWidget(type.element, _nameSizedBox, _uriBasic);
111+
102112
bool isKDebugMode(Element? element) =>
103113
element != null &&
104114
element.name == 'kDebugMode' &&

‎test_data/mock_packages/flutter/lib/src/widgets/basic.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,14 @@ class Row extends Flex {
122122
});
123123
}
124124

125+
class SizedBox extends SingleChildRenderObjectWidget {
126+
const SizedBox({Key? key, double? width, double? height, Widget? child});
127+
const SizedBox.expand({Key? key, Widget? child});
128+
const SizedBox.shrink({Key? key, Widget? child});
129+
SizedBox.fromSize({Key? key, Widget? child, Size? size});
130+
const SizedBox.square({Key? key, Widget? child, double? dimension});
131+
}
132+
125133
class Transform extends SingleChildRenderObjectWidget {
126134
const Transform({
127135
Key key,

‎test_data/rules/sized_box_shrink_expand.dart

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// test w/ `dart test -N sized_box_shrink_expand`
6+
7+
import 'package:flutter/foundation.dart';
8+
import 'package:flutter/widgets.dart';
9+
10+
Widget sizedBoxWithZeroWidthZeroHeight() {
11+
return SizedBox( // LINT
12+
height: 0,
13+
width: 0,
14+
child: Container(),
15+
);
16+
}
17+
18+
Widget sizedBoxWithInfiniteWidthInfiniteHeight() {
19+
return SizedBox( // LINT
20+
height: double.infinity,
21+
width: double.infinity,
22+
child: Container(),
23+
);
24+
}
25+
26+
Widget sizedBoxWithInfiniteWidthzeroHeight() {
27+
return SizedBox( // OK
28+
height: 0,
29+
width: double.infinity,
30+
child: Container(),
31+
);
32+
}
33+
34+
Widget sizedBoxWithZeroWidthInfiniteHeight() {
35+
return SizedBox( // OK
36+
height: double.infinity,
37+
width: 0,
38+
child: Container(),
39+
);
40+
}
41+
42+
Widget sizedBoxWithMixedWidthsAndHeights() {
43+
return SizedBox( // OK
44+
height: 26,
45+
width: 42,
46+
child: Container(),
47+
);
48+
}
49+
50+
Widget sizedBoxWithZeroWidth() {
51+
return SizedBox( // OK
52+
width: 0,
53+
child: Container(),
54+
);
55+
}
56+
57+
Widget sizedBoxWithInfiniteWidth() {
58+
return SizedBox( // OK
59+
width: double.infinity,
60+
child: Container(),
61+
);
62+
}
63+
64+
Widget sizedBoxWithZeroHeight() {
65+
return SizedBox( // OK
66+
height: 0,
67+
child: Container(),
68+
);
69+
}
70+
71+
Widget sizedBoxWithInfiniteHeight() {
72+
return SizedBox( // OK
73+
height: double.infinity,
74+
child: Container(),
75+
);
76+
}

0 commit comments

Comments
 (0)