[clang-tidy] Simplify diagnostics for UniqueptrResetRelease check

[njames93]

Summary

Tweak the diagnostics to create small replacements rather than grabbing source text from the lexer.
Also simplified the diagnostic message.

[aaron.ballman]

LGTM aside from a nit with the diagnostic wording.

clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp

115

clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp

 
 namespace clang {
 namespace tidy {
 namespace misc {
 
 void UniqueptrResetReleaseCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       cxxMemberCallExpr(
-          on(expr().bind("left")), callee(memberExpr().bind("reset_member")),
-          callee(
-              cxxMethodDecl(hasName("reset"),
-                            ofClass(cxxRecordDecl(hasName("::std::unique_ptr"),
-                                                  decl().bind("left_class"))))),
-          has(ignoringParenImpCasts(cxxMemberCallExpr(
+          callee(memberExpr(
+                     member(cxxMethodDecl(
+                         hasName("reset"),
+                                               decl().bind("left_class"))))))
+                     .bind("reset_member")),
+          hasArgument(
+              0, ignoringParenImpCasts(cxxMemberCallExpr(
                      on(expr().bind("right")),
-              callee(memberExpr().bind("release_member")),
-              callee(cxxMethodDecl(
+                     callee(memberExpr(member(cxxMethodDecl(
                                            hasName("release"),
-                  ofClass(cxxRecordDecl(hasName("::std::unique_ptr"),
-                                        decl().bind("right_class")))))))))
+                                           ofClass(cxxRecordDecl(
+                                               hasName("::std::unique_ptr"),
+                                               decl().bind("right_class"))))))
+                                .bind("release_member"))))))
           .bind("reset_call"),
       this);
 }
 
 namespace {
 const Type *getDeleterForUniquePtr(const MatchFinder::MatchResult &Result,
                                    StringRef ID) {
   const auto *Class =

 void UniqueptrResetReleaseCheck::check(const MatchFinder::MatchResult &Result) {
   if (!areDeletersCompatible(Result))
     return;
 
   const auto *ResetMember = Result.Nodes.getNodeAs<MemberExpr>("reset_member");
   const auto *ReleaseMember =
       Result.Nodes.getNodeAs<MemberExpr>("release_member");
   const auto *Right = Result.Nodes.getNodeAs<Expr>("right");
-  const auto *Left = Result.Nodes.getNodeAs<Expr>("left");
   const auto *ResetCall =
       Result.Nodes.getNodeAs<CXXMemberCallExpr>("reset_call");
 
-  std::string LeftText = std::string(clang::Lexer::getSourceText(
-      CharSourceRange::getTokenRange(Left->getSourceRange()),
-      *Result.SourceManager, getLangOpts()));
-  std::string RightText = std::string(clang::Lexer::getSourceText(
-      CharSourceRange::getTokenRange(Right->getSourceRange()),
-      *Result.SourceManager, getLangOpts()));
+  StringRef AssignmentText = " = ";
+  StringRef TrailingText = "";
+  if (ReleaseMember->isArrow()) {
+    AssignmentText = " = std::move(*";
+    TrailingText = ")";
+  } else if (!Right->isRValue()) {
+    AssignmentText = " = std::move(";
+    TrailingText = ")";
+  }
 
+  auto D = diag(ResetMember->getExprLoc(),
+                "prefer 'unique_ptr<>' assignment over 'release' and 'reset'");
   if (ResetMember->isArrow())
-    LeftText = "*" + LeftText;
-  if (ReleaseMember->isArrow())
-    RightText = "*" + RightText;
-  bool IsMove = false;
-  // Even if x was rvalue, *x is not rvalue anymore.
-  if (!Right->isRValue() || ReleaseMember->isArrow()) {
-    RightText = "std::move(" + RightText + ")";
-    IsMove = true;
-  }
-
-  std::string NewText = LeftText + " = " + RightText;
-
-  diag(ResetMember->getExprLoc(),
-       "prefer ptr = %select{std::move(ptr2)|ReturnUnique()}0 over "
-       "ptr.reset(%select{ptr2|ReturnUnique()}0.release())")
-      << !IsMove
-      << FixItHint::CreateReplacement(
-             CharSourceRange::getTokenRange(ResetCall->getSourceRange()),
-             NewText);
+    D << FixItHint::CreateInsertion(ResetMember->getBeginLoc(), "*");
+  D << FixItHint::CreateReplacement(
+           CharSourceRange::getCharRange(ResetMember->getOperatorLoc(),
+                                         Right->getBeginLoc()),
+           AssignmentText)
+           CharSourceRange::getTokenRange(ReleaseMember->getOperatorLoc(),
+                                          ResetCall->getEndLoc()),
+           TrailingText);
 }
 
 } // namespace misc
 } // namespace tidy
 } // namespace clang

clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp

 
 void f() {
   std::unique_ptr<Foo> a, b;
   std::unique_ptr<Bar> c;
   std::unique_ptr<Foo> *x = &a;
   std::unique_ptr<Foo> *y = &b;
 
   a.reset(b.release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr = std::move(ptr2) over ptr.reset(ptr2.release()) [misc-uniqueptr-reset-release]
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer 'unique_ptr<>' assignment over 'release' and 'reset' [misc-uniqueptr-reset-release]
   // CHECK-FIXES: a = std::move(b);
   a.reset(c.release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr = std::move(ptr2)
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer 'unique_ptr<>' assignment
   // CHECK-FIXES: a = std::move(c);
   a.reset(Create().release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr = ReturnUnique() over ptr.reset(ReturnUnique().release()) [misc-uniqueptr-reset-release]
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer 'unique_ptr<>' assignment
   // CHECK-FIXES: a = Create();
   x->reset(y->release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: prefer ptr = std::move(ptr2)
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: prefer 'unique_ptr<>' assignment
   // CHECK-FIXES: *x = std::move(*y);
   Look().reset(Look().release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr = std::move(ptr2)
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer 'unique_ptr<>' assignment
   // CHECK-FIXES: Look() = std::move(Look());
   Get()->reset(Get()->release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr = std::move(ptr2)
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer 'unique_ptr<>' assignment
   // CHECK-FIXES: *Get() = std::move(*Get());
 
   std::unique_ptr<Bar, FooFunc> func_a, func_b;
   func_a.reset(func_b.release());
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr = std::move(ptr2)
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer 'unique_ptr<>' assignment
   // CHECK-FIXES: func_a = std::move(func_b);
 }
 
 void negatives() {
   std::unique_ptr<Foo> src;
   struct OtherDeleter {};
   std::unique_ptr<Foo, OtherDeleter> dest;
   dest.reset(src.release());
 
   std::unique_ptr<Bar, FooFunc> func_a;
   std::unique_ptr<Bar, BarFunc> func_b;
   func_a.reset(func_b.release());
 }