From ec71d4c6c038e19b83e225733d57f9281b3e5526 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 b5ae53d9..b70dbe43 100644 --- a/src/indexer.h +++ b/src/indexer.h @@ -199,7 +199,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 58e078a2..63fcbaef 100644 --- a/src/messages/textDocument_documentSymbol.cc +++ b/src/messages/textDocument_documentSymbol.cc @@ -64,11 +64,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(); } @@ -100,92 +105,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; @@ -195,9 +209,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;