From e1ec052d53c98294548b88f65d7e09dad537c163 Mon Sep 17 00:00:00 2001 From: Joe Blaauboer Date: Mon, 28 Nov 2022 13:08:37 -0500 Subject: clang-tidy module: add check for string concatenation Co-Authored-by: Kyle Edwards --- Utilities/ClangTidyModule/CMakeLists.txt | 2 + Utilities/ClangTidyModule/Module.cxx | 3 + .../StringConcatenationUseCmstrcatCheck.cxx | 179 +++++++++++++++++++++ .../StringConcatenationUseCmstrcatCheck.h | 34 ++++ 4 files changed, 218 insertions(+) create mode 100644 Utilities/ClangTidyModule/StringConcatenationUseCmstrcatCheck.cxx create mode 100644 Utilities/ClangTidyModule/StringConcatenationUseCmstrcatCheck.h diff --git a/Utilities/ClangTidyModule/CMakeLists.txt b/Utilities/ClangTidyModule/CMakeLists.txt index 6fc54b1..97c176f 100644 --- a/Utilities/ClangTidyModule/CMakeLists.txt +++ b/Utilities/ClangTidyModule/CMakeLists.txt @@ -16,6 +16,8 @@ add_library(cmake-clang-tidy-module MODULE OstringstreamUseCmstrcatCheck.cxx OstringstreamUseCmstrcatCheck.h + StringConcatenationUseCmstrcatCheck.cxx + StringConcatenationUseCmstrcatCheck.h UseBespokeEnumClassCheck.cxx UseBespokeEnumClassCheck.h UseCmstrlenCheck.cxx diff --git a/Utilities/ClangTidyModule/Module.cxx b/Utilities/ClangTidyModule/Module.cxx index c747cee..4dd7dcd 100644 --- a/Utilities/ClangTidyModule/Module.cxx +++ b/Utilities/ClangTidyModule/Module.cxx @@ -4,6 +4,7 @@ #include #include "OstringstreamUseCmstrcatCheck.h" +#include "StringConcatenationUseCmstrcatCheck.h" #include "UseBespokeEnumClassCheck.h" #include "UseCmstrlenCheck.h" #include "UseCmsysFstreamCheck.h" @@ -25,6 +26,8 @@ public: CheckFactories.registerCheck( "cmake-ostringstream-use-cmstrcat"); CheckFactories.registerCheck("cmake-use-pragma-once"); + CheckFactories.registerCheck( + "cmake-string-concatenation-use-cmstrcat"); } }; diff --git a/Utilities/ClangTidyModule/StringConcatenationUseCmstrcatCheck.cxx b/Utilities/ClangTidyModule/StringConcatenationUseCmstrcatCheck.cxx new file mode 100644 index 0000000..df14c83 --- /dev/null +++ b/Utilities/ClangTidyModule/StringConcatenationUseCmstrcatCheck.cxx @@ -0,0 +1,179 @@ +/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying + file Copyright.txt or https://cmake.org/licensing for details. */ +#include "StringConcatenationUseCmstrcatCheck.h" + +#include + +#include +#include + +namespace clang { +namespace tidy { +namespace cmake { +using namespace ast_matchers; + +StringConcatenationUseCmstrcatCheck::StringConcatenationUseCmstrcatCheck( + StringRef Name, ClangTidyContext* Context) + : ClangTidyCheck(Name, Context) +{ +} + +void StringConcatenationUseCmstrcatCheck::registerMatchers(MatchFinder* Finder) +{ + auto IsString = expr(hasType(qualType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(classTemplateSpecializationDecl( + hasName("::std::basic_string"), + hasTemplateArgument( + 0, templateArgument(refersToType(asString("char"))))))))))); + + auto IsChar = expr(hasType(asString("char"))); + + auto IsCharPtr = expr(hasType(pointerType(pointee(asString("const char"))))); + + auto IsStringConcat = + cxxOperatorCallExpr(hasOperatorName("+"), + anyOf(allOf(hasLHS(IsString), hasRHS(IsString)), + allOf(hasLHS(IsString), hasRHS(IsChar)), + allOf(hasLHS(IsString), hasRHS(IsCharPtr)), + allOf(hasLHS(IsChar), hasRHS(IsString)), + allOf(hasLHS(IsCharPtr), hasRHS(IsString)))); + + auto IsStringAppend = cxxOperatorCallExpr( + hasOperatorName("+="), hasLHS(IsString), + anyOf(hasRHS(IsString), hasRHS(IsChar), hasRHS(IsCharPtr))); + + auto IsStringConcatWithLHS = + cxxOperatorCallExpr( + IsStringConcat, + optionally(hasLHS(materializeTemporaryExpr( + has(cxxBindTemporaryExpr(has(IsStringConcat.bind("lhs")))))))) + .bind("concat"); + + auto IsStringAppendWithRHS = + cxxOperatorCallExpr( + IsStringAppend, + optionally(hasRHS(materializeTemporaryExpr(has(implicitCastExpr( + has(cxxBindTemporaryExpr(has(IsStringConcat.bind("rhs")))))))))) + .bind("append"); + + Finder->addMatcher(IsStringConcatWithLHS, this); + Finder->addMatcher(IsStringAppendWithRHS, this); +} + +void StringConcatenationUseCmstrcatCheck::check( + const MatchFinder::MatchResult& Result) +{ + const CXXOperatorCallExpr* AppendNode = + Result.Nodes.getNodeAs("append"); + const CXXOperatorCallExpr* ConcatNode = + Result.Nodes.getNodeAs("concat"); + + if (AppendNode != nullptr) { + if (AppendNode->getBeginLoc().isValid()) { + assert(InProgressExprChains.find(AppendNode) == + InProgressExprChains.end()); + + ExprChain TmpExprChain = + std::make_pair(OperatorType::PlusEquals, + std::vector{ AppendNode }); + const CXXOperatorCallExpr* RHSNode = + Result.Nodes.getNodeAs("rhs"); + + if (RHSNode != nullptr) { + if (RHSNode->getBeginLoc().isValid()) { + InProgressExprChains[RHSNode] = std::move(TmpExprChain); + } + } else { + issueCorrection(TmpExprChain, Result); + } + } + } + + if (ConcatNode != nullptr) { + if (ConcatNode->getBeginLoc().isValid()) { + ExprChain TmpExprChain; + + if (!(InProgressExprChains.find(ConcatNode) == + InProgressExprChains.end())) { + TmpExprChain = std::move(InProgressExprChains[ConcatNode]); + InProgressExprChains.erase(ConcatNode); + if (TmpExprChain.first == OperatorType::PlusEquals) { + TmpExprChain.second.insert(TmpExprChain.second.begin() + 1, + ConcatNode); + } else { + TmpExprChain.second.insert(TmpExprChain.second.begin(), ConcatNode); + } + } else { + TmpExprChain = std::make_pair( + OperatorType::Plus, + std::vector{ ConcatNode }); + } + + const CXXOperatorCallExpr* LHSNode = + Result.Nodes.getNodeAs("lhs"); + + if (LHSNode != nullptr) { + if (LHSNode->getBeginLoc().isValid()) { + InProgressExprChains[LHSNode] = std::move(TmpExprChain); + } + } else { + issueCorrection(TmpExprChain, Result); + } + } + } +} + +void StringConcatenationUseCmstrcatCheck::issueCorrection( + const ExprChain& Chain, const MatchFinder::MatchResult& Result) +{ + std::vector FixIts; + const CXXOperatorCallExpr* ExprNode; + std::vector::const_iterator It = + Chain.second.begin(); + + if (Chain.first == OperatorType::PlusEquals) { + ExprNode = *It; + StringRef LHS = Lexer::getSourceText( + CharSourceRange::getTokenRange(ExprNode->getArg(0)->getSourceRange()), + Result.Context->getSourceManager(), Result.Context->getLangOpts()); + + FixIts.push_back(FixItHint::CreateReplacement( + ExprNode->getExprLoc(), "= cmStrCat(" + LHS.str() + ",")); + It++; + } else { + ExprNode = *It; + FixIts.push_back( + FixItHint::CreateInsertion(ExprNode->getBeginLoc(), "cmStrCat(")); + } + + while (It != std::end(Chain.second)) { + ExprNode = *It; + FixIts.push_back( + FixItHint::CreateReplacement(ExprNode->getOperatorLoc(), ",")); + It++; + } + It--; + ExprNode = *It; + + StringRef LastToken = Lexer::getSourceText( + CharSourceRange::getTokenRange(ExprNode->getArg(1)->getSourceRange()), + Result.Context->getSourceManager(), Result.Context->getLangOpts()); + FixIts.push_back(FixItHint::CreateInsertion( + ExprNode->getEndLoc().getLocWithOffset(LastToken.str().size()), ")")); + + It = Chain.second.begin(); + ExprNode = *It; + + if (Chain.first == OperatorType::PlusEquals) { + this->diag(ExprNode->getOperatorLoc(), + "use cmStrCat() instead of string append") + << FixIts; + } else { + this->diag(ExprNode->getBeginLoc(), + "use cmStrCat() instead of string concatenation") + << FixIts; + } +} +} +} +} diff --git a/Utilities/ClangTidyModule/StringConcatenationUseCmstrcatCheck.h b/Utilities/ClangTidyModule/StringConcatenationUseCmstrcatCheck.h new file mode 100644 index 0000000..43ff539 --- /dev/null +++ b/Utilities/ClangTidyModule/StringConcatenationUseCmstrcatCheck.h @@ -0,0 +1,34 @@ +/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying + file Copyright.txt or https://cmake.org/licensing for details. */ +#pragma once + +#include +#include + +namespace clang { +namespace tidy { +namespace cmake { +class StringConcatenationUseCmstrcatCheck : public ClangTidyCheck +{ +public: + StringConcatenationUseCmstrcatCheck(StringRef Name, + ClangTidyContext* Context); + void registerMatchers(ast_matchers::MatchFinder* Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult& Result) override; + +private: + enum class OperatorType + { + Plus, + PlusEquals + }; + typedef std::pair> + ExprChain; + std::map InProgressExprChains; + + void issueCorrection(const ExprChain& ExprChain, + const ast_matchers::MatchFinder::MatchResult& Result); +}; +} +} +} -- cgit v0.12 From c6c8616468d63c5a39d7018f07e8586a89ae7025 Mon Sep 17 00:00:00 2001 From: Sean Orner Date: Mon, 28 Nov 2022 13:37:52 -0500 Subject: clang-tidy module: add tests for string concatenation check --- Utilities/ClangTidyModule/Tests/CMakeLists.txt | 1 + ...ake-string-concatenation-use-cmstrcat-fixit.cxx | 36 +++++++ ...ke-string-concatenation-use-cmstrcat-stdout.txt | 113 +++++++++++++++++++++ .../cmake-string-concatenation-use-cmstrcat.cxx | 36 +++++++ 4 files changed, 186 insertions(+) create mode 100644 Utilities/ClangTidyModule/Tests/cmake-string-concatenation-use-cmstrcat-fixit.cxx create mode 100644 Utilities/ClangTidyModule/Tests/cmake-string-concatenation-use-cmstrcat-stdout.txt create mode 100644 Utilities/ClangTidyModule/Tests/cmake-string-concatenation-use-cmstrcat.cxx diff --git a/Utilities/ClangTidyModule/Tests/CMakeLists.txt b/Utilities/ClangTidyModule/Tests/CMakeLists.txt index b53d5d2..8220f39 100644 --- a/Utilities/ClangTidyModule/Tests/CMakeLists.txt +++ b/Utilities/ClangTidyModule/Tests/CMakeLists.txt @@ -15,3 +15,4 @@ add_run_clang_tidy_test(cmake-use-cmsys-fstream) add_run_clang_tidy_test(cmake-use-bespoke-enum-class) add_run_clang_tidy_test(cmake-ostringstream-use-cmstrcat) add_run_clang_tidy_test(cmake-use-pragma-once) +add_run_clang_tidy_test(cmake-string-concatenation-use-cmstrcat) diff --git a/Utilities/ClangTidyModule/Tests/cmake-string-concatenation-use-cmstrcat-fixit.cxx b/Utilities/ClangTidyModule/Tests/cmake-string-concatenation-use-cmstrcat-fixit.cxx new file mode 100644 index 0000000..79aecd4 --- /dev/null +++ b/Utilities/ClangTidyModule/Tests/cmake-string-concatenation-use-cmstrcat-fixit.cxx @@ -0,0 +1,36 @@ +#include + +template +std::string cmStrCat(Args&&... args) +{ + return ""; +} + +std::string a = "This is a string variable"; +std::string b = " and this is a string variable"; +std::string concat; + +// Correction needed +void test1() +{ + concat = cmStrCat(a, b); + concat = cmStrCat(a, " and this is a string literal"); + concat = cmStrCat(a, 'O'); + concat = cmStrCat("This is a string literal", b); + concat = cmStrCat('O', a); + concat = cmStrCat(a, " and this is a string literal", 'O', b); + + concat = cmStrCat(concat, b); + concat = cmStrCat(concat, " and this is a string literal"); + concat = cmStrCat(concat, 'o'); + concat = cmStrCat(concat, b, " and this is a string literal ", 'o', b); +} + +// No correction needed +void test2() +{ + a = b; + a = "This is a string literal"; + a = 'X'; + cmStrCat(a, b); +} diff --git a/Utilities/ClangTidyModule/Tests/cmake-string-concatenation-use-cmstrcat-stdout.txt b/Utilities/ClangTidyModule/Tests/cmake-string-concatenation-use-cmstrcat-stdout.txt new file mode 100644 index 0000000..3cfdef8 --- /dev/null +++ b/Utilities/ClangTidyModule/Tests/cmake-string-concatenation-use-cmstrcat-stdout.txt @@ -0,0 +1,113 @@ +cmake-string-concatenation-use-cmstrcat.cxx:16:12: warning: use cmStrCat() instead of string concatenation [cmake-string-concatenation-use-cmstrcat] + concat = a + b; + ^ ~ + cmStrCat( , ) +cmake-string-concatenation-use-cmstrcat.cxx:16:12: note: FIX-IT applied suggested code changes +cmake-string-concatenation-use-cmstrcat.cxx:16:14: note: FIX-IT applied suggested code changes + concat = a + b; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:16:17: note: FIX-IT applied suggested code changes + concat = a + b; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:17:12: warning: use cmStrCat() instead of string concatenation [cmake-string-concatenation-use-cmstrcat] + concat = a + " and this is a string literal"; + ^ ~ + cmStrCat( , ) +cmake-string-concatenation-use-cmstrcat.cxx:17:12: note: FIX-IT applied suggested code changes +cmake-string-concatenation-use-cmstrcat.cxx:17:14: note: FIX-IT applied suggested code changes + concat = a + " and this is a string literal"; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:17:47: note: FIX-IT applied suggested code changes + concat = a + " and this is a string literal"; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:18:12: warning: use cmStrCat() instead of string concatenation [cmake-string-concatenation-use-cmstrcat] + concat = a + 'O'; + ^ ~ + cmStrCat( , ) +cmake-string-concatenation-use-cmstrcat.cxx:18:12: note: FIX-IT applied suggested code changes +cmake-string-concatenation-use-cmstrcat.cxx:18:14: note: FIX-IT applied suggested code changes + concat = a + 'O'; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:18:19: note: FIX-IT applied suggested code changes + concat = a + 'O'; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:19:12: warning: use cmStrCat() instead of string concatenation [cmake-string-concatenation-use-cmstrcat] + concat = "This is a string literal" + b; + ^ ~ + cmStrCat( , ) +cmake-string-concatenation-use-cmstrcat.cxx:19:12: note: FIX-IT applied suggested code changes +cmake-string-concatenation-use-cmstrcat.cxx:19:39: note: FIX-IT applied suggested code changes + concat = "This is a string literal" + b; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:19:42: note: FIX-IT applied suggested code changes + concat = "This is a string literal" + b; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:20:12: warning: use cmStrCat() instead of string concatenation [cmake-string-concatenation-use-cmstrcat] + concat = 'O' + a; + ^ ~ + cmStrCat( , ) +cmake-string-concatenation-use-cmstrcat.cxx:20:12: note: FIX-IT applied suggested code changes +cmake-string-concatenation-use-cmstrcat.cxx:20:16: note: FIX-IT applied suggested code changes + concat = 'O' + a; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:20:19: note: FIX-IT applied suggested code changes + concat = 'O' + a; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:21:12: warning: use cmStrCat() instead of string concatenation [cmake-string-concatenation-use-cmstrcat] + concat = a + " and this is a string literal" + 'O' + b; + ^ ~ ~ ~ + cmStrCat( , , , ) +cmake-string-concatenation-use-cmstrcat.cxx:21:12: note: FIX-IT applied suggested code changes +cmake-string-concatenation-use-cmstrcat.cxx:21:14: note: FIX-IT applied suggested code changes + concat = a + " and this is a string literal" + 'O' + b; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:21:48: note: FIX-IT applied suggested code changes + concat = a + " and this is a string literal" + 'O' + b; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:21:54: note: FIX-IT applied suggested code changes + concat = a + " and this is a string literal" + 'O' + b; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:21:57: note: FIX-IT applied suggested code changes + concat = a + " and this is a string literal" + 'O' + b; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:23:10: warning: use cmStrCat() instead of string append [cmake-string-concatenation-use-cmstrcat] + concat += b; + ^~ + = cmStrCat(concat, ) +cmake-string-concatenation-use-cmstrcat.cxx:23:10: note: FIX-IT applied suggested code changes +cmake-string-concatenation-use-cmstrcat.cxx:23:14: note: FIX-IT applied suggested code changes + concat += b; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:24:10: warning: use cmStrCat() instead of string append [cmake-string-concatenation-use-cmstrcat] + concat += " and this is a string literal"; + ^~ + = cmStrCat(concat, ) +cmake-string-concatenation-use-cmstrcat.cxx:24:10: note: FIX-IT applied suggested code changes +cmake-string-concatenation-use-cmstrcat.cxx:24:44: note: FIX-IT applied suggested code changes + concat += " and this is a string literal"; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:25:10: warning: use cmStrCat() instead of string append [cmake-string-concatenation-use-cmstrcat] + concat += 'o'; + ^~ + = cmStrCat(concat, ) +cmake-string-concatenation-use-cmstrcat.cxx:25:10: note: FIX-IT applied suggested code changes +cmake-string-concatenation-use-cmstrcat.cxx:25:16: note: FIX-IT applied suggested code changes + concat += 'o'; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:26:10: warning: use cmStrCat() instead of string append [cmake-string-concatenation-use-cmstrcat] + concat += b + " and this is a string literal " + 'o' + b; + ^~ ~ ~ ~ + = cmStrCat(concat, , , , ) +cmake-string-concatenation-use-cmstrcat.cxx:26:10: note: FIX-IT applied suggested code changes +cmake-string-concatenation-use-cmstrcat.cxx:26:15: note: FIX-IT applied suggested code changes + concat += b + " and this is a string literal " + 'o' + b; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:26:50: note: FIX-IT applied suggested code changes + concat += b + " and this is a string literal " + 'o' + b; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:26:56: note: FIX-IT applied suggested code changes + concat += b + " and this is a string literal " + 'o' + b; + ^ +cmake-string-concatenation-use-cmstrcat.cxx:26:59: note: FIX-IT applied suggested code changes + concat += b + " and this is a string literal " + 'o' + b; + ^ diff --git a/Utilities/ClangTidyModule/Tests/cmake-string-concatenation-use-cmstrcat.cxx b/Utilities/ClangTidyModule/Tests/cmake-string-concatenation-use-cmstrcat.cxx new file mode 100644 index 0000000..13a20ac --- /dev/null +++ b/Utilities/ClangTidyModule/Tests/cmake-string-concatenation-use-cmstrcat.cxx @@ -0,0 +1,36 @@ +#include + +template +std::string cmStrCat(Args&&... args) +{ + return ""; +} + +std::string a = "This is a string variable"; +std::string b = " and this is a string variable"; +std::string concat; + +// Correction needed +void test1() +{ + concat = a + b; + concat = a + " and this is a string literal"; + concat = a + 'O'; + concat = "This is a string literal" + b; + concat = 'O' + a; + concat = a + " and this is a string literal" + 'O' + b; + + concat += b; + concat += " and this is a string literal"; + concat += 'o'; + concat += b + " and this is a string literal " + 'o' + b; +} + +// No correction needed +void test2() +{ + a = b; + a = "This is a string literal"; + a = 'X'; + cmStrCat(a, b); +} -- cgit v0.12 From 5ad111e595c2e1642fdebd3f7048a5aca664035f Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Mon, 28 Nov 2022 13:55:59 -0500 Subject: clang-tidy: disable string concatenation check --- .clang-tidy | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-tidy b/.clang-tidy index 9ce5f42..c790467 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -44,6 +44,7 @@ readability-*,\ -readability-uppercase-literal-suffix,\ cmake-*,\ -cmake-ostringstream-use-cmstrcat,\ +-cmake-string-concatenation-use-cmstrcat,\ -cmake-use-bespoke-enum-class,\ " HeaderFilterRegex: 'Source/cm[^/]*\.(h|hxx|cxx)$' -- cgit v0.12