Fix crash on mixes_in_class_methods for undeclared (sorbet#5835)

* Fix crash on mixes_in_class_methods for undeclared The crash looked like this: Exception::raise(): Should never happen Backtrace: #3 0xccda75 sorbet::core::ClassOrModule::isClass() #4 0x13de160 sorbet::resolver::(anonymous namespace)::ResolveConstantsWalk::resolveClassMethodsJob<>() #5 0x13d2407 sorbet::resolver::(anonymous namespace)::ResolveConstantsWalk::resolveConstants<>() #6 0x13d0a15 sorbet::resolver::Resolver::run() #7 0xeaf34b sorbet::realmain::pipeline::resolve() #8 0xb1b1ae sorbet::realmain::realmain() #9 0xb15a22 main #10 0x7ffff7c65083 __libc_start_main #11 0xb1592e _start * Fix error message
angellist · May 27, 2022 · 7bc6f67 · 7bc6f67
1 parent eedf4a9
commit 7bc6f67
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 13 deletions.
diff --git a/resolver/resolver.cc b/resolver/resolver.cc
@@ -999,7 +999,6 @@ class ResolveConstantsWalk {
             return;
         }
 
-        auto encounteredError = false;
         for (auto i = 0; i < numPosArgs; ++i) {
             auto &arg = send->getPosArg(i);
             if (arg.isSelfReference()) {
@@ -1019,28 +1018,34 @@ class ResolveConstantsWalk {
                 }
                 continue;
             }
+            auto idLoc = core::Loc(todo.file, id->loc);
             if (!id->symbol.isClassOrModule()) {
-                if (auto e =
-                        gs.beginError(core::Loc(todo.file, id->loc), core::errors::Resolver::InvalidMixinDeclaration)) {
+                if (auto e = gs.beginError(idLoc, core::errors::Resolver::InvalidMixinDeclaration)) {
                     e.setHeader("Argument to `{}` must be statically resolvable to a module", send->fun.show(gs));
                 }
                 continue;
             }
-            if (id->symbol.asClassOrModuleRef().data(gs)->isClass()) {
-                if (auto e =
-                        gs.beginError(core::Loc(todo.file, id->loc), core::errors::Resolver::InvalidMixinDeclaration)) {
+            auto idSymbol = id->symbol.asClassOrModuleRef();
+            if (idSymbol.data(gs)->isUndeclared()) {
+                if (auto e = gs.beginError(idLoc, core::errors::Resolver::InvalidMixinDeclaration)) {
+                    e.setHeader("`{}` is declared implicitly, but must be defined as a `{}` explicitly",
+                                id->symbol.show(gs), "module");
+                    e.addErrorLine(idSymbol.data(gs)->loc(), "Defined implicitly here");
+                    e.addErrorNote("`{}` has the potential to be a `{}`, which is not allowed with `{}`",
+                                   id->symbol.show(gs), "class", send->fun.show(gs));
+                }
+                continue;
+            }
+            if (idSymbol.data(gs)->isClass()) {
+                if (auto e = gs.beginError(idLoc, core::errors::Resolver::InvalidMixinDeclaration)) {
                     e.setHeader("`{}` is a class, not a module; Only modules may be mixins", id->symbol.show(gs));
                 }
-                encounteredError = true;
+                continue;
             }
             if (id->symbol == owner) {
-                if (auto e =
-                        gs.beginError(core::Loc(todo.file, id->loc), core::errors::Resolver::InvalidMixinDeclaration)) {
+                if (auto e = gs.beginError(idLoc, core::errors::Resolver::InvalidMixinDeclaration)) {
                     e.setHeader("Must not pass your self to `{}`", send->fun.show(gs));
                 }
-                encounteredError = true;
-            }
-            if (encounteredError) {
                 continue;
             }
 
@@ -1059,7 +1064,7 @@ class ResolveConstantsWalk {
                     arg.flags.isBlock = true;
                 }
 
-                auto type = core::make_type<core::ClassType>(id->symbol.asClassOrModuleRef());
+                auto type = core::make_type<core::ClassType>(idSymbol);
                 auto &elems = (core::cast_type<core::TupleType>(mixMethod.data(gs)->resultType))->elems;
                 // Make sure we are not adding existing symbols to our tuple
                 if (absl::c_find(elems, type) == elems.end()) {

diff --git a/test/testdata/resolver/mixes_in_class_module_not_set.rb b/test/testdata/resolver/mixes_in_class_module_not_set.rb
@@ -0,0 +1,7 @@
+# typed: strict
+
+module A::B
+  extend T::Helpers
+
+  mixes_in_class_methods(A) # error: `A` is declared implicitly, but must be defined as a `module` explicitly
+end