Skip to content
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

Remove avoid_null_checks_in_equality_operators from recommended #200

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

Remove avoid_null_checks_in_equality_operators from recommended #200

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 #201

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

4 participants