From 35ca011f9b835a3a09d6a501ca696ff611dd5f1a Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Sun, 2 May 2021 20:24:30 +0200 Subject: [PATCH] Fixed nested namespace handling --- src/cx/util.cc | 63 ++++++++++----- src/cx/util.h | 6 +- src/puml/class_diagram_generator.h | 4 + src/uml/class_diagram_visitor.cc | 119 +++++++++++++++++++++-------- tests/t00015/.clanguml | 12 +++ tests/t00015/t00015.cc | 25 ++++++ tests/t00015/test_case.h | 50 ++++++++++++ tests/test_cases.cc | 1 + tests/test_cases.yaml | 3 + 9 files changed, 229 insertions(+), 54 deletions(-) create mode 100644 tests/t00015/.clanguml create mode 100644 tests/t00015/t00015.cc create mode 100644 tests/t00015/test_case.h diff --git a/src/cx/util.cc b/src/cx/util.cc index 3adbf2ab..941d4a0f 100644 --- a/src/cx/util.cc +++ b/src/cx/util.cc @@ -24,6 +24,8 @@ #include #include +#include + namespace clanguml { namespace cx { namespace util { @@ -35,7 +37,8 @@ std::string to_string(CXString &&cxs) return r; } -std::string full_name(const cppast::cpp_entity &e) +std::string full_name( + const std::vector ¤t_ns, const cppast::cpp_entity &e) { if (e.name().empty()) return ""; @@ -43,25 +46,16 @@ std::string full_name(const cppast::cpp_entity &e) // parameters don't have a full name return e.name(); - std::string scopes; + std::vector fn; - for (auto cur = e.parent(); cur; cur = cur.value().parent()) - // prepend each scope, if there is any - if (cur.value().kind() == cppast::cpp_entity_kind::namespace_t) - type_safe::with(cur.value().scope_name(), - [&](const cppast::cpp_scope_name &cur_scope) { - scopes = cur_scope.name() + "::" + scopes; - }); + for (const auto &ns : current_ns) { + if (!ns.empty()) + fn.push_back(ns); + } - if (e.kind() == cppast::cpp_entity_kind::class_t) { - auto &c = static_cast(e); - return scopes /*+ c.semantic_scope()*/ + c.name(); - } - else if (e.kind() == cppast::cpp_entity_kind::class_template_t) { - return scopes; - } - else - return scopes + e.name(); + fn.push_back(e.name()); + + return fmt::format("{}", fmt::join(fn, "::")); } std::string full_name(const cppast::cpp_type &t, @@ -87,7 +81,8 @@ std::string ns(const cppast::cpp_entity &e) auto it = e.parent(); while (it) { if (it.value().kind() == cppast::cpp_entity_kind::namespace_t) { - res.push_back(it.value().name()); + if (!it.value().name().empty()) + res.push_back(it.value().name()); } it = it.value().parent(); } @@ -162,14 +157,40 @@ std::string ns(const cppast::cpp_type &t, const cppast::cpp_entity_index &idx) } } -std::string fully_prefixed(const cppast::cpp_entity &e) +std::string fully_prefixed( + const std::vector ¤t_ns, const cppast::cpp_entity &e) { + if (e.name().find("::") != std::string::npos) { + // the name already contains namespace, but it could be not + // absolute, i.e. relative to some supernamespace of current + // namespace context + std::list res; + + for (const auto &n : clanguml::util::split(e.name(), "::")) + res.push_back(n); + + std::list prefix_ns; + for (const auto &n : current_ns) { + if (!n.empty() && n != res.front()) + prefix_ns.push_back(n); + else + break; + } + + prefix_ns.reverse(); + for (const auto &n : prefix_ns) + res.push_front(n); + + return fmt::format("{}", fmt::join(res, "::")); + } + std::vector res{e.name()}; auto it = e.parent(); while (it) { if (it.value().kind() == cppast::cpp_entity_kind::namespace_t) { - res.push_back(it.value().name()); + if (!it.value().name().empty()) + res.push_back(it.value().name()); } it = it.value().parent(); } diff --git a/src/cx/util.h b/src/cx/util.h index b0735481..a523f617 100644 --- a/src/cx/util.h +++ b/src/cx/util.h @@ -40,12 +40,14 @@ namespace util { */ std::string to_string(CXString &&cxs); -std::string full_name(const cppast::cpp_entity &e); +std::string full_name( + const std::vector ¤t_ns, const cppast::cpp_entity &e); std::string full_name(const cppast::cpp_type &t, const cppast::cpp_entity_index &idx, bool inside_class); -std::string fully_prefixed(const cppast::cpp_entity &e); +std::string fully_prefixed( + const std::vector ¤t_ns, const cppast::cpp_entity &e); const cppast::cpp_type &unreferenced(const cppast::cpp_type &t); diff --git a/src/puml/class_diagram_generator.h b/src/puml/class_diagram_generator.h index 69251c20..bd1d9441 100644 --- a/src/puml/class_diagram_generator.h +++ b/src/puml/class_diagram_generator.h @@ -216,6 +216,10 @@ public: r.destination.find("@") != std::string::npos) { destination = m_model.usr_to_name( m_config.using_namespace, r.destination); + + // If something went wrong and we have an empty destination + // generate the relationship but comment it out for + // debugging if (destination.empty()) { ostr << "' "; destination = r.destination; diff --git a/src/uml/class_diagram_visitor.cc b/src/uml/class_diagram_visitor.cc index 76002429..cbc44914 100644 --- a/src/uml/class_diagram_visitor.cc +++ b/src/uml/class_diagram_visitor.cc @@ -73,8 +73,22 @@ void tu_visitor::operator()(const cppast::cpp_entity &file) { cppast::visit(file, [&, this](const cppast::cpp_entity &e, cppast::visitor_info info) { - if (e.kind() == cppast::cpp_entity_kind::class_t) { - LOG_DBG("========== Visiting '{}' - {}", cx::util::full_name(e), + if (e.kind() == cppast::cpp_entity_kind::namespace_t) { + if (info.event == + cppast::visitor_info::container_entity_enter) { + LOG_DBG("========== Visiting '{}' - {}", e.name(), + cppast::to_string(e.kind())); + ctx.namespace_.push_back(e.name()); + } + else { + LOG_DBG("========== Leaving '{}' - {}", e.name(), + cppast::to_string(e.kind())); + ctx.namespace_.pop_back(); + } + } + else if (e.kind() == cppast::cpp_entity_kind::class_t) { + LOG_DBG("========== Visiting '{}' - {}", + cx::util::full_name(ctx.namespace_, e), cppast::to_string(e.kind())); auto &cls = static_cast(e); @@ -87,35 +101,40 @@ void tu_visitor::operator()(const cppast::cpp_entity &file) return; } } - if (ctx.config.should_include(cx::util::fully_prefixed(cls))) + if (ctx.config.should_include( + cx::util::fully_prefixed(ctx.namespace_, cls))) process_class_declaration(cls); } else if (e.kind() == cppast::cpp_entity_kind::enum_t) { - LOG_DBG("========== Visiting '{}' - {}", cx::util::full_name(e), + LOG_DBG("========== Visiting '{}' - {}", + cx::util::full_name(ctx.namespace_, e), cppast::to_string(e.kind())); auto &enm = static_cast(e); - if (ctx.config.should_include(cx::util::fully_prefixed(enm))) + if (ctx.config.should_include( + cx::util::fully_prefixed(ctx.namespace_, enm))) process_enum_declaration(enm); } else if (e.kind() == cppast::cpp_entity_kind::type_alias_t) { - LOG_DBG("========== Visiting '{}' - {}", cx::util::full_name(e), + LOG_DBG("========== Visiting '{}' - {}", + cx::util::full_name(ctx.namespace_, e), cppast::to_string(e.kind())); auto &ta = static_cast(e); type_alias t; - t.alias = cx::util::full_name(ta); + t.alias = cx::util::full_name(ctx.namespace_, ta); t.underlying_type = cx::util::full_name(ta.underlying_type(), ctx.entity_index, cx::util::is_inside_class(e)); - ctx.add_type_alias(cx::util::full_name(ta), + ctx.add_type_alias(cx::util::full_name(ctx.namespace_, ta), type_safe::ref(ta.underlying_type())); ctx.d.add_type_alias(std::move(t)); } else if (e.kind() == cppast::cpp_entity_kind::alias_template_t) { - LOG_DBG("========== Visiting '{}' - {}", cx::util::full_name(e), + LOG_DBG("========== Visiting '{}' - {}", + cx::util::full_name(ctx.namespace_, e), cppast::to_string(e.kind())); auto &at = static_cast(e); @@ -141,7 +160,7 @@ void tu_visitor::operator()(const cppast::cpp_entity &file) void tu_visitor::process_enum_declaration(const cppast::cpp_enum &enm) { enum_ e; - e.name = cx::util::full_name(enm); + e.name = cx::util::full_name(ctx.namespace_, enm); for (const auto &ev : enm) { if (ev.kind() == cppast::cpp_entity_kind::enum_value_t) { @@ -155,7 +174,8 @@ void tu_visitor::process_enum_declaration(const cppast::cpp_enum &enm) if (cur.value().kind() == cppast::cpp_entity_kind::class_t) { class_relationship containment; containment.type = relationship_t::kContainment; - containment.destination = cx::util::full_name(cur.value()); + containment.destination = + cx::util::full_name(ctx.namespace_, cur.value()); e.relationships.emplace_back(std::move(containment)); LOG_DBG("Added relationship {} +-- {}", e.name, @@ -171,7 +191,7 @@ void tu_visitor::process_class_declaration(const cppast::cpp_class &cls) { class_ c; c.is_struct = cls.class_kind() == cppast::cpp_class_kind::struct_t; - c.name = cx::util::full_name(cls); + c.name = cx::util::full_name(ctx.namespace_, cls); cppast::cpp_access_specifier_kind last_access_specifier = cppast::cpp_access_specifier_kind::cpp_private; @@ -241,7 +261,7 @@ void tu_visitor::process_class_declaration(const cppast::cpp_class &cls) // Process class bases for (auto &base : cls.bases()) { class_parent cp; - cp.name = clanguml::cx::util::fully_prefixed(base); + cp.name = clanguml::cx::util::fully_prefixed(ctx.namespace_, base); cp.is_virtual = base.is_virtual(); switch (base.access_specifier()) { @@ -299,7 +319,8 @@ void tu_visitor::process_class_declaration(const cppast::cpp_class &cls) if (cur.value().kind() == cppast::cpp_entity_kind::class_t) { class_relationship containment; containment.type = relationship_t::kContainment; - containment.destination = cx::util::full_name(cur.value()); + containment.destination = + cx::util::full_name(ctx.namespace_, cur.value()); c.add_relationship(std::move(containment)); LOG_DBG("Added relationship {} +-- {}", @@ -335,10 +356,10 @@ void tu_visitor::process_field_with_template_instantiation( .size()) { // Here we need the name of the primary template with full namespace // prefix to apply config inclusion filters - auto primary_template_name = - cx::util::full_name(template_instantiation_type.primary_template() - .get(ctx.entity_index)[0] - .get()); + auto primary_template_name = cx::util::full_name(ctx.namespace_, + template_instantiation_type.primary_template() + .get(ctx.entity_index)[0] + .get()); LOG_DBG("Maybe building instantiation for: {}{}", primary_template_name, cppast::to_string(tr)); @@ -564,7 +585,27 @@ void tu_visitor::process_function_parameter( { method_parameter mp; mp.name = param.name(); - mp.type = cppast::to_string(param.type()); + const auto ¶m_type = + cppast::remove_cv(cx::util::unreferenced(param.type())); + if (param_type.kind() == cppast::cpp_type_kind::template_instantiation_t) { + // Template instantiation parameters are not fully prefixed + // so we have to deduce the correct namespace prefix of the + // template which is being instantiated + mp.type = cppast::to_string(param.type()); + + auto &template_instantiation_type = + static_cast( + param_type); + auto &primary_template_entity = + template_instantiation_type.primary_template(); + + auto trawname = cppast::to_string(template_instantiation_type); + auto pte = cx::util::fully_prefixed(ctx.namespace_, + primary_template_entity.get(ctx.entity_index)[0].get()); + } + else { + mp.type = cppast::to_string(param.type()); + } auto dv = param.default_value(); if (dv) @@ -613,7 +654,7 @@ void tu_visitor::process_function_parameter( .size()) { // Here we need the name of the primary template with full // namespace prefix to apply config inclusion filters - auto primary_template_name = cx::util::full_name( + auto primary_template_name = cx::util::full_name(ctx.namespace_, template_instantiation_type.primary_template() .get(ctx.entity_index)[0] .get()); @@ -638,7 +679,7 @@ void tu_visitor::process_function_parameter( rr.destination = tinst.usr; rr.type = relationship_t::kDependency; rr.label = ""; - LOG_DBG("Adding field instantiation relationship {} {} {} : {}", + LOG_DBG("Adding field dependency relationship {} {} {} : {}", rr.destination, model::class_diagram::to_string(rr.type), c.usr, rr.label); c.add_relationship(std::move(rr)); @@ -731,7 +772,7 @@ void tu_visitor::process_friend(const cppast::cpp_friend &f, class_ &parent) cppast::is_templated(f.entity().value()), cppast::to_string(f.entity().value().kind())); - name = cx::util::full_name(f.entity().value()); + name = cx::util::full_name(ctx.namespace_, f.entity().value()); } if (!ctx.config.should_include(name)) @@ -849,24 +890,23 @@ void tu_visitor::find_relationships(const cppast::cpp_type &t_, } } -class_ tu_visitor::build_template_instantiation(/*const cppast::cpp_entity &e,*/ +class_ tu_visitor::build_template_instantiation( const cppast::cpp_template_instantiation_type &t) { - auto full_template_name = cx::util::full_name( - t.primary_template().get(ctx.entity_index)[0].get()); + const auto &primary_template_ref = + static_cast( + t.primary_template().get(ctx.entity_index)[0].get()) + .class_(); + + auto full_template_name = + cx::util::full_name(ctx.namespace_, primary_template_ref); LOG_DBG("Found template instantiation: {} ({}) ..|> {}, {}", cppast::to_string(t), cppast::to_string(t.canonical()), t.primary_template().name(), full_template_name); class_ tinst; - const auto &primary_template_ref = - static_cast( - t.primary_template().get(ctx.entity_index)[0].get()) - .class_(); - tinst.name = util::split( - cppast::to_string(t), "<")[0]; // primary_template_ref.name(); if (full_template_name.back() == ':') tinst.name = full_template_name + tinst.name; @@ -879,6 +919,19 @@ class_ tu_visitor::build_template_instantiation(/*const cppast::cpp_entity &e,*/ LOG_WARN( "No user data for base template {}", primary_template_ref.name()); + // Extract namespace from base template name + auto ns_toks = clanguml::util::split( + tinst.base_template_usr.substr(0, tinst.base_template_usr.find('<')), + "::"); + + std::string ns; + if (ns_toks.size() > 1) { + ns = fmt::format( + "{}::", fmt::join(ns_toks.begin(), ns_toks.end() - 1, "::")); + } + + tinst.name = ns + util::split(cppast::to_string(t), "<")[0]; + tinst.is_template_instantiation = true; for (const auto &targ : t.arguments().value()) { @@ -905,6 +958,10 @@ class_ tu_visitor::build_template_instantiation(/*const cppast::cpp_entity &e,*/ } tinst.usr = tinst.full_name(ctx.config.using_namespace); + if (tinst.usr.substr(0, tinst.usr.find('<')).find("::") == + std::string::npos) { + tinst.usr = ns + tinst.usr; + } return tinst; } diff --git a/tests/t00015/.clanguml b/tests/t00015/.clanguml new file mode 100644 index 00000000..38a6ceb5 --- /dev/null +++ b/tests/t00015/.clanguml @@ -0,0 +1,12 @@ +compilation_database_dir: .. +output_directory: puml +diagrams: + t00015_class: + type: class + glob: + - ../../tests/t00015/t00015.cc + using_namespace: + - clanguml::t00015 + include: + namespaces: + - clanguml::t00015 diff --git a/tests/t00015/t00015.cc b/tests/t00015/t00015.cc new file mode 100644 index 00000000..968c3a53 --- /dev/null +++ b/tests/t00015/t00015.cc @@ -0,0 +1,25 @@ +namespace clanguml { +namespace t00015 { + +namespace ns1::ns2 { +class A { +}; + +namespace { +class Anon final : public A { +}; +} +} + +namespace ns3 { + +namespace ns1::ns2 { +class Anon : public t00015::ns1::ns2::A { +}; +} + +class B : public ns1::ns2::Anon { +}; +} +} +} diff --git a/tests/t00015/test_case.h b/tests/t00015/test_case.h new file mode 100644 index 00000000..24e034d7 --- /dev/null +++ b/tests/t00015/test_case.h @@ -0,0 +1,50 @@ +/** + * tests/t00015/test_case.cc + * + * Copyright (c) 2021 Bartek Kryza + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +TEST_CASE("t00015", "[test-case][class]") +{ + auto [config, db] = load_config("t00015"); + + auto diagram = config.diagrams["t00015_class"]; + + REQUIRE(diagram->name == "t00015_class"); + + REQUIRE(diagram->include.namespaces.size() == 1); + REQUIRE_THAT(diagram->include.namespaces, + VectorContains(std::string{"clanguml::t00015"})); + + REQUIRE(diagram->exclude.namespaces.size() == 0); + + REQUIRE(diagram->should_include("clanguml::t00015::ns1::ns2::A")); + + auto model = generate_class_diagram(db, diagram); + + REQUIRE(model.name == "t00015_class"); + + auto puml = generate_class_puml(diagram, model); + AliasMatcher _A(puml); + + REQUIRE_THAT(puml, StartsWith("@startuml")); + REQUIRE_THAT(puml, EndsWith("@enduml\n")); + REQUIRE_THAT(puml, IsClass(_A("ns1::ns2::A"))); + REQUIRE_THAT(puml, IsClass(_A("ns1::ns2::Anon"))); + REQUIRE_THAT(puml, IsClass(_A("ns3::B"))); + + save_puml( + "./" + config.output_directory + "/" + diagram->name + ".puml", puml); +} diff --git a/tests/test_cases.cc b/tests/test_cases.cc index c39f2f71..865e72c3 100644 --- a/tests/test_cases.cc +++ b/tests/test_cases.cc @@ -118,6 +118,7 @@ using namespace clanguml::test::matchers; #include "t00012/test_case.h" #include "t00013/test_case.h" #include "t00014/test_case.h" +#include "t00015/test_case.h" // // Sequence diagram tests diff --git a/tests/test_cases.yaml b/tests/test_cases.yaml index 56893373..33e70e19 100644 --- a/tests/test_cases.yaml +++ b/tests/test_cases.yaml @@ -39,6 +39,9 @@ test_cases: - name: t00014 title: Alias template instantiation description: + - name: t00015 + title: Namespace fun + description: Sequence diagrams: - name: t20001 title: Basic sequence diagram