Skip to content

Commit 06fb470

Browse files
pqCommit Queue
authored andcommitted
[element model] migrate avoid_null_checks_in_equality_operators
This includes an incomplete migration of `canonicalElement`. I *think* it's sufficient for this lint but please check me. (It handles parens minimally.) Looking ahead, some of the complexity in `canonicalElement` may go away in the new world but I'm still wrapping my head around it. (Some relevant context: #35343.) Bug: https://github.com/dart-lang/linter/issues/5099 Change-Id: Ie2ea18f173ae2a95cab0d3b3d5b82442fce6f026 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/391201 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Phil Quitslund <[email protected]>
1 parent cf1f2e5 commit 06fb470

File tree

3 files changed

+40
-10
lines changed

3 files changed

+40
-10
lines changed

pkg/linter/lib/src/extensions.dart

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,21 @@ extension AstNodeNullableExtension on AstNode? {
100100
return null;
101101
}
102102

103+
Element2? get canonicalElement2 {
104+
// TODO(pq): can this be replaced w/ a use of an `ElementLocator2`?
105+
var self = this;
106+
if (self is Expression) {
107+
var node = self.unParenthesized;
108+
if (node is Identifier) {
109+
return node.element;
110+
} else if (node is PropertyAccess) {
111+
// TODO(pq): implement.
112+
return null;
113+
}
114+
}
115+
return null;
116+
}
117+
103118
/// Whether the expression is null-aware, or if one of its recursive targets
104119
/// is null-aware.
105120
bool get containsNullAwareInvocationInChain {

pkg/linter/lib/src/rules/avoid_null_checks_in_equality_operators.dart

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import 'package:analyzer/dart/ast/ast.dart';
66
import 'package:analyzer/dart/ast/token.dart';
77
import 'package:analyzer/dart/ast/visitor.dart';
8-
import 'package:analyzer/dart/element/element.dart';
8+
import 'package:analyzer/dart/element/element2.dart';
99
import 'package:analyzer/dart/element/nullability_suffix.dart';
1010

1111
import '../analyzer.dart';
@@ -16,18 +16,19 @@ const _desc = r"Don't check for `null` in custom `==` operators.";
1616
bool _isComparingEquality(TokenType tokenType) =>
1717
tokenType == TokenType.BANG_EQ || tokenType == TokenType.EQ_EQ;
1818

19-
bool _isComparingParameterWithNull(BinaryExpression node, Element? parameter) =>
19+
bool _isComparingParameterWithNull(
20+
BinaryExpression node, Element2? parameter) =>
2021
_isComparingEquality(node.operator.type) &&
2122
((node.leftOperand.isNullLiteral &&
2223
_isParameter(node.rightOperand, parameter)) ||
2324
(node.rightOperand.isNullLiteral &&
2425
_isParameter(node.leftOperand, parameter)));
2526

26-
bool _isParameter(Expression expression, Element? parameter) =>
27-
expression.canonicalElement == parameter;
27+
bool _isParameter(Expression expression, Element2? parameter) =>
28+
expression.canonicalElement2 == parameter;
2829

2930
bool _isParameterWithQuestionQuestion(
30-
BinaryExpression node, Element? parameter) =>
31+
BinaryExpression node, Element2? parameter) =>
3132
node.operator.type == TokenType.QUESTION_QUESTION &&
3233
_isParameter(node.leftOperand, parameter);
3334

@@ -51,7 +52,7 @@ class AvoidNullChecksInEqualityOperators extends LintRule {
5152
}
5253

5354
class _BodyVisitor extends RecursiveAstVisitor<void> {
54-
final Element? parameter;
55+
final Element2? parameter;
5556
final LintRule rule;
5657

5758
_BodyVisitor(this.parameter, this.rule);
@@ -68,7 +69,7 @@ class _BodyVisitor extends RecursiveAstVisitor<void> {
6869
@override
6970
visitMethodInvocation(MethodInvocation node) {
7071
if (node.operator?.type == TokenType.QUESTION_PERIOD &&
71-
node.target.canonicalElement == parameter) {
72+
node.target.canonicalElement2 == parameter) {
7273
rule.reportLint(node);
7374
}
7475
super.visitMethodInvocation(node);
@@ -77,7 +78,7 @@ class _BodyVisitor extends RecursiveAstVisitor<void> {
7778
@override
7879
visitPropertyAccess(PropertyAccess node) {
7980
if (node.operator.type == TokenType.QUESTION_PERIOD &&
80-
node.target.canonicalElement == parameter) {
81+
node.target.canonicalElement2 == parameter) {
8182
rule.reportLint(node);
8283
}
8384
super.visitPropertyAccess(node);
@@ -100,11 +101,11 @@ class _Visitor extends SimpleAstVisitor<void> {
100101
return;
101102
}
102103

103-
var parameter = parameters.first.declaredElement?.canonicalElement;
104+
var parameter = parameters.first.declaredFragment?.element;
104105

105106
// Analyzer will produce UNNECESSARY_NULL_COMPARISON_FALSE|TRUE
106107
// See: https://github.com/dart-lang/linter/issues/2864
107-
if (parameter is VariableElement &&
108+
if (parameter is FormalParameterElement &&
108109
parameter.type.nullabilitySuffix != NullabilitySuffix.question) {
109110
return;
110111
}

pkg/linter/test/rules/avoid_null_checks_in_equality_operators_test.dart

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,20 @@ class C {
7373
]);
7474
}
7575

76+
test_nullableParameter_eqeqNull_not_parens() async {
77+
await assertDiagnostics(r'''
78+
class C {
79+
String foo = '';
80+
@override
81+
operator ==(Object? other) =>
82+
!((other) == null) && (other) is C && foo == (other.foo);
83+
}
84+
''', [
85+
error(WarningCode.NON_NULLABLE_EQUALS_PARAMETER, 52, 2),
86+
lint(85, 15),
87+
]);
88+
}
89+
7690
test_nullableParameter_fieldComparisonOnLocal() async {
7791
await assertDiagnostics(r'''
7892
class C {

0 commit comments

Comments
 (0)