From 5255fd17856f02ce20354dbb2078e01eed3bdf9f Mon Sep 17 00:00:00 2001 From: Bartek Kryza Date: Sun, 18 Dec 2022 17:11:14 +0100 Subject: [PATCH] Fixed path filtering --- .clang-uml | 2 + src/common/model/diagram_filter.cc | 32 ++++++++------- src/config/config.cc | 8 ++++ .../plantuml/sequence_diagram_generator.cc | 39 +++++++++++++------ src/sequence_diagram/model/participant.cc | 18 +++++++++ src/sequence_diagram/model/participant.h | 2 +- .../visitor/translation_unit_visitor.cc | 9 +++-- tests/t20001/t20001.cc | 1 - tests/t20017/.clang-uml | 3 ++ tests/t40001/.clang-uml | 2 +- tests/t40002/.clang-uml | 8 ++-- tests/t40003/.clang-uml | 4 +- tests/test_config_data/filters.yml | 16 ++++---- tests/test_filters.cc | 22 +++++------ uml/main_sequence_diagram.yml | 30 ++++++++++++++ 15 files changed, 139 insertions(+), 57 deletions(-) create mode 100644 uml/main_sequence_diagram.yml diff --git a/.clang-uml b/.clang-uml index 6e6d2e75..f515256c 100644 --- a/.clang-uml +++ b/.clang-uml @@ -16,6 +16,8 @@ diagrams: include!: uml/class_model_class_diagram.yml sequence_model_class: include!: uml/sequence_model_class_diagram.yml + main_sequence: + include!: uml/main_sequence_diagram.yml sequence_diagram_visitor_sequence: include!: uml/sequence_diagram_visitor_sequence_diagram.yml class_diagram_generator_sequence: diff --git a/src/common/model/diagram_filter.cc b/src/common/model/diagram_filter.cc index 592dd173..8d515ac5 100644 --- a/src/common/model/diagram_filter.cc +++ b/src/common/model/diagram_filter.cc @@ -353,13 +353,25 @@ paths_filter::paths_filter(filter_t type, const std::filesystem::path &root, : filter_visitor{type} , root_{root} { - for (auto &&path : p) { + for (const auto &path : p) { std::filesystem::path absolute_path; - if (path.is_relative()) + if (path.string().empty() || path.string() == ".") + absolute_path = root; + else if (path.is_relative()) absolute_path = root / path; + else + absolute_path = path; - absolute_path = absolute_path.lexically_normal(); + try { + absolute_path = + std::filesystem::canonical(absolute_path.lexically_normal()); + } + catch (std::filesystem::filesystem_error &e) { + LOG_WARN("Cannot add non-existent path {} to paths filter", + path.string()); + continue; + } paths_.emplace_back(std::move(absolute_path)); } @@ -430,7 +442,7 @@ void diagram_filter::init_filters(const config::diagram &c) filter_t::kInclusive, c.include().access)); add_inclusive_filter(std::make_unique( - filter_t::kInclusive, c.base_directory(), c.include().paths)); + filter_t::kInclusive, c.relative_to(), c.include().paths)); // Include any of these matches even if one them does not match std::vector> element_filters; @@ -474,21 +486,11 @@ void diagram_filter::init_filters(const config::diagram &c) for (auto &&path : c.include().dependants) { std::filesystem::path dep_path{path}; - if (dep_path.is_relative()) { - dep_path = c.base_directory() / path; - dep_path = relative(dep_path, c.relative_to()); - } - dependants.emplace_back(dep_path.lexically_normal().string()); } for (auto &&path : c.include().dependencies) { std::filesystem::path dep_path{path}; - if (dep_path.is_relative()) { - dep_path = c.base_directory() / path; - dep_path = relative(dep_path, c.relative_to()); - } - dependencies.emplace_back(dep_path.lexically_normal().string()); } @@ -518,7 +520,7 @@ void diagram_filter::init_filters(const config::diagram &c) filter_t::kExclusive, c.exclude().namespaces)); add_exclusive_filter(std::make_unique( - filter_t::kExclusive, c.base_directory(), c.exclude().paths)); + filter_t::kExclusive, c.relative_to(), c.exclude().paths)); add_exclusive_filter(std::make_unique( filter_t::kExclusive, c.exclude().elements)); diff --git a/src/config/config.cc b/src/config/config.cc index 7891b2f5..b6b37b6a 100644 --- a/src/config/config.cc +++ b/src/config/config.cc @@ -595,6 +595,11 @@ template <> struct convert { get_option(node, rhs.combine_free_functions_into_file_participants); get_option(node, rhs.relative_to); get_option(node, rhs.participants_order); + get_option(node, rhs.generate_method_arguments); + + // Ensure relative_to has a value + if (!rhs.relative_to.has_value) + rhs.relative_to.set(std::filesystem::current_path()); rhs.initialize_type_aliases(); @@ -630,6 +635,9 @@ template <> struct convert { get_option(node, rhs.relative_to); get_option(node, rhs.generate_system_headers); + if (!rhs.relative_to) + rhs.relative_to.set(std::filesystem::current_path()); + // Convert the path in relative_to to an absolute path, with respect // to the directory where the `.clang-uml` configuration file is located if (rhs.relative_to) { diff --git a/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc b/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc index 8e285299..339c3390 100644 --- a/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc +++ b/src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc @@ -58,20 +58,28 @@ void generator::generate_call(const message &m, std::ostream &ostr) const std::string message; + model::function::message_render_mode render_mode = + model::function::message_render_mode::full; + + if (m_config.generate_method_arguments() == + config::method_arguments::abbreviated) + render_mode = model::function::message_render_mode::abbreviated; + else if (m_config.generate_method_arguments() == + config::method_arguments::none) + render_mode = model::function::message_render_mode::no_arguments; + if (to.value().type_name() == "method") { message = dynamic_cast(to.value()) - .message_name(model::function::message_render_mode::full); + .message_name(render_mode); } else if (m_config.combine_free_functions_into_file_participants()) { if (to.value().type_name() == "function") { - message = - dynamic_cast(to.value()) - .message_name(model::function::message_render_mode::full); + message = dynamic_cast(to.value()) + .message_name(render_mode); } else if (to.value().type_name() == "function_template") { - message = - dynamic_cast(to.value()) - .message_name(model::function::message_render_mode::full); + message = dynamic_cast(to.value()) + .message_name(render_mode); } } @@ -302,7 +310,7 @@ void generator::generate_participant( if (is_participant_generated(file_id)) return; - [[maybe_unused]] const auto &relative_to = + const auto &relative_to = std::filesystem::canonical(m_config.relative_to()); auto participant_name = std::filesystem::relative( @@ -388,13 +396,22 @@ void generator::generate(std::ostream &ostr) const std::string from_alias = generate_alias(from.value()); + model::function::message_render_mode render_mode = + model::function::message_render_mode::full; + + if (m_config.generate_method_arguments() == + config::method_arguments::abbreviated) + render_mode = model::function::message_render_mode::abbreviated; + else if (m_config.generate_method_arguments() == + config::method_arguments::none) + render_mode = + model::function::message_render_mode::no_arguments; + if (from.value().type_name() == "method" || m_config.combine_free_functions_into_file_participants()) { ostr << "[->" << " " << from_alias << " : " - << from.value().message_name( - model::function::message_render_mode::full) - << std::endl; + << from.value().message_name(render_mode) << std::endl; } ostr << "activate " << from_alias << std::endl; diff --git a/src/sequence_diagram/model/participant.cc b/src/sequence_diagram/model/participant.cc index fdf61a28..6c7f1d92 100644 --- a/src/sequence_diagram/model/participant.cc +++ b/src/sequence_diagram/model/participant.cc @@ -196,6 +196,12 @@ std::string function::message_name(message_render_mode mode) const if (mode == message_render_mode::no_arguments) { return fmt::format("{}(){}", name(), is_const() ? " const" : ""); } + else if (mode == message_render_mode::abbreviated) { + return fmt::format("{}({}){}", name(), + clanguml::util::abbreviate( + fmt::format("{}", fmt::join(parameters_, ",")), 15), + is_const() ? " const" : ""); + } return fmt::format("{}({}){}", name(), fmt::join(parameters_, ","), is_const() ? " const" : ""); @@ -259,6 +265,12 @@ std::string method::message_name(message_render_mode mode) const return fmt::format("{}{}(){}{}", style, method_name(), is_const() ? " const" : "", style); } + else if (mode == message_render_mode::abbreviated) { + return fmt::format("{}({}){}", name(), + clanguml::util::abbreviate( + fmt::format("{}", fmt::join(parameters(), ",")), 15), + is_const() ? " const" : ""); + } return fmt::format("{}{}({}){}{}", style, method_name(), fmt::join(parameters(), ","), is_const() ? " const" : "", style); @@ -330,6 +342,12 @@ std::string function_template::message_name(message_render_mode mode) const return fmt::format( "{}{}(){}", name(), template_params, is_const() ? " const" : ""); } + else if (mode == message_render_mode::abbreviated) { + return fmt::format("{}({}){}", name(), + clanguml::util::abbreviate( + fmt::format("{}", fmt::join(parameters(), ",")), 15), + is_const() ? " const" : ""); + } return fmt::format("{}{}({}){}", name(), template_params, fmt::join(parameters(), ","), is_const() ? " const" : ""); diff --git a/src/sequence_diagram/model/participant.h b/src/sequence_diagram/model/participant.h index dce84dec..7a46b740 100644 --- a/src/sequence_diagram/model/participant.h +++ b/src/sequence_diagram/model/participant.h @@ -135,7 +135,7 @@ struct lambda : public class_ { }; struct function : public participant { - enum class message_render_mode { full, no_arguments }; + enum class message_render_mode { full, abbreviated, no_arguments }; function(const common::model::namespace_ &using_namespace); diff --git a/src/sequence_diagram/visitor/translation_unit_visitor.cc b/src/sequence_diagram/visitor/translation_unit_visitor.cc index b0691110..13d3b9a8 100644 --- a/src/sequence_diagram/visitor/translation_unit_visitor.cc +++ b/src/sequence_diagram/visitor/translation_unit_visitor.cc @@ -274,7 +274,7 @@ bool translation_unit_visitor::VisitCXXMethodDecl(clang::CXXMethodDecl *m) LOG_DBG("Getting method's class with local id {}", parent_decl->getID()); if (!get_participant(parent_decl)) { - LOG_INFO("Cannot find parent class_ for method {} in class {}", + LOG_DBG("Cannot find parent class_ for method {} in class {}", m->getQualifiedNameAsString(), m->getParent()->getQualifiedNameAsString()); return true; @@ -490,10 +490,8 @@ bool translation_unit_visitor::VisitLambdaExpr(clang::LambdaExpr *expr) auto m_ptr = std::make_unique( config().using_namespace()); - common::model::namespace_ ns{c_ptr->get_namespace()}; auto method_name = "operator()"; m_ptr->set_method_name(method_name); - ns.pop_back(); m_ptr->set_class_id(cls_id); m_ptr->set_class_full_name(c_ptr->full_name(false)); @@ -1158,6 +1156,11 @@ bool translation_unit_visitor::process_function_call_expression( if (!diagram().should_include(callee_name)) return false; + // Skip free functions declared in files outside of included paths + if (config().combine_free_functions_into_file_participants() && + !diagram().should_include(common::model::source_file{m.file()})) + return false; + std::unique_ptr f_ptr; if (!get_unique_id(callee_function->getID()).has_value()) { diff --git a/tests/t20001/t20001.cc b/tests/t20001/t20001.cc index 488c19fe..2c7cdaca 100644 --- a/tests/t20001/t20001.cc +++ b/tests/t20001/t20001.cc @@ -1,5 +1,4 @@ #include -#include #include namespace clanguml { diff --git a/tests/t20017/.clang-uml b/tests/t20017/.clang-uml index 0aa577ba..a5a65e48 100644 --- a/tests/t20017/.clang-uml +++ b/tests/t20017/.clang-uml @@ -10,6 +10,9 @@ diagrams: include: namespaces: - clanguml::t20017 + # This only affects free function participants + paths: + - t20017.cc using_namespace: - clanguml::t20017 start_from: diff --git a/tests/t40001/.clang-uml b/tests/t40001/.clang-uml index 918e7f64..ef2a4a41 100644 --- a/tests/t40001/.clang-uml +++ b/tests/t40001/.clang-uml @@ -15,7 +15,7 @@ diagrams: include: # Include only headers belonging to these paths paths: - - ../../../tests/t40001 + - . plantuml: before: - "' t40001 test include diagram" diff --git a/tests/t40002/.clang-uml b/tests/t40002/.clang-uml index 8daf8264..7a96f63b 100644 --- a/tests/t40002/.clang-uml +++ b/tests/t40002/.clang-uml @@ -11,13 +11,13 @@ diagrams: # Render the paths relative to this directory relative_to: ../../../tests/t40002 include: - # Include only files belonging to these paths + # Include only files belonging to these paths relative to relative_to paths: - - ../../../tests/t40002 + - . exclude: paths: - # Exclude single header - - ../../../tests/t40002/include/lib2/lib2_detail.h + # Exclude single header relative to relative_to + - include/lib2/lib2_detail.h plantuml: before: - "' t40002 test include diagram" \ No newline at end of file diff --git a/tests/t40003/.clang-uml b/tests/t40003/.clang-uml index 64f03790..4e28cc22 100644 --- a/tests/t40003/.clang-uml +++ b/tests/t40003/.clang-uml @@ -13,10 +13,10 @@ diagrams: include: # Include only files which depend on t1.h dependants: - - ../../../tests/t40003/include/dependants/t1.h + - include/dependants/t1.h # and dependencies of t2.cc dependencies: - - ../../../tests/t40003/src/dependencies/t2.cc + - src/dependencies/t2.cc plantuml: before: - "' t40003 test include diagram" \ No newline at end of file diff --git a/tests/test_config_data/filters.yml b/tests/test_config_data/filters.yml index f6bca1e2..478892c3 100644 --- a/tests/test_config_data/filters.yml +++ b/tests/test_config_data/filters.yml @@ -3,18 +3,18 @@ output_directory: output diagrams: include_test: - type: class + type: include + relative_to: ../../../src glob: - src/**/*.cc - src/**/*.h include: paths: - - dir1 - - dir2/dir1 - - file1.h + - class_diagram/ + - common + - util + - main.cc exclude: paths: - - dir1/file9.h - - dir2/dir1/file9.h - - dir1/dir3 - - file9.h \ No newline at end of file + - sequence_diagram + - util/error.h \ No newline at end of file diff --git a/tests/test_filters.cc b/tests/test_filters.cc index 99942631..df1d4d30 100644 --- a/tests/test_filters.cc +++ b/tests/test_filters.cc @@ -32,11 +32,6 @@ TEST_CASE("Test diagram paths filter", "[unit-test]") using clanguml::common::model::diagram_filter; using clanguml::common::model::source_file; - auto make_path = [](std::string_view p) { - return source_file{ - std::filesystem::current_path() / "test_config_data" / p}; - }; - auto cfg = clanguml::config::load("./test_config_data/filters.yml"); CHECK(cfg.diagrams.size() == 1); @@ -45,10 +40,15 @@ TEST_CASE("Test diagram paths filter", "[unit-test]") diagram_filter filter(diagram, config); - CHECK(filter.should_include(make_path("dir1/file1.h"))); - CHECK(filter.should_include(make_path("dir1/dir2/file1.h"))); - CHECK(filter.should_include(make_path("dir1/dir2/dir3/dir4/file1.h"))); - CHECK_FALSE(filter.should_include(make_path("dir1/file9.h"))); - CHECK_FALSE(filter.should_include(make_path("dir1/dir3/file1.h"))); - CHECK_FALSE(filter.should_include(make_path("dir2/dir1/file9.h"))); + auto make_path = [&](std::string_view p) { + return source_file{config.relative_to() / p}; + }; + + CHECK(filter.should_include( + make_path("class_diagram/visitor/translation_unit_visitor.h"))); + CHECK(filter.should_include(make_path("main.cc"))); + CHECK(filter.should_include(make_path("util/util.cc"))); + CHECK_FALSE(filter.should_include(make_path("util/error.h"))); + CHECK_FALSE(filter.should_include( + make_path("sequence_diagram/visitor/translation_unit_visitor.h"))); } diff --git a/uml/main_sequence_diagram.yml b/uml/main_sequence_diagram.yml new file mode 100644 index 00000000..dd51132b --- /dev/null +++ b/uml/main_sequence_diagram.yml @@ -0,0 +1,30 @@ +type: sequence +combine_free_functions_into_file_participants: true +generate_method_arguments: none +glob: + - src/main.cc +include: + paths: + - src +exclude: + namespaces: + - std + - fmt + - spdlog + - clang + - llvm + - nlohmann + - CLI + - __gnu_cxx + - inja + elements: + - clanguml::config::option + paths: + - src/util/util.h +using_namespace: + - clanguml +plantuml: + before: + - 'title clang-uml main function sequence diagram' +start_from: + - function: main(int,const char **) \ No newline at end of file