Skip to content

feat: Add support for hover docs. #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 21, 2022
Merged

feat: Add support for hover docs. #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions scip_indexer/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ cc_library(
"//cfg",
"//common",
"//core",
"//main/lsp",
"//proto",
"//sorbet_version",
"@com_google_absl//absl/synchronization",
Expand Down
132 changes: 106 additions & 26 deletions scip_indexer/SCIPIndexer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/str_replace.h"
#include "absl/strings/str_split.h"
#include "absl/synchronization/mutex.h"
#include "spdlog/fmt/fmt.h"
Expand All @@ -27,6 +28,7 @@
#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"

Expand Down Expand Up @@ -165,6 +167,7 @@ class GemMetadata final {
class NamedSymbolRef final {
core::SymbolRef selfOrOwner;
core::NameRef name;
core::TypePtr type;

public:
enum class Kind {
Expand All @@ -175,7 +178,7 @@ class NamedSymbolRef final {
};

private:
NamedSymbolRef(core::SymbolRef s, core::NameRef n, Kind k) : selfOrOwner(s), name(n) {
NamedSymbolRef(core::SymbolRef s, core::NameRef n, core::TypePtr t, Kind k) : selfOrOwner(s), name(n), type(t) {
switch (k) {
case Kind::ClassOrModule:
ENFORCE(s.isClassOrModule());
Expand Down Expand Up @@ -209,19 +212,19 @@ class NamedSymbolRef final {
}

static NamedSymbolRef classOrModule(core::SymbolRef self) {
return NamedSymbolRef(self, {}, Kind::ClassOrModule);
return NamedSymbolRef(self, {}, {}, Kind::ClassOrModule);
}

static NamedSymbolRef undeclaredField(core::SymbolRef owner, core::NameRef name) {
return NamedSymbolRef(owner, name, Kind::UndeclaredField);
static NamedSymbolRef undeclaredField(core::SymbolRef owner, core::NameRef name, core::TypePtr type) {
return NamedSymbolRef(owner, name, type, Kind::UndeclaredField);
}

static NamedSymbolRef staticField(core::SymbolRef self) {
return NamedSymbolRef(self, {}, Kind::StaticField);
return NamedSymbolRef(self, {}, {}, Kind::StaticField);
}

static NamedSymbolRef method(core::SymbolRef self) {
return NamedSymbolRef(self, {}, Kind::Method);
return NamedSymbolRef(self, {}, {}, Kind::Method);
}

Kind kind() const {
Expand Down Expand Up @@ -257,8 +260,61 @@ class NamedSymbolRef final {
return this->selfOrOwner;
}

vector<string> docStrings(const core::GlobalState &gs, core::Loc loc) {
vector<string> docs;
string markdown = "";
switch (this->kind()) {
case Kind::UndeclaredField: {
markdown = fmt::format("{} = T.let(_, {})", this->name.show(gs), this->type.show(gs));
break;
}
case Kind::StaticField: {
auto fieldRef = this->selfOrOwner.asFieldRef();
markdown =
fmt::format("{} = T.let(_, {})", fieldRef.showFullName(gs), fieldRef.data(gs)->resultType.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();
markdown = realmain::lsp::prettyTypeForMethod(gs, ref, ref.data(gs)->owner.data(gs)->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());
}
}
return docs;
}

// Returns OK if we were able to compute a symbol for the expression.
absl::Status symbolForExpr(const core::GlobalState &gs, const GemMetadata &metadata, scip::Symbol &symbol) const {
absl::Status symbolForExpr(const core::GlobalState &gs, const GemMetadata &metadata, scip::Symbol &symbol,
optional<core::Loc> loc) const {
// Don't set symbol.scheme and package.manager here because those are hard-coded to 'scip-ruby' and 'gem'
// anyways.
scip::Package package;
Expand Down Expand Up @@ -364,6 +420,7 @@ core::Loc trimColonColonPrefix(const core::GlobalState &gs, core::Loc baseLoc) {
class SCIPState {
string symbolScratchBuffer;
UnorderedMap<NamedSymbolRef, string> symbolStringCache;
UnorderedMap<uint32_t, core::TypePtr> localTypes;
GemMetadata gemMetadata;

public:
Expand Down Expand Up @@ -408,7 +465,7 @@ class SCIPState {
status = scip::utils::emitSymbolString(*symbol, this->symbolScratchBuffer);
} else {
scip::Symbol symbol;
status = symRef.symbolForExpr(gs, this->gemMetadata, symbol);
status = symRef.symbolForExpr(gs, this->gemMetadata, symbol, nullopt);
if (!status.ok()) {
return status;
}
Expand All @@ -423,11 +480,14 @@ class SCIPState {

private:
absl::Status saveDefinitionImpl(const core::GlobalState &gs, core::FileRef file, const string &symbolString,
core::Loc occLoc) {
core::Loc occLoc, const vector<string> &docs) {
ENFORCE(!symbolString.empty());
occLoc = trimColonColonPrefix(gs, occLoc);
scip::SymbolInformation symbolInfo;
symbolInfo.set_symbol(symbolString);
for (auto &doc : docs) {
symbolInfo.add_documentation(doc);
}
this->symbolMap[file].push_back(symbolInfo);

scip::Occurrence occurrence;
Expand Down Expand Up @@ -492,11 +552,18 @@ class SCIPState {
}

public:
absl::Status saveDefinition(const core::GlobalState &gs, core::FileRef file, OwnedLocal occ) {
absl::Status saveDefinition(const core::GlobalState &gs, core::FileRef file, OwnedLocal occ, core::TypePtr type) {
if (this->cacheOccurrence(gs, file, occ, scip::SymbolRole::Definition)) {
return absl::OkStatus();
}
return this->saveDefinitionImpl(gs, file, occ.toString(gs, file), core::Loc(file, occ.offsets));
vector<string> docStrings;
auto loc = core::Loc(file, occ.offsets);
if (type) {
auto var = loc.source(gs);
ENFORCE(var.has_value(), "Failed to find source text for definition of local variable");
docStrings.push_back(fmt::format("```ruby\n{} = T.let(_, {})\n```", var.value(), type.show(gs)));
}
return this->saveDefinitionImpl(gs, file, occ.toString(gs, file), loc, docStrings);
}

// Save definition when you have a sorbet Symbol.
Expand All @@ -506,7 +573,8 @@ class SCIPState {
std::optional<core::LocOffsets> loc = std::nullopt) {
// TODO:(varun) Should we cache here too to avoid emitting duplicate definitions?
scip::Symbol symbol;
auto status = symRef.symbolForExpr(gs, this->gemMetadata, symbol);
auto occLoc = loc.has_value() ? core::Loc(file, loc.value()) : symRef.symbolLoc(gs);
auto status = symRef.symbolForExpr(gs, this->gemMetadata, symbol, occLoc);
if (!status.ok()) {
return status;
}
Expand All @@ -515,10 +583,7 @@ class SCIPState {
return valueOrStatus.status();
}
string &symbolString = *valueOrStatus.value();

auto occLoc = loc.has_value() ? core::Loc(file, loc.value()) : symRef.symbolLoc(gs);

return this->saveDefinitionImpl(gs, file, symbolString, occLoc);
return this->saveDefinitionImpl(gs, file, symbolString, occLoc, symRef.docStrings(gs, occLoc));
}

absl::Status saveReference(const core::GlobalState &gs, core::FileRef file, OwnedLocal occ, int32_t symbol_roles) {
Expand Down Expand Up @@ -617,7 +682,8 @@ class AliasMap final {
if (sym == core::Symbols::Magic_undeclaredFieldStub()) {
ENFORCE(!bind.loc.empty());
this->map.insert( // no trim(...) because undeclared fields shouldn't have ::
{bind.bind.variable, {NamedSymbolRef::undeclaredField(klass, instr->name), bind.loc, false}});
{bind.bind.variable,
{NamedSymbolRef::undeclaredField(klass, instr->name, bind.bind.type), bind.loc, false}});
continue;
}
if (sym.isStaticField(gs)) {
Expand Down Expand Up @@ -685,6 +751,7 @@ class CFGTraversal final {
// in the block.
UnorderedMap<const cfg::BasicBlock *, UnorderedSet<cfg::LocalRef>> blockLocals;
UnorderedMap<cfg::LocalRef, uint32_t> functionLocals;
UnorderedMap<uint32_t, core::TypePtr> localTypes;
AliasMap aliasMap;

// Local variable counter that is reset for every function.
Expand All @@ -697,9 +764,11 @@ class CFGTraversal final {
: blockLocals(), functionLocals(), aliasMap(), scipState(scipState), ctx(ctx) {}

private:
void addLocal(const cfg::BasicBlock *bb, cfg::LocalRef localRef) {
uint32_t addLocal(const cfg::BasicBlock *bb, cfg::LocalRef localRef) {
this->counter++;
this->blockLocals[bb].insert(localRef);
this->functionLocals[localRef] = ++this->counter;
this->functionLocals[localRef] = this->counter;
return this->counter;
}

static core::LocOffsets lhsLocIfPresent(const cfg::Binding &binding) {
Expand All @@ -717,8 +786,10 @@ class CFGTraversal final {
// Emit an occurrence for a local variable if applicable.
//
// Returns true if an occurrence was emitted.
//
// The type should be provided if we have an lvalue.
bool emitLocalOccurrence(const cfg::CFG &cfg, const cfg::BasicBlock *bb, cfg::LocalOccurrence local,
ValueCategory category) {
ValueCategory category, core::TypePtr type) {
auto localRef = local.variable;
auto localVar = localRef.data(cfg);
auto symRef = this->aliasMap.try_consume(localRef);
Expand All @@ -733,7 +804,9 @@ class CFGTraversal final {
if (!this->functionLocals.contains(localRef)) {
isDefinition = true; // If we're seeing this for the first time in topological order,
// The current block must have a definition for the variable.
this->addLocal(bb, localRef);
auto id = this->addLocal(bb, localRef);
ENFORCE(type, "missing type for lvalue");
this->localTypes[id] = type;
}
// The variable wasn't passed in as an argument, and hasn't already been recorded
// as a local in the block. So this must be a definition line.
Expand Down Expand Up @@ -774,9 +847,14 @@ class CFGTraversal final {
}
} else {
uint32_t localId = this->functionLocals[localRef];
auto it = this->localTypes.find(localId);
core::TypePtr type{};
if (it != this->localTypes.end()) {
type = it->second;
}
if (isDefinition) {
status = this->scipState.saveDefinition(this->ctx.state, this->ctx.file,
OwnedLocal{this->ctx.owner, localId, loc});
OwnedLocal{this->ctx.owner, localId, loc}, type);
} else {
status = this->scipState.saveReference(this->ctx.state, this->ctx.file,
OwnedLocal{this->ctx.owner, localId, loc}, referenceRole);
Expand Down Expand Up @@ -856,12 +934,12 @@ class CFGTraversal final {
if (binding.value.tag() != cfg::Tag::Alias && binding.value.tag() != cfg::Tag::ArgPresent) {
// Emit occurrence information for the LHS
auto occ = cfg::LocalOccurrence{binding.bind.variable, lhsLocIfPresent(binding)};
this->emitLocalOccurrence(cfg, bb, occ, ValueCategory::LValue);
this->emitLocalOccurrence(cfg, bb, occ, ValueCategory::LValue, binding.bind.type);
}
// Emit occurrence information for the RHS
auto emitLocal = [this, &cfg, &bb, &binding](cfg::LocalRef local) -> void {
(void)this->emitLocalOccurrence(cfg, bb, cfg::LocalOccurrence{local, binding.loc},
ValueCategory::RValue);
ValueCategory::RValue, core::TypePtr());
};
switch (binding.value.tag()) {
case cfg::Tag::Literal: {
Expand All @@ -879,7 +957,8 @@ class CFGTraversal final {

// Emit reference for the receiver, if present.
if (send->recv.loc.exists() && !send->recv.loc.empty()) {
this->emitLocalOccurrence(cfg, bb, send->recv.occurrence(), ValueCategory::RValue);
this->emitLocalOccurrence(cfg, bb, send->recv.occurrence(), ValueCategory::RValue,
core::TypePtr());
}

// Emit reference for the method being called
Expand Down Expand Up @@ -919,7 +998,8 @@ class CFGTraversal final {
// and the first one is a write. Instead of emitting two occurrences, it'd be nice to emit
// a combined read-write occurrence. However, that would require complicating the code a
// bit, so let's leave it as-is for now.
this->emitLocalOccurrence(cfg, bb, arg.occurrence(), ValueCategory::RValue);
this->emitLocalOccurrence(cfg, bb, arg.occurrence(), ValueCategory::RValue,
core::TypePtr());
}

break;
Expand Down
Loading