From f5816e3be301041865ebfc5611758b514ba1795d Mon Sep 17 00:00:00 2001 From: Riatre Foo Date: Thu, 11 Oct 2018 04:32:59 +0800 Subject: [PATCH] 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. --- src/indexer.h | 3 +- src/messages/textDocument_documentSymbol.cc | 142 +++++++++++--------- 2 files changed, 80 insertions(+), 65 deletions(-) diff --git a/src/indexer.h b/src/indexer.h index 8f26822c..3783996f 100644 --- a/src/indexer.h +++ b/src/indexer.h @@ -211,7 +211,8 @@ struct VarDef : NameMixin { return spell && (parent_kind == lsSymbolKind::Function || parent_kind == lsSymbolKind::Method || - parent_kind == lsSymbolKind::StaticMethod) && + parent_kind == lsSymbolKind::StaticMethod || + parent_kind == lsSymbolKind::Constructor) && storage == clang::SC_None; } diff --git a/src/messages/textDocument_documentSymbol.cc b/src/messages/textDocument_documentSymbol.cc index ec9fed12..cc2535eb 100644 --- a/src/messages/textDocument_documentSymbol.cc +++ b/src/messages/textDocument_documentSymbol.cc @@ -76,11 +76,16 @@ struct Out_HierarchicalDocumentSymbol }; MAKE_REFLECT_STRUCT(Out_HierarchicalDocumentSymbol, jsonrpc, id, result); -bool IgnoreType(const QueryType::Def *def) { +template +bool Ignore(const Def *def) { + return false; +} +template <> +bool Ignore(const QueryType::Def *def) { return !def || def->kind == lsSymbolKind::TypeParameter; } - -bool IgnoreVar(const QueryVar::Def *def) { +template<> +bool Ignore(const QueryVar::Def *def) { return !def || def->is_local(); } @@ -112,92 +117,101 @@ struct Handler_TextDocumentDocumentSymbol std::sort(out.result.begin(), out.result.end()); pipeline::WriteStdout(kMethodType, out); } else if (g_config->client.hierarchicalDocumentSymbolSupport) { - std::unordered_map< - SymbolIdx, std::pair>> - sym2ds; + std::unordered_map> sym2ds; + std::vector> funcs; + std::vector> types; for (auto [sym, refcnt] : symbol2refcnt) { if (refcnt <= 0) continue; auto r = sym2ds.try_emplace(SymbolIdx{sym.usr, sym.kind}); if (!r.second) continue; - auto &kv = r.first->second; - kv.second = std::make_unique(); - lsDocumentSymbol &ds = *kv.second; + auto &ds = r.first->second; + ds = std::make_unique(); + const void *def_ptr = nullptr; WithEntity(db, sym, [&](const auto &entity) { auto *def = entity.AnyDef(); if (!def) return; - ds.name = def->Name(false); - ds.detail = def->Name(true); + ds->name = def->Name(false); + ds->detail = def->Name(true); + + // Try to find a definition with spell first. + const void *candidate_def_ptr = nullptr; 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) - break; - ds.kind = def.kind; + continue; if (auto ls_range = GetLsRange(wfile, def.spell->extent)) - ds.range = *ls_range; + ds->range = *ls_range; else - break; + continue; if (auto ls_range = GetLsRange(wfile, def.spell->range)) - ds.selectionRange = *ls_range; + ds->selectionRange = *ls_range; else - break; - kv.first = static_cast(&def); + continue; + 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 && - IgnoreType((const QueryType::Def *)kv.first)) || - (sym.kind == SymbolKind::Var && - IgnoreVar((const QueryVar::Def *)kv.first)))) - kv.first = nullptr; - if (!kv.first) { - kv.second.reset(); + if (!def_ptr) { + ds.reset(); 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) - continue; - lsDocumentSymbol &ds = *def_ds.second; - switch (sym.kind) { - case SymbolKind::Func: { - auto &def = *static_cast(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; + + for (auto &[def, ds] : funcs) + for (Usr 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)); } - case SymbolKind::Type: { - auto &def = *static_cast(def_ds.first); - for (Usr usr1 : def.funcs) { - auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Func}); - if (it != sym2ds.end() && it->second.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; + for (auto &[def, ds] : types) { + for (Usr usr1 : def->funcs) { + auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Func}); + if (it != sym2ds.end() && it->second) + ds->children.push_back(std::move(it->second)); } - default: - break; + for (Usr usr1 : def->types) { + 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.id = request->id; - for (auto &[sym, def_ds] : sym2ds) - if (def_ds.second) - out.result.push_back(std::move(def_ds.second)); + for (auto &[_, ds] : sym2ds) + if (ds) + out.result.push_back(std::move(ds)); pipeline::WriteStdout(kMethodType, out); } else { Out_TextDocumentDocumentSymbol out; @@ -207,9 +221,9 @@ struct Handler_TextDocumentDocumentSymbol if (std::optional info = GetSymbolInfo(db, sym, false)) { if ((sym.kind == SymbolKind::Type && - IgnoreType(db->GetType(sym).AnyDef())) || + Ignore(db->GetType(sym).AnyDef())) || (sym.kind == SymbolKind::Var && - IgnoreVar(db->GetVar(sym).AnyDef()))) + Ignore(db->GetVar(sym).AnyDef()))) continue; if (auto loc = GetLsLocation(db, working_files, sym, file_id)) { info->location = *loc;