From 81c7ce71df127267f3c56571e7dd3dd3b8159f4e Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Sun, 28 May 2023 18:09:01 +0200 Subject: [PATCH] Improved skipping of empty packages in class diagrams --- .clang-uml | 2 + .../plantuml/class_diagram_generator.cc | 14 +++++-- src/class_diagram/model/diagram.h | 4 -- src/class_diagram/visitor/template_builder.cc | 4 +- src/common/clang_utils.cc | 15 +++++++ src/common/clang_utils.h | 3 ++ src/common/model/nested_trait.h | 28 ++++++++++++- .../visitor/translation_unit_visitor.cc | 34 +++++++++++++--- src/main.cc | 9 +++-- tests/t00036/.clang-uml | 5 ++- tests/t00036/t00036.cc | 10 +++++ tests/t00036/test_case.h | 3 ++ tests/t00065/module2/module2.h | 1 + tests/t00065/test_case.h | 40 +++++-------------- uml/diagram_element_hierarchy_diagram.yml | 2 - 15 files changed, 121 insertions(+), 53 deletions(-) diff --git a/.clang-uml b/.clang-uml index 964717a0..f1ef7d47 100644 --- a/.clang-uml +++ b/.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: diff --git a/src/class_diagram/generators/plantuml/class_diagram_generator.cc b/src/class_diagram/generators/plantuml/class_diagram_generator.cc index 34f5ff37..59f831b5 100644 --- a/src/class_diagram/generators/plantuml/class_diagram_generator.cc +++ b/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(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(*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(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(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(p.get()); cls) { diff --git a/src/class_diagram/model/diagram.h b/src/class_diagram/model/diagram.h index 47b42602..e8a999ac 100644 --- a/src/class_diagram/model/diagram.h +++ b/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; } diff --git a/src/class_diagram/visitor/template_builder.cc b/src/class_diagram/visitor/template_builder.cc index 46be21b3..13e57ee2 100644 --- a/src/class_diagram/visitor/template_builder.cc +++ b/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_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)); } } diff --git a/src/common/clang_utils.cc b/src/common/clang_utils.cc index 8e44133d..4e0e87f6 100644 --- a/src/common/clang_utils.cc +++ b/src/common/clang_utils.cc @@ -736,4 +736,19 @@ std::vector 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 diff --git a/src/common/clang_utils.h b/src/common/clang_utils.h index 1aa906a4..59bd1782 100644 --- a/src/common/clang_utils.h +++ b/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); diff --git a/src/common/model/nested_trait.h b/src/common/model/nested_trait.h index 6433b8d0..c4949e39 100644 --- a/src/common/model/nested_trait.h +++ b/src/common/model/nested_trait.h @@ -70,7 +70,9 @@ public: if (parent && dynamic_cast *>(&parent.value())) return dynamic_cast &>(parent.value()) .template add_element(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 bool all_of(F &&f) const + { + return std::all_of( + elements_.cbegin(), elements_.cend(), [f](const auto &e) { + const auto *package_ptr = + dynamic_cast *>(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 *>(e.get()); + return package_ptr != nullptr && package_ptr->is_empty(); + }); + } auto begin() { return elements_.begin(); } auto end() { return elements_.end(); } diff --git a/src/common/visitor/translation_unit_visitor.cc b/src/common/visitor/translation_unit_visitor.cc index d786db7e..82014933 100644 --- a/src/common/visitor/translation_unit_visitor.cc +++ b/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()); - element.set_file_relative(util::path_to_url( - std::filesystem::relative(element.file(), relative_to_path_) - .string())); - element.set_line(source_manager_.getSpellingLineNumber(location)); - element.set_location_id(location.getHashValue()); + file = source_manager_.getFilename(location).str(); + line = source_manager_.getSpellingLineNumber(location); + column = source_manager_.getSpellingColumnNumber(location); + + if (file.empty()) { + // 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 \ No newline at end of file diff --git a/src/main.cc b/src/main.cc index 5ffb49e8..7ddb7609 100644 --- a/src/main.cc +++ b/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 #include -#include -#include #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( diff --git a/tests/t00036/.clang-uml b/tests/t00036/.clang-uml index 7d6d5449..0c09a1c0 100644 --- a/tests/t00036/.clang-uml +++ b/tests/t00036/.clang-uml @@ -10,4 +10,7 @@ diagrams: - clanguml::t00036 include: namespaces: - - clanguml::t00036 \ No newline at end of file + - clanguml::t00036 + exclude: + subclasses: + - clanguml::t00036::ns2::ns22::D \ No newline at end of file diff --git a/tests/t00036/t00036.cc b/tests/t00036/t00036.cc index cb0ec2fa..7fa9131c 100644 --- a/tests/t00036/t00036.cc +++ b/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 diff --git a/tests/t00036/test_case.h b/tests/t00036/test_case.h index 4f04dda3..385ff0e4 100644 --- a/tests/t00036/test_case.h +++ b/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"), "+a_int")); diff --git a/tests/t00065/module2/module2.h b/tests/t00065/module2/module2.h index bca56d84..2aeb2b27 100644 --- a/tests/t00065/module2/module2.h +++ b/tests/t00065/module2/module2.h @@ -15,6 +15,7 @@ template struct C { template struct D { T t; + C c; }; } diff --git a/tests/t00065/test_case.h b/tests/t00065/test_case.h index a90d050e..d8004889 100644 --- a/tests/t00065/test_case.h +++ b/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, IsClassTemplate("A", "T,P,CMP,int N")); - - // Check concepts - // REQUIRE_THAT(puml, IsConcept(_A("AConcept"))); - // REQUIRE_THAT(puml, - // IsConceptRequirement( - // _A("AConcept"), "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("foo"))); - - // Check if all fields exist - // REQUIRE_THAT(puml, (IsField("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"), _A("F"))); + REQUIRE_THAT(puml, IsPackage("module1")); + REQUIRE_THAT(puml, IsPackage("module2")); + REQUIRE_THAT(puml, IsPackage("submodule1a")); + REQUIRE_THAT(puml, IsPackage("concepts")); save_puml( config.output_directory() + "/" + diagram->name + ".puml", puml); diff --git a/uml/diagram_element_hierarchy_diagram.yml b/uml/diagram_element_hierarchy_diagram.yml index ca6df6bc..60d1792c 100644 --- a/uml/diagram_element_hierarchy_diagram.yml +++ b/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