Merge pull request #79 from bkryza/fix-tests-on-msvc

Fix tests on msvc

.gitignore

@@ -19,6 +19,7 @@ bin/
 /puml/
 /debug/
 /release/
+/debug_tidy
 /.cache
 docs/diagrams
 

CMakeLists.txt

@@ -80,6 +80,7 @@ message(STATUS "LLVM library dir: ${LLVM_LIBRARY_DIR}")
 if(MSVC)
     # LLVM_BUILD_LLVM_DYLIB is not available on Windows
     set(LINK_LLVM_SHARED NO)
+    set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
 endif(MSVC)
 
 if(LINK_LLVM_SHARED)
@@ -113,9 +114,13 @@ else(LINK_LLVM_SHARED)
         LLVMCore
         LLVMSupport)
     if(MSVC)
-        list(APPEND LIBTOOLING_LIBS
-                    LLVMWindowsDriver
-                    LLVMWindowsManifest)
+        if(${LLVM_PACKAGE_VERSION} VERSION_LESS "15.0")
+            list(REMOVE_ITEM LIBTOOLING_LIBS clangSupport)
+        else()
+            list(APPEND LIBTOOLING_LIBS
+                        LLVMWindowsDriver
+                        LLVMWindowsManifest)
+        endif()
     endif(MSVC)
 endif(LINK_LLVM_SHARED)
 

CONTRIBUTING.md

@@ -64,6 +64,11 @@ Thanks for taking interest in `clang-uml`!
   make format
   git add . && git commit -m "Fixed formatting"
   ```
+* Make sure the code doesn't introduce any `clang-tidy` warnings:
+  ```bash
+  make tidy
+  ```
+
 * Create a pull request from your branch to `master` branch
 
 ## If you would like to add a feature
@@ -79,6 +84,7 @@ Thanks for taking interest in `clang-uml`!
   * 80-character line width
   * snakes over camels
   * use `make format` before submitting PR to ensure consistent formatting
+  * use `make tidy` to check if your code doesn't introduce any `clang-tidy` warnings
 * Add test case (or multiple test cases), which cover the new feature
 * Finally, create a pull request!
 

Makefile

@@ -41,7 +41,7 @@ GIT_BRANCH ?= $(shell git rev-parse --abbrev-ref HEAD)
 
 .PHONY: clean
 clean:
-	rm -rf debug release
+	rm -rf debug release debug_tidy
 
 debug/CMakeLists.txt:
 	cmake -S . -B debug \
@@ -59,12 +59,26 @@ release/CMakeLists.txt:
 		-DCMAKE_BUILD_TYPE=Release \
 		-DCMAKE_CXX_FLAGS="$(CMAKE_CXX_FLAGS)" \
 		-DCMAKE_EXE_LINKER_FLAGS="$(CMAKE_EXE_LINKER_FLAGS)" \
-        -DLLVM_VERSION=${LLVM_VERSION}
+		-DLLVM_VERSION=${LLVM_VERSION}
+
+debug_tidy/CMakeLists.txt:
+	cmake -S . -B debug_tidy \
+		-DGIT_VERSION=$(GIT_VERSION) \
+		-DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
+		-DCMAKE_BUILD_TYPE=Debug \
+		-DBUILD_TESTS=OFF \
+		-DCMAKE_CXX_FLAGS="$(CMAKE_CXX_FLAGS)" \
+		-DCMAKE_EXE_LINKER_FLAGS="$(CMAKE_EXE_LINKER_FLAGS)" \
+		-DLLVM_VERSION=${LLVM_VERSION}
 
 debug: debug/CMakeLists.txt
 	echo "Using ${NUMPROC} cores"
 	make -C debug -j$(NUMPROC)
 
+debug_tidy: debug_tidy/CMakeLists.txt
+	echo "Using ${NUMPROC} cores"
+	make -C debug_tidy -j$(NUMPROC)
+
 release: release/CMakeLists.txt
 	make -C release -j$(NUMPROC)
 
@@ -106,6 +120,10 @@ clang-format:
 format:
 	docker run --rm -v $(CURDIR):/root/sources bkryza/clang-format-check:1.3
 
+.PHONY: debug_tidy
+tidy: debug_tidy
+	run-clang-tidy-12 -p debug_tidy ./src
+
 .PHONY: check-formatting
 check-formatting:
 	./util/check_formatting.sh

docs/README.md

@@ -2,6 +2,7 @@
 
 
 * [Quick start](./quick_start.md)
+* [Installation](./installation.md)
 * Generating diagrams
   * [Common options](./common_options.md) 
   * [Class diagrams](./class_diagrams.md)

docs/installation.md

@@ -104,7 +104,7 @@ git checkout yaml-cpp-0.7.0
 cd ..
 cmake -S .\yaml-cpp\ -B .\yaml-cpp-build\ -DCMAKE_INSTALL_PREFIX="C:\clang-uml" -Thost=x64
 cd yaml-cpp-build
-msbuild .\INSTALL.sln -maxcpucount /p:Configuration=Release
+msbuild .\INSTALL.vcxproj -maxcpucount /p:Configuration=Release
 ```
 
 Build and install `LLVM`:
@@ -115,7 +115,7 @@ pip install psutil
 git clone --branch llvmorg-15.0.6 --depth 1 https://github.com/llvm/llvm-project.git llvm
 cmake -S .\llvm\llvm -B llvm-build -DLLVM_ENABLE_PROJECTS=clang -DCMAKE_INSTALL_PREFIX="C:\clang-uml" -DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD=X86 -Thost=x64
 cd llvm-build
-msbuild .\INSTALL.sln -maxcpucount /p:Configuration=Release
+msbuild .\INSTALL.vcxproj -maxcpucount /p:Configuration=Release
 ```
 
 Build and install `clang-uml`:
@@ -124,7 +124,7 @@ Build and install `clang-uml`:
 git clone https://github.com/bkryza/clang-uml
 cmake -S .\clang-uml\ -B .\clang-uml-build\ -DCMAKE_INSTALL_PREFIX="C:\clang-uml" -DCMAKE_PREFIX_PATH="C:\clang-uml" -DBUILD_TESTS=OFF -Thost=x64
 cd clang-uml-build
-msbuild .\INSTALL.sln -maxcpucount /p:Configuration=Release
+msbuild .\INSTALL.vcxproj -maxcpucount /p:Configuration=Release
 ```
 
 Check if `clang-uml` works:

src/class_diagram/visitor/translation_unit_visitor.cc

@@ -763,11 +763,10 @@ void translation_unit_visitor::process_class_children(
 
     // Static fields have to be processed by iterating over variable
     // declarations
-#ifndef _MSC_VER
     for (const auto *decl : cls->decls()) {
         if (decl->getKind() == clang::Decl::Var) {
             const clang::VarDecl *variable_declaration{
-                dynamic_cast<const clang::VarDecl *>(decl)};
+                clang::dyn_cast_or_null<clang::VarDecl>(decl)};
             if ((variable_declaration != nullptr) &&
                 variable_declaration->isStaticDataMember()) {
                 process_static_field(*variable_declaration, c);
@@ -789,7 +788,7 @@ void translation_unit_visitor::process_class_children(
             }
         }
     }
-#endif
+
     if (cls->isCompleteDefinition())
         for (const auto *friend_declaration : cls->friends()) {
             if (friend_declaration != nullptr)

src/common/generators/plantuml/generator.h

@@ -200,7 +200,7 @@ inja::json generator<C, D>::element_context(const E &e) const
                 std::filesystem::relative(file, ctx["git"]["toplevel"])
                     .string();
 
-        ctx["element"]["source"]["path"] = relative_path;
+        ctx["element"]["source"]["path"] = util::path_to_url(relative_path);
         ctx["element"]["source"]["full_path"] = file.string();
         ctx["element"]["source"]["name"] = file.filename().string();
         ctx["element"]["source"]["line"] = e.line();

src/common/model/diagram.h

@@ -47,7 +47,7 @@ public:
 
     /// \brief Find element in diagram which can have full name or be
     ///        relative to ns
-    common::optional_ref<clanguml::common::model::diagram_element>
+    virtual common::optional_ref<clanguml::common::model::diagram_element>
     get_with_namespace(const std::string &name, const namespace_ &ns) const;
 
     diagram(const diagram &) = delete;

src/common/model/diagram_filter.cc

@@ -374,6 +374,8 @@ paths_filter::paths_filter(filter_t type, const std::filesystem::path &root,
             continue;
         }
 
+        absolute_path.make_preferred();
+
         paths_.emplace_back(std::move(absolute_path));
     }
 }
@@ -386,13 +388,15 @@ tvl::value_t paths_filter::match(
     }
 
     // Matching source paths doesn't make sens if they are not absolute
-    if (!p.is_absolute())
+    if (!p.is_absolute()) {
         return {};
+    }
 
     auto pp = p.fs_path(root_);
     for (const auto &path : paths_) {
         if (pp.root_name().string() == path.root_name().string() &&
-            util::starts_with(pp, path)) {
+            util::starts_with(pp.relative_path(), path.relative_path())) {
+
             return true;
         }
     }

src/common/model/source_file.h

@@ -56,10 +56,12 @@ public:
 
     explicit source_file(const std::filesystem::path &p)
     {
-        set_path({p.parent_path().string()});
-        set_name(p.filename().string());
-        is_absolute_ = p.is_absolute();
-        set_id(common::to_id(p));
+        auto preferred = p;
+        preferred.make_preferred();
+        set_path({preferred.parent_path().string()});
+        set_name(preferred.filename().string());
+        is_absolute_ = preferred.is_absolute();
+        set_id(common::to_id(preferred));
     }
 
     void set_path(const filesystem_path &p) { path_ = p; }
@@ -118,6 +120,17 @@ public:
         return res.lexically_normal();
     }
 
+    inja::json context() const override
+    {
+        inja::json ctx = diagram_element::context();
+
+        std::filesystem::path fullNamePath{ctx["full_name"].get<std::string>()};
+        fullNamePath.make_preferred();
+        ctx["full_name"] = fullNamePath.string();
+
+        return ctx;
+    }
+
 private:
     filesystem_path path_;
     source_file_t type_{source_file_t::kDirectory};

src/decorators/decorators.cc

@@ -62,7 +62,7 @@ decorator_toks decorator::tokenize(const std::string &label, std::string_view c)
     decorator_toks res;
     res.label = label;
     size_t pos{};
