Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: No warning if the number of elements in the spread operator using the if statement is more than two. #74

Merged
merged 16 commits into from
Nov 13, 2024
Merged
37 changes: 35 additions & 2 deletions packages/altive_lints/example/lints/avoid_single_child.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class MyWidget extends StatelessWidget {

@override
Widget build(BuildContext context) {
final isValued = Random().nextBool();
final random = Random();
return ListView(
children: [
Expand Down Expand Up @@ -59,7 +60,6 @@ class MyWidget extends StatelessWidget {
if (random.nextBool()) const Text('Hello World'),
],
),
// expect_lint: avoid_single_child
Column(
children: [
if (random.nextBool())
Expand All @@ -70,14 +70,47 @@ class MyWidget extends StatelessWidget {
],
],
),
// expect_lint: avoid_single_child
Column(
children: [
if(isValued)...[
Container(),
]else...[
Container(),
],
],
),
// expect_lint: avoid_single_child
Column(
children: [
if(isValued)...[
Container(),
],
],
),
Column(
children: [
if(isValued)...[
Container(),
Container(),
],
],
),
Column(
children: [
if(isValued)...[
Container(),
],
Container(),
],
),
Column(
children: random.nextBool()
? [
const Text('Hello World'),
]
: [
const Text('Hello'),
const Text('World'),
],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

badge
This is not the subject of this Issue, but this pattern has one element each, so lint is the expected value.

),
if (random.nextBool())
Expand Down
19 changes: 19 additions & 0 deletions packages/altive_lints/lib/src/lints/avoid_single_child.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,18 @@ class AvoidSingleChild extends DartLintRule {
if (childrenList.elements.length != 1) {
return;
}
for (final element in childrenList.elements) {
if (element is IfElement) {
if (_hasSingleChild(element.thenElement)) {
return;
}

if (element.elseElement != null &&
_hasSingleChild(element.elseElement!)) {
return;
}
}
}
final element = childrenList.elements.first;
if (element is ForElement) {
return;
Expand All @@ -85,4 +96,12 @@ class AvoidSingleChild extends DartLintRule {
}
});
}

bool _hasSingleChild(CollectionElement element) {
if (element is SpreadElement && element.expression is ListLiteral) {
final spreadElement = element.expression as ListLiteral;
return spreadElement.elements.length != 1;
boywithdv marked this conversation as resolved.
Show resolved Hide resolved
}
Copy link
Member

@riscait riscait Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

badge
@boywithdv
What does the name _hasSingleChild mean when length != 1, what does the name _hasSingleChild mean?It seems intuitive if length == 1...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, at a quick glance, it seems that the function name does not match the actual situation.

The function name _hasSingleChild seems to return true in the case of a single element, so the function name should be changed to _hasMultipleChild to make it clear that it returns true when there are multiple child elements.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Names are important. Make sure you name it with proper thought and self-review to make sure it is correct.

return false;
}
}