diff --git a/src/common/visitor/translation_unit_visitor.h b/src/common/visitor/translation_unit_visitor.h index 6f271832..7f7d8242 100644 --- a/src/common/visitor/translation_unit_visitor.h +++ b/src/common/visitor/translation_unit_visitor.h @@ -265,23 +265,29 @@ public: * @param comment clang::RawComment pointer * @param de Reference to clang::DiagnosticsEngine * @param element Reference to element to be updated + * @return Comment with uml directives stripped from it */ - void process_comment(const clang::RawComment *comment, - clang::DiagnosticsEngine &de, + [[maybe_unused]] std::string process_comment( + const clang::RawComment *comment, clang::DiagnosticsEngine &de, clanguml::common::model::decorated_element &e) { - if (comment != nullptr) { - auto [it, inserted] = processed_comments_.emplace(comment); + if (comment == nullptr) + return {}; - if (!inserted) - return; + auto [it, inserted] = processed_comments_.emplace(comment); - // Process clang-uml decorators in the comments - // TODO: Refactor to use standard block comments processable by - // clang comments - e.add_decorators(decorators::parse( - comment->getFormattedText(source_manager_, de))); - } + if (!inserted) + return {}; + + // Process clang-uml decorators in the comments + // TODO: Refactor to use standard block comments processable by + // clang comments + const auto &[decorators, stripped_comment] = + decorators::parse(comment->getFormattedText(source_manager_, de)); + + e.add_decorators(decorators); + + return stripped_comment; } /** @@ -332,6 +338,12 @@ public: */ const ConfigT &config() const { return config_; } +protected: + std::set &processed_comments() + { + return processed_comments_; + } + private: // Reference to the output diagram model DiagramT &diagram_; diff --git a/src/decorators/decorators.cc b/src/decorators/decorators.cc index 0c39d214..090bfeb9 100644 --- a/src/decorators/decorators.cc +++ b/src/decorators/decorators.cc @@ -187,10 +187,12 @@ std::shared_ptr call::from_string(std::string_view c) return res; } -std::vector> parse( +std::pair>, std::string> parse( std::string documentation_block, const std::string &clanguml_tag) { std::vector> res; + std::string stripped_comment; + const std::string begin_tag{"@" + clanguml_tag}; const auto begin_tag_size = begin_tag.size(); @@ -202,12 +204,20 @@ std::vector> parse( const std::string_view block_view{documentation_block}; auto pos = block_view.find("@" + clanguml_tag + "{"); + + if (pos == std::string::npos) { + // This comment had no uml directives + return {{}, util::trim(documentation_block)}; + } + + size_t last_end_pos{0}; while (pos < documentation_block.size()) { auto c_begin = pos + begin_tag_size; - auto c_end = documentation_block.find('}', c_begin); + auto c_end = block_view.find('}', c_begin); - if (c_end == std::string::npos) - return res; + if (c_end == std::string::npos) { + return {res, util::trim(stripped_comment)}; + } auto com = decorator::from_string(block_view.substr(c_begin + 1, c_end - 2)); @@ -215,10 +225,15 @@ std::vector> parse( if (com) res.emplace_back(std::move(com)); + const auto in_between_length = pos - last_end_pos; + stripped_comment += block_view.substr(last_end_pos, in_between_length); + + last_end_pos = pos + (c_end - c_begin + begin_tag_size + 1); + pos = block_view.find("@" + clanguml_tag + "{", c_end); } - return res; + return {res, util::trim(stripped_comment)}; }; } // namespace clanguml::decorators diff --git a/src/decorators/decorators.h b/src/decorators/decorators.h index b745f87a..4afbffac 100644 --- a/src/decorators/decorators.h +++ b/src/decorators/decorators.h @@ -156,9 +156,10 @@ struct call : public decorator { * * @param documentation_block Documentation block extracted from source code * @param clanguml_tag Name of the clanguml tag (default `uml`) - * @return List of clang-uml decorators extracted from comment + * @return Pair of: a list of clang-uml decorators extracted from comment and + * a comment stripped of any uml directives */ -std::vector> parse( +std::pair>, std::string> parse( std::string documentation_block, const std::string &clanguml_tag = "uml"); } // namespace decorators diff --git a/src/sequence_diagram/model/message.cc b/src/sequence_diagram/model/message.cc index fad5ca19..012aecc0 100644 --- a/src/sequence_diagram/model/message.cc +++ b/src/sequence_diagram/model/message.cc @@ -59,7 +59,11 @@ const std::string &message::return_type() const { return return_type_; } const std::optional &message::comment() const { return comment_; } -void message::set_comment(std::string c) { comment_ = std::move(c); } +void message::set_comment(std::string c) +{ + if (!c.empty()) + comment_ = std::move(c); +} void message::set_comment(const std::optional &c) { diff --git a/src/sequence_diagram/visitor/translation_unit_visitor.cc b/src/sequence_diagram/visitor/translation_unit_visitor.cc index ce866539..63c0750d 100644 --- a/src/sequence_diagram/visitor/translation_unit_visitor.cc +++ b/src/sequence_diagram/visitor/translation_unit_visitor.cc @@ -1047,24 +1047,28 @@ bool translation_unit_visitor::VisitCallExpr(clang::CallExpr *expr) set_source_location(*expr, m); - process_comment(clanguml::common::get_expression_raw_comment( - source_manager(), *context().get_ast_context(), expr), - context().get_ast_context()->getDiagnostics(), m); + const auto *raw_expr_comment = clanguml::common::get_expression_raw_comment( + source_manager(), *context().get_ast_context(), expr); + const auto stripped_comment = process_comment( + raw_expr_comment, context().get_ast_context()->getDiagnostics(), m); if (m.skip()) return true; auto generated_message_from_comment = generate_message_from_comment(m); - if (!generated_message_from_comment && !should_include(expr)) + if (!generated_message_from_comment && !should_include(expr)) { + processed_comments().erase(raw_expr_comment); return true; + } if (context().is_expr_in_current_control_statement_condition(expr)) { m.set_message_scope(common::model::message_scope_t::kCondition); } if (generated_message_from_comment) { - // Do nothing + LOG_DBG( + "Message for this call expression is taken from comment directive"); } // // Call to an overloaded operator @@ -1148,11 +1152,7 @@ bool translation_unit_visitor::VisitCallExpr(clang::CallExpr *expr) // Add message to diagram if (m.from() > 0 && m.to() > 0) { - if (!generated_message_from_comment) { - auto expr_comment = get_expression_comment(source_manager(), - *context().get_ast_context(), context().caller_id(), expr); - m.set_comment(expr_comment); - } + m.set_comment(stripped_comment); if (diagram().sequences().find(m.from()) == diagram().sequences().end()) { @@ -2167,10 +2167,17 @@ std::optional translation_unit_visitor::get_expression_comment( if (raw_comment == nullptr) return {}; - if (!processed_comments_.emplace(caller_id, raw_comment).second) { + if (!processed_comments_by_caller_id_.emplace(caller_id, raw_comment) + .second) { return {}; } - return raw_comment->getFormattedText(sm, sm.getDiagnostics()); + const auto &[decorators, stripped_comment] = decorators::parse( + raw_comment->getFormattedText(sm, sm.getDiagnostics())); + + if (stripped_comment.empty()) + return {}; + + return stripped_comment; } } // namespace clanguml::sequence_diagram::visitor diff --git a/src/sequence_diagram/visitor/translation_unit_visitor.h b/src/sequence_diagram/visitor/translation_unit_visitor.h index e76f90d7..10c779df 100644 --- a/src/sequence_diagram/visitor/translation_unit_visitor.h +++ b/src/sequence_diagram/visitor/translation_unit_visitor.h @@ -511,7 +511,7 @@ private: already_visited_in_static_declaration_{}; mutable std::set> - processed_comments_; + processed_comments_by_caller_id_; template_builder_t template_builder_; }; diff --git a/tests/t20048/.clang-uml b/tests/t20048/.clang-uml new file mode 100644 index 00000000..0b3d7e10 --- /dev/null +++ b/tests/t20048/.clang-uml @@ -0,0 +1,14 @@ +add_compile_flags: + - -fparse-all-comments +diagrams: + t20048_sequence: + type: sequence + glob: + - t20048.cc + generate_message_comments: true + include: + namespaces: + - clanguml::t20048 + using_namespace: clanguml::t20048 + from: + - function: "clanguml::t20048::tmain()" \ No newline at end of file diff --git a/tests/t20048/t20048.cc b/tests/t20048/t20048.cc new file mode 100644 index 00000000..cd04fa1c --- /dev/null +++ b/tests/t20048/t20048.cc @@ -0,0 +1,40 @@ +#include + +namespace clanguml { +namespace t20048 { + +int a1(int x) { return x + 1; } + +int a2(int x) { return x + 2; } + +int a3(int x) { return x + 3; } + +int a4(int x) { return x + 4; } + +int a5(int x) { return x + 5; } + +int a6(int x) { return x + 6; } + +int a7(int x) { return x + 7; } + +int tmain() +{ + // a1() adds `1` to the result of a2() + auto res = a1(a2(a3(0))); + + // This lambda calls a4() which adds `4` to it's argument + res = [](auto &&x) { return a4(x); }(0); + + // a5() adds `1` to the result of a6() + res = a5( + // a6() adds `1` to its argument + a6(0)); + + // a7() is called via add std::async + // \uml{call clanguml::t20048::a7(int)} + res = std::async(a7, 10).get(); + + return 0; +} +} +} \ No newline at end of file diff --git a/tests/t20048/test_case.h b/tests/t20048/test_case.h new file mode 100644 index 00000000..b9a45e86 --- /dev/null +++ b/tests/t20048/test_case.h @@ -0,0 +1,91 @@ +/** + * tests/t20048/test_case.h + * + * Copyright (c) 2021-2024 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("t20048", "[test-case][sequence]") +{ + auto [config, db] = load_config("t20048"); + + auto diagram = config.diagrams["t20048_sequence"]; + + REQUIRE(diagram->name == "t20048_sequence"); + + auto model = generate_sequence_diagram(*db, diagram); + + REQUIRE(model->name() == "t20048_sequence"); + + { + auto src = generate_sequence_puml(diagram, *model); + AliasMatcher _A(src); + + REQUIRE_THAT(src, StartsWith("@startuml")); + REQUIRE_THAT(src, EndsWith("@enduml\n")); + + // Check if all calls exist + REQUIRE_THAT(src, HasCall(_A("tmain()"), _A("a3(int)"), "")); + REQUIRE_THAT(src, HasCall(_A("tmain()"), _A("a2(int)"), "")); + REQUIRE_THAT(src, HasCall(_A("tmain()"), _A("a1(int)"), "")); + REQUIRE_THAT(src, HasCall(_A("tmain()"), _A("a5(int)"), "")); + REQUIRE_THAT(src, HasCall(_A("tmain()"), _A("a6(int)"), "")); + REQUIRE_THAT(src, HasCall(_A("tmain()"), _A("a7(int)"), "")); + + REQUIRE_THAT(src, + HasCall(_A("tmain()"), _A("tmain()::(lambda t20048.cc:26:11)"), + "operator()()")); + REQUIRE_THAT(src, + HasCall( + _A("tmain()::(lambda t20048.cc:26:11)"), _A("a4(int)"), "")); + + REQUIRE_THAT(src, + HasMessageComment(_A("tmain()"), + "a1\\(\\) adds `1` to the result\\n" + "of a2\\(\\)")); + REQUIRE_THAT(src, + HasMessageComment(_A("tmain()"), + "This lambda calls a4\\(\\) which\\n" + "adds `4` to it's argument")); + REQUIRE_THAT(src, + HasMessageComment( + _A("tmain()"), "a6\\(\\) adds `1` to its argument")); + REQUIRE_THAT(src, + HasMessageComment(_A("tmain()"), + "a5\\(\\) adds `1` to the result\\n" + "of a6\\(\\)")); + REQUIRE_THAT(src, + HasMessageComment( + _A("tmain()"), "a7\\(\\) is called via add std::async")); + + save_puml(config.output_directory(), diagram->name + ".puml", src); + } + + { + auto j = generate_sequence_json(diagram, *model); + + using namespace json; + + save_json(config.output_directory(), diagram->name + ".json", j); + } + + { + auto src = generate_sequence_mermaid(diagram, *model); + + mermaid::AliasMatcher _A(src); + using mermaid::IsClass; + + save_mermaid(config.output_directory(), diagram->name + ".mmd", src); + } +} \ No newline at end of file diff --git a/tests/test_cases.cc b/tests/test_cases.cc index cb715570..1d24eec7 100644 --- a/tests/test_cases.cc +++ b/tests/test_cases.cc @@ -474,6 +474,7 @@ using namespace clanguml::test::matchers; #include "t20045/test_case.h" #include "t20046/test_case.h" #include "t20047/test_case.h" +#include "t20048/test_case.h" /// /// Package diagram tests diff --git a/tests/test_decorator_parser.cc b/tests/test_decorator_parser.cc index 5ef6b19a..2334cf4f 100644 --- a/tests/test_decorator_parser.cc +++ b/tests/test_decorator_parser.cc @@ -20,6 +20,7 @@ #include "decorators/decorators.h" #include "catch.h" +#include "util/util.h" TEST_CASE("Test decorator parser on regular comment", "[unit-test]") { @@ -35,9 +36,10 @@ TEST_CASE("Test decorator parser on regular comment", "[unit-test]") using namespace clanguml::decorators; - auto decorators = parse(comment); + auto [decorators, stripped] = parse(comment); CHECK(decorators.empty()); + CHECK(clanguml::util::trim(comment) == stripped); } TEST_CASE("Test decorator parser on note", "[unit-test]") @@ -62,7 +64,7 @@ TEST_CASE("Test decorator parser on note", "[unit-test]") using namespace clanguml::decorators; - auto decorators = parse(comment); + auto [decorators, stripped] = parse(comment); CHECK(decorators.size() == 4); @@ -86,6 +88,15 @@ TEST_CASE("Test decorator parser on note", "[unit-test]") CHECK(n4); CHECK(n4->position == "left"); CHECK(n4->text == "This is a comment"); + + CHECK(stripped == R"(\brief This is a comment. + + This is a longer comment. + + \ + + \param a int an int + \param b float a float)"); } TEST_CASE("Test decorator parser on note with custom tag", "[unit-test]") @@ -110,7 +121,7 @@ TEST_CASE("Test decorator parser on note with custom tag", "[unit-test]") using namespace clanguml::decorators; - auto decorators = parse(comment, "clanguml"); + auto [decorators, stripped] = parse(comment, "clanguml"); CHECK(decorators.size() == 4); @@ -134,6 +145,15 @@ TEST_CASE("Test decorator parser on note with custom tag", "[unit-test]") CHECK(n4); CHECK(n4->position == "left"); CHECK(n4->text == "This is a comment"); + + CHECK(stripped == R"(\brief This is a comment. + + This is a longer comment. + + \ + + \param a int an int + \param b float a float)"); } TEST_CASE("Test decorator parser on style", "[unit-test]") @@ -144,7 +164,7 @@ TEST_CASE("Test decorator parser on style", "[unit-test]") using namespace clanguml::decorators; - auto decorators = parse(comment); + auto [decorators, stripped] = parse(comment); CHECK(decorators.size() == 1); @@ -152,6 +172,7 @@ TEST_CASE("Test decorator parser on style", "[unit-test]") CHECK(n1); CHECK(n1->spec == "#green,dashed,thickness=4"); + CHECK(stripped.empty()); } TEST_CASE("Test decorator parser on aggregation", "[unit-test]") @@ -162,7 +183,7 @@ TEST_CASE("Test decorator parser on aggregation", "[unit-test]") using namespace clanguml::decorators; - auto decorators = parse(comment); + auto [decorators, stripped] = parse(comment); CHECK(decorators.size() == 1); @@ -170,6 +191,7 @@ TEST_CASE("Test decorator parser on aggregation", "[unit-test]") CHECK(n1); CHECK(n1->multiplicity == "0..1:0..*"); + CHECK(stripped.empty()); } TEST_CASE("Test decorator parser on skip", "[unit-test]") @@ -180,13 +202,14 @@ TEST_CASE("Test decorator parser on skip", "[unit-test]") using namespace clanguml::decorators; - auto decorators = parse(comment); + auto [decorators, stripped] = parse(comment); CHECK(decorators.size() == 1); auto n1 = std::dynamic_pointer_cast(decorators.at(0)); CHECK(n1); + CHECK(stripped.empty()); } TEST_CASE("Test decorator parser on skiprelationship", "[unit-test]") @@ -197,13 +220,14 @@ TEST_CASE("Test decorator parser on skiprelationship", "[unit-test]") using namespace clanguml::decorators; - auto decorators = parse(comment); + auto [decorators, stripped] = parse(comment); CHECK(decorators.size() == 1); auto n1 = std::dynamic_pointer_cast(decorators.at(0)); CHECK(n1); + CHECK(stripped.empty()); } TEST_CASE("Test decorator parser on diagram scope", "[unit-test]") @@ -215,7 +239,7 @@ TEST_CASE("Test decorator parser on diagram scope", "[unit-test]") using namespace clanguml::decorators; - auto decorators = parse(comment); + auto [decorators, stripped] = parse(comment); CHECK(decorators.size() == 1); @@ -232,4 +256,21 @@ TEST_CASE("Test decorator parser on diagram scope", "[unit-test]") CHECK(n1->applies_to_diagram("diagram2")); CHECK(!n1->applies_to_diagram("diagram4")); + CHECK(stripped.empty()); +} + +TEST_CASE("Test invalid comment - unterminated curly brace", "[unit-test]") +{ + std::string comment = R"( + Test test test + \uml{call clanguml::test:aa() + )"; + + using namespace clanguml::decorators; + + auto [decorators, stripped] = parse(comment); + + CHECK(decorators.size() == 0); + + CHECK(stripped.empty()); }