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

diff --git a/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp b/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
index bc152f1..bbf8603 100644 (file)
--- 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 c023521..c2b5e4d 100644 (file)
--- 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 */;{{$}}
 }