diff --git a/example/all.yaml b/example/all.yaml index f96836c1c..40a034ea7 100644 --- a/example/all.yaml +++ b/example/all.yaml @@ -100,6 +100,7 @@ linter: - no_logic_in_create_state - no_runtimeType_toString - non_constant_identifier_names + - non_nullable_equals_parameter - noop_primitive_operations - null_check_on_nullable_type_parameter - null_closures diff --git a/lib/src/rules.dart b/lib/src/rules.dart index 9183371dd..950021fd4 100644 --- a/lib/src/rules.dart +++ b/lib/src/rules.dart @@ -103,6 +103,7 @@ import 'rules/no_leading_underscores_for_local_identifiers.dart'; import 'rules/no_logic_in_create_state.dart'; import 'rules/no_runtimeType_toString.dart'; import 'rules/non_constant_identifier_names.dart'; +import 'rules/non_nullable_equals_parameter.dart'; import 'rules/noop_primitive_operations.dart'; import 'rules/null_check_on_nullable_type_parameter.dart'; import 'rules/null_closures.dart'; @@ -322,6 +323,7 @@ void registerLintRules({bool inTestMode = false}) { ..register(NoDefaultCases()) ..register(NoDuplicateCaseValues()) ..register(NonConstantIdentifierNames()) + ..register(NonNullableEqualsParameter()) ..register(NoLeadingUnderscoresForLibraryPrefixes()) ..register(NoLeadingUnderscoresForLocalIdentifiers()) ..register(NoLogicInCreateState()) diff --git a/lib/src/rules/non_nullable_equals_parameter.dart b/lib/src/rules/non_nullable_equals_parameter.dart new file mode 100644 index 000000000..664a89024 --- /dev/null +++ b/lib/src/rules/non_nullable_equals_parameter.dart @@ -0,0 +1,104 @@ +// Copyright (c) 2022, 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:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; + +import '../analyzer.dart'; + +const _desc = r'The parameter type of `==` operators should be non-nullable.'; + +const _details = r''' +The parameter type of `==` operators should be non-nullable. + +`null` is never passed to `operator ==`, so the parameter type of an `==` +operator override should always be non-nullable. + +**BAD:** +```dart +class C { + String key; + B(this.key); + @override + operator ==(Object? other) => other is C && other.key == key; +} +``` + +**GOOD:** +```dart +class C { + String key; + B(this.key); + @override + operator ==(Object other) => other is C && other.key == key; +} +``` +'''; + +class NonNullableEqualsParameter extends LintRule { + static const LintCode code = LintCode('non_nullable_equals_parameter', + 'The parameter should have a non-nullable type.', + correctionMessage: 'Try using a non-nullable type.'); + + NonNullableEqualsParameter() + : super( + name: 'non_nullable_equals_parameter', + description: _desc, + details: _details, + group: Group.style); + + @override + LintCode get lintCode => code; + + @override + void registerNodeProcessors( + NodeLintRegistry registry, LinterContext context) { + var visitor = _Visitor(this, context); + registry.addMethodDeclaration(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + final LintRule rule; + + final LinterContext context; + + _Visitor(this.rule, this.context); + + @override + void visitMethodDeclaration(MethodDeclaration node) { + if (node.name.type != TokenType.EQ_EQ) { + return; + } + + var parameters = node.parameters; + if (parameters == null) { + return; + } + + if (parameters.parameters.length != 1) { + return; + } + + var parameter = parameters.parameters.first; + var parameterElement = parameter.declaredElement; + + if (parameterElement == null) { + return; + } + + var type = parameterElement.type; + + if (!type.isDartCoreObject && !type.isDynamic) { + // There is no legal way to define a nullable parameter type, which is not + // `dynamic` or `Object?`. + return; + } + + if (context.typeSystem.isNullable(parameterElement.type)) { + rule.reportLint(parameter); + } + } +} diff --git a/test/rules/all.dart b/test/rules/all.dart index d15aada21..6f7ded78a 100644 --- a/test/rules/all.dart +++ b/test/rules/all.dart @@ -51,6 +51,8 @@ import 'missing_whitespace_between_adjacent_strings_test.dart' as missing_whitespace_between_adjacent_strings; import 'non_constant_identifier_names_test.dart' as non_constant_identifier_names; +import 'non_nullable_equals_parameter_test.dart' + as non_nullable_equals_parameter; import 'null_closures_test.dart' as null_closures; import 'omit_local_variable_types_test.dart' as omit_local_variable_types; import 'overridden_fields_test.dart' as overridden_fields; @@ -130,6 +132,7 @@ void main() { literal_only_boolean_expressions.main(); missing_whitespace_between_adjacent_strings.main(); non_constant_identifier_names.main(); + non_nullable_equals_parameter.main(); null_closures.main(); omit_local_variable_types.main(); overridden_fields.main(); diff --git a/test/rules/non_nullable_equals_parameter_test.dart b/test/rules/non_nullable_equals_parameter_test.dart new file mode 100644 index 000000000..331639a64 --- /dev/null +++ b/test/rules/non_nullable_equals_parameter_test.dart @@ -0,0 +1,101 @@ +// Copyright (c) 2022, 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(NonNullableEqualsParameterTest); + }); +} + +@reflectiveTest +class NonNullableEqualsParameterTest extends LintRuleTest { + @override + String get lintRule => 'non_nullable_equals_parameter'; + + test_class_nonNullableParameter() async { + await assertNoDiagnostics(r''' +class C { + bool operator ==(Object other) => other is C; +} +'''); + } + + test_class_nonNullableParameter_inherited() async { + await assertNoDiagnostics(r''' +class C { + bool operator ==(Object other) => other is C; +} +class D extends C { + bool operator ==(other) => other is D; +} +'''); + } + + test_class_nonNullableParameter_narrowedType() async { + await assertNoDiagnostics(r''' +class C { + bool operator ==(covariant int other) => other is C; +} +'''); + } + + test_class_nullableParameter() async { + await assertDiagnostics(r''' +class C { + bool operator ==(Object? other) => other is C; +} +''', [ + lint(29, 13), + ]); + } + + test_class_nullableParameter_dynamic() async { + await assertDiagnostics(r''' +class C { + bool operator ==(dynamic other) => other is C; +} +''', [ + lint(29, 13), + ]); + } + + test_class_nullableParameter_dynamicAndInherited() async { + await assertDiagnostics(r''' +class C { + bool operator ==(dynamic other) => other is C; +} +class D extends C { + bool operator ==(other) => other is D; +} +''', [ + lint(29, 13), + lint(100, 5), + ]); + } + + test_class_nullableParameter_nonObject() async { + await assertDiagnostics(r''' +class C { + bool operator ==(num? other) => false; +} +''', [ + error(CompileTimeErrorCode.INVALID_OVERRIDE, 26, 2), + // No lint. + ]); + } + + test_mixin_nullableParameter() async { + await assertDiagnostics(r''' +mixin M { + bool operator ==(Object? other) => other is M; +} +''', [ + lint(29, 13), + ]); + } +}