Fixed nested namespace handling

src/cx/util.cc

@@ -24,6 +24,8 @@
 #include <cppast/cpp_template.hpp>
 #include <spdlog/spdlog.h>
 
+#include <list>
+
 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<std::string> &current_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<std::string> 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<const cppast::cpp_class &>(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<std::string> &current_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<std::string> res;
+
+        for (const auto &n : clanguml::util::split(e.name(), "::"))
+            res.push_back(n);
+
+        std::list<std::string> 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<std::string> 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();
     }

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<std::string> &current_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<std::string> &current_ns, const cppast::cpp_entity &e);
 
 const cppast::cpp_type &unreferenced(const cppast::cpp_type &t);
 

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;

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<const cppast::cpp_class &>(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<const cppast::cpp_enum &>(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<const cppast::cpp_type_alias &>(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<const cppast::cpp_alias_template &>(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 &param_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<const cppast::cpp_template_instantiation_type &>(
+                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<const cppast::cpp_class_template &>(
+            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<const cppast::cpp_class_template &>(
-            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;
 }

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

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 {
+};
+}
+}
+}

tests/t00015/test_case.h

@@ -0,0 +1,50 @@
+/**
+ * tests/t00015/test_case.cc
+ *
+ * Copyright (c) 2021 Bartek Kryza <bkryza@gmail.com>
+ *
+ * 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);
+}

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

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