From d039b2d12c3f52d32d73cd891b501ec1c505ae91 Mon Sep 17 00:00:00 2001
From: Varun Gandhi <varun.gandhi@sourcegraph.com>
Date: Tue, 20 Sep 2022 11:44:31 +0800
Subject: [PATCH 1/9] cleanup: Move debug logging code to separate file.

---
 scip_indexer/BUILD          |  1 +
 scip_indexer/Debug.cc       | 24 ++++++++++++++++++++++++
 scip_indexer/Debug.h        | 25 +++++++++++++++++++++++--
 scip_indexer/SCIPIndexer.cc | 23 -----------------------
 4 files changed, 48 insertions(+), 25 deletions(-)
 create mode 100644 scip_indexer/Debug.cc

diff --git a/scip_indexer/BUILD b/scip_indexer/BUILD
index c43a4875a..95904f878 100644
--- a/scip_indexer/BUILD
+++ b/scip_indexer/BUILD
@@ -28,6 +28,7 @@ cc_library(
 cc_library(
     name = "scip_indexer",
     srcs = [
+        "Debug.cc",
         "Debug.h",
         "SCIPIndexer.cc",
     ],
diff --git a/scip_indexer/Debug.cc b/scip_indexer/Debug.cc
new file mode 100644
index 000000000..864afc810
--- /dev/null
+++ b/scip_indexer/Debug.cc
@@ -0,0 +1,24 @@
+
+#include <string>
+
+#include "core/GlobalState.h"
+#include "core/Loc.h"
+
+#include "scip_indexer/Debug.h"
+
+namespace sorbet::scip_indexer {
+
+void _log_debug(const sorbet::core::GlobalState &gs, sorbet::core::Loc loc, std::string s) {
+    if (auto e = gs.beginError(loc, SCIPRubyDebug)) {
+        auto lines = absl::StrSplit(s, '\n');
+        for (auto line = lines.begin(); line != lines.end(); line++) {
+            auto text = std::string(line->begin(), line->length());
+            if (line == lines.begin()) {
+                e.setHeader("[scip-ruby] {}", text);
+            } else {
+                e.addErrorNote("{}", text);
+            }
+        }
+    }
+}
+} // namespace sorbet::scip_indexer
diff --git a/scip_indexer/Debug.h b/scip_indexer/Debug.h
index b69ca828a..215f98875 100644
--- a/scip_indexer/Debug.h
+++ b/scip_indexer/Debug.h
@@ -1,8 +1,14 @@
-#include <ostream>
+#ifndef SORBET_SCIP_DEBUG
+#define SORBET_SCIP_DEBUG
+
+#include <sstream>
 #include <string>
 #include <vector>
 
+#include "absl/strings/str_split.h"
+
 #include "common/common.h"
+#include "core/Error.h"
 
 template <typename K, typename V, typename Fn> std::string map_to_string(const sorbet::UnorderedMap<K, V> m, Fn f) {
     std::ostringstream out;
@@ -45,4 +51,19 @@ template <typename T, typename Fn> std::string vec_to_string(const std::vector<T
     }
     out << "]";
     return out.str();
-}
\ No newline at end of file
+}
+
+namespace sorbet::scip_indexer {
+
+constexpr sorbet::core::ErrorClass SCIPRubyDebug{400, sorbet::core::StrictLevel::False};
+
+void _log_debug(const sorbet::core::GlobalState &gs, sorbet::core::Loc loc, std::string s);
+} // namespace sorbet::scip_indexer
+
+#ifndef NDEBUG
+#define LOG_DEBUG(__gs, __loc, __s) sorbet::scip_indexer::_log_debug(__gs, __loc, __s)
+#else
+#define LOG_DEBUG(__gs, __s)
+#endif
+
+#endif // SORBET_SCIP_DEBUG
\ No newline at end of file
diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc
index 9e0e213af..cc1a6ced0 100644
--- a/scip_indexer/SCIPIndexer.cc
+++ b/scip_indexer/SCIPIndexer.cc
@@ -58,29 +58,6 @@ static uint32_t fnv1a_32(const string &s) {
 
 namespace sorbet::scip_indexer {
 
-constexpr sorbet::core::ErrorClass SCIPRubyDebug{400, sorbet::core::StrictLevel::False};
-
-void _log_debug(const sorbet::core::GlobalState &gs, sorbet::core::Loc loc, std::string s) {
-    if (auto e = gs.beginError(loc, SCIPRubyDebug)) {
-        auto lines = absl::StrSplit(s, '\n');
-        for (auto line = lines.begin(); line != lines.end(); line++) {
-            auto text = string(line->begin(), line->length());
-            if (line == lines.begin()) {
-                e.setHeader("[scip-ruby] {}", text);
-            } else {
-                e.addErrorNote("{}", text);
-            }
-        }
-    }
-}
-
-#ifndef NDEBUG
-#define LOG_DEBUG(__gs, __loc, __s) _log_debug(__gs, __loc, __s)
-#else
-#define LOG_DEBUG(__gs, __s) \
-    {}
-#endif
-
 // TODO(varun): This is an inline workaround for https://github.com/sorbet/sorbet/issues/5925
 // I've not changed the main definition because I didn't bother to rerun the tests with the change.
 static bool isTemporary(const core::GlobalState &gs, const core::LocalVariable &var) {

From 2bc7af82e76e5a45644a461d74c6e0720af78347 Mon Sep 17 00:00:00 2001
From: Varun Gandhi <varun.gandhi@sourcegraph.com>
Date: Tue, 20 Sep 2022 12:10:25 +0800
Subject: [PATCH 2/9] cleanup: Create separate type UntypedGenericSymbolRef.

---
 scip_indexer/BUILD            |   2 +
 scip_indexer/SCIPIndexer.cc   | 114 ++++++----------------------------
 scip_indexer/SCIPSymbolRef.cc |  81 ++++++++++++++++++++++++
 scip_indexer/SCIPSymbolRef.h  |  86 +++++++++++++++++++++++++
 4 files changed, 188 insertions(+), 95 deletions(-)
 create mode 100644 scip_indexer/SCIPSymbolRef.cc
 create mode 100644 scip_indexer/SCIPSymbolRef.h

diff --git a/scip_indexer/BUILD b/scip_indexer/BUILD
index 95904f878..ddfe832fe 100644
--- a/scip_indexer/BUILD
+++ b/scip_indexer/BUILD
@@ -31,6 +31,8 @@ cc_library(
         "Debug.cc",
         "Debug.h",
         "SCIPIndexer.cc",
+        "SCIPSymbolRef.cc",
+        "SCIPSymbolRef.h",
     ],
     linkstatic = select({
         "@com_stripe_ruby_typer//tools/config:linkshared": 0,
diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc
index cc1a6ced0..d2d905fb9 100644
--- a/scip_indexer/SCIPIndexer.cc
+++ b/scip_indexer/SCIPIndexer.cc
@@ -34,6 +34,7 @@
 #include "sorbet_version/sorbet_version.h"
 
 #include "scip_indexer/Debug.h"
+#include "scip_indexer/SCIPSymbolRef.h"
 #include "scip_indexer/SCIPUtils.h"
 
 using namespace std;
@@ -91,32 +92,6 @@ struct OwnedLocal {
     }
 };
 
-class GemMetadata final {
-    string _name;
-    string _version;
-
-    GemMetadata(string name, string version) : _name(name), _version(version) {}
-
-public:
-    GemMetadata &operator=(const GemMetadata &) = default;
-
-    static GemMetadata tryParseOrDefault(string metadata) {
-        vector<string> v = absl::StrSplit(metadata, '@');
-        if (v.size() != 2 || v[0].empty() || v[1].empty()) {
-            return GemMetadata{"TODO", "TODO"};
-        }
-        return GemMetadata{v[0], v[1]};
-    }
-
-    const string &name() const {
-        return this->_name;
-    }
-
-    const string &version() const {
-        return this->_version;
-    }
-};
-
 bool isSorbetInternal(const core::GlobalState &gs, core::SymbolRef sym) {
     UnorderedSet<core::SymbolRef> visited;
     auto classT = core::Symbols::T().data(gs)->lookupSingletonClass(gs);
@@ -179,6 +154,7 @@ class NamedSymbolRef final {
                 ENFORCE(!n.exists());
                 return;
             case Kind::UndeclaredField:
+                ENFORCE(s.isClassOrModule());
                 ENFORCE(n.exists());
                 return;
             case Kind::Method:
@@ -239,6 +215,18 @@ class NamedSymbolRef final {
         return Kind::ClassOrModule;
     }
 
+    UntypedGenericSymbolRef withoutType() const {
+        switch (this->kind()) {
+            case Kind::UndeclaredField:
+                ENFORCE(this->selfOrOwner.isClassOrModule());
+                return UntypedGenericSymbolRef::undeclared(this->selfOrOwner.asClassOrModuleRef(), this->name);
+            case Kind::Method:
+            case Kind::ClassOrModule:
+            case Kind::DeclaredField:
+                return UntypedGenericSymbolRef::declared(this->selfOrOwner);
+        }
+    }
+
     /// Display a NamedSymbolRef for debugging.
     string showRaw(const core::GlobalState &gs) const {
         switch (this->kind()) {
@@ -332,70 +320,6 @@ class NamedSymbolRef final {
 #undef CHECK_TYPE
     }
 
-    // Try to compute a scip::Symbol for this NamedSymbolRef.
-    absl::Status symbolForExpr(const core::GlobalState &gs, const GemMetadata &metadata, optional<core::Loc> loc,
-                               scip::Symbol &symbol) const {
-        // Don't set symbol.scheme and package.manager here because
-        // those are hard-coded to 'scip-ruby' and 'gem' anyways.
-        scip::Package package;
-        package.set_name(metadata.name());
-        package.set_version(metadata.version());
-        *symbol.mutable_package() = move(package);
-
-        InlinedVector<scip::Descriptor, 4> descriptors;
-        auto cur = this->selfOrOwner;
-        while (cur != core::Symbols::root()) {
-            // NOTE(varun): The current scheme will cause multiple 'definitions' for the same
-            // entity if it is present in different files, because the path is not encoded
-            // in the descriptor whose parent is the root. This matches the semantics of
-            // RubyMine, but we may want to revisit this if it is problematic for classes
-            // that are extended in lots of places.
-            scip::Descriptor descriptor;
-            *descriptor.mutable_name() = cur.name(gs).show(gs);
-            ENFORCE(!descriptor.name().empty());
-            // TODO(varun): Are the scip descriptor kinds correct?
-            switch (cur.kind()) {
-                case core::SymbolRef::Kind::Method:
-                    // NOTE(varun): There is a separate isOverloaded field in the flags field,
-                    // despite SO/docs saying that Ruby doesn't support method overloading,
-                    // Technically, we should better understand how this works and set the
-                    // disambiguator based on that. However, right now, an extension's
-                    // type-checking function is not run if a method is overloaded,
-                    // (see pipeline.cc), so it's unclear if we need to care about that.
-                    descriptor.set_suffix(scip::Descriptor::Method);
-                    break;
-                case core::SymbolRef::Kind::ClassOrModule:
-                    descriptor.set_suffix(scip::Descriptor::Type);
-                    break;
-                case core::SymbolRef::Kind::TypeArgument:
-                    descriptor.set_suffix(scip::Descriptor::TypeParameter);
-                    break;
-                case core::SymbolRef::Kind::FieldOrStaticField:
-                    descriptor.set_suffix(scip::Descriptor::Term);
-                    break;
-                case core::SymbolRef::Kind::TypeMember: // TODO: What does TypeMember mean?
-                    descriptor.set_suffix(scip::Descriptor::Type);
-                    break;
-                default:
-                    return absl::InvalidArgumentError("unexpected expr type for symbol computation");
-            }
-            descriptors.push_back(move(descriptor));
-            cur = cur.owner(gs);
-        }
-        while (!descriptors.empty()) {
-            *symbol.add_descriptors() = move(descriptors.back());
-            descriptors.pop_back();
-        }
-        if (this->name != core::NameRef::noName()) {
-            scip::Descriptor descriptor;
-            descriptor.set_suffix(scip::Descriptor::Term);
-            *descriptor.mutable_name() = this->name.shortName(gs);
-            ENFORCE(!descriptor.name().empty());
-            *symbol.add_descriptors() = move(descriptor);
-        }
-        return absl::OkStatus();
-    }
-
     core::Loc symbolLoc(const core::GlobalState &gs) const {
         switch (this->kind()) {
             case Kind::Method: {
@@ -461,7 +385,7 @@ using OccurrenceCache = UnorderedMap<pair<core::LocOffsets, /*SymbolRole*/ int32
 /// The states are implicitly merged at the time of emitting the index.
 class SCIPState {
     string symbolScratchBuffer;
-    UnorderedMap<NamedSymbolRef, string> symbolStringCache;
+    UnorderedMap<UntypedGenericSymbolRef, string> symbolStringCache;
 
     /// Cache of occurrences for locals that have been emitted in this function.
     ///
@@ -518,7 +442,7 @@ class SCIPState {
     /// If the returned value is as success, the pointer is non-null.
     ///
     /// The argument symbol is used instead of recomputing from scratch if it is non-null.
-    absl::StatusOr<const string *> saveSymbolString(const core::GlobalState &gs, NamedSymbolRef symRef,
+    absl::StatusOr<const string *> saveSymbolString(const core::GlobalState &gs, UntypedGenericSymbolRef symRef,
                                                     const scip::Symbol *symbol) {
         auto pair = this->symbolStringCache.find(symRef);
         if (pair != this->symbolStringCache.end()) {
@@ -673,11 +597,11 @@ class SCIPState {
 
         auto occLoc = loc.has_value() ? core::Loc(file, loc.value()) : symRef.symbolLoc(gs);
         scip::Symbol symbol;
-        auto status = symRef.symbolForExpr(gs, this->gemMetadata, occLoc, symbol);
+        auto status = symRef.withoutType().symbolForExpr(gs, this->gemMetadata, occLoc, symbol);
         if (!status.ok()) {
             return status;
         }
-        absl::StatusOr<const string *> valueOrStatus(this->saveSymbolString(gs, symRef, &symbol));
+        absl::StatusOr<const string *> valueOrStatus(this->saveSymbolString(gs, symRef.withoutType(), &symbol));
         if (!valueOrStatus.ok()) {
             return valueOrStatus.status();
         }
@@ -718,7 +642,7 @@ class SCIPState {
         }
         auto &gs = ctx.state;
         auto file = ctx.file;
-        absl::StatusOr<const string *> valueOrStatus(this->saveSymbolString(gs, symRef, nullptr));
+        absl::StatusOr<const string *> valueOrStatus(this->saveSymbolString(gs, symRef.withoutType(), nullptr));
         if (!valueOrStatus.ok()) {
             return valueOrStatus.status();
         }
diff --git a/scip_indexer/SCIPSymbolRef.cc b/scip_indexer/SCIPSymbolRef.cc
new file mode 100644
index 000000000..5ff5a5b85
--- /dev/null
+++ b/scip_indexer/SCIPSymbolRef.cc
@@ -0,0 +1,81 @@
+// NOTE: Protobuf headers should go first since they use poisoned functions.
+#include "proto/SCIP.pb.h"
+
+#include <optional>
+#include <string>
+#include <vector>
+
+#include "absl/status/status.h"
+
+#include "core/Loc.h"
+
+#include "scip_indexer/SCIPSymbolRef.h"
+
+using namespace std;
+
+namespace sorbet::scip_indexer {
+
+// Try to compute a scip::Symbol for this value.
+absl::Status UntypedGenericSymbolRef::symbolForExpr(const core::GlobalState &gs, const GemMetadata &metadata,
+                                                    optional<core::Loc> loc, scip::Symbol &symbol) const {
+    // Don't set symbol.scheme and package.manager here because
+    // those are hard-coded to 'scip-ruby' and 'gem' anyways.
+    scip::Package package;
+    package.set_name(metadata.name());
+    package.set_version(metadata.version());
+    *symbol.mutable_package() = move(package);
+
+    InlinedVector<scip::Descriptor, 4> descriptors;
+    auto cur = this->selfOrOwner;
+    while (cur != core::Symbols::root()) {
+        // NOTE(varun): The current scheme will cause multiple 'definitions' for the same
+        // entity if it is present in different files, because the path is not encoded
+        // in the descriptor whose parent is the root. This matches the semantics of
+        // RubyMine, but we may want to revisit this if it is problematic for classes
+        // that are extended in lots of places.
+        scip::Descriptor descriptor;
+        *descriptor.mutable_name() = cur.name(gs).show(gs);
+        ENFORCE(!descriptor.name().empty());
+        // TODO(varun): Are the scip descriptor kinds correct?
+        switch (cur.kind()) {
+            case core::SymbolRef::Kind::Method:
+                // NOTE(varun): There is a separate isOverloaded field in the flags field,
+                // despite SO/docs saying that Ruby doesn't support method overloading,
+                // Technically, we should better understand how this works and set the
+                // disambiguator based on that. However, right now, an extension's
+                // type-checking function is not run if a method is overloaded,
+                // (see pipeline.cc), so it's unclear if we need to care about that.
+                descriptor.set_suffix(scip::Descriptor::Method);
+                break;
+            case core::SymbolRef::Kind::ClassOrModule:
+                descriptor.set_suffix(scip::Descriptor::Type);
+                break;
+            case core::SymbolRef::Kind::TypeArgument:
+                descriptor.set_suffix(scip::Descriptor::TypeParameter);
+                break;
+            case core::SymbolRef::Kind::FieldOrStaticField:
+                descriptor.set_suffix(scip::Descriptor::Term);
+                break;
+            case core::SymbolRef::Kind::TypeMember: // TODO: What does TypeMember mean?
+                descriptor.set_suffix(scip::Descriptor::Type);
+                break;
+            default:
+                return absl::InvalidArgumentError("unexpected expr type for symbol computation");
+        }
+        descriptors.push_back(move(descriptor));
+        cur = cur.owner(gs);
+    }
+    while (!descriptors.empty()) {
+        *symbol.add_descriptors() = move(descriptors.back());
+        descriptors.pop_back();
+    }
+    if (this->name != core::NameRef::noName()) {
+        scip::Descriptor descriptor;
+        descriptor.set_suffix(scip::Descriptor::Term);
+        *descriptor.mutable_name() = this->name.shortName(gs);
+        ENFORCE(!descriptor.name().empty());
+        *symbol.add_descriptors() = move(descriptor);
+    }
+    return absl::OkStatus();
+}
+} // namespace sorbet::scip_indexer
\ No newline at end of file
diff --git a/scip_indexer/SCIPSymbolRef.h b/scip_indexer/SCIPSymbolRef.h
new file mode 100644
index 000000000..fa1ed6c2e
--- /dev/null
+++ b/scip_indexer/SCIPSymbolRef.h
@@ -0,0 +1,86 @@
+#ifndef SORBET_SCIP_SYMBOL_REF
+#define SORBET_SCIP_SYMBOL_REF
+
+#include <optional>
+#include <string>
+#include <vector>
+
+#include "absl/status/status.h"
+#include "absl/strings/str_split.h"
+
+#include "common/common.h"
+#include "core/Loc.h"
+#include "core/NameRef.h"
+#include "core/SymbolRef.h"
+
+namespace scip { // Avoid needlessly including protobuf header here.
+class Symbol;
+}
+
+namespace sorbet::scip_indexer {
+
+class GemMetadata final {
+    std::string _name;
+    std::string _version;
+
+    GemMetadata(std::string name, std::string version) : _name(name), _version(version) {}
+
+public:
+    GemMetadata &operator=(const GemMetadata &) = default;
+
+    static GemMetadata tryParseOrDefault(std::string metadata) {
+        std::vector<std::string> v = absl::StrSplit(metadata, '@');
+        if (v.size() != 2 || v[0].empty() || v[1].empty()) {
+            return GemMetadata{"TODO", "TODO"};
+        }
+        return GemMetadata{v[0], v[1]};
+    }
+
+    const std::string &name() const {
+        return this->_name;
+    }
+
+    const std::string &version() const {
+        return this->_version;
+    }
+};
+
+// Simplified version of NamedSymbolRef that doesn't care about types.
+//
+// Primarily for use in storing/looking up information in maps/sets,
+// as type information for fields can be refined based on control flow.
+class UntypedGenericSymbolRef final {
+    sorbet::core::SymbolRef selfOrOwner;
+    sorbet::core::NameRef name;
+
+    UntypedGenericSymbolRef(sorbet::core::SymbolRef selfOrOwner, sorbet::core::NameRef name)
+        : selfOrOwner(selfOrOwner), name(name) {}
+
+public:
+    bool operator==(const UntypedGenericSymbolRef &other) const {
+        return this->selfOrOwner == other.selfOrOwner && this->name == other.name;
+    }
+
+    template <typename H> friend H AbslHashValue(H h, const UntypedGenericSymbolRef &x) {
+        return H::combine(std::move(h), x.selfOrOwner, x.name);
+    }
+
+    static UntypedGenericSymbolRef declared(sorbet::core::SymbolRef sym) {
+        ENFORCE(sym.exists());
+        return UntypedGenericSymbolRef(sym, {});
+    }
+
+    static UntypedGenericSymbolRef undeclared(sorbet::core::ClassOrModuleRef klass, sorbet::core::NameRef name) {
+        ENFORCE(klass.exists());
+        ENFORCE(name.exists())
+        return UntypedGenericSymbolRef(klass, name);
+    }
+
+    // Try to compute a scip::Symbol for this value.
+    absl::Status symbolForExpr(const core::GlobalState &gs, const GemMetadata &metadata, std::optional<core::Loc> loc,
+                               scip::Symbol &symbol) const;
+};
+
+} // namespace sorbet::scip_indexer
+
+#endif // SORBET_SCIP_SYMBOL_REF
\ No newline at end of file

From 94b3fb0f1a54c31494e21dddeafd7014b740b54d Mon Sep 17 00:00:00 2001
From: Varun Gandhi <varun.gandhi@sourcegraph.com>
Date: Tue, 20 Sep 2022 12:15:08 +0800
Subject: [PATCH 3/9] cleanup: Remove useless ENFORCE calls.

---
 scip_indexer/SCIPIndexer.cc | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc
index d2d905fb9..7d2abf289 100644
--- a/scip_indexer/SCIPIndexer.cc
+++ b/scip_indexer/SCIPIndexer.cc
@@ -240,7 +240,6 @@ class NamedSymbolRef final {
             case Kind::Method:
                 return fmt::format("Method {}", this->selfOrOwner.showFullName(gs));
         }
-        ENFORCE(false, "impossible");
     }
 
     core::SymbolRef asSymbolRef() const {
@@ -257,7 +256,6 @@ class NamedSymbolRef final {
             case Kind::Method:
                 return isSorbetInternal(gs, this->asSymbolRef());
         }
-        ENFORCE(false, "impossible");
     }
 
     vector<string> docStrings(const core::GlobalState &gs, core::TypePtr fieldType, core::Loc loc) {
@@ -1000,8 +998,6 @@ class CFGTraversal final {
                 }
                 break;
             }
-            default:
-                ENFORCE(false, "unhandled case of ValueCategory")
         }
         ENFORCE(this->functionLocals.contains(localRef), "should've added local earlier if it was missing");
         absl::Status status;

From bcd73300070f6d1dc01dc26bdce286fef9328839 Mon Sep 17 00:00:00 2001
From: Varun Gandhi <varun.gandhi@sourcegraph.com>
Date: Tue, 20 Sep 2022 12:16:35 +0800
Subject: [PATCH 4/9] cleanup: Remove needless macro with lambda.

---
 scip_indexer/SCIPIndexer.cc | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc
index 7d2abf289..358d69965 100644
--- a/scip_indexer/SCIPIndexer.cc
+++ b/scip_indexer/SCIPIndexer.cc
@@ -259,22 +259,23 @@ class NamedSymbolRef final {
     }
 
     vector<string> docStrings(const core::GlobalState &gs, core::TypePtr fieldType, core::Loc loc) {
-#define CHECK_TYPE(type, name) \
-    ENFORCE(type, "missing type for {} in file {}\n{}\n", name, loc.file().data(gs).path(), loc.toString(gs))
+        auto checkType = [&gs, &loc](core::TypePtr ty, const std::string &name) {
+            ENFORCE(ty, "missing type for {} in file {}\n{}\n", name, loc.file().data(gs).path(), loc.toString(gs));
+        };
 
         vector<string> docs;
         string markdown = "";
         switch (this->kind()) {
             case Kind::UndeclaredField: {
                 auto name = this->name.show(gs);
-                CHECK_TYPE(fieldType, name);
+                checkType(fieldType, name);
                 markdown = fmt::format("{} ({})", name, fieldType.show(gs));
                 break;
             }
             case Kind::DeclaredField: {
                 auto fieldRef = this->selfOrOwner.asFieldRef();
                 auto name = fieldRef.showFullName(gs);
-                CHECK_TYPE(fieldType, name);
+                checkType(fieldType, name);
                 markdown = fmt::format("{} ({})", name, fieldType.show(gs));
                 break;
             }
@@ -296,7 +297,7 @@ class NamedSymbolRef final {
             case Kind::Method: {
                 auto ref = this->selfOrOwner.asMethodRef();
                 auto resultType = ref.data(gs)->owner.data(gs)->resultType;
-                CHECK_TYPE(resultType, fmt::format("result type for {}", ref.showFullName(gs)));
+                checkType(resultType, fmt::format("result type for {}", ref.showFullName(gs)));
                 markdown = realmain::lsp::prettyTypeForMethod(gs, ref, resultType, nullptr, nullptr);
                 // FIXME(varun): For some reason, it looks like a bunch of public methods
                 // get marked as private here. Avoid printing misleading info until we fix that.
@@ -315,7 +316,6 @@ class NamedSymbolRef final {
             }
         }
         return docs;
-#undef CHECK_TYPE
     }
 
     core::Loc symbolLoc(const core::GlobalState &gs) const {

From 7a0cffa6796424674db1d10ad43175257b357f41 Mon Sep 17 00:00:00 2001
From: Varun Gandhi <varun.gandhi@sourcegraph.com>
Date: Tue, 20 Sep 2022 12:16:57 +0800
Subject: [PATCH 5/9] cleanup: Remove unused type.

---
 scip_indexer/SCIPIndexer.cc | 2 --
 1 file changed, 2 deletions(-)

diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc
index 358d69965..bff152582 100644
--- a/scip_indexer/SCIPIndexer.cc
+++ b/scip_indexer/SCIPIndexer.cc
@@ -376,8 +376,6 @@ enum class Emitted {
     Earlier,
 };
 
-using OccurrenceCache = UnorderedMap<pair<core::LocOffsets, /*SymbolRole*/ int32_t>, uint32_t>;
-
 /// Per-thread state storing information to be emitting in a SCIP index.
 ///
 /// The states are implicitly merged at the time of emitting the index.

From b39463ba797c31c9ef745b6ab85a3f9906a1c915 Mon Sep 17 00:00:00 2001
From: Varun Gandhi <varun.gandhi@sourcegraph.com>
Date: Tue, 20 Sep 2022 12:25:38 +0800
Subject: [PATCH 6/9] cleanup: Use InlinedVector for doc strings.

---
 scip_indexer/SCIPIndexer.cc | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc
index bff152582..00934ff89 100644
--- a/scip_indexer/SCIPIndexer.cc
+++ b/scip_indexer/SCIPIndexer.cc
@@ -112,6 +112,8 @@ bool isSorbetInternal(const core::GlobalState &gs, core::SymbolRef sym) {
     return false;
 }
 
+template <typename T> using SmallVec = InlinedVector<T, 2>;
+
 // A wrapper type to handle both top-level symbols (like classes) as well as
 // "inner symbols" like fields (@x). In a statically typed language, field
 // symbols are like any other symbols, but in Ruby, they aren't (necessarily)
@@ -258,12 +260,11 @@ class NamedSymbolRef final {
         }
     }
 
-    vector<string> docStrings(const core::GlobalState &gs, core::TypePtr fieldType, core::Loc loc) {
+    void saveDocStrings(const core::GlobalState &gs, core::TypePtr fieldType, core::Loc loc, SmallVec<string> &docs) {
         auto checkType = [&gs, &loc](core::TypePtr ty, const std::string &name) {
             ENFORCE(ty, "missing type for {} in file {}\n{}\n", name, loc.file().data(gs).path(), loc.toString(gs));
         };
 
-        vector<string> docs;
         string markdown = "";
         switch (this->kind()) {
             case Kind::UndeclaredField: {
@@ -315,7 +316,6 @@ class NamedSymbolRef final {
                 docs.push_back(doc.value());
             }
         }
-        return docs;
     }
 
     core::Loc symbolLoc(const core::GlobalState &gs) const {
@@ -466,7 +466,7 @@ class SCIPState {
     }
 
 private:
-    Emitted saveSymbolInfo(core::FileRef file, const string &symbolString, const vector<string> &docs) {
+    Emitted saveSymbolInfo(core::FileRef file, const string &symbolString, const SmallVec<string> &docs) {
         if (this->emittedSymbols.contains({file, symbolString})) {
             return Emitted::Earlier;
         }
@@ -480,7 +480,7 @@ class SCIPState {
     }
 
     absl::Status saveDefinitionImpl(const core::GlobalState &gs, core::FileRef file, const string &symbolString,
-                                    core::Loc occLoc, const vector<string> &docs) {
+                                    core::Loc occLoc, const SmallVec<string> &docs) {
         ENFORCE(!symbolString.empty());
 
         auto emitted = this->saveSymbolInfo(file, symbolString, docs);
@@ -506,7 +506,7 @@ class SCIPState {
     }
 
     void saveReferenceImpl(const core::GlobalState &gs, core::FileRef file, const string &symbolString,
-                           const vector<string> &overrideDocs, core::LocOffsets occLocOffsets, int32_t symbol_roles) {
+                           const SmallVec<string> &overrideDocs, core::LocOffsets occLocOffsets, int32_t symbol_roles) {
         ENFORCE(!symbolString.empty());
         auto occLoc = trimColonColonPrefix(gs, core::Loc(file, occLocOffsets));
         scip::Occurrence occurrence;
@@ -573,7 +573,7 @@ class SCIPState {
         if (this->cacheOccurrence(gs, file, occ, scip::SymbolRole::Definition)) {
             return absl::OkStatus();
         }
-        vector<string> docStrings;
+        SmallVec<string> docStrings;
         auto loc = core::Loc(file, occ.offsets);
         if (type) {
             auto var = loc.source(gs);
@@ -602,8 +602,10 @@ class SCIPState {
             return valueOrStatus.status();
         }
         const string &symbolString = *valueOrStatus.value();
-        return this->saveDefinitionImpl(gs, file, symbolString, occLoc,
-                                        symRef.docStrings(gs, symRef.definitionType(), occLoc));
+
+        SmallVec<string> docs;
+        symRef.saveDocStrings(gs, symRef.definitionType(), occLoc, docs);
+        return this->saveDefinitionImpl(gs, file, symbolString, occLoc, docs);
     }
 
     absl::Status saveReference(const core::GlobalState &gs, core::FileRef file, OwnedLocal occ,
@@ -611,7 +613,7 @@ class SCIPState {
         if (this->cacheOccurrence(gs, file, occ, symbol_roles)) {
             return absl::OkStatus();
         }
-        vector<string> overrideDocs;
+        SmallVec<string> overrideDocs;
         auto loc = core::Loc(file, occ.offsets);
         if (overrideType.has_value()) {
             ENFORCE(overrideType.value(), "forgot to fold type to nullopt earlier: {}\n{}\n", file.data(gs).path(),
@@ -644,7 +646,7 @@ class SCIPState {
         }
         const string &symbolString = *valueOrStatus.value();
 
-        vector<string> overrideDocs{};
+        SmallVec<string> overrideDocs{};
         using Kind = NamedSymbolRef::Kind;
         switch (symRef.kind()) {
             case Kind::ClassOrModule:
@@ -653,7 +655,7 @@ class SCIPState {
             case Kind::UndeclaredField:
             case Kind::DeclaredField:
                 if (overrideType.has_value()) {
-                    overrideDocs = symRef.docStrings(gs, overrideType.value(), loc);
+                    symRef.saveDocStrings(gs, overrideType.value(), loc, overrideDocs);
                 }
         }
         this->saveReferenceImpl(gs, file, symbolString, overrideDocs, occLoc, symbol_roles);

From 640849399c91568ccff3214455d224b5adfc71b2 Mon Sep 17 00:00:00 2001
From: Varun Gandhi <varun.gandhi@sourcegraph.com>
Date: Tue, 20 Sep 2022 12:29:18 +0800
Subject: [PATCH 7/9] cleanup: Explicitly initialize fields in SCIPState.

---
 scip_indexer/SCIPIndexer.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc
index 00934ff89..a9846cb6a 100644
--- a/scip_indexer/SCIPIndexer.cc
+++ b/scip_indexer/SCIPIndexer.cc
@@ -423,7 +423,9 @@ class SCIPState {
     vector<scip::SymbolInformation> externalSymbols;
 
 public:
-    SCIPState(GemMetadata metadata) : symbolScratchBuffer(), symbolStringCache(), gemMetadata(metadata) {}
+    SCIPState(GemMetadata metadata)
+        : symbolScratchBuffer(), symbolStringCache(), localOccurrenceCache(), symbolOccurrenceCache(),
+          gemMetadata(metadata), occurrenceMap(), emittedSymbols(), symbolMap(), documents(), externalSymbols() {}
     ~SCIPState() = default;
     SCIPState(SCIPState &&) = default;
     SCIPState &operator=(SCIPState &&other) = default;
@@ -890,6 +892,8 @@ optional<core::TypePtr> computeOverrideType(core::TypePtr definitionType, core::
 }
 
 /// Convenience type to handle CFG traversal and recording info in SCIPState.
+///
+/// Any caches that are not specific to a traversal should be added to SCIPState.
 class CFGTraversal final {
     // A map from each basic block to the locals in it.
     //

From 26e97feb60f48c4ddfba79d879a984fa7e46d540 Mon Sep 17 00:00:00 2001
From: Varun Gandhi <varun.gandhi@sourcegraph.com>
Date: Tue, 20 Sep 2022 12:34:18 +0800
Subject: [PATCH 8/9] cleanup: Rename NamedSymbolRef to GenericSymbolRef.

---
 scip_indexer/SCIPIndexer.cc  | 68 ++++++++++++++++++------------------
 scip_indexer/SCIPSymbolRef.h |  2 +-
 2 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc
index a9846cb6a..6696d4740 100644
--- a/scip_indexer/SCIPIndexer.cc
+++ b/scip_indexer/SCIPIndexer.cc
@@ -121,8 +121,8 @@ template <typename T> using SmallVec = InlinedVector<T, 2>;
 // So Sorbet represents them with a separate name on the side.
 //
 // Structurally, this is similar to the Alias instruction. One key difference
-// is that the SymbolRef may refer to the owner in some situations.
-class NamedSymbolRef final {
+// is that a GenericSymbolRef may refer to the owner in some situations.
+class GenericSymbolRef final {
     core::SymbolRef selfOrOwner;
 
     /// Name of the symbol, which may or may not exist.
@@ -144,7 +144,7 @@ class NamedSymbolRef final {
     };
 
 private:
-    NamedSymbolRef(core::SymbolRef s, core::NameRef n, core::TypePtr t, Kind k)
+    GenericSymbolRef(core::SymbolRef s, core::NameRef n, core::TypePtr t, Kind k)
         : selfOrOwner(s), name(n), _definitionType(t) {
         switch (k) {
             case Kind::ClassOrModule:
@@ -166,38 +166,38 @@ class NamedSymbolRef final {
     }
 
 public:
-    NamedSymbolRef(const NamedSymbolRef &) = default;
-    NamedSymbolRef(NamedSymbolRef &&) = default;
-    NamedSymbolRef &operator=(const NamedSymbolRef &) = default;
-    NamedSymbolRef &operator=(NamedSymbolRef &&) = default;
+    GenericSymbolRef(const GenericSymbolRef &) = default;
+    GenericSymbolRef(GenericSymbolRef &&) = default;
+    GenericSymbolRef &operator=(const GenericSymbolRef &) = default;
+    GenericSymbolRef &operator=(GenericSymbolRef &&) = default;
 
-    friend bool operator==(const NamedSymbolRef &lhs, const NamedSymbolRef &rhs) {
+    friend bool operator==(const GenericSymbolRef &lhs, const GenericSymbolRef &rhs) {
         return lhs.selfOrOwner == rhs.selfOrOwner && lhs.name == rhs.name;
     }
 
-    friend bool operator<(const NamedSymbolRef &lhs, const NamedSymbolRef &rhs) {
+    friend bool operator<(const GenericSymbolRef &lhs, const GenericSymbolRef &rhs) {
         return lhs.selfOrOwner.rawId() < rhs.selfOrOwner.rawId() ||
                (lhs.selfOrOwner == rhs.selfOrOwner && lhs.name.rawId() < rhs.name.rawId());
     }
 
-    template <typename H> friend H AbslHashValue(H h, const NamedSymbolRef &c) {
+    template <typename H> friend H AbslHashValue(H h, const GenericSymbolRef &c) {
         return H::combine(std::move(h), c.selfOrOwner, c.name);
     }
 
-    static NamedSymbolRef classOrModule(core::SymbolRef self) {
-        return NamedSymbolRef(self, {}, {}, Kind::ClassOrModule);
+    static GenericSymbolRef classOrModule(core::SymbolRef self) {
+        return GenericSymbolRef(self, {}, {}, Kind::ClassOrModule);
     }
 
-    static NamedSymbolRef undeclaredField(core::SymbolRef owner, core::NameRef name, core::TypePtr type) {
-        return NamedSymbolRef(owner, name, type, Kind::UndeclaredField);
+    static GenericSymbolRef undeclaredField(core::SymbolRef owner, core::NameRef name, core::TypePtr type) {
+        return GenericSymbolRef(owner, name, type, Kind::UndeclaredField);
     }
 
-    static NamedSymbolRef declaredField(core::SymbolRef self, core::TypePtr type) {
-        return NamedSymbolRef(self, {}, type, Kind::DeclaredField);
+    static GenericSymbolRef declaredField(core::SymbolRef self, core::TypePtr type) {
+        return GenericSymbolRef(self, {}, type, Kind::DeclaredField);
     }
 
-    static NamedSymbolRef method(core::SymbolRef self) {
-        return NamedSymbolRef(self, {}, {}, Kind::Method);
+    static GenericSymbolRef method(core::SymbolRef self) {
+        return GenericSymbolRef(self, {}, {}, Kind::Method);
     }
 
     core::TypePtr definitionType() const {
@@ -229,7 +229,7 @@ class NamedSymbolRef final {
         }
     }
 
-    /// Display a NamedSymbolRef for debugging.
+    /// Display a GenericSymbolRef for debugging.
     string showRaw(const core::GlobalState &gs) const {
         switch (this->kind()) {
             case Kind::UndeclaredField:
@@ -397,7 +397,7 @@ class SCIPState {
     ///
     /// This is mainly present to avoid emitting duplicate occurrences
     /// for DSL-like constructs like prop/def_delegator.
-    UnorderedSet<tuple<NamedSymbolRef, core::Loc, /*SymbolRole*/ int32_t>> symbolOccurrenceCache;
+    UnorderedSet<tuple<GenericSymbolRef, core::Loc, /*SymbolRole*/ int32_t>> symbolOccurrenceCache;
     // ^ Naively, I would think that that shouldn't happen because we don't traverse
     // rewriter-synthesized method bodies, but it does seem to happen.
     //
@@ -559,7 +559,7 @@ class SCIPState {
         return true;
     }
 
-    bool cacheOccurrence(const core::GlobalState &gs, core::Loc loc, NamedSymbolRef sym, int32_t symbolRoles) {
+    bool cacheOccurrence(const core::GlobalState &gs, core::Loc loc, GenericSymbolRef sym, int32_t symbolRoles) {
         // Optimization:
         //   Avoid emitting duplicate def/refs for symbols.
         // This can happen with constructs like:
@@ -588,7 +588,7 @@ class SCIPState {
     // Save definition when you have a sorbet Symbol.
     // Meant for methods, fields etc., but not local variables.
     // TODO(varun): Should we always pass in the location instead of sometimes only?
-    absl::Status saveDefinition(const core::GlobalState &gs, core::FileRef file, NamedSymbolRef symRef,
+    absl::Status saveDefinition(const core::GlobalState &gs, core::FileRef file, GenericSymbolRef symRef,
                                 optional<core::LocOffsets> loc = nullopt) {
         // In practice, there doesn't seem to be any situation which triggers
         // a duplicate definition being emitted, so skip calling cacheOccurrence here.
@@ -628,7 +628,7 @@ class SCIPState {
         return absl::OkStatus();
     }
 
-    absl::Status saveReference(const core::Context &ctx, NamedSymbolRef symRef, optional<core::TypePtr> overrideType,
+    absl::Status saveReference(const core::Context &ctx, GenericSymbolRef symRef, optional<core::TypePtr> overrideType,
                                core::LocOffsets occLoc, int32_t symbol_roles) {
         // HACK: Reduce noise due to <static-init> in snapshots.
         if (ctx.owner.name(ctx) == core::Names::staticInit()) {
@@ -649,7 +649,7 @@ class SCIPState {
         const string &symbolString = *valueOrStatus.value();
 
         SmallVec<string> overrideDocs{};
-        using Kind = NamedSymbolRef::Kind;
+        using Kind = GenericSymbolRef::Kind;
         switch (symRef.kind()) {
             case Kind::ClassOrModule:
             case Kind::Method:
@@ -763,7 +763,7 @@ findUnresolvedFieldTransitive(const core::GlobalState &gs, core::Loc loc, core::
 // Loosely inspired by AliasesAndKeywords in IREmitterContext.cc
 class AliasMap final {
 public:
-    using Impl = UnorderedMap<cfg::LocalRef, tuple<NamedSymbolRef, core::LocOffsets, /*emitted*/ bool>>;
+    using Impl = UnorderedMap<cfg::LocalRef, tuple<GenericSymbolRef, core::LocOffsets, /*emitted*/ bool>>;
 
 private:
     Impl map;
@@ -806,7 +806,7 @@ class AliasMap final {
                         if (klass.exists()) {
                             this->map.insert( // no trim(...) because undeclared fields shouldn't have ::
                                 {bind.bind.variable,
-                                 {NamedSymbolRef::undeclaredField(klass, instr->name, bind.bind.type), bind.loc,
+                                 {GenericSymbolRef::undeclaredField(klass, instr->name, bind.bind.type), bind.loc,
                                   false}});
                         }
                     } else if (absl::holds_alternative<core::SymbolRef>(result)) {
@@ -814,7 +814,7 @@ class AliasMap final {
                         if (fieldSym.exists()) {
                             this->map.insert(
                                 {bind.bind.variable,
-                                 {NamedSymbolRef::declaredField(fieldSym, bind.bind.type), trim(bind.loc), false}});
+                                 {GenericSymbolRef::declaredField(fieldSym, bind.bind.type), trim(bind.loc), false}});
                         }
                     } else {
                         ENFORCE(false, "Should've handled all cases of variant earlier");
@@ -825,7 +825,7 @@ class AliasMap final {
                     ENFORCE(!bind.loc.empty());
                     this->map.insert(
                         {bind.bind.variable,
-                         {NamedSymbolRef::declaredField(instr->what, bind.bind.type), trim(bind.loc), false}});
+                         {GenericSymbolRef::declaredField(instr->what, bind.bind.type), trim(bind.loc), false}});
                     continue;
                 }
                 // Outside of definition contexts for classes & modules,
@@ -845,13 +845,13 @@ class AliasMap final {
                         // all the 'internal' stuff here?
                         continue;
                     }
-                    this->map.insert({bind.bind.variable, {NamedSymbolRef::classOrModule(sym), trim(loc), false}});
+                    this->map.insert({bind.bind.variable, {GenericSymbolRef::classOrModule(sym), trim(loc), false}});
                 }
             }
         }
     }
 
-    optional<pair<NamedSymbolRef, core::LocOffsets>> try_consume(cfg::LocalRef localRef) {
+    optional<pair<GenericSymbolRef, core::LocOffsets>> try_consume(cfg::LocalRef localRef) {
         auto it = this->map.find(localRef);
         if (it == this->map.end()) {
             return nullopt;
@@ -922,7 +922,7 @@ class CFGTraversal final {
 
     // Map for storing the type at the original site of definition for a local variable.
     //
-    // Performs the role of definitionType on NamedSymbolRef but for locals.
+    // Performs the role of definitionType on GenericSymbolRef but for locals.
     //
     // NOTE: Subsequent references may have different types.
     UnorderedMap<uint32_t, core::TypePtr> localDefinitionType;
@@ -1164,7 +1164,7 @@ class CFGTraversal final {
                                     // TODO(varun): For arrays, hashes etc., try to identify if the function
                                     // matches a known operator (e.g. []=), and emit an appropriate
                                     // 'WriteAccess' symbol role for it.
-                                    auto status = this->scipState.saveReference(ctx, NamedSymbolRef::method(funSym),
+                                    auto status = this->scipState.saveReference(ctx, GenericSymbolRef::method(funSym),
                                                                                 nullopt, send->funLoc, 0);
                                     ENFORCE(status.ok());
                                 }
@@ -1236,7 +1236,7 @@ class CFGTraversal final {
         // See NOTE[alias-handling].
         AliasMap::Impl map;
         this->aliasMap.extract(map);
-        using SymbolWithLoc = pair<NamedSymbolRef, core::LocOffsets>;
+        using SymbolWithLoc = pair<GenericSymbolRef, core::LocOffsets>;
         vector<SymbolWithLoc> todo;
         for (auto &[_, value] : map) {
             auto &[namedSym, loc, emitted] = value;
@@ -1406,7 +1406,7 @@ class SCIPSemanticExtension : public SemanticExtension {
         }
         auto scipState = this->getSCIPState();
         if (methodDef.name != core::Names::staticInit()) {
-            auto status = scipState->saveDefinition(gs, file, scip_indexer::NamedSymbolRef::method(methodDef.symbol));
+            auto status = scipState->saveDefinition(gs, file, scip_indexer::GenericSymbolRef::method(methodDef.symbol));
             ENFORCE(status.ok());
         }
 
diff --git a/scip_indexer/SCIPSymbolRef.h b/scip_indexer/SCIPSymbolRef.h
index fa1ed6c2e..1b02ae263 100644
--- a/scip_indexer/SCIPSymbolRef.h
+++ b/scip_indexer/SCIPSymbolRef.h
@@ -45,7 +45,7 @@ class GemMetadata final {
     }
 };
 
-// Simplified version of NamedSymbolRef that doesn't care about types.
+// Simplified version of GenericSymbolRef that doesn't care about types.
 //
 // Primarily for use in storing/looking up information in maps/sets,
 // as type information for fields can be refined based on control flow.

From 7f952674e324549bcf98a83713b522507b2db018 Mon Sep 17 00:00:00 2001
From: Varun Gandhi <varun.gandhi@sourcegraph.com>
Date: Tue, 20 Sep 2022 12:50:47 +0800
Subject: [PATCH 9/9] cleanup: Move GenericSymbolRef to SCIPSymbolRef.{h,cc}

---
 scip_indexer/SCIPIndexer.cc   | 246 ----------------------------------
 scip_indexer/SCIPSymbolRef.cc | 114 ++++++++++++++++
 scip_indexer/SCIPSymbolRef.h  | 146 ++++++++++++++++++++
 3 files changed, 260 insertions(+), 246 deletions(-)

diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc
index 6696d4740..eea2fbfb5 100644
--- a/scip_indexer/SCIPIndexer.cc
+++ b/scip_indexer/SCIPIndexer.cc
@@ -29,7 +29,6 @@
 #include "core/Loc.h"
 #include "core/SymbolRef.h"
 #include "core/Symbols.h"
-#include "main/lsp/lsp.h"
 #include "main/pipeline/semantic_extension/SemanticExtension.h"
 #include "sorbet_version/sorbet_version.h"
 
@@ -92,251 +91,6 @@ struct OwnedLocal {
     }
 };
 
-bool isSorbetInternal(const core::GlobalState &gs, core::SymbolRef sym) {
-    UnorderedSet<core::SymbolRef> visited;
-    auto classT = core::Symbols::T().data(gs)->lookupSingletonClass(gs);
-    while (sym.exists() && !visited.contains(sym)) {
-        if (sym.isClassOrModule()) {
-            auto klass = sym.asClassOrModuleRef();
-            if (klass == core::Symbols::Sorbet_Private() || klass == core::Symbols::T() || klass == classT) {
-                return true;
-            }
-            auto name = klass.data(gs)->name;
-            if (name == core::Names::Constants::Opus()) {
-                return true;
-            }
-        }
-        visited.insert(sym);
-        sym = sym.owner(gs);
-    }
-    return false;
-}
-
-template <typename T> using SmallVec = InlinedVector<T, 2>;
-
-// A wrapper type to handle both top-level symbols (like classes) as well as
-// "inner symbols" like fields (@x). In a statically typed language, field
-// symbols are like any other symbols, but in Ruby, they aren't (necessarily)
-// declared ahead-of-time (you can declare them with @x = T.let(…, …) though).
-// So Sorbet represents them with a separate name on the side.
-//
-// Structurally, this is similar to the Alias instruction. One key difference
-// is that a GenericSymbolRef may refer to the owner in some situations.
-class GenericSymbolRef final {
-    core::SymbolRef selfOrOwner;
-
-    /// Name of the symbol, which may or may not exist.
-    core::NameRef name;
-
-    /// The type of the symbol at its definition, if applicable.
-    ///
-    /// References to this symbol may have a different type,
-    /// because you can change the type of a field, including within
-    /// the same basic block.
-    core::TypePtr _definitionType;
-
-public:
-    enum class Kind {
-        ClassOrModule,
-        UndeclaredField,
-        DeclaredField,
-        Method,
-    };
-
-private:
-    GenericSymbolRef(core::SymbolRef s, core::NameRef n, core::TypePtr t, Kind k)
-        : selfOrOwner(s), name(n), _definitionType(t) {
-        switch (k) {
-            case Kind::ClassOrModule:
-                ENFORCE(s.isClassOrModule());
-                ENFORCE(!n.exists());
-                return;
-            case Kind::DeclaredField:
-                ENFORCE(s.isFieldOrStaticField());
-                ENFORCE(!n.exists());
-                return;
-            case Kind::UndeclaredField:
-                ENFORCE(s.isClassOrModule());
-                ENFORCE(n.exists());
-                return;
-            case Kind::Method:
-                ENFORCE(s.isMethod());
-                ENFORCE(!n.exists());
-        }
-    }
-
-public:
-    GenericSymbolRef(const GenericSymbolRef &) = default;
-    GenericSymbolRef(GenericSymbolRef &&) = default;
-    GenericSymbolRef &operator=(const GenericSymbolRef &) = default;
-    GenericSymbolRef &operator=(GenericSymbolRef &&) = default;
-
-    friend bool operator==(const GenericSymbolRef &lhs, const GenericSymbolRef &rhs) {
-        return lhs.selfOrOwner == rhs.selfOrOwner && lhs.name == rhs.name;
-    }
-
-    friend bool operator<(const GenericSymbolRef &lhs, const GenericSymbolRef &rhs) {
-        return lhs.selfOrOwner.rawId() < rhs.selfOrOwner.rawId() ||
-               (lhs.selfOrOwner == rhs.selfOrOwner && lhs.name.rawId() < rhs.name.rawId());
-    }
-
-    template <typename H> friend H AbslHashValue(H h, const GenericSymbolRef &c) {
-        return H::combine(std::move(h), c.selfOrOwner, c.name);
-    }
-
-    static GenericSymbolRef classOrModule(core::SymbolRef self) {
-        return GenericSymbolRef(self, {}, {}, Kind::ClassOrModule);
-    }
-
-    static GenericSymbolRef undeclaredField(core::SymbolRef owner, core::NameRef name, core::TypePtr type) {
-        return GenericSymbolRef(owner, name, type, Kind::UndeclaredField);
-    }
-
-    static GenericSymbolRef declaredField(core::SymbolRef self, core::TypePtr type) {
-        return GenericSymbolRef(self, {}, type, Kind::DeclaredField);
-    }
-
-    static GenericSymbolRef method(core::SymbolRef self) {
-        return GenericSymbolRef(self, {}, {}, Kind::Method);
-    }
-
-    core::TypePtr definitionType() const {
-        return this->_definitionType;
-    }
-
-    Kind kind() const {
-        if (this->name.exists()) {
-            return Kind::UndeclaredField;
-        }
-        if (this->selfOrOwner.isFieldOrStaticField()) {
-            return Kind::DeclaredField;
-        }
-        if (this->selfOrOwner.isMethod()) {
-            return Kind::Method;
-        }
-        return Kind::ClassOrModule;
-    }
-
-    UntypedGenericSymbolRef withoutType() const {
-        switch (this->kind()) {
-            case Kind::UndeclaredField:
-                ENFORCE(this->selfOrOwner.isClassOrModule());
-                return UntypedGenericSymbolRef::undeclared(this->selfOrOwner.asClassOrModuleRef(), this->name);
-            case Kind::Method:
-            case Kind::ClassOrModule:
-            case Kind::DeclaredField:
-                return UntypedGenericSymbolRef::declared(this->selfOrOwner);
-        }
-    }
-
-    /// Display a GenericSymbolRef for debugging.
-    string showRaw(const core::GlobalState &gs) const {
-        switch (this->kind()) {
-            case Kind::UndeclaredField:
-                return fmt::format("UndeclaredField(owner: {}, name: {})", this->selfOrOwner.showFullName(gs),
-                                   this->name.toString(gs));
-            case Kind::DeclaredField:
-                return fmt::format("DeclaredField {}", this->selfOrOwner.showFullName(gs));
-            case Kind::ClassOrModule:
-                return fmt::format("ClassOrModule {}", this->selfOrOwner.showFullName(gs));
-            case Kind::Method:
-                return fmt::format("Method {}", this->selfOrOwner.showFullName(gs));
-        }
-    }
-
-    core::SymbolRef asSymbolRef() const {
-        ENFORCE(this->kind() != Kind::UndeclaredField);
-        return this->selfOrOwner;
-    }
-
-    bool isSorbetInternalClassOrMethod(const core::GlobalState &gs) const {
-        switch (this->kind()) {
-            case Kind::UndeclaredField:
-            case Kind::DeclaredField:
-                return false;
-            case Kind::ClassOrModule:
-            case Kind::Method:
-                return isSorbetInternal(gs, this->asSymbolRef());
-        }
-    }
-
-    void saveDocStrings(const core::GlobalState &gs, core::TypePtr fieldType, core::Loc loc, SmallVec<string> &docs) {
-        auto checkType = [&gs, &loc](core::TypePtr ty, const std::string &name) {
-            ENFORCE(ty, "missing type for {} in file {}\n{}\n", name, loc.file().data(gs).path(), loc.toString(gs));
-        };
-
-        string markdown = "";
-        switch (this->kind()) {
-            case Kind::UndeclaredField: {
-                auto name = this->name.show(gs);
-                checkType(fieldType, name);
-                markdown = fmt::format("{} ({})", name, fieldType.show(gs));
-                break;
-            }
-            case Kind::DeclaredField: {
-                auto fieldRef = this->selfOrOwner.asFieldRef();
-                auto name = fieldRef.showFullName(gs);
-                checkType(fieldType, name);
-                markdown = fmt::format("{} ({})", name, fieldType.show(gs));
-                break;
-            }
-            case Kind::ClassOrModule: {
-                auto ref = this->selfOrOwner.asClassOrModuleRef();
-                auto classOrModule = ref.data(gs);
-                if (classOrModule->isClass()) {
-                    auto super = classOrModule->superClass();
-                    if (super.exists() && super != core::Symbols::Object()) {
-                        markdown = fmt::format("class {} < {}", ref.show(gs), super.show(gs));
-                    } else {
-                        markdown = fmt::format("class {}", ref.show(gs));
-                    }
-                } else {
-                    markdown = fmt::format("module {}", ref.show(gs));
-                }
-                break;
-            }
-            case Kind::Method: {
-                auto ref = this->selfOrOwner.asMethodRef();
-                auto resultType = ref.data(gs)->owner.data(gs)->resultType;
-                checkType(resultType, fmt::format("result type for {}", ref.showFullName(gs)));
-                markdown = realmain::lsp::prettyTypeForMethod(gs, ref, resultType, nullptr, nullptr);
-                // FIXME(varun): For some reason, it looks like a bunch of public methods
-                // get marked as private here. Avoid printing misleading info until we fix that.
-                // https://github.com/sourcegraph/scip-ruby/issues/33
-                markdown = absl::StrReplaceAll(markdown, {{"private def", "def"}, {"; end", ""}});
-                break;
-            }
-        }
-        if (!markdown.empty()) {
-            docs.push_back(fmt::format("```ruby\n{}\n```", markdown));
-        }
-        auto whatFile = loc.file();
-        if (whatFile.exists()) {
-            if (auto doc = realmain::lsp::findDocumentation(whatFile.data(gs).source(), loc.beginPos())) {
-                docs.push_back(doc.value());
-            }
-        }
-    }
-
-    core::Loc symbolLoc(const core::GlobalState &gs) const {
-        switch (this->kind()) {
-            case Kind::Method: {
-                auto method = this->selfOrOwner.asMethodRef().data(gs);
-                if (!method->nameLoc.exists() || method->nameLoc.empty()) {
-                    return method->loc();
-                }
-                return method->nameLoc;
-            }
-            case Kind::ClassOrModule:
-            case Kind::DeclaredField:
-                return this->selfOrOwner.loc(gs);
-            case Kind::UndeclaredField:
-                ENFORCE(false, "case UndeclaredField should not be triggered here");
-                return core::Loc();
-        }
-    }
-};
-
 InlinedVector<int32_t, 4> fromSorbetLoc(const core::GlobalState &gs, core::Loc loc) {
     ENFORCE(!loc.empty());
     auto [start, end] = loc.position(gs);
diff --git a/scip_indexer/SCIPSymbolRef.cc b/scip_indexer/SCIPSymbolRef.cc
index 5ff5a5b85..0b18be141 100644
--- a/scip_indexer/SCIPSymbolRef.cc
+++ b/scip_indexer/SCIPSymbolRef.cc
@@ -6,8 +6,10 @@
 #include <vector>
 
 #include "absl/status/status.h"
+#include "absl/strings/str_replace.h"
 
 #include "core/Loc.h"
+#include "main/lsp/lsp.h"
 
 #include "scip_indexer/SCIPSymbolRef.h"
 
@@ -78,4 +80,116 @@ absl::Status UntypedGenericSymbolRef::symbolForExpr(const core::GlobalState &gs,
     }
     return absl::OkStatus();
 }
+
+string GenericSymbolRef::showRaw(const core::GlobalState &gs) const {
+    switch (this->kind()) {
+        case Kind::UndeclaredField:
+            return fmt::format("UndeclaredField(owner: {}, name: {})", this->selfOrOwner.showFullName(gs),
+                               this->name.toString(gs));
+        case Kind::DeclaredField:
+            return fmt::format("DeclaredField {}", this->selfOrOwner.showFullName(gs));
+        case Kind::ClassOrModule:
+            return fmt::format("ClassOrModule {}", this->selfOrOwner.showFullName(gs));
+        case Kind::Method:
+            return fmt::format("Method {}", this->selfOrOwner.showFullName(gs));
+    }
+}
+
+bool GenericSymbolRef::isSorbetInternal(const core::GlobalState &gs, core::SymbolRef sym) {
+    UnorderedSet<core::SymbolRef> visited;
+    auto classT = core::Symbols::T().data(gs)->lookupSingletonClass(gs);
+    while (sym.exists() && !visited.contains(sym)) {
+        if (sym.isClassOrModule()) {
+            auto klass = sym.asClassOrModuleRef();
+            if (klass == core::Symbols::Sorbet_Private() || klass == core::Symbols::T() || klass == classT) {
+                return true;
+            }
+            auto name = klass.data(gs)->name;
+            if (name == core::Names::Constants::Opus()) {
+                return true;
+            }
+        }
+        visited.insert(sym);
+        sym = sym.owner(gs);
+    }
+    return false;
+}
+
+void GenericSymbolRef::saveDocStrings(const core::GlobalState &gs, core::TypePtr fieldType, core::Loc loc,
+                                      SmallVec<string> &docs) const {
+    auto checkType = [&gs, &loc](core::TypePtr ty, const std::string &name) {
+        ENFORCE(ty, "missing type for {} in file {}\n{}\n", name, loc.file().data(gs).path(), loc.toString(gs));
+    };
+
+    string markdown = "";
+    switch (this->kind()) {
+        case Kind::UndeclaredField: {
+            auto name = this->name.show(gs);
+            checkType(fieldType, name);
+            markdown = fmt::format("{} ({})", name, fieldType.show(gs));
+            break;
+        }
+        case Kind::DeclaredField: {
+            auto fieldRef = this->selfOrOwner.asFieldRef();
+            auto name = fieldRef.showFullName(gs);
+            checkType(fieldType, name);
+            markdown = fmt::format("{} ({})", name, fieldType.show(gs));
+            break;
+        }
+        case Kind::ClassOrModule: {
+            auto ref = this->selfOrOwner.asClassOrModuleRef();
+            auto classOrModule = ref.data(gs);
+            if (classOrModule->isClass()) {
+                auto super = classOrModule->superClass();
+                if (super.exists() && super != core::Symbols::Object()) {
+                    markdown = fmt::format("class {} < {}", ref.show(gs), super.show(gs));
+                } else {
+                    markdown = fmt::format("class {}", ref.show(gs));
+                }
+            } else {
+                markdown = fmt::format("module {}", ref.show(gs));
+            }
+            break;
+        }
+        case Kind::Method: {
+            auto ref = this->selfOrOwner.asMethodRef();
+            auto resultType = ref.data(gs)->owner.data(gs)->resultType;
+            checkType(resultType, fmt::format("result type for {}", ref.showFullName(gs)));
+            markdown = realmain::lsp::prettyTypeForMethod(gs, ref, resultType, nullptr, nullptr);
+            // FIXME(varun): For some reason, it looks like a bunch of public methods
+            // get marked as private here. Avoid printing misleading info until we fix that.
+            // https://github.com/sourcegraph/scip-ruby/issues/33
+            markdown = absl::StrReplaceAll(markdown, {{"private def", "def"}, {"; end", ""}});
+            break;
+        }
+    }
+    if (!markdown.empty()) {
+        docs.push_back(fmt::format("```ruby\n{}\n```", markdown));
+    }
+    auto whatFile = loc.file();
+    if (whatFile.exists()) {
+        if (auto doc = realmain::lsp::findDocumentation(whatFile.data(gs).source(), loc.beginPos())) {
+            docs.push_back(doc.value());
+        }
+    }
+}
+
+core::Loc GenericSymbolRef::symbolLoc(const core::GlobalState &gs) const {
+    switch (this->kind()) {
+        case Kind::Method: {
+            auto method = this->selfOrOwner.asMethodRef().data(gs);
+            if (!method->nameLoc.exists() || method->nameLoc.empty()) {
+                return method->loc();
+            }
+            return method->nameLoc;
+        }
+        case Kind::ClassOrModule:
+        case Kind::DeclaredField:
+            return this->selfOrOwner.loc(gs);
+        case Kind::UndeclaredField:
+            ENFORCE(false, "case UndeclaredField should not be triggered here");
+            return core::Loc();
+    }
+}
+
 } // namespace sorbet::scip_indexer
\ No newline at end of file
diff --git a/scip_indexer/SCIPSymbolRef.h b/scip_indexer/SCIPSymbolRef.h
index 1b02ae263..ca0674116 100644
--- a/scip_indexer/SCIPSymbolRef.h
+++ b/scip_indexer/SCIPSymbolRef.h
@@ -12,6 +12,8 @@
 #include "core/Loc.h"
 #include "core/NameRef.h"
 #include "core/SymbolRef.h"
+#include "core/Symbols.h"
+#include "core/TypePtr.h"
 
 namespace scip { // Avoid needlessly including protobuf header here.
 class Symbol;
@@ -19,6 +21,8 @@ class Symbol;
 
 namespace sorbet::scip_indexer {
 
+template <typename T> using SmallVec = InlinedVector<T, 2>;
+
 class GemMetadata final {
     std::string _name;
     std::string _version;
@@ -81,6 +85,148 @@ class UntypedGenericSymbolRef final {
                                scip::Symbol &symbol) const;
 };
 
+// A wrapper type to handle both top-level symbols (like classes) as well as
+// "inner symbols" like fields (@x). In a statically typed language, field
+// symbols are like any other symbols, but in Ruby, they aren't (necessarily)
+// declared ahead-of-time (you can declare them with @x = T.let(…, …) though).
+// So Sorbet represents them with a separate name on the side.
+//
+// Structurally, this is similar to the Alias instruction. One key difference
+// is that a GenericSymbolRef may refer to the owner in some situations.
+class GenericSymbolRef final {
+    core::SymbolRef selfOrOwner;
+
+    /// Name of the symbol, which may or may not exist.
+    core::NameRef name;
+
+    /// The type of the symbol at its definition, if applicable.
+    ///
+    /// References to this symbol may have a different type,
+    /// because you can change the type of a field, including within
+    /// the same basic block.
+    core::TypePtr _definitionType;
+
+public:
+    enum class Kind {
+        ClassOrModule,
+        UndeclaredField,
+        DeclaredField,
+        Method,
+    };
+
+private:
+    GenericSymbolRef(core::SymbolRef s, core::NameRef n, core::TypePtr t, Kind k)
+        : selfOrOwner(s), name(n), _definitionType(t) {
+        switch (k) {
+            case Kind::ClassOrModule:
+                ENFORCE(s.isClassOrModule());
+                ENFORCE(!n.exists());
+                return;
+            case Kind::DeclaredField:
+                ENFORCE(s.isFieldOrStaticField());
+                ENFORCE(!n.exists());
+                return;
+            case Kind::UndeclaredField:
+                ENFORCE(s.isClassOrModule());
+                ENFORCE(n.exists());
+                return;
+            case Kind::Method:
+                ENFORCE(s.isMethod());
+                ENFORCE(!n.exists());
+        }
+    }
+
+public:
+    GenericSymbolRef(const GenericSymbolRef &) = default;
+    GenericSymbolRef(GenericSymbolRef &&) = default;
+    GenericSymbolRef &operator=(const GenericSymbolRef &) = default;
+    GenericSymbolRef &operator=(GenericSymbolRef &&) = default;
+
+    friend bool operator==(const GenericSymbolRef &lhs, const GenericSymbolRef &rhs) {
+        return lhs.selfOrOwner == rhs.selfOrOwner && lhs.name == rhs.name;
+    }
+
+    friend bool operator<(const GenericSymbolRef &lhs, const GenericSymbolRef &rhs) {
+        return lhs.selfOrOwner.rawId() < rhs.selfOrOwner.rawId() ||
+               (lhs.selfOrOwner == rhs.selfOrOwner && lhs.name.rawId() < rhs.name.rawId());
+    }
+
+    template <typename H> friend H AbslHashValue(H h, const GenericSymbolRef &c) {
+        return H::combine(std::move(h), c.selfOrOwner, c.name);
+    }
+
+    static GenericSymbolRef classOrModule(core::SymbolRef self) {
+        return GenericSymbolRef(self, {}, {}, Kind::ClassOrModule);
+    }
+
+    static GenericSymbolRef undeclaredField(core::SymbolRef owner, core::NameRef name, core::TypePtr type) {
+        return GenericSymbolRef(owner, name, type, Kind::UndeclaredField);
+    }
+
+    static GenericSymbolRef declaredField(core::SymbolRef self, core::TypePtr type) {
+        return GenericSymbolRef(self, {}, type, Kind::DeclaredField);
+    }
+
+    static GenericSymbolRef method(core::SymbolRef self) {
+        return GenericSymbolRef(self, {}, {}, Kind::Method);
+    }
+
+    core::TypePtr definitionType() const {
+        return this->_definitionType;
+    }
+
+    Kind kind() const {
+        if (this->name.exists()) {
+            return Kind::UndeclaredField;
+        }
+        if (this->selfOrOwner.isFieldOrStaticField()) {
+            return Kind::DeclaredField;
+        }
+        if (this->selfOrOwner.isMethod()) {
+            return Kind::Method;
+        }
+        return Kind::ClassOrModule;
+    }
+
+    UntypedGenericSymbolRef withoutType() const {
+        switch (this->kind()) {
+            case Kind::UndeclaredField:
+                ENFORCE(this->selfOrOwner.isClassOrModule());
+                return UntypedGenericSymbolRef::undeclared(this->selfOrOwner.asClassOrModuleRef(), this->name);
+            case Kind::Method:
+            case Kind::ClassOrModule:
+            case Kind::DeclaredField:
+                return UntypedGenericSymbolRef::declared(this->selfOrOwner);
+        }
+    }
+
+    /// Display a GenericSymbolRef for debugging.
+    std::string showRaw(const core::GlobalState &gs) const;
+
+    core::SymbolRef asSymbolRef() const {
+        ENFORCE(this->kind() != Kind::UndeclaredField);
+        return this->selfOrOwner;
+    }
+
+    static bool isSorbetInternal(const core::GlobalState &gs, core::SymbolRef sym);
+
+    bool isSorbetInternalClassOrMethod(const core::GlobalState &gs) const {
+        switch (this->kind()) {
+            case Kind::UndeclaredField:
+            case Kind::DeclaredField:
+                return false;
+            case Kind::ClassOrModule:
+            case Kind::Method:
+                return isSorbetInternal(gs, this->asSymbolRef());
+        }
+    }
+
+    void saveDocStrings(const core::GlobalState &gs, core::TypePtr fieldType, core::Loc loc,
+                        SmallVec<std::string> &docs) const;
+
+    core::Loc symbolLoc(const core::GlobalState &gs) const;
+};
+
 } // namespace sorbet::scip_indexer
 
 #endif // SORBET_SCIP_SYMBOL_REF
\ No newline at end of file