From cbe8e32ff456a3496653dc27291d727a066a60c8 Mon Sep 17 00:00:00 2001 From: Varun Gandhi Date: Mon, 18 Jul 2022 19:08:39 +0800 Subject: [PATCH 1/3] fix: Fix bug in NamedSymbolRef's kind() method. --- scip_indexer/SCIPIndexer.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc index 4473385273..e03bbf0d1b 100644 --- a/scip_indexer/SCIPIndexer.cc +++ b/scip_indexer/SCIPIndexer.cc @@ -230,6 +230,9 @@ class NamedSymbolRef final { if (this->selfOrOwner.isFieldOrStaticField()) { return Kind::StaticField; } + if (this->selfOrOwner.isMethod()) { + return Kind::Method; + } return Kind::ClassOrModule; } From 37766f845d40f7f13bbe8ed0fdda522027dad3cd Mon Sep 17 00:00:00 2001 From: Varun Gandhi Date: Mon, 18 Jul 2022 19:09:29 +0800 Subject: [PATCH 2/3] cleanup: Add more useful error message for an enforce. --- scip_indexer/SCIPIndexer.cc | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc index e03bbf0d1b..2759a6a299 100644 --- a/scip_indexer/SCIPIndexer.cc +++ b/scip_indexer/SCIPIndexer.cc @@ -236,6 +236,21 @@ class NamedSymbolRef final { return Kind::ClassOrModule; } + 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::StaticField: + return fmt::format("StaticField {}", 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)); + } + ENFORCE(false, "impossible"); + } + core::SymbolRef asSymbolRef() const { ENFORCE(this->kind() != Kind::UndeclaredField); return this->selfOrOwner; @@ -957,9 +972,11 @@ class CFGTraversal final { } } // Sort for determinism - fast_sort(todo, [](const SymbolWithLoc &p1, const SymbolWithLoc &p2) -> bool { + fast_sort(todo, [&](const SymbolWithLoc &p1, const SymbolWithLoc &p2) -> bool { ENFORCE(p1.second.beginPos() != p2.second.beginPos(), - "Different alias instructions should correspond to different start offsets"); + "found alias instructions with same start offset in {}, source:\n{}\nsym1 = {}, sym2 = {}\n", + file.data(gs).path(), core::Loc(file, p1.second).toString(gs), p1.first.showRaw(gs), + p2.first.showRaw(gs)); return p1.second.beginPos() < p2.second.beginPos(); }); // NOTE:(varun) Not 100% sure if emitting a reference here. Here's why it's written this From fd056f8a7ea004db8e0b7bfc8eddce05baea122e Mon Sep 17 00:00:00 2001 From: Varun Gandhi Date: Mon, 18 Jul 2022 19:10:09 +0800 Subject: [PATCH 3/3] fix: Fix bug where LocOffsets in AliasMap were incorrect. --- scip_indexer/SCIPIndexer.cc | 12 +++++++++--- test/scip/testdata/types.rb | 5 +++++ test/scip/testdata/types.snapshot.rb | 8 ++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 test/scip/testdata/types.rb create mode 100644 test/scip/testdata/types.snapshot.rb diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc index 2759a6a299..9709c33c96 100644 --- a/scip_indexer/SCIPIndexer.cc +++ b/scip_indexer/SCIPIndexer.cc @@ -596,6 +596,11 @@ class AliasMap final { auto &gs = ctx.state; auto method = ctx.owner; auto klass = method.owner(gs); + // Make sure that the offsets we store here match the offsets we use + // in saveDefinition/saveReference. + auto trim = [&](core::LocOffsets loc) -> core::LocOffsets { + return trimColonColonPrefix(gs, core::Loc(ctx.file, loc)).offsets(); + }; for (auto &bb : cfg.basicBlocks) { for (auto &bind : bb->exprs) { auto *instr = cfg::cast_instruction(bind.value); @@ -610,12 +615,13 @@ class AliasMap final { } if (sym == core::Symbols::Magic_undeclaredFieldStub()) { ENFORCE(!bind.loc.empty()); - this->map.insert( + this->map.insert( // no trim(...) because undeclared fields shouldn't have :: {bind.bind.variable, {NamedSymbolRef::undeclaredField(klass, instr->name), bind.loc, false}}); continue; } if (sym.isStaticField(gs)) { - this->map.insert({bind.bind.variable, {NamedSymbolRef::staticField(instr->what), bind.loc, false}}); + this->map.insert( + {bind.bind.variable, {NamedSymbolRef::staticField(instr->what), trim(bind.loc), false}}); continue; } // Outside of definition contexts for classes & modules, @@ -632,7 +638,7 @@ class AliasMap final { if (!loc.exists() || loc.empty()) { // For special classes like Sorbet::Private::Static continue; } - this->map.insert({bind.bind.variable, {NamedSymbolRef::classOrModule(sym), loc, false}}); + this->map.insert({bind.bind.variable, {NamedSymbolRef::classOrModule(sym), trim(loc), false}}); } } } diff --git a/test/scip/testdata/types.rb b/test/scip/testdata/types.rb new file mode 100644 index 0000000000..5fa55f2a88 --- /dev/null +++ b/test/scip/testdata/types.rb @@ -0,0 +1,5 @@ +# typed: true + +def f() + T.let(true, T::Boolean) +end diff --git a/test/scip/testdata/types.snapshot.rb b/test/scip/testdata/types.snapshot.rb new file mode 100644 index 0000000000..094f6c7245 --- /dev/null +++ b/test/scip/testdata/types.snapshot.rb @@ -0,0 +1,8 @@ + # typed: true + + def f() +#^^^^^^^ definition scip-ruby gem TODO TODO Object#f(). + T.let(true, T::Boolean) +# ^ reference scip-ruby gem TODO TODO T# +# ^^^^^^^ reference scip-ruby gem TODO TODO T#Boolean. + end