Improved skipping of empty packages in class diagrams

.clang-uml

@@ -14,6 +14,8 @@ diagrams:
     include!: uml/common_model_class_diagram.yml
   class_model_class:
     include!: uml/class_model_class_diagram.yml
+  diagram_element_hierarchy_class:
+    include!: uml/diagram_element_hierarchy_diagram.yml
   sequence_model_class:
     include!: uml/sequence_model_class_diagram.yml
   main_sequence:

src/class_diagram/generators/plantuml/class_diagram_generator.cc

@@ -721,9 +721,14 @@ void generator::generate_relationships(
 {
     for (const auto &subpackage : p) {
         if (dynamic_cast<package *>(subpackage.get()) != nullptr) {
-            // TODO: add option - generate_empty_packages
+            // TODO: add option - generate_empty_packages, currently
+            //       packages which do not contain anything but other packages
+            //       are skipped
             const auto &sp = dynamic_cast<package &>(*subpackage);
-            if (!sp.is_empty())
+            if (!sp.is_empty() &&
+                !sp.all_of([this](const common::model::element &e) {
+                    return !m_model.should_include(e);
+                }))
                 generate_relationships(sp, ostr);
         }
         else if (dynamic_cast<class_ *>(subpackage.get()) != nullptr) {
@@ -774,7 +779,10 @@ void generator::generate_top_level_elements(std::ostream &ostr) const
 {
     for (const auto &p : m_model) {
         if (auto *pkg = dynamic_cast<package *>(p.get()); pkg) {
-            if (!pkg->is_empty())
+            if (!pkg->is_empty() &&
+                !pkg->all_of([this](const common::model::element &e) {
+                    return !m_model.should_include(e);
+                }))
                 generate(*pkg, ostr);
         }
         else if (auto *cls = dynamic_cast<class_ *>(p.get()); cls) {

src/class_diagram/model/diagram.h

@@ -187,7 +187,6 @@ bool diagram::add_with_filesystem_path(
 
     const auto base_name = e->name();
     const auto full_name = e->full_name(false);
-    const auto id = e->id();
     auto &e_ref = *e;
 
     if (add_element(parent_path, std::move(e))) {
@@ -195,9 +194,6 @@ bool diagram::add_with_filesystem_path(
         return true;
     }
 
-    LOG_WARN(
-        "Cannot add {} {} with id {} due to: {}", element_type, base_name, id);
-
     return false;
 }
 

src/class_diagram/visitor/template_builder.cc

@@ -1044,6 +1044,8 @@ template_builder::try_as_template_specialization_type(
     }
 
     if (diagram().should_include(nested_template_instantiation_full_name)) {
+        visitor_.set_source_location(
+            *template_decl, *nested_template_instantiation);
         visitor_.add_class(std::move(nested_template_instantiation));
     }
 
@@ -1155,7 +1157,7 @@ std::optional<template_parameter> template_builder::try_as_record_type(
                 if (parent.has_value())
                     parent.value()->add_relationship(
                         {relationship_t::kDependency, tag_argument->id()});
-
+                visitor_.set_source_location(*template_decl, *tag_argument);
                 visitor_.add_class(std::move(tag_argument));
             }
         }

src/common/clang_utils.cc

@@ -736,4 +736,19 @@ std::vector<std::string> tokenize_unexposed_template_parameter(
     return result;
 }
 
+bool parse_source_location(const std::string &location_str, std::string &file,
+    unsigned &line, unsigned &column)
+{
+    auto tokens = util::split(location_str, ":");
+
+    if (tokens.size() < 3)
+        return false;
+
+    file = tokens.at(0);
+    line = std::stoi(tokens.at(1));
+    column = std::stoi(tokens.at(2));
+
+    return true;
+}
+
 } // namespace clanguml::common

src/common/clang_utils.h

@@ -170,6 +170,9 @@ void if_dyn_cast(P pointer, F &&func)
     }
 }
 
+bool parse_source_location(const std::string &location_str, std::string &file,
+    unsigned &line, unsigned &column);
+
 bool is_type_parameter(const std::string &t);
 
 bool is_qualifier(const std::string &q);

src/common/model/nested_trait.h

@@ -70,7 +70,9 @@ public:
         if (parent && dynamic_cast<nested_trait<T, Path> *>(&parent.value()))
             return dynamic_cast<nested_trait<T, Path> &>(parent.value())
                 .template add_element<V>(std::move(p));
-        spdlog::info("No parent element found at: {}", path.to_string());
+
+        LOG_INFO("No parent element found at: {}", path.to_string());
+
         throw std::runtime_error(
             "No parent element found for " + path.to_string());
     }
@@ -135,7 +137,29 @@ public:
             elements_.end();
     }
 
-    bool is_empty() const { return elements_.empty(); }
+    template <typename F> bool all_of(F &&f) const
+    {
+        return std::all_of(
+            elements_.cbegin(), elements_.cend(), [f](const auto &e) {
+                const auto *package_ptr =
+                    dynamic_cast<nested_trait<T, Path> *>(e.get());
+
+                if (package_ptr != nullptr)
+                    return package_ptr->all_of(f);
+
+                return f(*e);
+            });
+    }
+
+    bool is_empty() const
+    {
+        return elements_.empty() ||
+            std::all_of(elements_.cbegin(), elements_.cend(), [](auto &e) {
+                const auto *package_ptr =
+                    dynamic_cast<nested_trait<T, Path> *>(e.get());
+                return package_ptr != nullptr && package_ptr->is_empty();
+            });
+    }
 
     auto begin() { return elements_.begin(); }
     auto end() { return elements_.end(); }

src/common/visitor/translation_unit_visitor.cc

@@ -85,14 +85,36 @@ void translation_unit_visitor::set_source_location(
     const clang::SourceLocation &location,
     clanguml::common::model::source_location &element)
 {
+    std::string file;
+    unsigned line{};
+    [[maybe_unused]] unsigned column{};
+
     if (location.isValid()) {
-        element.set_file(source_manager_.getFilename(location).str());
+        file = source_manager_.getFilename(location).str();
-        element.set_file_relative(util::path_to_url(
+        line = source_manager_.getSpellingLineNumber(location);
-            std::filesystem::relative(element.file(), relative_to_path_)
+        column = source_manager_.getSpellingColumnNumber(location);
-                .string()));
+
-        element.set_line(source_manager_.getSpellingLineNumber(location));
+        if (file.empty()) {
-        element.set_location_id(location.getHashValue());
+            // Why do I have to do this?
+            parse_source_location(
+                location.printToString(source_manager()), file, line, column);
+        }
+    }
+    else {
+        auto success = parse_source_location(
+            location.printToString(source_manager()), file, line, column);
+        if (!success) {
+            LOG_DBG("Failed to extract source location for element from {}",
+                location.printToString(source_manager_));
+            return;
         }
     }
 
+    element.set_file(file);
+    element.set_file_relative(util::path_to_url(
+        std::filesystem::relative(element.file(), relative_to_path_).string()));
+    element.set_line(line);
+    element.set_location_id(location.getHashValue());
+}
+
 } // namespace clanguml::common::visitor

src/main.cc

@@ -19,7 +19,6 @@
 #include "cli/cli_handler.h"
 #include "common/compilation_database.h"
 #include "common/generators/generators.h"
-#include "include_diagram/generators/plantuml/include_diagram_generator.h"
 #include "util/query_driver_output_extractor.h"
 #include "util/util.h"
 
@@ -33,8 +32,6 @@
 #include <spdlog/spdlog.h>
 
 #include <cstring>
-#include <iostream>
-#include <util/thread_pool_executor.h>
 
 #ifdef ENABLE_BACKWARD_CPP
 namespace backward {
@@ -55,6 +52,12 @@ int main(int argc, const char *argv[])
     if (res == cli::cli_flow_t::kError)
         return 1;
 
+#if !defined(NDEBUG)
+    // Catch invalid logger message formats, e.g. missing arguments
+    spdlog::set_error_handler(
+        [](const std::string & /*msg*/) { assert(0 == 1); });
+#endif
+
     try {
         const auto db =
             clanguml::common::compilation_database::auto_detect_from_directory(

tests/t00036/.clang-uml

@@ -11,3 +11,6 @@ diagrams:
     include:
       namespaces:
         - clanguml::t00036
+    exclude:
+      subclasses:
+        - clanguml::t00036::ns2::ns22::D

tests/t00036/t00036.cc

@@ -27,8 +27,18 @@ namespace ns22 {
 // TODO: Fix for incomplete struct C declaration "struct C;"
 struct C { };
 
+struct D { };
+
 }
 }
 
+namespace ns3 {
+namespace ns33 {
+namespace detail {
+struct DImpl : public ns2::ns22::D { };
+}
+} // namespace ns33
+} // namespace ns3
+
 } // namespace t00036
 } // namespace clanguml

tests/t00036/test_case.h

@@ -40,8 +40,11 @@ TEST_CASE("t00036", "[test-case][class]")
         REQUIRE_THAT(puml, IsEnum(_A("E")));
         REQUIRE_THAT(puml, IsClass(_A("B")));
         REQUIRE_THAT(puml, IsClass(_A("C")));
+        REQUIRE_THAT(puml, !IsClass(_A("DImpl")));
         REQUIRE_THAT(puml, IsPackage("ns111"));
         REQUIRE_THAT(puml, IsPackage("ns22"));
+        REQUIRE_THAT(puml, !IsPackage("ns3"));
+        REQUIRE_THAT(puml, !IsPackage("ns33"));
 
         REQUIRE_THAT(puml, IsAggregation(_A("B"), _A("A<int>"), "+a_int"));
 

tests/t00065/module2/module2.h

@@ -15,6 +15,7 @@ template <typename T> struct C {
 
 template <bconcept T> struct D {
     T t;
+    C<int> c;
 };
 
 }

tests/t00065/test_case.h

@@ -36,38 +36,16 @@ TEST_CASE("t00065", "[test-case][class]")
         REQUIRE_THAT(puml, EndsWith("@enduml\n"));
 
         // Check if all classes exist
-        //    REQUIRE_THAT(puml, IsClass(_A("AAA")));
+        REQUIRE_THAT(puml, IsClass(_A("R")));
+        REQUIRE_THAT(puml, IsClass(_A("A")));
+        REQUIRE_THAT(puml, IsClass(_A("AImpl")));
+        REQUIRE_THAT(puml, IsEnum(_A("XYZ")));
+        REQUIRE_THAT(puml, IsEnum(_A("ABC")));
 
-        // Check if class templates exist
+        REQUIRE_THAT(puml, IsPackage("module1"));
-        // REQUIRE_THAT(puml, IsClassTemplate("A", "T,P,CMP,int N"));
+        REQUIRE_THAT(puml, IsPackage("module2"));
-
+        REQUIRE_THAT(puml, IsPackage("submodule1a"));
-        // Check concepts
+        REQUIRE_THAT(puml, IsPackage("concepts"));
-        // REQUIRE_THAT(puml, IsConcept(_A("AConcept<T>")));
-        // REQUIRE_THAT(puml,
-        //    IsConceptRequirement(
-        //        _A("AConcept<T,P>"), "sizeof (T) > sizeof (P)"));
-
-        // Check if all enums exist
-        // REQUIRE_THAT(puml, IsEnum(_A("Lights")));
-
-        // Check if all inner classes exist
-        // REQUIRE_THAT(puml, IsInnerClass(_A("A"), _A("AA")));
-
-        // Check if all inheritance relationships exist
-        // REQUIRE_THAT(puml, IsBaseClass(_A("Base"), _A("Child")));
-
-        // Check if all methods exist
-        // REQUIRE_THAT(puml, (IsMethod<Public, Const>("foo")));
-
-        // Check if all fields exist
-        // REQUIRE_THAT(puml, (IsField<Private>("private_member", "int")));
-
-        // Check if all relationships exist
-        // REQUIRE_THAT(puml, IsAssociation(_A("D"), _A("A"), "-as"));
-        // REQUIRE_THAT(puml, IsDependency(_A("R"), _A("B")));
-        // REQUIRE_THAT(puml, IsAggregation(_A("R"), _A("D"), "-ag"));
-        // REQUIRE_THAT(puml, IsComposition(_A("R"), _A("D"), "-ac"));
-        // REQUIRE_THAT(puml, IsInstantiation(_A("ABCD::F<T>"), _A("F<int>")));
 
         save_puml(
             config.output_directory() + "/" + diagram->name + ".puml", puml);

uml/diagram_element_hierarchy_diagram.yml

@@ -1,6 +1,4 @@
 type: class
-#include_relations_also_as_members: false
-#generate_method_arguments: none
 generate_packages: true
 glob:
   - src/common/model/*.cc