Skip to content

Remove avoid_null_checks_in_equality_operators from recommended #829

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
srawlins opened this issue Aug 15, 2024 · 6 comments
Closed

Remove avoid_null_checks_in_equality_operators from recommended #829

srawlins opened this issue Aug 15, 2024 · 6 comments

Comments

@srawlins
Copy link
Member

We introduced a new Warning called NON_NULLABLE_EQUALS_PARAMETER a few releases ago. It warns when the parameter of an operator == override has a nullable type:

"The parameter type of '==' operators should be non-nullable."

I didn't realize it at the time, but that new warning, plus null safety, basically replace the avoid_null_checks_in_equality_operators lint rule. This rule reports doing any null-check work on a nullable parameter of an operator == override.

As I found out when I migrated the tests from the legacy framework, the rule is a bit non-sensical now because of the redundancy. Every test case either has a WarningCode.NON_NULLABLE_EQUALS_PARAMETER, if it features a nullable parameter, or it features another warning like WarningCode.UNNECESSARY_NULL_COMPARISON_TRUE or StaticWarningCode.INVALID_NULL_AWARE_OPERATOR, if the parameter is non-nullable and is compared to null.

@devoncarew
Copy link
Member

cc @natebosch, @goderbauer, and @lrhn for thoughts

@lrhn
Copy link
Member

lrhn commented Sep 4, 2024

Are there exceptions to the NON_NULLABLE_EQUALS_PARAMETER warning. For example of the parameter type is neither nullable nor non-nullable?

class C<T extends C<T>?> {
  bool operator ==(covariant T o) => ...
}

where T is potentially nuallble. Or even

extension type Maybe(Object? o) { 
  bool check(C me) { ... }
}

class C {
  bool operator ==(covariant Maybe other) => other.check(me);
}

where you won't get a warning, and you can't really provide a non-potentially nullable type with the same effect.
In that case, the author may feel inclined to do an == null check. (On the other hand, that's such a convoluted case, that I doubt anyone will hit it, and if they do, I'm not sure I want to help them.)

Probably fine.

@natebosch
Copy link
Member

Removing SGTM.

@devoncarew
Copy link
Member

resolution: sgtm to remove

@srawlins
Copy link
Member Author

srawlins commented Sep 5, 2024

Are there exceptions to the NON_NULLABLE_EQUALS_PARAMETER warning. For example of the parameter type is neither nullable nor non-nullable?

I get "invalid override" for this example, and for the extension type example; it seems even with the "covariant" keyword, these are illegal? But no "NON_NULLABLE_EQUALS_PARAMETER" warning. Given this code:

class C<T extends C<T>?> {
  bool operator ==(covariant T o) => false;
}

class D {
  bool operator ==(covariant int? o) => false;
}

void main() {}

both CFE and analyzer report that C.== and D.== are invalid. NON_NULLABLE_EQUALS_PARAMETER is only reported when the parameter type is Object? or dynamic.

@devoncarew
Copy link
Member

closed by dart-archive/lints#201

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
Archived in project
Development

No branches or pull requests

5 participants