lint rule: Remove the avoid_null_checks_in_equality_operators rule
Fixes https://github.com/dart-lang/linter/issues/5063 Change-Id: I85a3c9e1a568d55ce1e21d0f1fee4ce1c83292f4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/389300 Commit-Queue: Samuel Rawlins <srawlins@google.com> Reviewed-by: Phil Quitslund <pquitslund@google.com>
This commit is contained in:
committed by
Commit Queue
parent
89f66223f1
commit
5572900016
@@ -12,7 +12,6 @@ import 'package:analyzer/src/error/codes.g.dart';
|
||||
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
|
||||
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
|
||||
import 'package:analyzer_plugin/utilities/range_factory.dart';
|
||||
import 'package:linter/src/lint_codes.dart';
|
||||
|
||||
class RemoveComparison extends ResolvedCorrectionProducer {
|
||||
@override
|
||||
@@ -51,8 +50,7 @@ class RemoveComparison extends ResolvedCorrectionProducer {
|
||||
return errorCode == WarningCode.UNNECESSARY_NAN_COMPARISON_TRUE ||
|
||||
errorCode == WarningCode.UNNECESSARY_NULL_COMPARISON_ALWAYS_NULL_TRUE ||
|
||||
errorCode == WarningCode.UNNECESSARY_NULL_COMPARISON_NEVER_NULL_TRUE ||
|
||||
errorCode == WarningCode.UNNECESSARY_TYPE_CHECK_TRUE ||
|
||||
errorCode == LinterLintCode.avoid_null_checks_in_equality_operators;
|
||||
errorCode == WarningCode.UNNECESSARY_TYPE_CHECK_TRUE;
|
||||
}
|
||||
|
||||
@override
|
||||
|
||||
@@ -1923,8 +1923,6 @@ LintCode.avoid_js_rounded_ints:
|
||||
status: noFix
|
||||
LintCode.avoid_multiple_declarations_per_line:
|
||||
status: hasFix
|
||||
LintCode.avoid_null_checks_in_equality_operators:
|
||||
status: hasFix
|
||||
LintCode.avoid_positional_boolean_parameters:
|
||||
status: noFix
|
||||
notes: |-
|
||||
|
||||
@@ -313,9 +313,6 @@ final _builtInLintProducers = <LintCode, List<ProducerGenerator>>{
|
||||
LinterLintCode.avoid_multiple_declarations_per_line: [
|
||||
SplitMultipleDeclarations.new,
|
||||
],
|
||||
LinterLintCode.avoid_null_checks_in_equality_operators: [
|
||||
RemoveComparison.new,
|
||||
],
|
||||
LinterLintCode.avoid_print: [
|
||||
MakeConditionalOnDebugMode.new,
|
||||
RemovePrint.new,
|
||||
|
||||
@@ -12,10 +12,10 @@ import 'fix_processor.dart';
|
||||
void main() {
|
||||
defineReflectiveSuite(() {
|
||||
defineReflectiveTests(RemoveComparisonTest);
|
||||
defineReflectiveTests(RemoveTypeCheckTest);
|
||||
defineReflectiveTests(RemoveTypeCheckBulkTest);
|
||||
defineReflectiveTests(RemoveNullCheckComparisonTest);
|
||||
defineReflectiveTests(RemoveNullCheckComparisonBulkTest);
|
||||
defineReflectiveTests(RemoveNullCheckComparisonTest);
|
||||
defineReflectiveTests(RemoveTypeCheckBulkTest);
|
||||
defineReflectiveTests(RemoveTypeCheckTest);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -610,7 +610,7 @@ class Person {
|
||||
final String name = '';
|
||||
|
||||
@override
|
||||
operator ==(Object? other) =>
|
||||
operator ==(Object other) =>
|
||||
other != null &&
|
||||
other is Person &&
|
||||
name == other.name;
|
||||
@@ -620,7 +620,7 @@ class Person2 {
|
||||
final String name = '';
|
||||
|
||||
@override
|
||||
operator ==(Object? other) =>
|
||||
operator ==(Object other) =>
|
||||
other != null &&
|
||||
other is Person &&
|
||||
name == other.name;
|
||||
@@ -631,7 +631,7 @@ class Person {
|
||||
final String name = '';
|
||||
|
||||
@override
|
||||
operator ==(Object? other) =>
|
||||
operator ==(Object other) =>
|
||||
other is Person &&
|
||||
name == other.name;
|
||||
}
|
||||
@@ -640,7 +640,7 @@ class Person2 {
|
||||
final String name = '';
|
||||
|
||||
@override
|
||||
operator ==(Object? other) =>
|
||||
operator ==(Object other) =>
|
||||
other is Person &&
|
||||
name == other.name;
|
||||
}
|
||||
|
||||
@@ -27,7 +27,6 @@ linter:
|
||||
- avoid_init_to_null
|
||||
- avoid_js_rounded_ints
|
||||
- avoid_multiple_declarations_per_line
|
||||
- avoid_null_checks_in_equality_operators
|
||||
- avoid_positional_boolean_parameters
|
||||
- avoid_print
|
||||
- avoid_private_typedef_functions
|
||||
|
||||
@@ -240,13 +240,6 @@ class LinterLintCode extends LintCode {
|
||||
"Try splitting the variable declarations into multiple lines.",
|
||||
);
|
||||
|
||||
static const LintCode avoid_null_checks_in_equality_operators =
|
||||
LinterLintCode(
|
||||
LintNames.avoid_null_checks_in_equality_operators,
|
||||
"Unnecessary null comparison in implementation of '=='.",
|
||||
correctionMessage: "Try removing the comparison.",
|
||||
);
|
||||
|
||||
static const LintCode avoid_positional_boolean_parameters = LinterLintCode(
|
||||
LintNames.avoid_positional_boolean_parameters,
|
||||
"'bool' parameters should be named parameters.",
|
||||
|
||||
@@ -2,113 +2,19 @@
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
import 'package:analyzer/dart/ast/ast.dart';
|
||||
import 'package:analyzer/dart/ast/token.dart';
|
||||
import 'package:analyzer/dart/ast/visitor.dart';
|
||||
import 'package:analyzer/dart/element/element.dart';
|
||||
import 'package:analyzer/dart/element/nullability_suffix.dart';
|
||||
import 'package:pub_semver/pub_semver.dart';
|
||||
|
||||
import '../analyzer.dart';
|
||||
import '../extensions.dart';
|
||||
|
||||
const _desc = r"Don't check for `null` in custom `==` operators.";
|
||||
|
||||
bool _isComparingEquality(TokenType tokenType) =>
|
||||
tokenType == TokenType.BANG_EQ || tokenType == TokenType.EQ_EQ;
|
||||
|
||||
bool _isComparingParameterWithNull(BinaryExpression node, Element? parameter) =>
|
||||
_isComparingEquality(node.operator.type) &&
|
||||
((node.leftOperand.isNullLiteral &&
|
||||
_isParameter(node.rightOperand, parameter)) ||
|
||||
(node.rightOperand.isNullLiteral &&
|
||||
_isParameter(node.leftOperand, parameter)));
|
||||
|
||||
bool _isParameter(Expression expression, Element? parameter) =>
|
||||
expression.canonicalElement == parameter;
|
||||
|
||||
bool _isParameterWithQuestionQuestion(
|
||||
BinaryExpression node, Element? parameter) =>
|
||||
node.operator.type == TokenType.QUESTION_QUESTION &&
|
||||
_isParameter(node.leftOperand, parameter);
|
||||
|
||||
class AvoidNullChecksInEqualityOperators extends LintRule {
|
||||
AvoidNullChecksInEqualityOperators()
|
||||
: super(
|
||||
name: LintNames.avoid_null_checks_in_equality_operators,
|
||||
description: _desc,
|
||||
);
|
||||
name: LintNames.avoid_null_checks_in_equality_operators,
|
||||
description: _desc,
|
||||
state: State.removed(since: Version(3, 7, 0)));
|
||||
|
||||
@override
|
||||
LintCode get lintCode =>
|
||||
LinterLintCode.avoid_null_checks_in_equality_operators;
|
||||
|
||||
@override
|
||||
void registerNodeProcessors(
|
||||
NodeLintRegistry registry, LinterContext context) {
|
||||
var visitor = _Visitor(this);
|
||||
registry.addMethodDeclaration(this, visitor);
|
||||
}
|
||||
}
|
||||
|
||||
class _BodyVisitor extends RecursiveAstVisitor<void> {
|
||||
final Element? parameter;
|
||||
final LintRule rule;
|
||||
|
||||
_BodyVisitor(this.parameter, this.rule);
|
||||
|
||||
@override
|
||||
visitBinaryExpression(BinaryExpression node) {
|
||||
if (_isParameterWithQuestionQuestion(node, parameter) ||
|
||||
_isComparingParameterWithNull(node, parameter)) {
|
||||
rule.reportLint(node);
|
||||
}
|
||||
super.visitBinaryExpression(node);
|
||||
}
|
||||
|
||||
@override
|
||||
visitMethodInvocation(MethodInvocation node) {
|
||||
if (node.operator?.type == TokenType.QUESTION_PERIOD &&
|
||||
node.target.canonicalElement == parameter) {
|
||||
rule.reportLint(node);
|
||||
}
|
||||
super.visitMethodInvocation(node);
|
||||
}
|
||||
|
||||
@override
|
||||
visitPropertyAccess(PropertyAccess node) {
|
||||
if (node.operator.type == TokenType.QUESTION_PERIOD &&
|
||||
node.target.canonicalElement == parameter) {
|
||||
rule.reportLint(node);
|
||||
}
|
||||
super.visitPropertyAccess(node);
|
||||
}
|
||||
}
|
||||
|
||||
class _Visitor extends SimpleAstVisitor<void> {
|
||||
final LintRule rule;
|
||||
|
||||
_Visitor(this.rule);
|
||||
|
||||
@override
|
||||
void visitMethodDeclaration(MethodDeclaration node) {
|
||||
var parameters = node.parameters?.parameters;
|
||||
if (parameters == null) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (node.name.type != TokenType.EQ_EQ || parameters.length != 1) {
|
||||
return;
|
||||
}
|
||||
|
||||
var parameter = parameters.first.declaredElement?.canonicalElement;
|
||||
|
||||
// Analyzer will produce UNNECESSARY_NULL_COMPARISON_FALSE|TRUE
|
||||
// See: https://github.com/dart-lang/linter/issues/2864
|
||||
if (parameter is VariableElement &&
|
||||
parameter.type.nullabilitySuffix != NullabilitySuffix.question) {
|
||||
return;
|
||||
}
|
||||
|
||||
node.body.accept(_BodyVisitor(parameter, rule));
|
||||
}
|
||||
LintCode get lintCode => LinterLintCode.removed_lint;
|
||||
}
|
||||
|
||||
@@ -1455,6 +1455,7 @@ LintCode:
|
||||
addedIn: "2.0"
|
||||
categories: [style]
|
||||
hasPublishedDocs: false
|
||||
removedIn: "3.7"
|
||||
deprecatedDetails: |-
|
||||
**DON'T** check for `null` in custom `==` operators.
|
||||
|
||||
@@ -1482,6 +1483,8 @@ LintCode:
|
||||
operator ==(Object? other) => other is Person && name == other.name;
|
||||
}
|
||||
```
|
||||
|
||||
This rule has been removed.
|
||||
avoid_positional_boolean_parameters:
|
||||
problemMessage: "'bool' parameters should be named parameters."
|
||||
correctionMessage: "Try converting the parameter to a named parameter."
|
||||
|
||||
@@ -43,8 +43,6 @@ import 'avoid_init_to_null_test.dart' as avoid_init_to_null;
|
||||
import 'avoid_js_rounded_ints_test.dart' as avoid_js_rounded_ints;
|
||||
import 'avoid_multiple_declarations_per_line_test.dart'
|
||||
as avoid_multiple_declarations_per_line;
|
||||
import 'avoid_null_checks_in_equality_operators_test.dart'
|
||||
as avoid_null_checks_in_equality_operators;
|
||||
import 'avoid_positional_boolean_parameters_test.dart'
|
||||
as avoid_positional_boolean_parameters;
|
||||
import 'avoid_print_test.dart' as avoid_print;
|
||||
@@ -346,7 +344,6 @@ void main() {
|
||||
avoid_init_to_null.main();
|
||||
avoid_js_rounded_ints.main();
|
||||
avoid_multiple_declarations_per_line.main();
|
||||
avoid_null_checks_in_equality_operators.main();
|
||||
avoid_positional_boolean_parameters.main();
|
||||
avoid_print.main();
|
||||
avoid_private_typedef_functions.main();
|
||||
|
||||
@@ -1,125 +0,0 @@
|
||||
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
import 'package:test_reflective_loader/test_reflective_loader.dart';
|
||||
|
||||
import '../rule_test_support.dart';
|
||||
|
||||
main() {
|
||||
defineReflectiveSuite(() {
|
||||
defineReflectiveTests(AvoidNullChecksInEqualityOperatorsTest);
|
||||
});
|
||||
}
|
||||
|
||||
@reflectiveTest
|
||||
class AvoidNullChecksInEqualityOperatorsTest extends LintRuleTest {
|
||||
@override
|
||||
String get lintRule => LintNames.avoid_null_checks_in_equality_operators;
|
||||
|
||||
test_dynamicParameter_neNull() async {
|
||||
// https://github.com/dart-lang/linter/issues/2864
|
||||
await assertDiagnostics(r'''
|
||||
class C {
|
||||
String foo = '';
|
||||
@override
|
||||
operator ==(dynamic other) {
|
||||
return other != null && other is C && foo == other.foo;
|
||||
}
|
||||
}
|
||||
''', [
|
||||
error(WarningCode.NON_NULLABLE_EQUALS_PARAMETER, 52, 2),
|
||||
]);
|
||||
}
|
||||
|
||||
test_dynamicParameter_propertyAccess() async {
|
||||
await assertDiagnostics(r'''
|
||||
class C {
|
||||
String foo = '';
|
||||
@override
|
||||
operator ==(dynamic other) => other is C && foo == other.foo;
|
||||
}
|
||||
''', [
|
||||
error(WarningCode.NON_NULLABLE_EQUALS_PARAMETER, 52, 2),
|
||||
]);
|
||||
}
|
||||
|
||||
test_nonNullableParameter_neNull() async {
|
||||
// https://github.com/dart-lang/linter/issues/2864
|
||||
await assertDiagnostics(r'''
|
||||
class C {
|
||||
String foo = '';
|
||||
@override
|
||||
operator ==(Object other) {
|
||||
return other != null && other is C && foo == other.foo;
|
||||
}
|
||||
}
|
||||
''', [
|
||||
error(WarningCode.UNNECESSARY_NULL_COMPARISON_NEVER_NULL_TRUE, 88, 7),
|
||||
]);
|
||||
}
|
||||
|
||||
test_nullableParameter_eqeqNull_not() async {
|
||||
await assertDiagnostics(r'''
|
||||
class C {
|
||||
String foo = '';
|
||||
@override
|
||||
operator ==(Object? other) =>
|
||||
!(other == null) && other is C && foo == other.foo;
|
||||
}
|
||||
''', [
|
||||
error(WarningCode.NON_NULLABLE_EQUALS_PARAMETER, 52, 2),
|
||||
lint(85, 13),
|
||||
]);
|
||||
}
|
||||
|
||||
test_nullableParameter_fieldComparisonOnLocal() async {
|
||||
await assertDiagnostics(r'''
|
||||
class C {
|
||||
String foo;
|
||||
C(this.foo);
|
||||
@override
|
||||
operator ==(Object? other) {
|
||||
if (other is C) {
|
||||
var toCompare = other ?? C("");
|
||||
return toCompare.foo == foo;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
}
|
||||
''', [
|
||||
error(WarningCode.NON_NULLABLE_EQUALS_PARAMETER, 62, 2),
|
||||
lint(126, 14),
|
||||
error(StaticWarningCode.DEAD_NULL_AWARE_EXPRESSION, 135, 5),
|
||||
]);
|
||||
}
|
||||
|
||||
test_nullableParameter_neNull() async {
|
||||
await assertDiagnostics(r'''
|
||||
class C {
|
||||
String foo = '';
|
||||
@override
|
||||
operator ==(Object? other) =>
|
||||
other != null && other is C && foo == other.foo;
|
||||
}
|
||||
''', [
|
||||
error(WarningCode.NON_NULLABLE_EQUALS_PARAMETER, 52, 2),
|
||||
lint(83, 13),
|
||||
]);
|
||||
}
|
||||
|
||||
test_nullableParameter_nullAwarePropertyAccess() async {
|
||||
await assertDiagnostics(r'''
|
||||
class C {
|
||||
String foo = '';
|
||||
@override
|
||||
operator ==(Object? other) => other is C && foo == other?.foo;
|
||||
}
|
||||
''', [
|
||||
error(WarningCode.NON_NULLABLE_EQUALS_PARAMETER, 52, 2),
|
||||
lint(94, 10),
|
||||
error(StaticWarningCode.INVALID_NULL_AWARE_OPERATOR, 99, 2),
|
||||
]);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user