[clang-tools-extra] [clang-tidy] Fixes to readability-implicit-bool-conversion (PR #72050)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

<details>
<summary>Changes</summary>

- Fixed issue with invalid code being generated when static_cast is put into fix, and no space were added before it.
- Fixed issue with duplicate parentheses being added when double implicit cast is used.

Closes #<!-- -->71848 

---
Full diff: https://github.com/llvm/llvm-project/pull/72050.diff


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp (+18-4) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp (+9) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
index 69e6d73c4fcd7bb..5e71b2fc81de7f0 100644
--- a/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -173,16 +173,30 @@ StringRef getEquivalentBoolLiteralForExpr(const Expr *Expression,
   return {};
 }
 
+bool needsSpacePrefix(SourceLocation Loc, ASTContext &Context) {
+  SourceRange PrefixRange(Loc.getLocWithOffset(-1), Loc);
+  StringRef SpaceBeforeStmtStr = Lexer::getSourceText(
+      CharSourceRange::getCharRange(PrefixRange), Context.getSourceManager(),
+      Context.getLangOpts(), nullptr);
+  if (SpaceBeforeStmtStr.empty())
+    return true;
+
+  const StringRef AllowedCharacters(" \t\n\v\f\r(){}[]<>;,+=-|&~!^*/");
+  return SpaceBeforeStmtStr.rtrim(AllowedCharacters).size() ==
+         SpaceBeforeStmtStr.size();
+}
+
 void fixGenericExprCastFromBool(DiagnosticBuilder &Diag,
                                 const ImplicitCastExpr *Cast,
                                 ASTContext &Context, StringRef OtherType) {
   const Expr *SubExpr = Cast->getSubExpr();
-  bool NeedParens = !isa<ParenExpr>(SubExpr);
+  const bool NeedParens = !isa<ParenExpr>(SubExpr->IgnoreImplicit());
+  const bool NeedSpace = needsSpacePrefix(Cast->getBeginLoc(), Context);
 
   Diag << FixItHint::CreateInsertion(
-      Cast->getBeginLoc(),
-      (Twine("static_cast<") + OtherType + ">" + (NeedParens ? "(" : ""))
-          .str());
+      Cast->getBeginLoc(), (Twine() + (NeedSpace ? " " : "") + "static_cast<" +
+                            OtherType + ">" + (NeedParens ? "(" : ""))
+                               .str());
 
   if (NeedParens) {
     SourceLocation EndLoc = Lexer::getLocForEndOfToken(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index f49c412118e7d98..e0353e4344e9b6a 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -410,7 +410,8 @@ Changes in existing checks
 - Improved :doc:`readability-implicit-bool-conversion
   <clang-tidy/checks/readability/implicit-bool-conversion>` check to take
   do-while loops into account for the `AllowIntegerConditions` and
-  `AllowPointerConditions` options.
+  `AllowPointerConditions` options. Additionally, an issue with auto-fix
+  suggestions generating invalid code in certain scenarios has been resolved.
 
 - Improved :doc:`readability-non-const-parameter
   <clang-tidy/checks/readability/non-const-parameter>` check to ignore
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp
index 323cf813c047000..3929984204905d3 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion.cpp
@@ -478,3 +478,12 @@ namespace PR47000 {
   using IntType = int;
   int to_int2(bool x) { return IntType{x}; }
 }
+
+namespace PR71848 {
+  int fun() {
+    bool foo = false;
+    return( foo );
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicit conversion bool -> 'int' [readability-implicit-bool-conversion]
+// CHECK-FIXES: return static_cast<int>( foo );
+  }
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/72050