Skip to content

proposal: avoid_nullable_equals_parameter #58753

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

Closed
natebosch opened this issue Jun 2, 2022 · 8 comments
Closed

proposal: avoid_nullable_equals_parameter #58753

natebosch opened this issue Jun 2, 2022 · 8 comments
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-lint-proposal linter-status-pending P2 A bug or feature request we're likely to work on

Comments

@natebosch
Copy link
Member

Description

"The null value will never be passed to operator ==. Use a non-nullable Object parameter"

Details

A definition like bool operator ==(Object? other) causes other classes which override the interface to also accept a nullable argument, even though null will never be passed.

Kind

Style

Good Examples

  @override
  bool operator ==(Object other) =>

Bad Examples

  @override
  bool operator ==(Object? other) =>

Discussion

Mockito codegen currently fails to generate fakes for classes with a nullable argument to operator ==.

The main motivator is that the code is misleading - null can't show up so it's not worth making it look like it can.

@natebosch
Copy link
Member Author

This could perhaps supersede avoid_null_checks_in_equality_operators

If I understand correctly that lint only fires when the argument is nullable.

@natebosch
Copy link
Member Author

There is one place where using Object? is useful, it allows promotion to a generic type when the bound on the generic is also Object?. If the argument is Object then we can't promote to a generic type with no bound.

Example:

class C<T> {
  bool checkStuff(T other) => true;

  @override
  bool operator ==(Object other) => other is T && checkStuff(other);
  //                                                         ^^^^^
  // The argument type 'Object' can't be assigned to the parameter type 'T'.
}

I think it's probably better to find a different pattern for the body than to widen the argument type to get promotion.

@natebosch
Copy link
Member Author

natebosch commented Jun 13, 2022

Could also flag no argument dynamic as an unnecessary widening as well.

google/protobuf.dart#675

@srawlins
Copy link
Member

+100. One question:

"The null value will never be passed to operator ==. Use a non-nullable Object parameter"

Does this fact come from spec? Or just existing behavior of today's backends?

@bwilkerson
Copy link
Member

It's from the spec (the third bullet item):

17.27 Equality equality
Equality expressions test objects for equality.
...
Evaluation of an equality expression ee of the form e1 == e2 proceeds as follows:
• The expression e1 is evaluated to an object o1.
• The expression e2 is evaluated to an object o2.
• If either o1 or o2 is the null object (17.4), then ee evaluates to true if both o1 and o2 are the null object and to false otherwise. Otherwise,
• evaluation of ee is equivalent to the method invocation o1.==(o2).

@srawlins
Copy link
Member

Some flutter customer tests violate this so the change is being reverted. #51038

srawlins referenced this issue in srawlins/flutter_portal Jan 17, 2023
srawlins referenced this issue in flutter/cocoon Jan 17, 2023
Make operator == override parameters no-nullable. See dart-lang/linter#3441

Work towards dart-lang/sdk#51038.
srawlins referenced this issue in srawlins/flutter_reactive_ble Jan 17, 2023
@srawlins srawlins reopened this Jan 18, 2023
sfshaza2 referenced this issue in dart-lang/site-www Jan 18, 2023
auto-submit bot referenced this issue in flutter/cocoon Jan 18, 2023
* == override parameters are non-nullable

Make operator == override parameters no-nullable. See dart-lang/linter#3441

Work towards dart-lang/sdk#51038.

* Update fake_google_account.dart
@pq pq removed this from the Dart 3 beta 1 milestone Feb 2, 2023
@pq pq added this to the Dart 3 beta 2 milestone Feb 2, 2023
Taym95 referenced this issue in PhilipsHue/flutter_reactive_ble Feb 7, 2023
* Make operator == override parameters no-nullable. See dart-lang/linter#3441

Work towards dart-lang/sdk#51038.
@pq pq modified the milestones: Dart 3 beta 2, Dart 3 beta 3 Mar 13, 2023
Taym95 referenced this issue in PhilipsHue/flutter_reactive_ble Jul 6, 2023
* Make operator == override parameters no-nullable. See dart-lang/linter#3441

Work towards dart-lang/sdk#51038.
copybara-service bot referenced this issue Dec 14, 2023
…pe""

This reverts commit 885457e.

[analyzer] new warning for nullable '==' parameter type

This rule checks that a parameter to an `operator ==` implementation has
a non-nullable type.

I intentionally did not enforce, in this rule, that the parameter is
exactly `Object`. It is legal to narrow the parameter type to a
different non-nullable type, like `int`. I can't imagine doing it, but
it seems to be unrelated to whether the type should be nullable or not.

Fixes https://github.com/dart-lang/linter/issues/3441

Replaces dart-archive/linter#3923

Change-Id: Ic0be2bfebaf59b0336e9a3a58e5b7f5359eb8646
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291042
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Stephen Adams <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
@mateusfccp
Copy link
Contributor

Wasn't this solved by https://dart-review.googlesource.com/c/sdk/+/277700?

@srawlins
Copy link
Member

srawlins commented Mar 7, 2024

Yes, sorry. Landed, reverted, then re-landed with aad9e57. That commit message says "Fixes #58753" but that doesn't work cross-repos.

@srawlins srawlins closed this as completed Mar 7, 2024
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-lint-proposal linter-status-pending P2 A bug or feature request we're likely to work on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants