Skip to content

For unrelated_type_equality_checks (and other rules), how are extension types related to other things? #59299

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

Open
srawlins opened this issue Sep 14, 2023 · 1 comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-new-language-feature linter-set-core P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@srawlins
Copy link
Member

Here's a very simple example:

extension type ET(int it) {}

extension type ETnum(int it) implements num {}

void m1(int? a, ET b) {
  a == b;
  b == a;
}

void m2(int? a, ETnum b) {
  a == b;
  b == a;
}

An ET can be equal to an int?, and vice versa. An ETnum can be equal to a int?, and vice versa. It seems like they should be considered "related."

Similarly an extension type should be considered "related" to other extension types that it "implements."

One other big lint rule that this affects is collection_methods_unrelated_type:

extension type ET(int it) {}

extension type ETnum(int it) implements num {}

bool m1(List<int> list, ET a) => list.contains(a);
bool m2(List<ET> list, int a) => list.contains(a);
bool m3(List<int> list, ETnum a) => list.contains(a);
bool m4(List<ETnum> list, int a) => list.contains(a);

Should all of the code here be allowed? I think so...

So more formally, given a type T and an extension type V1 declared as extension type V1(T it) implements V2, V3, ...:

  • V1 is related to T.
  • T is related to V1.
  • V1 is related to V2, etc.
  • V2, etc. are related to V1.

But there might be something more necessary, like "V1 is related to any type which T is related to." Then if V1 is an extension type on int, it is related to num.

I'll note that the algorithm for "relatedness" is not super well-defined, and is pretty ad hoc, but these are important linter rules, and the existing relatedness algorithm has gotten us very far.

CC @pq @jacob314 @eernstg @lrhn

@eernstg
Copy link
Member

eernstg commented Sep 15, 2023

I do recognize that it would be possible to just ignore the extension type entirely, and do the test based on the representation type.

If the goal is to detect situations where two objects are compared for equality, but they are unlikely to be equal then this approach would be the most permissive. "We won't lint anything that might work" (OK, everything might work, essentially, but you know what I mean ;-).

However, I'm not convinced that this is the most useful approach. The point is that extension types can be used to make a compile-time distinction between different ways to use objects with the same run time type. For example:

extension type Meters(double value) {}
extension type Feet(double value) {}

extension Length on double {
  Meters get meter => Meters(this);
  Feet get feet => Feet(this);
}

void main() {
  print(1.5.meter == 1.5.feet); // 'true'!
}

If we don't lint the situation where a Meters and a Feet are compared for equality, we will certainly be able to get 'true' in some cases (so we haven't "wasted" the comparison), but it's still likely to be a bug that those two objects were compared in the first place.

Anyone who does want to compare the underlying representation objects could just say that: myMeters.value == myFeet.value.

I don't know exactly how the decision is made about which types are related and which ones are not, but I suspect that it would be useful to simply rely on the subtyping relationships of extension types, and ignore the representation type.

This means that we will accept fancyInt == 42 without a lint if fancyInt has static type FancyInt:

extension type FancyInt(int value) implements int {
  bool isPrime => ...;
  ...
}

In this case the author of the extension type indicated that it should be possible to assign a FancyInt to an int. In other words, no attempt has been made to protect a FancyInt against being seen as a plain int, and hence it's fine to compare it to another int.

We might have several different types like this (FancyInt and OtherFancyInt, both implements int), and we might want to consider those types as related, but I assume this will be handled in a useful manner by using the existing checks for this lint because we can create the same kind of subtyping situation using regular classes.

@bwilkerson bwilkerson added P2 A bug or feature request we're likely to work on linter-new-language-feature labels Sep 18, 2023
@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Apr 3, 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 20, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 20, 2024
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-new-language-feature linter-set-core P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants