Merge topic 'clang-tidy-module-cmstrlen-check-sizeof'

830eed374d CMake: fix sizeof string literal violations
c6368a23fe clang-tidy module: look for sizeof string literal in cmStrLen() check

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !7927

8 files changed, 96 insertions, 11 deletions

diff --git a/Source/CPack/cmCPackDragNDropGenerator.cxx b/Source/CPack/cmCPackDragNDropGenerator.cxx
index 0579066..68e7ba3 100644
--- a/Source/CPack/cmCPackDragNDropGenerator.cxx
+++ b/Source/CPack/cmCPackDragNDropGenerator.cxx
@@ -451,7 +451,7 @@ int cmCPackDragNDropGenerator::CreateDMG(const std::string& src_dir,
     mountpoint_regex.find(attach_output.c_str());
     std::string const temp_mount = mountpoint_regex.match(1);
     std::string const temp_mount_name =
-      temp_mount.substr(sizeof("/Volumes/") - 1);
+      temp_mount.substr(cmStrLen("/Volumes/"));
 
     // Remove dummy padding file so we have enough space on RW image ...
     std::ostringstream dummy_padding;
diff --git a/Source/cmExportFileGenerator.cxx b/Source/cmExportFileGenerator.cxx
index bd2b6af..c8e2cb8 100644
--- a/Source/cmExportFileGenerator.cxx
+++ b/Source/cmExportFileGenerator.cxx
@@ -669,8 +669,7 @@ void cmExportFileGenerator::ResolveTargetsInGeneratorExpression(
 
   while ((pos = input.find("$<TARGET_PROPERTY:", lastPos)) !=
          std::string::npos) {
-    std::string::size_type nameStartPos =
-      pos + sizeof("$<TARGET_PROPERTY:") - 1;
+    std::string::size_type nameStartPos = pos + cmStrLen("$<TARGET_PROPERTY:");
     std::string::size_type closePos = input.find('>', nameStartPos);
     std::string::size_type commaPos = input.find(',', nameStartPos);
     std::string::size_type nextOpenPos = input.find("$<", nameStartPos);
@@ -696,7 +695,7 @@ void cmExportFileGenerator::ResolveTargetsInGeneratorExpression(
   pos = 0;
   lastPos = pos;
   while ((pos = input.find("$<TARGET_NAME:", lastPos)) != std::string::npos) {
-    std::string::size_type nameStartPos = pos + sizeof("$<TARGET_NAME:") - 1;
+    std::string::size_type nameStartPos = pos + cmStrLen("$<TARGET_NAME:");
     std::string::size_type endPos = input.find('>', nameStartPos);
     if (endPos == std::string::npos) {
       errorString = "$<TARGET_NAME:...> expression incomplete";
@@ -721,7 +720,7 @@ void cmExportFileGenerator::ResolveTargetsInGeneratorExpression(
   lastPos = pos;
   while (errorString.empty() &&
          (pos = input.find("$<LINK_ONLY:", lastPos)) != std::string::npos) {
-    std::string::size_type nameStartPos = pos + sizeof("$<LINK_ONLY:") - 1;
+    std::string::size_type nameStartPos = pos + cmStrLen("$<LINK_ONLY:");
     std::string::size_type endPos = input.find('>', nameStartPos);
     if (endPos == std::string::npos) {
       errorString = "$<LINK_ONLY:...> expression incomplete";
diff --git a/Source/cmGeneratorExpression.cxx b/Source/cmGeneratorExpression.cxx
index b605350..c5ae31b 100644
--- a/Source/cmGeneratorExpression.cxx
+++ b/Source/cmGeneratorExpression.cxx
@@ -406,7 +406,7 @@ void cmGeneratorExpression::ReplaceInstallPrefix(
 
   while ((pos = input.find("$<INSTALL_PREFIX>", lastPos)) !=
          std::string::npos) {
-    std::string::size_type endPos = pos + sizeof("$<INSTALL_PREFIX>") - 1;
+    std::string::size_type endPos = pos + cmStrLen("$<INSTALL_PREFIX>");
     input.replace(pos, endPos - pos, replacement);
     lastPos = endPos;
   }
diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx
index 6065285..fae6d54 100644
--- a/Source/cmGeneratorTarget.cxx
+++ b/Source/cmGeneratorTarget.cxx
@@ -6718,7 +6718,7 @@ bool cmGeneratorTarget::IsLinkLookupScope(std::string const& n,
                                           cmLocalGenerator const*& lg) const
 {
   if (cmHasLiteralPrefix(n, CMAKE_DIRECTORY_ID_SEP)) {
-    cmDirectoryId const dirId = n.substr(sizeof(CMAKE_DIRECTORY_ID_SEP) - 1);
+    cmDirectoryId const dirId = n.substr(cmStrLen(CMAKE_DIRECTORY_ID_SEP));
     if (dirId.String.empty()) {
       lg = this->LocalGenerator;
       return true;
diff --git a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-fixit.cxx b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-fixit.cxx
index c93d557..cde0086 100644
--- a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-fixit.cxx
+++ b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-fixit.cxx
@@ -27,11 +27,16 @@ int main()
   (void)cmStrLen("Goodbye");
   (void)cmStrLen("Hola");
   (void)cmStrLen("Bonjour");
+  (void)(cmStrLen("Hallo"));
+  (void)(4 + cmStrLen("Hallo"));
+  (void)(cmStrLen("Hallo"));
+  (void)(4 + cmStrLen("Hallo"));
 
   // No correction needed
   (void)ns2::strlen("Salve");
   (void)cmStrLen("Konnichiwa");
   (void)strlen(s0);
+  (void)(sizeof("Hallo") - 2);
 
   return 0;
 }
diff --git a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-stdout.txt b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-stdout.txt
index 6c52ad5..d18822a 100644
--- a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-stdout.txt
+++ b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-stdout.txt
@@ -18,3 +18,35 @@ cmake-use-cmstrlen.cxx:29:9: warning: use cmStrLen() for string literals [cmake-
         ^~~~~~~~~~~
         cmStrLen
 cmake-use-cmstrlen.cxx:29:9: note: FIX-IT applied suggested code changes
+cmake-use-cmstrlen.cxx:30:10: warning: use cmStrLen() for string literals [cmake-use-cmstrlen]
+  (void)(sizeof("Hallo") - 1);
+         ^~~~~~          ~~~
+         cmStrLen
+cmake-use-cmstrlen.cxx:30:10: note: FIX-IT applied suggested code changes
+cmake-use-cmstrlen.cxx:30:26: note: FIX-IT applied suggested code changes
+  (void)(sizeof("Hallo") - 1);
+                         ^
+cmake-use-cmstrlen.cxx:31:14: warning: use cmStrLen() for string literals [cmake-use-cmstrlen]
+  (void)(4 + sizeof("Hallo") - 1);
+             ^~~~~~          ~~~
+             cmStrLen
+cmake-use-cmstrlen.cxx:31:14: note: FIX-IT applied suggested code changes
+cmake-use-cmstrlen.cxx:31:30: note: FIX-IT applied suggested code changes
+  (void)(4 + sizeof("Hallo") - 1);
+                             ^
+cmake-use-cmstrlen.cxx:32:10: warning: use cmStrLen() for string literals [cmake-use-cmstrlen]
+  (void)(sizeof "Hallo" - 1);
+         ^~~~~~         ~~~
+         cmStrLen(      )
+cmake-use-cmstrlen.cxx:32:10: note: FIX-IT applied suggested code changes
+cmake-use-cmstrlen.cxx:32:25: note: FIX-IT applied suggested code changes
+  (void)(sizeof "Hallo" - 1);
+                        ^
+cmake-use-cmstrlen.cxx:33:14: warning: use cmStrLen() for string literals [cmake-use-cmstrlen]
+  (void)(4 + sizeof "Hallo" - 1);
+             ^~~~~~         ~~~
+             cmStrLen(      )
+cmake-use-cmstrlen.cxx:33:14: note: FIX-IT applied suggested code changes
+cmake-use-cmstrlen.cxx:33:29: note: FIX-IT applied suggested code changes
+  (void)(4 + sizeof "Hallo" - 1);
+                            ^
diff --git a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen.cxx b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen.cxx
index f36262b..205bc9c 100644
--- a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen.cxx
+++ b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen.cxx
@@ -27,11 +27,16 @@ int main()
   (void)::strlen("Goodbye");
   (void)std::strlen("Hola");
   (void)ns1::strlen("Bonjour");
+  (void)(sizeof("Hallo") - 1);
+  (void)(4 + sizeof("Hallo") - 1);
+  (void)(sizeof "Hallo" - 1);
+  (void)(4 + sizeof "Hallo" - 1);
 
   // No correction needed
   (void)ns2::strlen("Salve");
   (void)cmStrLen("Konnichiwa");
   (void)strlen(s0);
+  (void)(sizeof("Hallo") - 2);
 
   return 0;
 }
diff --git a/Utilities/ClangTidyModule/UseCmstrlenCheck.cxx b/Utilities/ClangTidyModule/UseCmstrlenCheck.cxx
index 590d260..d4bae1f 100644
--- a/Utilities/ClangTidyModule/UseCmstrlenCheck.cxx
+++ b/Utilities/ClangTidyModule/UseCmstrlenCheck.cxx
@@ -17,17 +17,61 @@ UseCmstrlenCheck::UseCmstrlenCheck(StringRef Name, ClangTidyContext* Context)
 void UseCmstrlenCheck::registerMatchers(MatchFinder* Finder)
 {
   Finder->addMatcher(callExpr(callee(functionDecl(hasName("::strlen"))),
-                              callee(expr().bind("callee")),
+                              callee(expr().bind("strlen")),
                               hasArgument(0, stringLiteral())),
                      this);
+
+  auto IsSizeOfStringLiteral =
+    unaryExprOrTypeTraitExpr(
+      ofKind(UETT_SizeOf),
+      anyOf(has(parenExpr(has(stringLiteral())).bind("paren")),
+            has(stringLiteral())))
+      .bind("sizeOf");
+  Finder->addMatcher(
+    binaryOperator(
+      hasOperatorName("-"),
+      hasLHS(anyOf(
+        binaryOperator(hasOperatorName("+"), hasRHS(IsSizeOfStringLiteral)),
+        IsSizeOfStringLiteral)),
+      hasRHS(implicitCastExpr(has(integerLiteral(equals(1)).bind("literal")))))
+      .bind("sizeOfMinus"),
+    this);
 }
 
 void UseCmstrlenCheck::check(const MatchFinder::MatchResult& Result)
 {
-  const Expr* Node = Result.Nodes.getNodeAs<Expr>("callee");
+  const Expr* Strlen = Result.Nodes.getNodeAs<Expr>("strlen");
+  const BinaryOperator* SizeOfMinus =
+    Result.Nodes.getNodeAs<BinaryOperator>("sizeOfMinus");
+
+  if (Strlen) {
+    this->diag(Strlen->getBeginLoc(), "use cmStrLen() for string literals")
+      << FixItHint::CreateReplacement(Strlen->getSourceRange(), "cmStrLen");
+  }
+
+  if (SizeOfMinus) {
+    const ParenExpr* Paren = Result.Nodes.getNodeAs<ParenExpr>("paren");
+    const UnaryExprOrTypeTraitExpr* SizeOf =
+      Result.Nodes.getNodeAs<UnaryExprOrTypeTraitExpr>("sizeOf");
+    const IntegerLiteral* Literal =
+      Result.Nodes.getNodeAs<IntegerLiteral>("literal");
 
-  this->diag(Node->getBeginLoc(), "use cmStrLen() for string literals")
-    << FixItHint::CreateReplacement(Node->getSourceRange(), "cmStrLen");
+    std::vector<FixItHint> FixIts;
+    if (Paren) {
+      FixIts.push_back(
+        FixItHint::CreateReplacement(SizeOf->getOperatorLoc(), "cmStrLen"));
+      FixIts.push_back(FixItHint::CreateRemoval(
+        SourceRange(SizeOfMinus->getOperatorLoc(), Literal->getLocation())));
+    } else {
+      FixIts.push_back(
+        FixItHint::CreateReplacement(SizeOf->getOperatorLoc(), "cmStrLen("));
+      FixIts.push_back(FixItHint::CreateReplacement(
+        SourceRange(SizeOfMinus->getOperatorLoc(), Literal->getLocation()),
+        ")"));
+    }
+    this->diag(SizeOf->getOperatorLoc(), "use cmStrLen() for string literals")
+      << FixIts;
+  }
 }
 }
 }