Merge pull request #3 from bkryza/handle-unnamed-enums

Handle unnamed enums

.github/workflows/build.yml

@@ -10,8 +10,12 @@ jobs:
       uses: actions/checkout@v2
       with:
         submodules: recursive
+    - name: Install add-apt-repository
+      run: sudo apt-get install software-properties-common
+    - name: Add llvm repository
+      run: sudo add-apt-repository 'deb http://apt.llvm.org/focal/ llvm-toolchain-focal-12 main' && sudo apt update
     - name: Install deps
-      run: sudo apt-get install ccache cmake libyaml-cpp-dev libspdlog-dev clang-11 libclang-11-dev libclang-cpp11-dev
+      run: sudo apt-get install ccache cmake libyaml-cpp-dev libspdlog-dev clang-12 libclang-12-dev libclang-cpp12-dev
     - name: Build and unit test
       run: |
         make debug

CMakeLists.txt

@@ -15,7 +15,7 @@ set(CLANG_UML_INSTALL_BIN_DIR ${PROJECT_SOURCE_DIR}/bin)
 
 set(UML_HEADERS_DIR ${PROJECT_SOURCE_DIR}/src/uml)
 
-set(LLVM_PREFERRED_VERSION 11.0.0)
+set(LLVM_PREFERRED_VERSION 12.0.0)
 
 message(STATUS "Checking for spdlog...")
 find_package(spdlog REQUIRED)

README.md

@@ -13,6 +13,15 @@ TODO
 
 ## Usage
 
+### Generating compile commands database
+`clang-uml` requires an up-to-date
+[compile-commands.json](https://clang.llvm.org/docs/JSONCompilationDatabase.html)
+file, containing the list of commands used for compiling the source code.
+Nowadays, this file can be generated rather easily using multiple methods:
+  * For [CMake](https://cmake.org/) projects, simply invoke the `cmake` command
+    as `cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON ...`
+  * For Make projects checkout [compiledb](https://github.com/nickdiego/compiledb) or [Bear](https://github.com/rizsotto/Bear)
+
 ### Invocation
 
 ### Configuration file format and examples

docs/test_cases/t00002.md

@@ -52,6 +52,29 @@ public:
             a->foo_c();
     }
 
+private:
+    std::vector<A *> as;
+};
+
+//
+// NOTE: libclang fails on:
+//
+//   class D : public virtual B, public virtual C {
+//
+class E : virtual public B, virtual public C {
+public:
+    void foo_a() override
+    {
+        for (auto a : as)
+            a->foo_a();
+    }
+
+    void foo_c() override
+    {
+        for (auto a : as)
+            a->foo_c();
+    }
+
 private:
     std::vector<A *> as;
 };

src/main.cc

@@ -52,12 +52,15 @@ int main(int argc, const char *argv[])
 
     std::string config_path{".clanguml"};
     std::string compilation_database_dir{'.'};
+    std::vector<std::string> diagram_names{};
     bool verbose{false};
 
     app.add_option(
         "-c,--config", config_path, "Location of configuration file");
     app.add_option("-d,--compile-database", compilation_database_dir,
         "Location of configuration file");
+    app.add_option("-n,--diagram-name", diagram_names,
+        "List of diagram names to generate");
     app.add_flag("-v,--verbose", verbose, "Verbose logging");
 
     CLI11_PARSE(app, argc, argv);
@@ -80,6 +83,13 @@ int main(int argc, const char *argv[])
     cppast::libclang_compilation_database db2(config.compilation_database_dir);
 
     for (const auto &[name, diagram] : config.diagrams) {
+        // If there are any specific diagram names provided on the command line,
+        // and this diagram is not in that list - skip it
+        if (!diagram_names.empty() &&
+            std::find(diagram_names.begin(), diagram_names.end(), name) ==
+                diagram_names.end())
+            continue;
+
         using clanguml::config::class_diagram;
         using clanguml::config::sequence_diagram;
 

src/puml/class_diagram_generator.h

@@ -124,7 +124,7 @@ public:
         }
     }
 
