fix: Fix code navigation for enum values (#220)

With this patch, we reduce divergence from Sorbet by using a zero-length location
for the synthetic classes generated when rewriting enums. We also add a special case
for emitting definitions for enum values, since I wasn't able to figure out a better
way to do the same thing.

rewriter/TEnum.cc

@@ -150,10 +150,7 @@ std::optional<ProcessStatResult> processStat(core::MutableContext ctx, ast::Clas
 
     auto statLocZero = stat.loc().copyWithZeroLength();
     auto name = ctx.state.enterNameConstant(ctx.state.freshNameUnique(core::UniqueNameKind::TEnum, lhs->cnst, 1));
-    // For some reason, Sorbet uses a zero-length range here,
-    // but it seems like we need this for scip-ruby?
-    // https://github.com/sorbet/sorbet/pull/7092
-    auto classCnst = ast::MK::UnresolvedConstant(lhs->loc, ast::MK::EmptyTree(), name);
+    auto classCnst = ast::MK::UnresolvedConstant(statLocZero, ast::MK::EmptyTree(), name);
     ast::ClassDef::ANCESTORS_store parent;
     parent.emplace_back(klass->name.deepCopy());
     ast::ClassDef::RHS_store classRhs;

scip_indexer/SCIPIndexer.cc

@@ -968,14 +968,14 @@ class CFGTraversal final {
             auto symRef = this->aliasMap.try_consume(arg.variable);
             ENFORCE(symRef.has_value());
             auto [namedSym, _] = symRef.value();
-            auto check =
+            auto isDefinition =
                 isMethodFileStaticInit ||
                 (namedSym.kind() != GenericSymbolRef::Kind::Field &&
                  method == gs.lookupStaticInitForClass(namedSym.asSymbolRef().asClassOrModuleRef().data(gs)->owner,
                                                        /*allowMissing*/ true));
             absl::Status status;
             string kind;
-            if (check) {
+            if (isDefinition) {
                 status = this->scipState.saveDefinition(gs, file, namedSym, arg.loc);
                 kind = "definition";
             } else {
@@ -1161,8 +1161,24 @@ class CFGTraversal final {
         //   In this situation, M should count as a reference if we're mimicking RubyMine.
         //   Specifically, Go to Definition for modules seems to go to 'module M' even
         //   when other forms like 'class M::C' are present.
+        bool isMethodClassStaticInit = false;
+        auto methodOwner = method.owner(gs);
+        if (methodOwner.isClassOrModule() && methodOwner.asClassOrModuleRef().exists()) {
+            auto attached = methodOwner.asClassOrModuleRef().data(gs)->attachedClass(gs);
+            if (attached.exists()) {
+                isMethodClassStaticInit = method == gs.lookupStaticInitForClass(attached,
+                                                                                /*allowMissing*/ true);
+            }
+        }
         for (auto &[namedSym, loc] : todo) {
-            auto status = this->scipState.saveReference(ctx, namedSym, nullopt, loc, 0);
+            absl::Status status;
+            if (isMethodClassStaticInit && namedSym.isEnumConstant(gs)) {
+                // Enum constants don't have references in the <static-init> of the owner
+                // class, but they do have alias instructions, so record those as definitions.
+                status = this->scipState.saveDefinition(ctx, file, namedSym, loc);
+            } else {
+                status = this->scipState.saveReference(ctx, namedSym, nullopt, loc, 0);
+            }
             ENFORCE(status.ok(), "status: {}\n", status.message());
         }
     }
@@ -1361,10 +1377,17 @@ class SCIPSemanticExtension : public SemanticExtension {
         if (this->doNothing() || ast::isa_tree<ast::EmptyTree>(klass.name)) {
             return;
         }
-        auto scipState = this->getSCIPState();
+        auto nameLoc = klass.name.loc();
+        // The exists() case is present for defensiveness.
+        // The empty() check is present for enums where the definition generates
+        // a synthetic class with a zero-length location, see TEnum.cc.
+        if (!nameLoc.exists() || nameLoc.empty()) {
+            return;
+        }
 
-        auto status = scipState->saveDefinition(gs, file, scip_indexer::GenericSymbolRef::classOrModule(klass.symbol),
-                                                klass.name.loc());
+        auto scipState = this->getSCIPState();
+        auto status =
+            scipState->saveDefinition(gs, file, scip_indexer::GenericSymbolRef::classOrModule(klass.symbol), nameLoc);
         ENFORCE(status.ok());
         auto *expr = &klass.name;
         if (auto *constantLit = ast::cast_tree<ast::ConstantLit>(*expr)) {

scip_indexer/SCIPSymbolRef.h

@@ -169,6 +169,18 @@ class GenericSymbolRef final {
         return Kind::ClassOrModule;
     }
 
+    bool isEnumConstant(const core::GlobalState &gs) const {
+        if (this->kind() != Kind::Field) {
+            return false;
+        }
+        auto sym = this->selfOrOwner.asClassOrModuleRef();
+        if (!sym.exists()) {
+            return false;
+        }
+        auto super = sym.data(gs)->superClass();
+        return super == core::Symbols::T_Enum();
+    }
+
     UntypedGenericSymbolRef withoutType() const {
         switch (this->kind()) {
             case Kind::Field:

test/scip/testdata/enum.rb

@@ -1,10 +1,25 @@
-# typed: struct
+# typed: strict
 
 class X < T::Enum
   enums do
     A = new("A")
     B = new
+    C = B
   end
 
   All = T.let([A, B], T::Array[X])
 end
+
+# Adding more cases like this is not supported (c.f. isTEnum),
+# but let's at least add a test.
+class Y < X
+  enums do
+    D = new
+    E = B
+  end
+end
+
+def use_abc
+  x = X::A
+  return
+end

test/scip/testdata/enum.snapshot.rb

@@ -1,7 +1,6 @@
- # typed: struct
+ # typed: strict
 
  class X < T::Enum
-#      ^ reference [..] X#
 #      ^ definition [..] X#
 #      ^ definition [..] X#serialize().
 #          ^ reference [..] T#
@@ -10,23 +9,45 @@ class X < T::Enum
 #             ^^^^ reference [..] T#Enum#
    enums do
      A = new("A")
-#    ^ definition local 2~#119448696
-#    ^ definition [..] X#A#
-#    ^ reference [..] X#A#
-#    ^ reference [..] X#A.
+#    ^ definition [..] X#A.
 #        ^^^ reference [..] Class#new().
      B = new
-#    ^ definition local 5~#119448696
-#    ^ definition [..] X#B#
-#    ^ reference [..] X#B#
-#    ^ reference [..] X#B.
+#    ^ definition [..] X#B.
 #        ^^^ reference [..] Class#new().
+     C = B
+#    ^ definition [..] X#C.
+#        ^ reference [..] X#B.
    end
 
    All = T.let([A, B], T::Array[X])
 #  ^^^ definition [..] X#All.
 #               ^ reference [..] X#A.
 #                  ^ reference [..] X#B.
-#                         ^^^^^^^^ definition local 8~#119448696
+#                         ^^^^^^^^ definition local 4~#119448696
 #                               ^ reference [..] X#
  end
+
+ # Adding more cases like this is not supported (c.f. isTEnum),
+ # but let's at least add a test.
+ class Y < X
+#      ^ definition [..] Y#
+#          ^ reference [..] X#
+   enums do
+     D = new
+#    ^ definition [..] Y#D.
+#        ^^^ reference [..] Class#new().
+     E = B
+#    ^ definition [..] Y#E.
+#    ^^^^^ reference [..] Y#E.
+#        ^ reference [..] X#B.
+   end
+ end
+
+ def use_abc
+#    ^^^^^^^ definition [..] Object#use_abc().
+   x = X::A
+#  ^ definition local 1~#1971237871
+#      ^ reference [..] X#
+#         ^ reference [..] X#A.
+   return
+ end