clang-tidy module: look for sizeof string literal in cmStrLen() check

4 files changed, 90 insertions, 4 deletions

diff --git a/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-fixit.cxx b/Utilities/ClangTidyModule/Tests/cmake-use-cmstrlen-fixit.cxx
index c93d5576fb..cde00864f1 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 6c52ad5af9..d18822a4f4 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 f36262b5ea..205bc9cc85 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 590d260b4e..d4bae1f794 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;
+  }
 }
 }
 }