WIP: Add support for fields in mixins.

varungandhi-src · varungandhi-src · commit e4f99dd3ad09 · 2022-09-21T12:11:04.000+08:00
diff --git a/scip_indexer/SCIPFieldResolve.cc b/scip_indexer/SCIPFieldResolve.cc
@@ -1,5 +1,7 @@
 #include <vector>
 
+#include "absl/strings/ascii.h"
+
 #include "common/common.h"
 #include "core/GlobalState.h"
 #include "core/SymbolRef.h"
@@ -17,7 +19,7 @@ string FieldQueryResult::Data::showRaw(const core::GlobalState &gs) const {
         case Kind::FromDeclared:
             return this->originalSymbol().showRaw(gs);
         case Kind::FromUndeclared:
-            return fmt::format("In({})", this->originalClass().showFullName(gs));
+            return fmt::format("In({})", absl::StripAsciiWhitespace(this->originalClass().showFullName(gs)));
     }
 }
 
diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc
@@ -33,6 +33,7 @@
 #include "sorbet_version/sorbet_version.h"
 
 #include "scip_indexer/Debug.h"
+#include "scip_indexer/SCIPFieldResolve.h"
 #include "scip_indexer/SCIPSymbolRef.h"
 #include "scip_indexer/SCIPUtils.h"
 
