Skip to content

fix: Stop incorrectly emitting multiline occurrences #212

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
varungandhi-src merged 1 commit into from
Jul 9, 2024
+17 −9
Merged

fix: Stop incorrectly emitting multiline occurrences #212

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
20 changes: 15 additions & 5 deletions scip_indexer/SCIPIndexer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,19 @@ class SCIPState {
core::Loc occLoc, const SmallVec<string> &docs,
const SmallVec<scip::Relationship> &rels) {
ENFORCE(!symbolString.empty());

auto emitted = this->saveSymbolInfo(file, symbolString, docs, rels);

occLoc = trimColonColonPrefix(gs, occLoc);
auto range = sorbet::scip_indexer::fromSorbetLoc(gs, occLoc);
if (range.size() == 4) {
// Don't emit multiline occurrences; generally this indicates a bug in the indexer.
// FIXME: This causes us to miss the definition for the initialize method
// in the struct.rb test case.
return absl::OkStatus();
}
auto emitted = this->saveSymbolInfo(file, symbolString, docs, rels);
scip::Occurrence occurrence;
occurrence.set_symbol(symbolString);
occurrence.set_symbol_roles(scip::SymbolRole::Definition);
for (auto val : sorbet::scip_indexer::fromSorbetLoc(gs, occLoc)) {
for (auto val : range) {
occurrence.add_range(val);
}
switch (emitted) {
Expand All @@ -320,7 +325,12 @@ class SCIPState {
scip::Occurrence occurrence;
occurrence.set_symbol(symbolString);
occurrence.set_symbol_roles(symbol_roles);
for (auto val : sorbet::scip_indexer::fromSorbetLoc(gs, occLoc)) {
auto range = sorbet::scip_indexer::fromSorbetLoc(gs, occLoc);
if (range.size() == 4) {
// Don't emit multiline occurrences; generally this indicates a bug in the indexer.
return;
}
for (auto val : range) {
occurrence.add_range(val);
}
for (auto &doc : overrideDocs) {
Expand Down
4 changes: 0 additions & 4 deletions test/scip/testdata/struct.snapshot.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ def f
#^^^^^ reference [..] POINT#
#^^^^^ definition [..] POINT#
#^^^^^ definition [..] POINT#
#^^^^^^^^^^^^^^^^^^^^ reference [..] Struct#
#^^^^^^^^^^^^^^^^^^^^ definition local 2~#119448696
#^^^^^^^^^^^^^^^^^^^^ definition local 5~#119448696
#^^^^^^^^^^^^^^^^^^^^ definition [..] POINT#initialize().
# ^ definition [..] POINT#`x=`().
# ^ definition [..] POINT#x().
# ^ reference [..] BasicObject#
Expand Down
2 changes: 2 additions & 0 deletions test/scip_test_runner.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ struct SCIPRange final {
auto &r = protoRange;
if (r.size() == 4) {
*this = SCIPRange(SCIPPosition{r[0] + 1, r[1] + 1}, SCIPPosition{r[2] + 1, r[3] + 1});
return;
}
*this = SCIPRange(SCIPPosition{r[0] + 1, r[1] + 1}, SCIPPosition{r[0] + 1, r[2] + 1});
}
Expand Down Expand Up @@ -366,6 +367,7 @@ void formatSnapshot(const scip::Document &document, FormatOptions options, std::
auto occ = occurrences[occ_i];
auto range = SCIPRange(occ.range());
if (range.isMultiline()) { // FIXME(varun): Handle multiline occurrences.
ENFORCE(false, "Got multiline occurrence which shouldn't have been emitted: {}", range.toString());
continue;
}
bool isDefinition = ((unsigned(occ.symbol_roles()) & unsigned(scip::SymbolRole::Definition)) > 0);
Expand Down