From d1b83d5e0169a40eef8b4b6dcbd72721e673f772 Mon Sep 17 00:00:00 2001 From: Varun Gandhi Date: Tue, 12 Jul 2022 19:11:32 +0800 Subject: [PATCH] fix: Emit qualified names for symbols --- scip_indexer/SCIPIndexer.cc | 71 +++++++++++-------- scip_indexer/SCIPUtils.cc | 14 ++-- test/scip/testdata/args.snapshot.rb | 2 +- test/scip/testdata/arrays.snapshot.rb | 2 +- test/scip/testdata/classes.snapshot.rb | 36 +++++----- test/scip/testdata/for.snapshot.rb | 2 +- test/scip/testdata/functions.snapshot.rb | 6 +- test/scip/testdata/hashes.snapshot.rb | 2 +- .../loops_and_conditionals.snapshot.rb | 14 ++-- 9 files changed, 83 insertions(+), 66 deletions(-) diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc index cc4e5b4e55..f86df1e8e1 100644 --- a/scip_indexer/SCIPIndexer.cc +++ b/scip_indexer/SCIPIndexer.cc @@ -134,36 +134,49 @@ absl::Status symbolForExpr(const core::GlobalState &gs, core::SymbolRef symRef, package.set_version("TODO"); *symbol.mutable_package() = move(package); - scip::Descriptor descriptor; - *descriptor.mutable_name() = symRef.name(gs).show(gs); - - // TODO: Are the scip descriptor kinds correct? - switch (symRef.kind()) { - case core::SymbolRef::Kind::Method: - // NOTE: There is a separate isOverloaded field in the flags field, - // despite SO/docs saying that Ruby doesn't support method overloading, - // Technically, we should better understand how this works and set the - // disambiguator based on that. However, right now, an extension's - // type-checking function is not run if a method is overloaded, - // (see pipeline.cc), so it's unclear if we need to care about that. - descriptor.set_suffix(scip::Descriptor::Method); - break; - case core::SymbolRef::Kind::ClassOrModule: - descriptor.set_suffix(scip::Descriptor::Type); - break; - case core::SymbolRef::Kind::TypeArgument: - descriptor.set_suffix(scip::Descriptor::TypeParameter); - break; - case core::SymbolRef::Kind::FieldOrStaticField: - descriptor.set_suffix(scip::Descriptor::Term); - break; - case core::SymbolRef::Kind::TypeMember: // TODO: What does TypeMember mean? - descriptor.set_suffix(scip::Descriptor::Type); - break; - default: - return absl::InvalidArgumentError("unexpected expr type for symbol computation"); + InlinedVector descriptors; + auto cur = symRef; + while (cur != core::Symbols::root()) { + // NOTE:(varun) The current scheme will cause multiple 'definitions' for the same + // entity if it is present in different files, because the path is not encoded + // in the descriptor whose parent is the root. This matches the semantics of + // RubyMine, but we may want to revisit this if it is problematic for classes + // that are extended in lots of places. + scip::Descriptor descriptor; + *descriptor.mutable_name() = cur.name(gs).show(gs); + // TODO: Are the scip descriptor kinds correct? + switch (cur.kind()) { + case core::SymbolRef::Kind::Method: + // NOTE: There is a separate isOverloaded field in the flags field, + // despite SO/docs saying that Ruby doesn't support method overloading, + // Technically, we should better understand how this works and set the + // disambiguator based on that. However, right now, an extension's + // type-checking function is not run if a method is overloaded, + // (see pipeline.cc), so it's unclear if we need to care about that. + descriptor.set_suffix(scip::Descriptor::Method); + break; + case core::SymbolRef::Kind::ClassOrModule: + descriptor.set_suffix(scip::Descriptor::Type); + break; + case core::SymbolRef::Kind::TypeArgument: + descriptor.set_suffix(scip::Descriptor::TypeParameter); + break; + case core::SymbolRef::Kind::FieldOrStaticField: + descriptor.set_suffix(scip::Descriptor::Term); + break; + case core::SymbolRef::Kind::TypeMember: // TODO: What does TypeMember mean? + descriptor.set_suffix(scip::Descriptor::Type); + break; + default: + return absl::InvalidArgumentError("unexpected expr type for symbol computation"); + } + descriptors.push_back(move(descriptor)); + cur = cur.owner(gs); + } + while (!descriptors.empty()) { + *symbol.add_descriptors() = move(descriptors.back()); + descriptors.pop_back(); } - *symbol.add_descriptors() = move(descriptor); return absl::OkStatus(); } diff --git a/scip_indexer/SCIPUtils.cc b/scip_indexer/SCIPUtils.cc index 25e2937ca4..c6a6d61c28 100644 --- a/scip_indexer/SCIPUtils.cc +++ b/scip_indexer/SCIPUtils.cc @@ -54,12 +54,16 @@ absl::Status emitSymbolString(const scip::Symbol &symbol, string &out) { } else { out.append(". . "); } - if (symbol.descriptors_size() == 1) { - return emitDescriptorString(symbol.descriptors()[0], out); + if (symbol.descriptors_size() == 0) { + return absl::InvalidArgumentError("expected symbol to have at least 1 descriptor"); } - // FIXME(varun): Are we going to emit multiple descriptors per symbol? - return absl::InvalidArgumentError( - absl::StrCat("expected symbol to have 1 descriptor but found %d", symbol.descriptors_size())); + for (auto i = 0; i < symbol.descriptors_size(); ++i) { + auto status = emitDescriptorString(symbol.descriptors()[i], out); + if (!status.ok()) { + return status; + } + } + return absl::OkStatus(); } }; // end namespace scip::utils \ No newline at end of file diff --git a/test/scip/testdata/args.snapshot.rb b/test/scip/testdata/args.snapshot.rb index 090be2b27d..74a8245d4f 100644 --- a/test/scip/testdata/args.snapshot.rb +++ b/test/scip/testdata/args.snapshot.rb @@ -1,7 +1,7 @@ # typed: true def args(x, y) -#^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO args(). +#^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#args(). # ^ definition local 1~#2634721084 # ^ definition local 2~#2634721084 z = x + y diff --git a/test/scip/testdata/arrays.snapshot.rb b/test/scip/testdata/arrays.snapshot.rb index fadbe0479e..4d121c1639 100644 --- a/test/scip/testdata/arrays.snapshot.rb +++ b/test/scip/testdata/arrays.snapshot.rb @@ -1,7 +1,7 @@ # typed: true def arrays(a, i) -#^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO arrays(). +#^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#arrays(). # ^ definition local 1~#513334479 # ^ definition local 2~#513334479 a[0] = 0 diff --git a/test/scip/testdata/classes.snapshot.rb b/test/scip/testdata/classes.snapshot.rb index f07014c077..4698274c26 100644 --- a/test/scip/testdata/classes.snapshot.rb +++ b/test/scip/testdata/classes.snapshot.rb @@ -6,14 +6,14 @@ class C1 # ^^ definition scip-ruby gem TODO TODO C1# def f() -# ^^^^^^^ definition scip-ruby gem TODO TODO f(). +# ^^^^^^^ definition scip-ruby gem TODO TODO C1#f(). _a = C1.new # ^^ definition local 2~#3809224601 # ^^ reference scip-ruby gem TODO TODO C1# _b = M2::C2.new # ^^ definition local 5~#3809224601 # ^^ reference scip-ruby gem TODO TODO M2# -# ^^ reference scip-ruby gem TODO TODO C2# +# ^^ reference scip-ruby gem TODO TODO M2#C2# return end end @@ -21,24 +21,24 @@ def f() module M2 # ^^ definition scip-ruby gem TODO TODO M2# class C2 -# ^^ definition scip-ruby gem TODO TODO C2# +# ^^ definition scip-ruby gem TODO TODO M2#C2# end end class M3::C3 # ^^ definition scip-ruby gem TODO TODO M3# -# ^^ definition scip-ruby gem TODO TODO C3# +# ^^ definition scip-ruby gem TODO TODO M3#C3# end def local_class() -#^^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO local_class(). +#^^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#local_class(). localClass = Class.new # ^^^^^^^^^^ definition local 2~#552113551 # ^^^^^ reference scip-ruby gem TODO TODO Class# # Technically, this is not supported by Sorbet (https://srb.help/3001), # but make sure we don't crash or do something weird. def localClass.myMethod() -# ^^^^^^^^^^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO myMethod(). +# ^^^^^^^^^^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#myMethod(). ":)" end _c = localClass.new @@ -47,7 +47,7 @@ def localClass.myMethod() _m = localClass.myMethod # ^^ definition local 4~#552113551 # ^^^^^^^^^^ reference local 2~#552113551 -# ^^^^^^^^ reference scip-ruby gem TODO TODO myMethod(). +# ^^^^^^^^ reference scip-ruby gem TODO TODO Object#myMethod(). return end @@ -59,27 +59,27 @@ module M4 end def module_access() -#^^^^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO module_access(). +#^^^^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#module_access(). _ = M4::K # ^ definition local 2~#3353511840 # ^^ reference scip-ruby gem TODO TODO M4# -# ^ reference scip-ruby gem TODO TODO K. +# ^ reference scip-ruby gem TODO TODO M4#K. return end module M5 # ^^ definition scip-ruby gem TODO TODO M5# module M6 -# ^^ definition scip-ruby gem TODO TODO M6# +# ^^ definition scip-ruby gem TODO TODO M5#M6# def self.g() -# ^^^^^^^^^^^^ definition scip-ruby gem TODO TODO g(). +# ^^^^^^^^^^^^ definition scip-ruby gem TODO TODO M5##g(). end end def self.h() -# ^^^^^^^^^^^^ definition scip-ruby gem TODO TODO h(). +# ^^^^^^^^^^^^ definition scip-ruby gem TODO TODO #h(). M6.g() -# ^^ reference scip-ruby gem TODO TODO M6# +# ^^ reference scip-ruby gem TODO TODO M5#M6# return end end @@ -87,17 +87,17 @@ def self.h() class C7 # ^^ definition scip-ruby gem TODO TODO C7# module M8 -# ^^ definition scip-ruby gem TODO TODO M8# +# ^^ definition scip-ruby gem TODO TODO C7#M8# def self.i() -# ^^^^^^^^^^^^ definition scip-ruby gem TODO TODO i(). +# ^^^^^^^^^^^^ definition scip-ruby gem TODO TODO C7##i(). end end def j() -# ^^^^^^^ definition scip-ruby gem TODO TODO j(). +# ^^^^^^^ definition scip-ruby gem TODO TODO C7#j(). M8.j() -# ^^ reference scip-ruby gem TODO TODO M8# -# ^ reference scip-ruby gem TODO TODO j(). +# ^^ reference scip-ruby gem TODO TODO C7#M8# +# ^ reference scip-ruby gem TODO TODO C7#j(). return end end diff --git a/test/scip/testdata/for.snapshot.rb b/test/scip/testdata/for.snapshot.rb index 72fa62c99a..953d56f372 100644 --- a/test/scip/testdata/for.snapshot.rb +++ b/test/scip/testdata/for.snapshot.rb @@ -1,7 +1,7 @@ # typed: true def for_loop() -#^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO for_loop(). +#^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#for_loop(). y = 0 # ^ definition local 1~#1120785331 for x in [1, 2, 3] diff --git a/test/scip/testdata/functions.snapshot.rb b/test/scip/testdata/functions.snapshot.rb index 4b76e5bda9..5e95d962f6 100644 --- a/test/scip/testdata/functions.snapshot.rb +++ b/test/scip/testdata/functions.snapshot.rb @@ -1,7 +1,7 @@ # typed: true def globalFn1() -#^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO globalFn1(). +#^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#globalFn1(). x = 10 # ^ definition local 1~#3846536873 x @@ -9,10 +9,10 @@ def globalFn1() end def globalFn2() -#^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO globalFn2(). +#^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#globalFn2(). x = globalFn1() # ^ definition local 1~#3796204016 # ^^^^^^^^^^^^^^^ reference local 1~#3796204016 -# ^^^^^^^^^ reference scip-ruby gem TODO TODO globalFn1(). +# ^^^^^^^^^ reference scip-ruby gem TODO TODO Object#globalFn1(). end diff --git a/test/scip/testdata/hashes.snapshot.rb b/test/scip/testdata/hashes.snapshot.rb index b09eb30c1d..13f5c4f5a1 100644 --- a/test/scip/testdata/hashes.snapshot.rb +++ b/test/scip/testdata/hashes.snapshot.rb @@ -1,7 +1,7 @@ # typed: true def hashes(h, k) -#^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO hashes(). +#^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#hashes(). # ^ definition local 1~#1685166589 # ^ definition local 2~#1685166589 h["hello"] = "world" diff --git a/test/scip/testdata/loops_and_conditionals.snapshot.rb b/test/scip/testdata/loops_and_conditionals.snapshot.rb index 26aa0a85e3..e4e414c16e 100644 --- a/test/scip/testdata/loops_and_conditionals.snapshot.rb +++ b/test/scip/testdata/loops_and_conditionals.snapshot.rb @@ -1,7 +1,7 @@ # typed: true def if_elsif_else() -#^^^^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO if_elsif_else(). +#^^^^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#if_elsif_else(). x = 0 # ^ definition local 1~#2393773952 y = 0 @@ -43,7 +43,7 @@ def if_elsif_else() end def unless() -#^^^^^^^^^^^^ definition scip-ruby gem TODO TODO unless(). +#^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#unless(). z = 0 # ^ definition local 1~#2827997891 x = 1 @@ -66,7 +66,7 @@ def unless() end def case(x, y) -#^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO case(). +#^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#case(). # ^ definition local 1~#2602907825 # ^ definition local 2~#2602907825 case x @@ -91,7 +91,7 @@ def case(x, y) end def for(xs) -#^^^^^^^^^^^ definition scip-ruby gem TODO TODO for(). +#^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#for(). # ^^ definition local 1~#2901640080 for e in xs # ^ definition local 2~#2901640080 @@ -124,7 +124,7 @@ def for(xs) end def while(xs) -#^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO while(). +#^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#while(). # ^^ definition local 1~#231090382 i = 0 # ^ definition local 2~#231090382 @@ -161,7 +161,7 @@ def while(xs) end def until(xs) -#^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO until(). +#^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#until(). # ^^ definition local 1~#3132432719 i = 0 # ^ definition local 2~#3132432719 @@ -198,7 +198,7 @@ def until(xs) end def flip_flop(xs) -#^^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO flip_flop(). +#^^^^^^^^^^^^^^^^^ definition scip-ruby gem TODO TODO Object#flip_flop(). # ^^ definition local 1~#2191960030 # NOTE: flip-flops are unsupported (https://srb.help/3003) # Unlike redo, which somehow works, we fail to emit references