[clang-tools-extra] a1968d5 - Prevent abseil-cleanup-ctad check from stomping on surrounding context

[Yitzhak Mandelbaum via cfe-commits]

Author: CJ Johnson
Date: 2021-12-09T17:41:12Z
New Revision: a1968d5341d7b1ec7889955f230d0375548d165b

URL: https://github.com/llvm/llvm-project/commit/a1968d5341d7b1ec7889955f230d0375548d165b
DIFF: https://github.com/llvm/llvm-project/commit/a1968d5341d7b1ec7889955f230d0375548d165b.diff

LOG: Prevent abseil-cleanup-ctad check from stomping on surrounding context

This change applies two fixes to the abseil-cleanup-ctad check. It uses hasSingleDecl() to ensure only declStmt()s with one varDecl() are matched (leaving compount declStmt()s unchanged). It also addresses a bug in the handling of comments that surround the absl::MakeCleanup() calls by switching to the callArgs() combinator from Clang Transformer.

Reviewed By: ymandel

Differential Revision: https://reviews.llvm.org/D115452

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
    clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp b/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
index bc152f1dafa7..bbf860342fee 100644
--- a/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
+++ b/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
@@ -28,16 +28,14 @@ RewriteRule CleanupCtadCheckImpl() {
                              "deduction pattern in C++17 and higher");
 
   return makeRule(
-      declStmt(has(varDecl(
+      declStmt(hasSingleDecl(varDecl(
           hasType(autoType()), hasTypeLoc(typeLoc().bind("auto_type_loc")),
-          hasInitializer(traverse(
-              clang::TK_IgnoreUnlessSpelledInSource,
+          hasInitializer(hasDescendant(
               callExpr(callee(functionDecl(hasName("absl::MakeCleanup"))),
-                       argumentCountIs(1),
-                       hasArgument(0, expr().bind("make_cleanup_argument")))
+                       argumentCountIs(1))
                   .bind("make_cleanup_call")))))),
       {changeTo(node("auto_type_loc"), cat("absl::Cleanup")),
-       changeTo(node("make_cleanup_call"), cat(node("make_cleanup_argument")))},
+       changeTo(node("make_cleanup_call"), cat(callArgs("make_cleanup_call")))},
       warning_message);
 }
 

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp b/clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp
index c023521bb261..c2b5e4df985d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp
@@ -81,35 +81,25 @@ void test() {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup's class template argument deduction pattern in C++17 and higher
   // CHECK-FIXES: {{^}}  absl::Cleanup a = [] {};{{$}}
 
-  // Removes extra parens
-  auto b = absl::MakeCleanup(([] {}));
+  auto b = absl::MakeCleanup(std::function<void()>([] {}));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher
-  // CHECK-FIXES: {{^}}  absl::Cleanup b = [] {};{{$}}
+  // CHECK-FIXES: {{^}}  absl::Cleanup b = std::function<void()>([] {});{{$}}
 
-  auto c = absl::MakeCleanup(std::function<void()>([] {}));
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher
-  // CHECK-FIXES: {{^}}  absl::Cleanup c = std::function<void()>([] {});{{$}}
-
-  // Removes extra parens
-  auto d = absl::MakeCleanup((std::function<void()>([] {})));
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher
-  // CHECK-FIXES: {{^}}  absl::Cleanup d = std::function<void()>([] {});{{$}}
-
-  const auto e = absl::MakeCleanup([] {});
+  const auto c = absl::MakeCleanup([] {});
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher
-  // CHECK-FIXES: {{^}}  const absl::Cleanup e = [] {};{{$}}
+  // CHECK-FIXES: {{^}}  const absl::Cleanup c = [] {};{{$}}
 
-  // Removes extra parens
-  const auto f = absl::MakeCleanup(([] {}));
+  const auto d = absl::MakeCleanup(std::function<void()>([] {}));
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher
-  // CHECK-FIXES: {{^}}  const absl::Cleanup f = [] {};{{$}}
+  // CHECK-FIXES: {{^}}  const absl::Cleanup d = std::function<void()>([] {});{{$}}
 
-  const auto g = absl::MakeCleanup(std::function<void()>([] {}));
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher
-  // CHECK-FIXES: {{^}}  const absl::Cleanup g = std::function<void()>([] {});{{$}}
+  // Preserves extra parens
+  auto e = absl::MakeCleanup(([] {}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher
+  // CHECK-FIXES: {{^}}  absl::Cleanup e = ([] {});{{$}}
 
-  // Removes extra parens
-  const auto h = absl::MakeCleanup((std::function<void()>([] {})));
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher
-  // CHECK-FIXES: {{^}}  const absl::Cleanup h = std::function<void()>([] {});{{$}}
+  // Preserves comments
+  auto f = /* a */ absl::MakeCleanup(/* b */ [] { /* c */ } /* d */) /* e */;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher
+  // CHECK-FIXES: {{^}}  absl::Cleanup f = /* a */ /* b */ [] { /* c */ } /* d */ /* e */;{{$}}
 }