Detailed explanation for T.all type errors (sorbet#7832)

* add error details for T.all * PR comments * more test cases
Shopify · Apr 18, 2024 · 5fe5714 · 5fe5714
1 parent da74886
commit 5fe5714
Show file tree

Hide file tree

Showing 3 changed files with 153 additions and 3 deletions.
diff --git a/core/types/subtyping.cc b/core/types/subtyping.cc
@@ -1449,6 +1449,8 @@ bool Types::isSubType(const GlobalState &gs, const TypePtr &t1, const TypePtr &t
 template <class T>
 bool Types::isSubTypeUnderConstraint(const GlobalState &gs, TypeConstraint &constr, const TypePtr &t1,
                                      const TypePtr &t2, UntypedMode mode, T &errorDetailsCollector) {
+    constexpr auto shouldAddErrorDetails = std::is_same<T, ErrorSection::Collector>::value;
+
     if (t1 == t2) {
         return true;
     }
@@ -1472,8 +1474,29 @@ bool Types::isSubTypeUnderConstraint(const GlobalState &gs, TypeConstraint &cons
     }
 
     if (auto *a2 = cast_type<AndType>(t2)) { // 2, 5
-        return Types::isSubTypeUnderConstraint(gs, constr, t1, a2->left, mode, errorDetailsCollector) &&
-               Types::isSubTypeUnderConstraint(gs, constr, t1, a2->right, mode, errorDetailsCollector);
+        auto subCollectorLeft = errorDetailsCollector.newCollector();
+        auto isSubTypeOfLeft = Types::isSubTypeUnderConstraint(gs, constr, t1, a2->left, mode, subCollectorLeft);
+        if (!isSubTypeOfLeft) {
+            if constexpr (shouldAddErrorDetails) {
+                auto message = ErrorColors::format("`{}` is not a subtype of `{}` (the left side of the `{}`)",
+                                                   t1.show(gs), a2->left.show(gs), "T.all");
+                subCollectorLeft.message = message;
+                errorDetailsCollector.addErrorDetails(move(subCollectorLeft));
+            }
+            return isSubTypeOfLeft;
+        }
+
+        auto subCollectorRight = errorDetailsCollector.newCollector();
+        auto isSubTypeOfRight = Types::isSubTypeUnderConstraint(gs, constr, t1, a2->right, mode, subCollectorRight);
+        if constexpr (shouldAddErrorDetails) {
+            if (!isSubTypeOfRight) {
+                auto message = ErrorColors::format("`{}` is not a subtype of `{}` (the right side of the `{}`)",
+                                                   t1.show(gs), a2->right.show(gs), "T.all");
+                subCollectorRight.message = message;
+                errorDetailsCollector.addErrorDetails(move(subCollectorRight));
+            }
+        }
+        return isSubTypeOfRight;
     }
 
     auto *a1 = cast_type<AndType>(t1);

diff --git a/test/cli/detailed-errors/intersection.rb b/test/cli/detailed-errors/intersection.rb
@@ -0,0 +1,52 @@
+# typed: true
+
+extend T::Sig
+
+module M1
+end
+
+module M2
+end
+
+module M3
+end
+
+module M4
+end
+
+class C
+end
+
+class C1
+  include M1
+end
+
+class C2
+  include M2
+end
+
+class C124
+  include M1
+  include M2
+  include M4
+end
+
+class C123
+  include M1
+  include M2
+  include M3
+end
+
+class C134
+  include M1
+  include M3
+  include M4
+end
+
+T.let(C.new, T.all(M1, M2))
+T.let(C1.new, T.all(M1, M2))
+T.let(C2.new, T.all(M1, M2))
+T.let(C1.new, T.all(M1, M2, M3, M4))
+T.let(C123.new, T.all(M1, M2, M3, M4))
+T.let(C124.new, T.all(M1, M2, M3, M4))
+T.let(C134.new, T.all(M1, M2, M3, M4))
diff --git a/test/cli/detailed-errors/test.out b/test/cli/detailed-errors/test.out
@@ -45,6 +45,81 @@ type_members.rb:21: Argument does not have asserted type `A[Middle, Middle, Midd
     `Lower` is not a supertype of `Middle` for contravariant type member `A::Y`
     `Upper` is not a subtype of `Middle` for covariant type member `A::Z`
 
+intersection.rb:46: Argument does not have asserted type `T.all(M1, M2)` https://srb.help/7007
+    46 |T.let(C.new, T.all(M1, M2))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Got `C` originating from:
+    intersection.rb:46:
+    46 |T.let(C.new, T.all(M1, M2))
+              ^^^^^
+  Detailed explanation:
+    `C` is not a subtype of `M1` (the left side of the `T.all`)
+
+intersection.rb:47: Argument does not have asserted type `T.all(M1, M2)` https://srb.help/7007
+    47 |T.let(C1.new, T.all(M1, M2))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Got `C1` originating from:
+    intersection.rb:47:
+    47 |T.let(C1.new, T.all(M1, M2))
+              ^^^^^^
+  Detailed explanation:
+    `C1` is not a subtype of `M2` (the right side of the `T.all`)
+
+intersection.rb:48: Argument does not have asserted type `T.all(M1, M2)` https://srb.help/7007
+    48 |T.let(C2.new, T.all(M1, M2))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Got `C2` originating from:
+    intersection.rb:48:
+    48 |T.let(C2.new, T.all(M1, M2))
+              ^^^^^^
+  Detailed explanation:
+    `C2` is not a subtype of `M1` (the left side of the `T.all`)
+
+intersection.rb:49: Argument does not have asserted type `T.all(M1, M2, M3, M4)` https://srb.help/7007
+    49 |T.let(C1.new, T.all(M1, M2, M3, M4))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Got `C1` originating from:
+    intersection.rb:49:
+    49 |T.let(C1.new, T.all(M1, M2, M3, M4))
+              ^^^^^^
+  Detailed explanation:
+    `C1` is not a subtype of `T.all(M1, M2, M3)` (the left side of the `T.all`)
+      `C1` is not a subtype of `T.all(M1, M2)` (the left side of the `T.all`)
+        `C1` is not a subtype of `M2` (the right side of the `T.all`)
+
+intersection.rb:50: Argument does not have asserted type `T.all(M1, M2, M3, M4)` https://srb.help/7007
+    50 |T.let(C123.new, T.all(M1, M2, M3, M4))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Got `C123` originating from:
+    intersection.rb:50:
+    50 |T.let(C123.new, T.all(M1, M2, M3, M4))
+              ^^^^^^^^
+  Detailed explanation:
+    `C123` is not a subtype of `M4` (the right side of the `T.all`)
+
+intersection.rb:51: Argument does not have asserted type `T.all(M1, M2, M3, M4)` https://srb.help/7007
+    51 |T.let(C124.new, T.all(M1, M2, M3, M4))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Got `C124` originating from:
+    intersection.rb:51:
+    51 |T.let(C124.new, T.all(M1, M2, M3, M4))
+              ^^^^^^^^
+  Detailed explanation:
+    `C124` is not a subtype of `T.all(M1, M2, M3)` (the left side of the `T.all`)
+      `C124` is not a subtype of `M3` (the right side of the `T.all`)
+
+intersection.rb:52: Argument does not have asserted type `T.all(M1, M2, M3, M4)` https://srb.help/7007
+    52 |T.let(C134.new, T.all(M1, M2, M3, M4))
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Got `C134` originating from:
+    intersection.rb:52:
+    52 |T.let(C134.new, T.all(M1, M2, M3, M4))
+              ^^^^^^^^
+  Detailed explanation:
+    `C134` is not a subtype of `T.all(M1, M2, M3)` (the left side of the `T.all`)
+      `C134` is not a subtype of `T.all(M1, M2)` (the left side of the `T.all`)
+        `C134` is not a subtype of `M2` (the right side of the `T.all`)
+
 shapes.rb:10: Expected `{a: Integer, b: String}` but found `{}` for argument `x` https://srb.help/7002
     10 |takes_shape({})
                     ^^
@@ -200,4 +275,4 @@ tuples.rb:18: Expected `[Integer, String]` but found `[String("")]` for argument
     tuples.rb:18:
     18 |takes_tuple([''])
                     ^^^^
-Errors: 16
+Errors: 23