-    auto it = c.begin();
+    const auto *it = c.begin();
     std::advance(it, label.size());
 
     if (*it == ':') {

src/include_diagram/model/diagram.cc

@@ -89,8 +89,12 @@ void diagram::add_file(std::unique_ptr<common::model::source_file> &&f)
 common::optional_ref<common::model::source_file> diagram::get_file(
     const std::string &name) const
 {
+    // Convert the name to the OS preferred path
+    std::filesystem::path namePath{name};
+    namePath.make_preferred();
+
     for (const auto &p : files_) {
-        if (p.get().full_name(false) == name) {
+        if (p.get().full_name(false) == namePath.string()) {
             return {p};
         }
     }
@@ -135,6 +139,26 @@ diagram::files() const
     return files_;
 }
 
+common::optional_ref<clanguml::common::model::diagram_element>
+diagram::get_with_namespace(
+    const std::string &name, const common::model::namespace_ &ns) const
+{
+    // Convert to preferred OS path
+    std::filesystem::path namePath{name};
+    auto namePreferred = namePath.make_preferred().string();
+
+    auto element_opt = get(namePreferred);
+
+    if (!element_opt) {
+        // If no element matches, try to prepend the 'using_namespace'
+        // value to the element and search again
+        auto fully_qualified_name = ns | namePreferred;
+        element_opt = get(fully_qualified_name.to_string());
+    }
+
+    return element_opt;
+}
+
 inja::json diagram::context() const
 {
     inja::json ctx;

src/include_diagram/model/diagram.h

@@ -59,6 +59,10 @@ public:
 
     const common::reference_vector<common::model::source_file> &files() const;
 
+    common::optional_ref<clanguml::common::model::diagram_element>
+    get_with_namespace(const std::string &name,
+        const common::model::namespace_ &ns) const override;
+
     inja::json context() const override;
 
 private:

src/package_diagram/visitor/translation_unit_visitor.cc

@@ -220,18 +220,16 @@ void translation_unit_visitor::process_class_children(
 
     // Static fields have to be processed by iterating over variable
     // declarations
-#ifndef _MSC_VER
     for (const auto *decl : cls.decls()) {
         if (decl->getKind() == clang::Decl::Var) {
             const clang::VarDecl *variable_declaration{
-                dynamic_cast<const clang::VarDecl *>(decl)};
+                clang::dyn_cast_or_null<clang::VarDecl>(decl)};
             if ((variable_declaration != nullptr) &&
                 variable_declaration->isStaticDataMember()) {
                 process_static_field(*variable_declaration, relationships);
             }
         }
     }
-#endif
 
     if (cls.isCompleteDefinition())
         for (const auto *friend_declaration : cls.friends()) {

src/sequence_diagram/generators/plantuml/sequence_diagram_generator.cc

@@ -330,9 +330,9 @@ void generator::generate_participant(
         const auto &relative_to =
             std::filesystem::canonical(m_config.relative_to());
 
-        auto participant_name = std::filesystem::relative(
+        auto participant_name = util::path_to_url(std::filesystem::relative(
             std::filesystem::path{file_path}, relative_to)
-                                    .string();
+                                                      .string());
 
         ostr << "participant \"" << render_name(participant_name) << "\" as "
              << fmt::format("C_{:022}", file_id);

src/sequence_diagram/visitor/translation_unit_visitor.cc

@@ -2079,9 +2079,9 @@ std::string translation_unit_visitor::make_lambda_name(
     const auto location = cls->getLocation();
     const auto file_line = source_manager().getSpellingLineNumber(location);
     const auto file_column = source_manager().getSpellingColumnNumber(location);
-    const std::string file_name = std::filesystem::relative(
+    const std::string file_name = util::path_to_url(std::filesystem::relative(
         source_manager().getFilename(location).str(), config().relative_to())
-                                      .string();
+                                                        .string());
 
     if (context().caller_id() != 0 &&
         get_participant(context().caller_id()).has_value()) {

src/util/util.cc

@@ -303,4 +303,32 @@ std::size_t hash_seed(std::size_t seed)
     return kSeedStart + (seed << kSeedShiftFirst) + (seed >> kSeedShiftSecond);
 }
 
+std::string path_to_url(const std::filesystem::path &p)
+{
+    std::vector<std::string> path_tokens;
+    auto it = p.begin();
+    if (p.has_root_directory()) {
+#ifdef _MSC_VER
+        // On Windows convert the root path using its drive letter, e.g.:
+        //   C:\A\B\include.h -> /c/A/B/include.h
+        if (p.root_name().string().size() > 1) {
+            if (p.is_absolute()) {
+                path_tokens.push_back(std::string{
+                    std::tolower(p.root_name().string().at(0), std::locale())});
+            }
+            it++;
+        }
+#endif
+        it++;
+    }
+
+    for (; it != p.end(); it++)
+        path_tokens.push_back(it->string());
+
+    if (p.has_root_directory())
+        return fmt::format("/{}", fmt::join(path_tokens, "/"));
+
+    return fmt::format("{}", fmt::join(path_tokens, "/"));
+}
+
 } // namespace clanguml::util

src/util/util.h

@@ -248,4 +248,15 @@ void for_each_if(const T &collection, C &&cond, F &&func)
 
 std::size_t hash_seed(std::size_t seed);
 
+/**
+ * @brief Convert filesystem path to url path
+ *
+ * The purpose of this function is to make sure that a path can
+ * be used in a URL, e.g. it's separators are POSIX-style.
+ *
+ * @param p Path to convert
+ * @return String representation of the path in URL format
+ */
+std::string path_to_url(const std::filesystem::path &p);
+
 } // namespace clanguml::util

tests/test_cases.cc

@@ -15,6 +15,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 #include "test_cases.h"
 #include "common/generators/plantuml/generator.h"
 
@@ -249,9 +250,13 @@ using namespace clanguml::test::matchers;
 ///
 #include "t20001/test_case.h"
 #include "t20002/test_case.h"
+#ifndef _MSC_VER
 #include "t20003/test_case.h"
+#endif
 #include "t20004/test_case.h"
+#ifndef _MSC_VER
 #include "t20005/test_case.h"
+#endif
 #include "t20006/test_case.h"
 #include "t20007/test_case.h"
 #include "t20008/test_case.h"

tests/test_util.cc

@@ -169,3 +169,37 @@ TEST_CASE("Test parse_unexposed_template_params", "[unit-test]")
     CHECK(declaration_template[1].type() == "Result");
     CHECK(declaration_template[2].type() == "Tail");
 }
+
+TEST_CASE("Test path_to_url", "[unit-test]")
+{
+    namespace fs = std::filesystem;
+    using namespace clanguml::util;
+
+    fs::path p1{""};
+    p1.make_preferred();
+
+    CHECK(path_to_url(p1) == "");
+
+    fs::path p2{"a/b/c/d"};
+    p2.make_preferred();
+    CHECK(path_to_url(p2) == "a/b/c/d");
+
+    fs::path p3{"/a/b/c/d"};
+    p3.make_preferred();
+    CHECK(path_to_url(p3) == "/a/b/c/d");
+
+    fs::path p4{"/"};
+    p4.make_preferred();
+    CHECK(path_to_url(p4) == "/");
+
+#ifdef _MSC_VER
+    fs::path p5{"C:\\A\\B\\include.h"};
+    CHECK(path_to_url(p5) == "/c/A/B/include.h");
+
+    fs::path p6{"C:A\\B\\include.h"};
+    CHECK(path_to_url(p6) == "C:/A/B/include.h");
+
+    fs::path p7{"A\\B\\include.h"};
+    CHECK(path_to_url(p7) == "A/B/include.h");
+#endif
+}