Skip to content

Populate enclosing_range on SCIP definition Occurrences #250

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

Open
jjuliamolin wants to merge 1 commit into
base: scip-ruby/master
Choose a base branch
from
Open

Populate enclosing_range on SCIP definition Occurrences #250

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
8 changes: 8 additions & 0 deletions proto/SCIP.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,14 @@ message Occurrence {
SyntaxKind syntax_kind = 5;
// (optional) Diagnostics that have been reported for this specific range.
repeated Diagnostic diagnostics = 6;
// (optional) Using the same encoding as the sibling `range` field, half-open
// source range of the nearest non-trivial enclosing AST node. This range must
// enclose the `range` field.
//
// For definition occurrences, the enclosing range should indicate the
// start/end bounds of the entire definition AST node, including
// documentation.
repeated int32 enclosing_range = 7;
}

// Represents a diagnostic, such as a compiler error or warning, which should be
Expand Down
45 changes: 31 additions & 14 deletions scip_indexer/SCIPIndexer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,8 @@ class SCIPState {

absl::Status saveDefinitionImpl(const core::GlobalState &gs, core::FileRef file, const string &symbolString,
core::Loc occLoc, const SmallVec<string> &docs,
const SmallVec<scip::Relationship> &rels) {
const SmallVec<scip::Relationship> &rels,
optional<core::Loc> enclosingLoc = nullopt) {
ENFORCE(!symbolString.empty());
occLoc = trimColonColonPrefix(gs, occLoc);
auto range = sorbet::scip_indexer::fromSorbetLoc(gs, occLoc);
Expand All @@ -305,6 +306,12 @@ class SCIPState {
for (auto val : range) {
occurrence.add_range(val);
}
if (enclosingLoc.has_value() && !enclosingLoc->empty()) {
auto encRange = sorbet::scip_indexer::fromSorbetLoc(gs, enclosingLoc.value());
for (auto val : encRange) {
occurrence.add_enclosing_range(val);
}
}
switch (emitted) {
case Emitted::Now:
break;
Expand Down Expand Up @@ -396,7 +403,8 @@ class SCIPState {
}

public:
absl::Status saveDefinition(const core::GlobalState &gs, core::FileRef file, OwnedLocal occ, core::TypePtr type) {
absl::Status saveDefinition(const core::GlobalState &gs, core::FileRef file, OwnedLocal occ, core::TypePtr type,
optional<core::Loc> enclosingLoc = nullopt) {
if (this->cacheOccurrence(gs, file, occ, scip::SymbolRole::Definition)) {
return absl::OkStatus();
}
Expand All @@ -407,7 +415,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, {}, enclosingLoc);
}

void saveAliasRelationship(const core::GlobalState &gs, UntypedGenericSymbolRef aliasedSymbol,
Expand All @@ -423,7 +431,8 @@ class SCIPState {
// TODO(varun): Should we always pass in the location instead of sometimes only?
absl::Status saveDefinition(const core::GlobalState &gs, core::FileRef file, GenericSymbolRef symRef,
optional<UntypedGenericSymbolRef> aliasedSymbol,
optional<core::LocOffsets> loc = nullopt) {
optional<core::LocOffsets> loc = nullopt,
optional<core::Loc> enclosingLoc = nullopt) {
// In practice, there doesn't seem to be any situation which triggers
// a duplicate definition being emitted, so skip calling cacheOccurrence here.
auto occLoc = loc.has_value() ? core::Loc(file, loc.value()) : symRef.symbolLoc(gs);
Expand Down Expand Up @@ -452,7 +461,7 @@ class SCIPState {
this->saveAliasRelationship(gs, aliasedSymbol.value(), rels);
}

return this->saveDefinitionImpl(gs, file, symbolString, occLoc, docs, rels);
return this->saveDefinitionImpl(gs, file, symbolString, occLoc, docs, rels, enclosingLoc);
}

absl::Status saveReference(const core::GlobalState &gs, core::FileRef file, OwnedLocal occ,
Expand Down Expand Up @@ -868,10 +877,13 @@ class CFGTraversal final {
uint32_t counter = 0;
SCIPState &scipState;
core::Context ctx;
// The enclosing range for all occurrences emitted during this traversal
// (i.e. the method definition's full source range).
optional<core::Loc> enclosingLoc;

public:
CFGTraversal(SCIPState &scipState, core::Context ctx)
: blockLocals(), functionLocals(), aliasMap(), scipState(scipState), ctx(ctx) {}
CFGTraversal(SCIPState &scipState, core::Context ctx, optional<core::Loc> enclosingLoc = nullopt)
: blockLocals(), functionLocals(), aliasMap(), scipState(scipState), ctx(ctx), enclosingLoc(enclosingLoc) {}

private:
uint32_t addLocal(const cfg::BasicBlock *bb, cfg::LocalRef localRef) {
Expand Down Expand Up @@ -976,7 +988,7 @@ class CFGTraversal final {
defRefData.aliasRHS->toString(gs, cfg), file.data(gs).path());
}
}
status = this->scipState.saveDefinition(gs, file, namedSym, aliasedSymbol, loc);
status = this->scipState.saveDefinition(gs, file, namedSym, aliasedSymbol, loc, this->enclosingLoc);
} else {
auto overrideType = computeOverrideType(namedSym.definitionType(), type);
status = this->scipState.saveReference(ctx, namedSym, overrideType, loc, referenceRole);
Expand All @@ -995,7 +1007,8 @@ class CFGTraversal final {
}
}
if (isDefinition) {
status = this->scipState.saveDefinition(gs, file, OwnedLocal{this->ctx.owner, localId, loc}, type);
status = this->scipState.saveDefinition(gs, file, OwnedLocal{this->ctx.owner, localId, loc}, type,
this->enclosingLoc);
} else {
status = this->scipState.saveReference(gs, file, OwnedLocal{this->ctx.owner, localId, loc},
overrideType, referenceRole);
Expand Down Expand Up @@ -1059,7 +1072,8 @@ class CFGTraversal final {
absl::Status status;
string kind;
if (isDefinition) {
status = this->scipState.saveDefinition(gs, file, namedSym, /*aliasedSymbol*/ nullopt, arg.loc);
status = this->scipState.saveDefinition(gs, file, namedSym, /*aliasedSymbol*/ nullopt, arg.loc,
this->enclosingLoc);
kind = "definition";
} else {
status = this->scipState.saveReference(ctx, namedSym, nullopt, arg.loc, 0);
Expand Down Expand Up @@ -1255,7 +1269,8 @@ class CFGTraversal final {
if (isMethodClassStaticInit && namedSym.isEnumConstant(gs)) {
// Enum constants don't have references in the <static-init> of the owner
// class, but they do have alias instructions, so record those as definitions.
status = this->scipState.saveDefinition(ctx, file, namedSym, /*aliasSymbol*/ nullopt, loc);
status = this->scipState.saveDefinition(ctx, file, namedSym, /*aliasSymbol*/ nullopt, loc,
this->enclosingLoc);
} else {
status = this->scipState.saveReference(ctx, namedSym, nullopt, loc, 0);
}
Expand Down Expand Up @@ -1491,7 +1506,8 @@ class SCIPSemanticExtension : public SemanticExtension {

auto scipState = this->getSCIPState();
auto sym = scip_indexer::GenericSymbolRef::classOrModule(klass.symbol);
auto status = scipState->saveDefinition(gs, file, sym, /*aliasedSymbol*/ nullopt, nameLoc);
auto klassLoc = core::Loc(file, klass.loc);
auto status = scipState->saveDefinition(gs, file, sym, /*aliasedSymbol*/ nullopt, nameLoc, klassLoc);
ENFORCE(status.ok());
auto *expr = &klass.name;
if (auto *constantLit = ast::cast_tree<ast::ConstantLit>(*expr)) {
Expand All @@ -1513,9 +1529,10 @@ class SCIPSemanticExtension : public SemanticExtension {
return;
}
auto scipState = this->getSCIPState();
auto methodLoc = core::Loc(file, methodDef.loc);
if (methodDef.name != core::Names::staticInit()) {
auto sym = scip_indexer::GenericSymbolRef::method(methodDef.symbol);
auto status = scipState->saveDefinition(gs, file, sym, /*aliasedSymbol*/ nullopt);
auto status = scipState->saveDefinition(gs, file, sym, /*aliasedSymbol*/ nullopt, /*loc*/ nullopt, methodLoc);
ENFORCE(status.ok());
}

Expand All @@ -1534,7 +1551,7 @@ class SCIPSemanticExtension : public SemanticExtension {
// information repeatedly for each occurrence.

auto &scipStateRef = *scipState.get();
sorbet::scip_indexer::CFGTraversal traversal(scipStateRef, core::Context(gs, methodDef.symbol, file));
sorbet::scip_indexer::CFGTraversal traversal(scipStateRef, core::Context(gs, methodDef.symbol, file), methodLoc);
traversal.traverse(cfg);
scipStateRef.clearFunctionLocalCaches();
}
Expand Down
19 changes: 19 additions & 0 deletions test/scip/testdata/alias.snapshot.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,21 @@

class X
# ^ definition [..] X#
# enclosing_range 3:1-14:4
alias_method :am_aaa, :aaa
# ^^^^^^^^^^^^ reference [..] Module#alias_method().
alias :a_aaa :aaa

def aaa
# ^^^ definition [..] X#aaa().
# enclosing_range 7:3-9:6
puts "AAA"
# ^^^^ reference [..] Kernel#puts().
end

def check_alias
# ^^^^^^^^^^^ definition [..] X#check_alias().
# enclosing_range 11:3-13:6
return [am_aaa, a_aaa]
# ^^^^^^ reference [..] X#aaa().
# ^^^^^ reference [..] X#aaa().
Expand All @@ -22,15 +25,19 @@ def check_alias

module Mod1
# ^^^^ definition [..] Mod1#
# enclosing_range 16:1-18:4
ABC = 10
# ^^^ definition [..] Mod1#ABC.
# enclosing_range 16:1-16:12
# ^^^^^^^^ reference [..] Mod1#ABC.
end

module Mod2
# ^^^^ definition [..] Mod2#
# enclosing_range 20:1-22:4
FEG = Mod1::ABC
# ^^^ definition [..] Mod2#FEG.
# enclosing_range 20:1-20:12
# relation reference=[..] Mod1#ABC.
# ^^^^ reference [..] Mod1#
# ^^^ reference [..] Mod1#ABC.
Expand All @@ -39,7 +46,9 @@ module Mod2

def myfunction(myparam)
# ^^^^^^^^^^ definition [..] Object#myfunction().
# enclosing_range 24:1-26:4
# ^^^^^^^ definition local 1$3083414419
# enclosing_range 24:1-26:4
myparam + Mod2::FEG
# ^^^^^^^ reference local 1$3083414419
# ^^^^ reference [..] Mod2#
Expand All @@ -48,43 +57,53 @@ def myfunction(myparam)

class X < T::Enum
# ^ definition [..] X#
# enclosing_range 28:1-36:4
# ^ definition [..] X#serialize().
# enclosing_range 28:1-36:4
# ^ reference [..] T#
# ^^^^ reference [..] Module#public().
# ^^^^ reference [..] String#
# ^^^^ reference [..] T#Enum#
enums do
A = new("A")
# ^ definition [..] X#A.
# enclosing_range 28:1-28:18
# ^^^ reference [..] Class#new().
B = new
# ^ definition [..] X#B.
# enclosing_range 28:1-28:18
# ^^^ reference [..] Class#new().
C = B
# ^ definition [..] X#C.
# enclosing_range 28:1-28:18
# relation reference=[..] X#B.
# ^ reference [..] X#B.
end

All = T.let([A, B], T::Array[X])
# ^^^ definition [..] X#All.
# enclosing_range 28:1-28:18
# ^ reference [..] X#A.
# ^ reference [..] X#B.
# ^^^^^^^^ definition local 4$119448696
# enclosing_range 28:1-28:18
# ^ reference [..] X#
end

# Adding more cases like this is not supported (c.f. isTEnum),
# but let's at least add a test.
class Y < X
# ^ definition [..] Y#
# enclosing_range 40:1-45:4
# ^ reference [..] X#
enums do
D = new
# ^ definition [..] Y#D.
# enclosing_range 40:1-40:12
# ^^^ reference [..] Class#new().
E = B
# ^ definition [..] Y#E.
# enclosing_range 40:1-40:12
# relation reference=[..] X#B.
# ^^^^^ reference [..] Y#E.
# ^ reference [..] X#B.
Expand Down
10 changes: 10 additions & 0 deletions test/scip/testdata/args.snapshot.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@

def args(x, y)
# ^^^^ definition [..] Object#args().
# enclosing_range 3:1-11:4
# ^ definition local 1$2634721084
# enclosing_range 3:1-11:4
# ^ definition local 2$2634721084
# enclosing_range 3:1-11:4
z = x + y
# ^ definition local 3$2634721084
# enclosing_range 3:1-11:4
# ^ reference local 1$2634721084
# ^ reference local 2$2634721084
if x == 2
Expand All @@ -27,9 +31,13 @@ def args(x, y)

def keyword_args(w:, x: 3, y: [], **kwargs)
# ^^^^^^^^^^^^ definition [..] Object#keyword_args().
# enclosing_range 13:1-17:4
# ^^ definition local 1$3526982640
# enclosing_range 13:1-17:4
# ^^ definition local 2$3526982640
# enclosing_range 13:1-17:4
# ^^ definition local 3$3526982640
# enclosing_range 13:1-17:4
y << w + x
# ^ reference local 3$3526982640
# ^ reference local 1$3526982640
Expand All @@ -41,8 +49,10 @@ def keyword_args(w:, x: 3, y: [], **kwargs)

def use_kwargs
# ^^^^^^^^^^ definition [..] Object#use_kwargs().
# enclosing_range 19:1-24:4
h = { a: 3 }
# ^ definition local 1$571973038
# enclosing_range 19:1-24:4
keyword_args(w: 0, **h)
# ^^^^^^^^^^^^ reference [..] Object#keyword_args().
# ^ reference local 1$571973038
Expand Down
4 changes: 4 additions & 0 deletions test/scip/testdata/arrays.snapshot.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@

def arrays(a, i)
# ^^^^^^ definition [..] Object#arrays().
# enclosing_range 3:1-9:4
# ^ definition local 1$513334479
# enclosing_range 3:1-9:4
# ^ definition local 2$513334479
# enclosing_range 3:1-9:4
a[0] = 0
# ^ reference local 1$513334479
a[1] = a[2]
Expand All @@ -16,6 +19,7 @@ def arrays(a, i)
# ^ reference local 2$513334479
b = a[2..-1]
# ^ definition local 3$513334479
# enclosing_range 3:1-9:4
# ^ reference local 1$513334479
a << a[-1]
# ^ reference local 1$513334479
Expand Down
Loading