Fix hierarchical document symbol

1. Fixed a bug on building document symbol tree: As sym2ds was updated in
place, nested funcs/types may be moved into children of another
lsDocumentSymbol before itself got processed.

2. Namespaces only have declarations, in the old implementation it wasn't included in the result, making the result less hierarchical. This
commit fixes this by including the declarations of a symbol if no
definitions found.
This commit is contained in:
Riatre Foo 2018-10-11 04:32:59 +08:00 committed by Fangrui Song
parent 82deedf8c3
commit ec71d4c6c0
2 changed files with 80 additions and 65 deletions

View File

@ -199,7 +199,8 @@ struct VarDef : NameMixin<VarDef> {
return spell && return spell &&
(parent_kind == lsSymbolKind::Function || (parent_kind == lsSymbolKind::Function ||
parent_kind == lsSymbolKind::Method || parent_kind == lsSymbolKind::Method ||
parent_kind == lsSymbolKind::StaticMethod) && parent_kind == lsSymbolKind::StaticMethod ||
parent_kind == lsSymbolKind::Constructor) &&
storage == clang::SC_None; storage == clang::SC_None;
} }

View File

@ -64,11 +64,16 @@ struct Out_HierarchicalDocumentSymbol
}; };
MAKE_REFLECT_STRUCT(Out_HierarchicalDocumentSymbol, jsonrpc, id, result); MAKE_REFLECT_STRUCT(Out_HierarchicalDocumentSymbol, jsonrpc, id, result);
bool IgnoreType(const QueryType::Def *def) { template <typename Def>
bool Ignore(const Def *def) {
return false;
}
template <>
bool Ignore(const QueryType::Def *def) {
return !def || def->kind == lsSymbolKind::TypeParameter; return !def || def->kind == lsSymbolKind::TypeParameter;
} }
template<>
bool IgnoreVar(const QueryVar::Def *def) { bool Ignore(const QueryVar::Def *def) {
return !def || def->is_local(); return !def || def->is_local();
} }
@ -100,92 +105,101 @@ struct Handler_TextDocumentDocumentSymbol
std::sort(out.result.begin(), out.result.end()); std::sort(out.result.begin(), out.result.end());
pipeline::WriteStdout(kMethodType, out); pipeline::WriteStdout(kMethodType, out);
} else if (g_config->client.hierarchicalDocumentSymbolSupport) { } else if (g_config->client.hierarchicalDocumentSymbolSupport) {
std::unordered_map< std::unordered_map<SymbolIdx, std::unique_ptr<lsDocumentSymbol>> sym2ds;
SymbolIdx, std::pair<const void *, std::unique_ptr<lsDocumentSymbol>>> std::vector<std::pair<const QueryFunc::Def *, lsDocumentSymbol *>> funcs;
sym2ds; std::vector<std::pair<const QueryType::Def *, lsDocumentSymbol *>> types;
for (auto [sym, refcnt] : symbol2refcnt) { for (auto [sym, refcnt] : symbol2refcnt) {
if (refcnt <= 0) if (refcnt <= 0)
continue; continue;
auto r = sym2ds.try_emplace(SymbolIdx{sym.usr, sym.kind}); auto r = sym2ds.try_emplace(SymbolIdx{sym.usr, sym.kind});
if (!r.second) if (!r.second)
continue; continue;
auto &kv = r.first->second; auto &ds = r.first->second;
kv.second = std::make_unique<lsDocumentSymbol>(); ds = std::make_unique<lsDocumentSymbol>();
lsDocumentSymbol &ds = *kv.second; const void *def_ptr = nullptr;
WithEntity(db, sym, [&](const auto &entity) { WithEntity(db, sym, [&](const auto &entity) {
auto *def = entity.AnyDef(); auto *def = entity.AnyDef();
if (!def) if (!def)
return; return;
ds.name = def->Name(false); ds->name = def->Name(false);
ds.detail = def->Name(true); ds->detail = def->Name(true);
// Try to find a definition with spell first.
const void *candidate_def_ptr = nullptr;
for (auto &def : entity.def) for (auto &def : entity.def)
if (def.file_id == file_id) { if (def.file_id == file_id && !Ignore(&def)) {
ds->kind = def.kind;
if (def.kind == lsSymbolKind::Namespace)
candidate_def_ptr = &def;
if (!def.spell) if (!def.spell)
break; continue;
ds.kind = def.kind;
if (auto ls_range = GetLsRange(wfile, def.spell->extent)) if (auto ls_range = GetLsRange(wfile, def.spell->extent))
ds.range = *ls_range; ds->range = *ls_range;
else else
break; continue;
if (auto ls_range = GetLsRange(wfile, def.spell->range)) if (auto ls_range = GetLsRange(wfile, def.spell->range))
ds.selectionRange = *ls_range; ds->selectionRange = *ls_range;
else else
break; continue;
kv.first = static_cast<const void *>(&def); def_ptr = &def;
break;
} }
// Try to find a declaration.
if (!def_ptr && candidate_def_ptr)
for (auto &decl : entity.declarations)
if (decl.file_id == file_id) {
if (auto ls_range = GetLsRange(wfile, decl.extent))
ds->range = *ls_range;
else
continue;
if (auto ls_range = GetLsRange(wfile, decl.range))
ds->selectionRange = *ls_range;
else
continue;
def_ptr = candidate_def_ptr;
break;
}
}); });
if (kv.first && ((sym.kind == SymbolKind::Type && if (!def_ptr) {
IgnoreType((const QueryType::Def *)kv.first)) || ds.reset();
(sym.kind == SymbolKind::Var &&
IgnoreVar((const QueryVar::Def *)kv.first))))
kv.first = nullptr;
if (!kv.first) {
kv.second.reset();
continue; continue;
} }
if (sym.kind == SymbolKind::Func)
funcs.emplace_back((const QueryFunc::Def *)def_ptr, ds.get());
else if (sym.kind == SymbolKind::Type)
types.emplace_back((const QueryType::Def *)def_ptr, ds.get());
} }
for (auto &[sym, def_ds] : sym2ds) {
if (!def_ds.second) for (auto &[def, ds] : funcs)
continue; for (Usr usr1 : def->vars) {
lsDocumentSymbol &ds = *def_ds.second; auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Var});
switch (sym.kind) { if (it != sym2ds.end() && it->second)
case SymbolKind::Func: { ds->children.push_back(std::move(it->second));
auto &def = *static_cast<const QueryFunc::Def *>(def_ds.first);
for (Usr usr1 : def.vars) {
auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Var});
if (it != sym2ds.end() && it->second.second)
ds.children.push_back(std::move(it->second.second));
}
break;
} }
case SymbolKind::Type: { for (auto &[def, ds] : types) {
auto &def = *static_cast<const QueryType::Def *>(def_ds.first); for (Usr usr1 : def->funcs) {
for (Usr usr1 : def.funcs) { auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Func});
auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Func}); if (it != sym2ds.end() && it->second)
if (it != sym2ds.end() && it->second.second) ds->children.push_back(std::move(it->second));
ds.children.push_back(std::move(it->second.second));
}
for (Usr usr1 : def.types) {
auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Type});
if (it != sym2ds.end() && it->second.second)
ds.children.push_back(std::move(it->second.second));
}
for (auto [usr1, _] : def.vars) {
auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Var});
if (it != sym2ds.end() && it->second.second)
ds.children.push_back(std::move(it->second.second));
}
break;
} }
default: for (Usr usr1 : def->types) {
break; auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Type});
if (it != sym2ds.end() && it->second)
ds->children.push_back(std::move(it->second));
}
for (auto [usr1, _] : def->vars) {
auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Var});
if (it != sym2ds.end() && it->second)
ds->children.push_back(std::move(it->second));
} }
} }
Out_HierarchicalDocumentSymbol out; Out_HierarchicalDocumentSymbol out;
out.id = request->id; out.id = request->id;
for (auto &[sym, def_ds] : sym2ds) for (auto &[_, ds] : sym2ds)
if (def_ds.second) if (ds)
out.result.push_back(std::move(def_ds.second)); out.result.push_back(std::move(ds));
pipeline::WriteStdout(kMethodType, out); pipeline::WriteStdout(kMethodType, out);
} else { } else {
Out_TextDocumentDocumentSymbol out; Out_TextDocumentDocumentSymbol out;
@ -195,9 +209,9 @@ struct Handler_TextDocumentDocumentSymbol
if (std::optional<lsSymbolInformation> info = if (std::optional<lsSymbolInformation> info =
GetSymbolInfo(db, sym, false)) { GetSymbolInfo(db, sym, false)) {
if ((sym.kind == SymbolKind::Type && if ((sym.kind == SymbolKind::Type &&
IgnoreType(db->GetType(sym).AnyDef())) || Ignore(db->GetType(sym).AnyDef())) ||
(sym.kind == SymbolKind::Var && (sym.kind == SymbolKind::Var &&
IgnoreVar(db->GetVar(sym).AnyDef()))) Ignore(db->GetVar(sym).AnyDef())))
continue; continue;
if (auto loc = GetLsLocation(db, working_files, sym, file_id)) { if (auto loc = GetLsLocation(db, working_files, sym, file_id)) {
info->location = *loc; info->location = *loc;