Skip to content
This repository was archived by the owner on Jul 16, 2023. It is now read-only.

Commit e773d30

Browse files
authored
feat: add static code diagnostic check-for-equals-in-render-object-setters (#1003)
* feat: add static code diagnostic prefer-checking-for-equals-in-render-object-setters * chore: rename the rule * fix: handle not equal check
1 parent 3dd127e commit e773d30

File tree

9 files changed

+383
-0
lines changed

9 files changed

+383
-0
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## Unreleased
44

5+
* feat: add static code diagnostic [`check-for-equals-in-render-object-setters`](https://dartcodemetrics.dev/docs/rules/flutter/check-for-equals-in-render-object-setters).
56
* feat: add static code diagnostic [`avoid-redundant-async`](https://dartcodemetrics.dev/docs/rules/common/avoid-redundant-async).
67
* feat: add static code diagnostic [`prefer-correct-test-file-name`](https://dartcodemetrics.dev/docs/rules/common/prefer-correct-test-file-name).
78
* feat: add static code diagnostic [`prefer-iterable-of`](https://dartcodemetrics.dev/docs/rules/common/prefer-iterable-of).

lib/src/analyzers/lint_analyzer/rules/rules_factory.dart

+3
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import 'rules_list/avoid_unused_parameters/avoid_unused_parameters_rule.dart';
2828
import 'rules_list/avoid_wrapping_in_padding/avoid_wrapping_in_padding_rule.dart';
2929
import 'rules_list/ban_name/ban_name_rule.dart';
3030
import 'rules_list/binary_expression_operand_order/binary_expression_operand_order_rule.dart';
31+
import 'rules_list/check_for_equals_in_render_object_setters/check_for_equals_in_render_object_setters_rule.dart';
3132
import 'rules_list/component_annotation_arguments_ordering/component_annotation_arguments_ordering_rule.dart';
3233
import 'rules_list/double_literal_format/double_literal_format_rule.dart';
3334
import 'rules_list/format_comment/format_comment_rule.dart';
@@ -98,6 +99,8 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
9899
AvoidExpandedAsSpacerRule.ruleId: AvoidExpandedAsSpacerRule.new,
99100
BanNameRule.ruleId: BanNameRule.new,
100101
BinaryExpressionOperandOrderRule.ruleId: BinaryExpressionOperandOrderRule.new,
102+
CheckForEqualsInRenderObjectSettersRule.ruleId:
103+
CheckForEqualsInRenderObjectSettersRule.new,
101104
ComponentAnnotationArgumentsOrderingRule.ruleId:
102105
ComponentAnnotationArgumentsOrderingRule.new,
103106
DoubleLiteralFormatRule.ruleId: DoubleLiteralFormatRule.new,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// ignore_for_file: public_member_api_docs
2+
3+
import 'package:analyzer/dart/ast/ast.dart';
4+
import 'package:analyzer/dart/ast/visitor.dart';
5+
import 'package:analyzer/dart/element/element.dart';
6+
7+
import '../../../../../utils/flutter_types_utils.dart';
8+
import '../../../../../utils/node_utils.dart';
9+
import '../../../lint_utils.dart';
10+
import '../../../models/internal_resolved_unit_result.dart';
11+
import '../../../models/issue.dart';
12+
import '../../../models/severity.dart';
13+
import '../../models/flutter_rule.dart';
14+
import '../../rule_utils.dart';
15+
16+
part 'visitor.dart';
17+
18+
class CheckForEqualsInRenderObjectSettersRule extends FlutterRule {
19+
static const ruleId = 'check-for-equals-in-render-object-setters';
20+
21+
CheckForEqualsInRenderObjectSettersRule([
22+
Map<String, Object> config = const {},
23+
]) : super(
24+
id: ruleId,
25+
severity: readSeverity(config, Severity.warning),
26+
excludes: readExcludes(config),
27+
);
28+
29+
@override
30+
Iterable<Issue> check(InternalResolvedUnitResult source) {
31+
final visitor = _Visitor();
32+
33+
source.unit.visitChildren(visitor);
34+
35+
return visitor.declarations
36+
.map((declaration) => createIssue(
37+
rule: this,
38+
location: nodeLocation(node: declaration.node, source: source),
39+
message: declaration.errorMessage,
40+
))
41+
.toList(growable: false);
42+
}
43+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
part of 'check_for_equals_in_render_object_setters_rule.dart';
2+
3+
class _Visitor extends GeneralizingAstVisitor<void> {
4+
final _declarations = <_DeclarationInfo>[];
5+
6+
Iterable<_DeclarationInfo> get declarations => _declarations;
7+
8+
@override
9+
void visitClassDeclaration(ClassDeclaration node) {
10+
final classType = node.extendsClause?.superclass.type;
11+
if (!isRenderObjectOrSubclass(classType)) {
12+
return;
13+
}
14+
15+
final settersVisitor = _SettersVisitor();
16+
node.visitChildren(settersVisitor);
17+
18+
_declarations.addAll(settersVisitor.declarations);
19+
}
20+
}
21+
22+
class _SettersVisitor extends GeneralizingAstVisitor<void> {
23+
final _declarations = <_DeclarationInfo>[];
24+
25+
Iterable<_DeclarationInfo> get declarations => _declarations;
26+
27+
@override
28+
void visitMethodDeclaration(MethodDeclaration node) {
29+
if (!node.isSetter) {
30+
return;
31+
}
32+
33+
final body = node.body;
34+
35+
var notFirst = false;
36+
37+
if (body is BlockFunctionBody) {
38+
for (final statement in body.block.statements) {
39+
if (statement is IfStatement) {
40+
if (notFirst) {
41+
_declarations.add(_DeclarationInfo(
42+
node,
43+
'Equals check should come first in the block.',
44+
));
45+
46+
return;
47+
}
48+
49+
final returnVisitor = _ReturnVisitor();
50+
statement.visitChildren(returnVisitor);
51+
52+
final condition = statement.condition;
53+
if (condition is BinaryExpression) {
54+
if (!(_usesParameter(condition.leftOperand) ||
55+
_usesParameter(condition.rightOperand) &&
56+
returnVisitor.isValid)) {
57+
_declarations.add(_DeclarationInfo(
58+
node,
59+
'Equals check compares a wrong parameter or has no return statement.',
60+
));
61+
}
62+
}
63+
64+
return;
65+
}
66+
67+
if (statement is! AssertStatement &&
68+
statement is! VariableDeclarationStatement) {
69+
notFirst = true;
70+
}
71+
}
72+
}
73+
74+
if (notFirst) {
75+
_declarations.add(_DeclarationInfo(
76+
node,
77+
'Equals check is missing.',
78+
));
79+
}
80+
}
81+
82+
bool _usesParameter(Expression expression) =>
83+
expression is SimpleIdentifier &&
84+
expression.staticElement is ParameterElement;
85+
}
86+
87+
class _ReturnVisitor extends RecursiveAstVisitor<void> {
88+
bool _hasValidReturn = false;
89+
90+
bool _hasValidAssignment = false;
91+
92+
bool get isValid => _hasValidReturn || _hasValidAssignment;
93+
94+
@override
95+
void visitReturnStatement(ReturnStatement node) {
96+
super.visitReturnStatement(node);
97+
98+
final type = node.expression?.staticType;
99+
100+
if (type == null || type.isVoid) {
101+
_hasValidReturn = true;
102+
}
103+
}
104+
105+
@override
106+
void visitAssignmentExpression(AssignmentExpression node) {
107+
super.visitAssignmentExpression(node);
108+
109+
_hasValidAssignment = true;
110+
}
111+
}
112+
113+
class _DeclarationInfo {
114+
final Declaration node;
115+
final String errorMessage;
116+
117+
const _DeclarationInfo(this.node, this.errorMessage);
118+
}

lib/src/utils/flutter_types_utils.dart

+9
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ bool hasWidgetType(DartType type) =>
1212
bool isWidgetOrSubclass(DartType? type) =>
1313
_isWidget(type) || _isSubclassOfWidget(type);
1414

15+
bool isRenderObjectOrSubclass(DartType? type) =>
16+
_isRenderObject(type) || _isSubclassOfRenderObject(type);
17+
1518
bool isWidgetStateOrSubclass(DartType? type) =>
1619
_isWidgetState(type) || _isSubclassOfWidgetState(type);
1720

@@ -61,3 +64,9 @@ bool _isFuture(DartType type) =>
6164

6265
bool _isListenable(DartType type) =>
6366
type.getDisplayString(withNullability: false) == 'Listenable';
67+
68+
bool _isRenderObject(DartType? type) =>
69+
type?.getDisplayString(withNullability: false) == 'RenderObject';
70+
71+
bool _isSubclassOfRenderObject(DartType? type) =>
72+
type is InterfaceType && type.allSupertypes.any(_isRenderObject);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart';
2+
import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/check_for_equals_in_render_object_setters/check_for_equals_in_render_object_setters_rule.dart';
3+
import 'package:test/test.dart';
4+
5+
import '../../../../../helpers/rule_test_helper.dart';
6+
7+
const _examplePath =
8+
'check_for_equals_in_render_object_setters/examples/example.dart';
9+
10+
void main() {
11+
group('CheckForEqualsInRenderObjectSettersRule', () {
12+
test('initialization', () async {
13+
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
14+
final issues = CheckForEqualsInRenderObjectSettersRule().check(unit);
15+
16+
RuleTestHelper.verifyInitialization(
17+
issues: issues,
18+
ruleId: 'check-for-equals-in-render-object-setters',
19+
severity: Severity.warning,
20+
);
21+
});
22+
23+
test('reports about found issues', () async {
24+
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
25+
final issues = CheckForEqualsInRenderObjectSettersRule().check(unit);
26+
27+
RuleTestHelper.verifyIssues(
28+
issues: issues,
29+
startLines: [29, 37, 61],
30+
startColumns: [3, 3, 3],
31+
locationTexts: [
32+
'set dividerWidth(double value) {\n'
33+
' _dividerWidth = value;\n'
34+
' markNeedsLayout();\n'
35+
' }',
36+
'set dividerHeight(double value) {\n'
37+
' if (_dividerHeight == _dividerWidth) {\n'
38+
' return;\n'
39+
' }\n'
40+
'\n'
41+
' _dividerHeight = value;\n'
42+
' markNeedsLayout();\n'
43+
' }',
44+
'set spacing(double value) {\n'
45+
' _spacing = value;\n'
46+
'\n'
47+
' if (_spacing == value) {\n'
48+
' return;\n'
49+
' }\n'
50+
' markNeedsLayout();\n'
51+
' }',
52+
],
53+
messages: [
54+
'Equals check is missing.',
55+
'Equals check compares a wrong parameter or has no return statement.',
56+
'Equals check should come first in the block.',
57+
],
58+
);
59+
});
60+
});
61+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
class RenderObject {}
2+
3+
class RenderBox extends RenderObject {}
4+
5+
class SomeRenderBox extends RenderBox {
6+
int _page;
7+
int get page => _page;
8+
set page(int value) {
9+
if (value == _page) {
10+
return;
11+
}
12+
_page = value;
13+
markNeedsLayout();
14+
}
15+
16+
double get overflowSpacing => _overflowSpacing;
17+
double _overflowSpacing;
18+
set overflowSpacing(double value) {
19+
assert(value != null);
20+
if (_overflowSpacing == value) return;
21+
22+
_overflowSpacing = value;
23+
markNeedsLayout();
24+
}
25+
26+
double _dividerWidth;
27+
double get dividerWidth => _dividerWidth;
28+
// LINT
29+
set dividerWidth(double value) {
30+
_dividerWidth = value;
31+
markNeedsLayout();
32+
}
33+
34+
double _dividerHeight;
35+
double get dividerHeight => _dividerHeight;
36+
// LINT
37+
set dividerHeight(double value) {
38+
if (_dividerHeight == _dividerWidth) {
39+
return;
40+
}
41+
42+
_dividerHeight = value;
43+
markNeedsLayout();
44+
}
45+
46+
Clip get clipBehavior => _clipBehavior;
47+
Clip _clipBehavior = Clip.none;
48+
set clipBehavior(Clip value) {
49+
assert(value != null);
50+
if (value == _clipBehavior) {
51+
return;
52+
}
53+
_clipBehavior = value;
54+
markNeedsPaint();
55+
markNeedsSemanticsUpdate();
56+
}
57+
58+
double get spacing => _spacing;
59+
double _spacing;
60+
// LINT
61+
set spacing(double value) {
62+
_spacing = value;
63+
64+
if (_spacing == value) {
65+
return;
66+
}
67+
markNeedsLayout();
68+
}
69+
70+
bool get opaque => _opaque;
71+
bool _opaque;
72+
set opaque(bool value) {
73+
if (_opaque != value) {
74+
_opaque = value;
75+
markNeedsPaint();
76+
}
77+
}
78+
79+
void markNeedsLayout() {}
80+
81+
void markNeedsPaint() {}
82+
83+
void markNeedsSemanticsUpdate() {}
84+
}
85+
86+
enum Clip { none }

0 commit comments

Comments
 (0)