@@ -173,6 +174,11 @@ class SCIPState {
     UnorderedSet<pair<core::FileRef, string>> emittedSymbols;
     UnorderedMap<core::FileRef, vector<scip::SymbolInformation>> symbolMap;
 
+    // Stores the relationships that apply to a field or a method.
+    RelationshipsMap relationshipsMap;
+
+    FieldResolver fieldResolver;
+
     vector<scip::Document> documents;
     vector<scip::SymbolInformation> externalSymbols;
 
@@ -222,7 +228,8 @@ class SCIPState {
     }
 
 private:
-    Emitted saveSymbolInfo(core::FileRef file, const string &symbolString, const SmallVec<string> &docs) {
+    Emitted saveSymbolInfo(core::FileRef file, const string &symbolString, const SmallVec<string> &docs,
+                           const SmallVec<scip::Relationship> &rels) {
         if (this->emittedSymbols.contains({file, symbolString})) {
             return Emitted::Earlier;
         }
@@ -231,15 +238,19 @@ class SCIPState {
         for (auto &doc : docs) {
             symbolInfo.add_documentation(doc);
         }
+        for (auto &rel : rels) {
+            *symbolInfo.add_relationships() = rel;
+        }
         this->symbolMap[file].push_back(symbolInfo);
         return Emitted::Now;
     }
 
     absl::Status saveDefinitionImpl(const core::GlobalState &gs, core::FileRef file, const string &symbolString,
-                                    core::Loc occLoc, const SmallVec<string> &docs) {
+                                    core::Loc occLoc, const SmallVec<string> &docs,
+                                    const SmallVec<scip::Relationship> &rels) {
         ENFORCE(!symbolString.empty());
 
-        auto emitted = this->saveSymbolInfo(file, symbolString, docs);
+        auto emitted = this->saveSymbolInfo(file, symbolString, docs, rels);
 
         occLoc = trimColonColonPrefix(gs, occLoc);
         scip::Occurrence occurrence;
@@ -336,7 +347,7 @@ class SCIPState {
             ENFORCE(var.has_value(), "Failed to find source text for definition of local variable");
             docStrings.push_back(fmt::format("```ruby\n{} ({})\n```", var.value(), type.show(gs)));
         }
-        return this->saveDefinitionImpl(gs, file, occ.toSCIPString(gs, file), loc, docStrings);
+        return this->saveDefinitionImpl(gs, file, occ.toSCIPString(gs, file), loc, docStrings, {});
     }
 
     // Save definition when you have a sorbet Symbol.
@@ -349,19 +360,29 @@ class SCIPState {
 
         auto occLoc = loc.has_value() ? core::Loc(file, loc.value()) : symRef.symbolLoc(gs);
         scip::Symbol symbol;
-        auto status = symRef.withoutType().symbolForExpr(gs, this->gemMetadata, occLoc, symbol);
+        auto untypedSymRef = symRef.withoutType();
+        auto status = untypedSymRef.symbolForExpr(gs, this->gemMetadata, occLoc, symbol);
         if (!status.ok()) {
             return status;
         }
-        absl::StatusOr<const string *> valueOrStatus(this->saveSymbolString(gs, symRef.withoutType(), &symbol));
+        absl::StatusOr<const string *> valueOrStatus(this->saveSymbolString(gs, untypedSymRef, &symbol));
         if (!valueOrStatus.ok()) {
             return valueOrStatus.status();
         }
         const string &symbolString = *valueOrStatus.value();
 
         SmallVec<string> docs;
         symRef.saveDocStrings(gs, symRef.definitionType(), occLoc, docs);
-        return this->saveDefinitionImpl(gs, file, symbolString, occLoc, docs);
+
+        SmallVec<scip::Relationship> rels;
+        untypedSymRef.saveRelationships(this->relationshipsMap, rels,
+                                        [this, &gs](UntypedGenericSymbolRef sym, std::string &out) {
+                                            auto status = this->saveSymbolString(gs, sym, nullptr);
+                                            ENFORCE(status.ok());
+                                            out = *status.value();
+                                        });
+
+        return this->saveDefinitionImpl(gs, file, symbolString, occLoc, docs, rels);
     }
 
     absl::Status saveReference(const core::GlobalState &gs, core::FileRef file, OwnedLocal occ,
@@ -457,63 +478,6 @@ string format_ancestry(const core::GlobalState &gs, core::SymbolRef sym) {
     return out.str();
 }
 
-static absl::variant</*owner*/ core::ClassOrModuleRef, core::SymbolRef>
-findUnresolvedFieldTransitive(const core::GlobalState &gs, core::Loc loc, core::ClassOrModuleRef start,
-                              core::NameRef field) {
-    auto fieldText = field.shortName(gs);
-    auto isInstanceVar = fieldText.size() >= 2 && fieldText[0] == '@' && fieldText[1] != '@';
-    auto isClassInstanceVar = isInstanceVar && start.data(gs)->isSingletonClass(gs);
-    // Class instance variables are not inherited, unlike ordinary instance
-    // variables or class variables.
-    if (isClassInstanceVar) {
-        return start;
-    }
-    auto isClassVar = fieldText.size() >= 2 && fieldText[0] == '@' && fieldText[1] == '@';
-    if (isClassVar && !start.data(gs)->isSingletonClass(gs)) {
-        // Triggered when undeclared class variables are accessed from instance methods.
-        start = start.data(gs)->lookupSingletonClass(gs);
-    }
-
-    // TODO(varun): Should we add a cache here? It seems wasteful to redo
-    // work for every occurrence.
-    if (gs.unresolvedFields.find(start) == gs.unresolvedFields.end() ||
-        !gs.unresolvedFields.find(start)->second.contains(field)) {
-        // Triggered by code patterns like:
-        //   # top-level
-        //   def MyClass.method
-        //     # blah
-        //   end
-        // which is not supported by Sorbet.
-        LOG_DEBUG(gs, loc,
-                  fmt::format("couldn't find field {} in class {};\n"
-                              "are you using a code pattern like def MyClass.method which is unsupported by Sorbet?",
-                              field.exists() ? field.toString(gs) : "<non-existent>",
-                              start.exists() ? start.showFullName(gs) : "<non-existent>"));
-        // As a best-effort guess, assume that the definition is
-        // in this class but we somehow missed it.
-        return start;
-    }
-
-    auto best = start;
-    auto cur = start;
-    while (cur.exists()) {
-        auto klass = cur.data(gs);
-        auto sym = klass->findMember(gs, field);
-        if (sym.exists()) {
-            return sym;
-        }
-        auto it = gs.unresolvedFields.find(cur);
-        if (it != gs.unresolvedFields.end() && it->second.contains(field)) {
-            best = cur;
-        }
-        if (cur == klass->superClass()) { // FIXME(varun): Handle mix-ins
-            break;
-        }
-        cur = klass->superClass();
-    }
-    return best;
-}
-
 // Loosely inspired by AliasesAndKeywords in IREmitterContext.cc
 class AliasMap final {
 public:
@@ -528,7 +492,8 @@ class AliasMap final {
 public:
     AliasMap() = default;
 
-    void populate(const core::Context &ctx, const cfg::CFG &cfg) {
+    void populate(const core::Context &ctx, const cfg::CFG &cfg, FieldResolver &fieldResolver,
+                  RelationshipsMap &relMap) {
         this->map = {};
         auto &gs = ctx.state;
         auto method = ctx.owner;
@@ -553,25 +518,40 @@ class AliasMap final {
                 if (sym == core::Symbols::Magic_undeclaredFieldStub()) {
                     ENFORCE(!bind.loc.empty());
                     ENFORCE(klass.isClassOrModule());
-                    auto result = findUnresolvedFieldTransitive(ctx, ctx.locAt(bind.loc), klass.asClassOrModuleRef(),
-                                                                instr->name);
-                    if (absl::holds_alternative<core::ClassOrModuleRef>(result)) {
-                        auto klass = absl::get<core::ClassOrModuleRef>(result);
-                        if (klass.exists()) {
-                            this->map.insert( // no trim(...) because undeclared fields shouldn't have ::
-                                {bind.bind.variable,
-                                 {GenericSymbolRef::undeclaredField(klass, instr->name, bind.bind.type), bind.loc,
-                                  false}});
+                    auto [result, cacheHit] = fieldResolver.findUnresolvedFieldTransitive(
+                        ctx, ctx.locAt(bind.loc), {klass.asClassOrModuleRef(), instr->name});
+                    auto checkExists = [&](bool exists, const std::string &text) {
+                        ENFORCE(exists,
+                                "Returned non-existent {} from findUnresolvedFieldTransitive with start={}, "
+                                "field={}, file={}, loc={}",
+                                text, klass.exists() ? klass.toStringFullName(gs) : "<non-existent>",
+                                instr->name.exists() ? instr->name.toString(gs) : "<non-existent>",
+                                ctx.file.data(gs).path(), ctx.locAt(bind.loc).showRawLineColumn(gs));
+                    };
+                    switch (result.inherited.kind()) {
+                        case FieldQueryResult::Kind::FromUndeclared: {
+                            auto klass = result.inherited.originalClass();
+                            checkExists(klass.exists(), "class");
+                            auto namedSymRef = GenericSymbolRef::undeclaredField(klass, instr->name, bind.bind.type);
+                            if (!cacheHit) {
+                                // It may be the case that the mixin values are already stored because of the
+                                // traversal in some other function. In that case, don't bother overriding.
+                                relMap.insert({namedSymRef.withoutType(), result.mixedIn});
+                            }
+                            // no trim(...) because undeclared fields shouldn't have ::
+                            this->map.insert({bind.bind.variable, {namedSymRef, bind.loc, false}});
+                            break;
                         }
-                    } else if (absl::holds_alternative<core::SymbolRef>(result)) {
-                        auto fieldSym = absl::get<core::SymbolRef>(result);
-                        if (fieldSym.exists()) {
-                            this->map.insert(
-                                {bind.bind.variable,
-                                 {GenericSymbolRef::declaredField(fieldSym, bind.bind.type), trim(bind.loc), false}});
+                        case FieldQueryResult::Kind::FromDeclared: {
+                            auto fieldSym = result.inherited.originalSymbol();
+                            checkExists(fieldSym.exists(), "field");
+                            auto namedSymRef = GenericSymbolRef::declaredField(fieldSym, bind.bind.type);
+                            if (!cacheHit) {
+                                relMap.insert({namedSymRef.withoutType(), result.mixedIn});
+                            }
+                            this->map.insert({bind.bind.variable, {namedSymRef, trim(bind.loc), false}});
+                            break;
                         }
-                    } else {
-                        ENFORCE(false, "Should've handled all cases of variant earlier");
                     }
                     continue;
                 }
@@ -817,7 +797,9 @@ class CFGTraversal final {
 
 public:
     void traverse(const cfg::CFG &cfg) {
-        this->aliasMap.populate(this->ctx, cfg);
+        this->aliasMap.populate(this->ctx, cfg, this->scipState.fieldResolver, this->scipState.relationshipsMap);
+        fmt::print(stderr, "log: [traverse] relationshipsMap = {}\n",
+                   showRawRelationshipsMap(this->ctx.state, this->scipState.relationshipsMap));
         auto &gs = this->ctx.state;
         auto file = this->ctx.file;
         auto method = this->ctx.owner;
diff --git a/scip_indexer/SCIPSymbolRef.cc b/scip_indexer/SCIPSymbolRef.cc
@@ -6,17 +6,27 @@
 #include <vector>
 
 #include "absl/status/status.h"
+#include "absl/strings/ascii.h"
 #include "absl/strings/str_replace.h"
+#include "spdlog/fmt/fmt.h"
 
 #include "core/Loc.h"
 #include "main/lsp/lsp.h"
 
+#include "scip_indexer/Debug.h"
 #include "scip_indexer/SCIPSymbolRef.h"
 
 using namespace std;
 
 namespace sorbet::scip_indexer {
 
+string showRawRelationshipsMap(const core::GlobalState &gs, const RelationshipsMap &relMap) {
+    return showMap(relMap, [&gs](const UntypedGenericSymbolRef &ugsr, const auto &mixins) -> string {
+        return fmt::format("{}: {}", ugsr.showRaw(gs),
+                           showVec(*mixins.get(), [&gs](const auto &mixin) -> string { return mixin.showRaw(gs); }));
+    });
+}
+
 // 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 {
@@ -83,9 +93,40 @@ absl::Status UntypedGenericSymbolRef::symbolForExpr(const core::GlobalState &gs,
 
 string UntypedGenericSymbolRef::showRaw(const core::GlobalState &gs) const {
     if (this->name.exists()) {
-        return fmt::format("UGSR(owner: {}, name: {})", this->selfOrOwner.showFullName(gs), this->name.toString(gs));
+        return fmt::format("UGSR({}.{})", absl::StripAsciiWhitespace(this->selfOrOwner.showFullName(gs)),
+                           this->name.toString(gs));
+    }
+    return fmt::format("UGSR({})", absl::StripAsciiWhitespace(this->selfOrOwner.showFullName(gs)));
+}
+
+void UntypedGenericSymbolRef::saveRelationships(
+    const RelationshipsMap &relationshipMap, SmallVec<scip::Relationship> &rels,
+    const absl::FunctionRef<void(UntypedGenericSymbolRef, std::string &)> &saveSymbolString) const {
+    using SymbolKind = core::SymbolRef::Kind;
+    if (this->selfOrOwner.kind() != SymbolKind::FieldOrStaticField) {
+        return;
+    }
+    auto it = relationshipMap.find(*this);
+    if (it == relationshipMap.end()) {
+        return;
+    }
+    rels.reserve(it->second->size());
+    for (auto data : *it->second.get()) {
+        scip::Relationship rel;
+        rel.set_is_reference(true);
+        using Kind = FieldQueryResult::Kind;
+        switch (data.kind()) {
+            case Kind::FromUndeclared:
+                saveSymbolString(UntypedGenericSymbolRef::undeclared(data.originalClass(), this->name),
+                                 *rel.mutable_symbol());
+                break;
+            case Kind::FromDeclared:
+                saveSymbolString(UntypedGenericSymbolRef::declared(data.originalSymbol()), *rel.mutable_symbol());
+                break;
+        }
+        ENFORCE(!rel.symbol().empty());
+        rels.push_back(move(rel));
     }
-    return fmt::format("UGSR(symbol: {})", this->selfOrOwner.showFullName(gs));
 }
 
 string GenericSymbolRef::showRaw(const core::GlobalState &gs) const {
@@ -199,4 +240,4 @@ core::Loc GenericSymbolRef::symbolLoc(const core::GlobalState &gs) const {
     }
 }
 
-} // namespace sorbet::scip_indexer
+} // namespace sorbet::scip_indexer
diff --git a/scip_indexer/SCIPSymbolRef.h b/scip_indexer/SCIPSymbolRef.h
@@ -15,9 +15,12 @@
 #include "core/Symbols.h"
 #include "core/TypePtr.h"
 
+#include "scip_indexer/SCIPFieldResolve.h"
+
 namespace scip { // Avoid needlessly including protobuf header here.
 class Symbol;
-}
+class Relationship;
+} // namespace scip
 
 namespace sorbet::scip_indexer {
 
@@ -49,6 +52,12 @@ class GemMetadata final {
     }
 };
 
+class UntypedGenericSymbolRef;
+
+using RelationshipsMap = UnorderedMap<UntypedGenericSymbolRef, std::shared_ptr<std::vector<FieldQueryResult::Data>>>;
+
+std::string showRawRelationshipsMap(const core::GlobalState &gs, const RelationshipsMap &relMap);
+
 // Simplified version of GenericSymbolRef that doesn't care about types.
 //
 // Primarily for use in storing/looking up information in maps/sets,
@@ -84,6 +93,10 @@ class UntypedGenericSymbolRef final {
     absl::Status symbolForExpr(const core::GlobalState &gs, const GemMetadata &metadata, std::optional<core::Loc> loc,
                                scip::Symbol &symbol) const;
 
+    void
+    saveRelationships(const RelationshipsMap &relationshipMap, SmallVec<scip::Relationship> &rels,
+                      const absl::FunctionRef<void(UntypedGenericSymbolRef, std::string &)> &saveSymbolString) const;
+
     std::string showRaw(const core::GlobalState &gs) const;
 };
 
@@ -210,8 +223,10 @@ class GenericSymbolRef final {
         return this->selfOrOwner;
     }
 
+private:
     static bool isSorbetInternal(const core::GlobalState &gs, core::SymbolRef sym);
 
+public:
     bool isSorbetInternalClassOrMethod(const core::GlobalState &gs) const {
         switch (this->kind()) {
             case Kind::UndeclaredField:
@@ -231,4 +246,4 @@ class GenericSymbolRef final {
 
 } // namespace sorbet::scip_indexer
 
-#endif // SORBET_SCIP_SYMBOL_REF
+#endif // SORBET_SCIP_SYMBOL_REF
diff --git a/test/scip/testdata/mixin.rb b/test/scip/testdata/mixin.rb
diff --git a/test/scip/testdata/mixin.snapshot.rb b/test/scip/testdata/mixin.snapshot.rb

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,7 @@`
`1`	`1`	`#include <vector>`
`2`	`2`
	`3`	`+#include "absl/strings/ascii.h"`
	`4`	`+`
`3`	`5`	`#include "common/common.h"`
`4`	`6`	`#include "core/GlobalState.h"`
`5`	`7`	`#include "core/SymbolRef.h"`
`@@ -17,7 +19,7 @@ string FieldQueryResult::Data::showRaw(const core::GlobalState &gs) const {`
`17`	`19`	`case Kind::FromDeclared:`
`18`	`20`	`return this->originalSymbol().showRaw(gs);`
`19`	`21`	`case Kind::FromUndeclared:`
`20`		`- return fmt::format("In({})", this->originalClass().showFullName(gs));`
	`22`	`+ return fmt::format("In({})", absl::StripAsciiWhitespace(this->originalClass().showFullName(gs)));`
`21`	`23`	`}`
`22`	`24`	`}`
`23`	`25`