-    void generate_aliases(const class_ &c, std::ostream &ostr) const
+    void generate_alias(const class_ &c, std::ostream &ostr) const
     {
         std::string class_type{"class"};
         if (c.is_abstract())
@@ -135,6 +135,14 @@ public:
         ostr << "\" as " << c.alias() << std::endl;
     }
 
+    void generate_alias(const enum_ &e, std::ostream &ostr) const
+    {
+        ostr << "enum"
+             << " \"" << e.full_name(m_config.using_namespace);
+
+        ostr << "\" as " << e.alias() << std::endl;
+    }
+
     void generate(const class_ &c, std::ostream &ostr) const
     {
         std::string class_type{"class"};
@@ -199,54 +207,74 @@ public:
 
         if (m_config.should_include_relationship("inheritance"))
             for (const auto &b : c.bases) {
-                ostr << m_model.to_alias(m_config.using_namespace,
-                            ns_relative(m_config.using_namespace, b.name))
-                     << " <|-- "
-                     << m_model.to_alias(m_config.using_namespace,
-                            ns_relative(m_config.using_namespace, c.name))
-                     << std::endl;
+                std::stringstream relstr;
+                try {
+                    relstr << m_model.to_alias(m_config.using_namespace,
+                                  ns_relative(m_config.using_namespace, b.name))
+                           << " <|-- "
+                           << m_model.to_alias(m_config.using_namespace,
+                                  ns_relative(m_config.using_namespace, c.name))
+                           << std::endl;
+                    ostr << relstr.str();
+                }
+                catch (error::uml_alias_missing &e) {
+                    LOG_ERROR("Skipping inheritance relation from {} to {} due "
+                              "to: {}",
+                        b.name, c.name, e.what());
+                }
             }
 
         for (const auto &r : c.relationships) {
             if (!m_config.should_include_relationship(name(r.type)))
                 continue;
 
-            std::string destination;
-            if (r.destination.find("#") != std::string::npos ||
-                r.destination.find("@") != std::string::npos) {
-                destination = m_model.usr_to_name(
-                    m_config.using_namespace, r.destination);
+            std::stringstream relstr;
 
-                // If something went wrong and we have an empty destination
-                // generate the relationship but comment it out for
-                // debugging
-                if (destination.empty()) {
-                    ostr << "' ";
+            std::string destination;
+            try {
+                if (r.destination.find("#") != std::string::npos ||
+                    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()) {
+                        relstr << "' ";
+                        destination = r.destination;
+                    }
+                }
+                else {
                     destination = r.destination;
                 }
+
+                relstr << m_model.to_alias(m_config.using_namespace,
+                              ns_relative(m_config.using_namespace,
+                                  c.full_name(m_config.using_namespace)))
+                       << " " << to_string(r.type) << " "
+                       << m_model.to_alias(m_config.using_namespace,
+                              ns_relative(
+                                  m_config.using_namespace, destination));
+
+                if (!r.label.empty())
+                    relstr << " : " << r.label;
+
+                relstr << std::endl;
+                ostr << relstr.str();
             }
-            else {
-                destination = r.destination;
+            catch (error::uml_alias_missing &e) {
+                LOG_ERROR("Skipping {} relation from {} to {} due "
+                          "to: {}",
+                    to_string(r.type), c.full_name(m_config.using_namespace),
+                    destination, e.what());
             }
-
-            ostr << m_model.to_alias(m_config.using_namespace,
-                        ns_relative(m_config.using_namespace,
-                            c.full_name(m_config.using_namespace)))
-                 << " " << to_string(r.type) << " "
-                 << m_model.to_alias(m_config.using_namespace,
-                        ns_relative(m_config.using_namespace, destination));
-
-            if (!r.label.empty())
-                ostr << " : " << r.label;
-
-            ostr << std::endl;
         }
     }
 
     void generate(const enum_ &e, std::ostream &ostr) const
     {
-        ostr << "enum " << ns_relative(m_config.using_namespace, e.name) << " {"
-             << std::endl;
+        ostr << "enum " << e.alias() << " {" << std::endl;
 
         for (const auto &enum_constant : e.constants) {
             ostr << enum_constant << std::endl;
@@ -259,29 +287,39 @@ public:
                 continue;
 
             std::string destination;
-            if (r.destination.find("#") != std::string::npos ||
-                r.destination.find("@") != std::string::npos) {
-                destination = m_model.usr_to_name(
-                    m_config.using_namespace, r.destination);
-                if (destination.empty()) {
-                    ostr << "' ";
+            std::stringstream relstr;
+            try {
+                if (r.destination.find("#") != std::string::npos ||
+                    r.destination.find("@") != std::string::npos) {
+                    destination = m_model.usr_to_name(
+                        m_config.using_namespace, r.destination);
+                    if (destination.empty()) {
+                        relstr << "' ";
+                        destination = r.destination;
+                    }
+                }
+                else {
                     destination = r.destination;
                 }
+
+                relstr << m_model.to_alias(m_config.using_namespace,
+                              ns_relative(m_config.using_namespace, e.name))
+                       << " " << to_string(r.type) << " "
+                       << m_model.to_alias(m_config.using_namespace,
+                              ns_relative(
+                                  m_config.using_namespace, destination));
+
+                if (!r.label.empty())
+                    relstr << " : " << r.label;
+
+                relstr << std::endl;
+                ostr << relstr.str();
             }
-            else {
-                destination = r.destination;
+            catch (error::uml_alias_missing &ex) {
+                LOG_ERROR("Skipping {} relation from {} to {} due "
+                          "to: {}",
+                    to_string(r.type), e.name, destination, ex.what());
             }
-
-            ostr << m_model.to_alias(m_config.using_namespace,
-                        ns_relative(m_config.using_namespace, e.name))
-                 << " " << to_string(r.type) << " "
-                 << m_model.to_alias(m_config.using_namespace,
-                        ns_relative(m_config.using_namespace, destination));
-
-            if (!r.label.empty())
-                ostr << " : " << r.label;
-
-            ostr << std::endl;
         }
     }
 
@@ -294,7 +332,12 @@ public:
 
         if (m_config.should_include_entities("classes")) {
             for (const auto &c : m_model.classes) {
-                generate_aliases(c, ostr);
+                generate_alias(c, ostr);
+                ostr << std::endl;
+            }
+
+            for (const auto &e : m_model.enums) {
+                generate_alias(e, ostr);
                 ostr << std::endl;
             }
 

src/uml/class_diagram_model.h

@@ -17,6 +17,7 @@
  */
 #pragma once
 
+#include "util/error.h"
 #include "util/util.h"
 
 #include <clang-c/CXCompilationDatabase.h>
@@ -174,6 +175,13 @@ public:
 
     void add_relationship(class_relationship &&cr)
     {
+        if (cr.destination.empty()) {
+            LOG_WARN(
+                "Skipping relationship '{}' - {} - '{}' due empty destination",
+                cr.destination, to_string(cr.type), usr);
+            return;
+        }
+
         auto it = std::find(relationships.begin(), relationships.end(), cr);
         if (it == relationships.end())
             relationships.emplace_back(std::move(cr));
@@ -232,6 +240,17 @@ struct enum_ : public element {
     {
         return l.name == r.name;
     }
+
+    std::string full_name(
+        const std::vector<std::string> &using_namespaces) const
+    {
+        using namespace clanguml::util;
+
+        std::ostringstream ostr;
+        ostr << ns_relative(using_namespaces, name);
+
+        return ostr.str();
+    }
 };
 
 struct diagram {
@@ -281,7 +300,14 @@ struct diagram {
             }
         }
 
-        return full_name;
+        for (const auto &e : enums) {
+            if (e.full_name(using_namespaces) == full_name) {
+                return e.alias();
+            }
+        }
+
+        throw error::uml_alias_missing(
+            fmt::format("Missing alias for {}", full_name));
     }
 
     std::string usr_to_name(const std::vector<std::string> &using_namespaces,

src/uml/class_diagram_visitor.cc

@@ -659,9 +659,12 @@ void tu_visitor::process_template_method(
 {
     class_method m;
     m.name = util::trim(mf.name());
-    m.type = cppast::to_string(
-        static_cast<const cppast::cpp_member_function &>(mf.function())
-            .return_type());
+    if (mf.function().kind() == cppast::cpp_entity_kind::constructor_t)
+        m.type = "void";
+    else
+        m.type = cppast::to_string(
+            static_cast<const cppast::cpp_member_function &>(mf.function())
+                .return_type());
     m.is_pure_virtual = false;
     m.is_virtual = false;
     m.is_const = cppast::is_const(
@@ -747,16 +750,6 @@ void tu_visitor::process_function_parameter(
         // 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());
@@ -885,6 +878,13 @@ void tu_visitor::process_template_template_parameter(
 
 void tu_visitor::process_friend(const cppast::cpp_friend &f, class_ &parent)
 {
+    // Only process friends to other classes or class templates
+    if (!f.entity() ||
+        (f.entity().value().kind() != cppast::cpp_entity_kind::class_t) &&
+            (f.entity().value().kind() !=
+                cppast::cpp_entity_kind::class_template_t))
+        return;
+
     class_relationship r;
     r.type = relationship_t::kFriendship;
     r.label = "<<friend>>";

src/util/error.h

@@ -0,0 +1,29 @@
+/**
+ * src/util/error.h
+ *
+ * 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.
+ */
+#pragma once
+
+#include <stdexcept>
+
+namespace clanguml::error {
+struct uml_alias_missing : public virtual std::runtime_error {
+    uml_alias_missing(const std::string &message)
+        : std::runtime_error(message)
+    {
+    }
+};
+}

src/util/util.h

@@ -34,6 +34,10 @@ std::string trim(const std::string &s);
 #define __FILENAME__                                                           \
     (strrchr(__FILE__, '/') ? strrchr(__FILE__, '/') + 1 : __FILE__)
 
+#define LOG_ERROR(fmt__, ...)                                                  \
+    spdlog::error(std::string("[{}:{}] ") + fmt__, __FILENAME__, __LINE__,     \
+        ##__VA_ARGS__)
+
 #define LOG_WARN(fmt__, ...)                                                   \
     spdlog::warn(std::string("[{}:{}] ") + fmt__, __FILENAME__, __LINE__,      \
         ##__VA_ARGS__)

tests/t00002/t00002.cc

@@ -33,6 +33,29 @@ public:
             a->foo_c();
     }
 
+private:
+    std::vector<A *> as;
+};
+
+//
+// NOTE: libclang fails on:
+//
+//   class D : public virtual B, public virtual C {
+//
+class E : virtual public B, virtual public C {
+public:
+    void foo_a() override
+    {
+        for (auto a : as)
+            a->foo_a();
+    }
+
+    void foo_c() override
+    {
+        for (auto a : as)
+            a->foo_c();
+    }
+
 private:
     std::vector<A *> as;
 };

tests/t00004/test_case.h

@@ -46,10 +46,10 @@ TEST_CASE("t00004", "[test-case][class]")
     REQUIRE_THAT(puml, IsClass(_A("A")));
     REQUIRE_THAT(puml, IsClass(_A("AA")));
     REQUIRE_THAT(puml, IsClass(_A("AAA")));
-    REQUIRE_THAT(puml, IsEnum("Lights"));
+    REQUIRE_THAT(puml, IsEnum(_A("Lights")));
     REQUIRE_THAT(puml, IsInnerClass(_A("A"), _A("AA")));
     REQUIRE_THAT(puml, IsInnerClass(_A("AA"), _A("AAA")));
-    REQUIRE_THAT(puml, IsInnerClass(_A("AA"), "Lights"));
+    REQUIRE_THAT(puml, IsInnerClass(_A("AA"), _A("Lights")));
     REQUIRE_THAT(puml, (IsMethod<Public, Const>("foo")));
     REQUIRE_THAT(puml, (IsMethod<Public, Const>("foo2")));