Added check uniqueptr-delete-release to replace "delete x.release()" with "x = nullptr"

[alexfh]

Nice check! One comment (quite usual ;).

test/clang-tidy/readability-uniqueptr-delete-release.cpp

25

I'd anchor the pattern to the start of line to verify correct replacement of "delete ".

36

Please add tests for code in templates with multiple instantiations and for code in macros. I suspect these cases need special treatment, which is not currently done.

[sbenza]

added more test cases and fixed the check for them.

[aaron.ballman]

clang-tidy/readability/UniqueptrDeleteReleaseCheck.h

20

Is there a reason for = nullptr instead of .reset(); ?

[sbenza]

clang-tidy/readability/UniqueptrDeleteReleaseCheck.h

20

Preference of the author and initial reviewer?
They are equivalent.

[aaron.ballman]

LGTM!

clang-tidy/readability/UniqueptrDeleteReleaseCheck.h

20

While equivalent, one may be considered more readable than another (in either direction!). For me, personally, I prefer seeing .reset() because it's a reminder to me that this object is also being destroyed. I don't have overly strong opinions though, and don't think it's worth some sort of user-configurability.

clang-tidy/readability/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyReadabilityModule
   BracesAroundStatementsCheck.cpp
   ContainerSizeEmptyCheck.cpp
   ElseAfterReturnCheck.cpp
   FunctionSizeCheck.cpp
   IdentifierNamingCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantStringCStrCheck.cpp
   RedundantSmartptrGetCheck.cpp
   SimplifyBooleanExprCheck.cpp
+  UniqueptrDeleteReleaseCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTooling
   )

clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "ElseAfterReturnCheck.h"
 #include "FunctionSizeCheck.h"
 #include "IdentifierNamingCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
 #include "NamedParameterCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "SimplifyBooleanExprCheck.h"
+#include "UniqueptrDeleteReleaseCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
 class ReadabilityModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<BracesAroundStatementsCheck>(
         "readability-braces-around-statements");
     CheckFactories.registerCheck<ContainerSizeEmptyCheck>(
         "readability-container-size-empty");
     CheckFactories.registerCheck<ElseAfterReturnCheck>(
         "readability-else-after-return");
     CheckFactories.registerCheck<FunctionSizeCheck>(
         "readability-function-size");
     CheckFactories.registerCheck<IdentifierNamingCheck>(
         "readability-identifier-naming");
     CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
         "readability-inconsistent-declaration-parameter-name");
+    CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
+        "readability-uniqueptr-delete-release");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "readability-named-parameter");
     CheckFactories.registerCheck<RedundantSmartptrGetCheck>(
         "readability-redundant-smartptr-get");
     CheckFactories.registerCheck<RedundantStringCStrCheck>(
         "readability-redundant-string-cstr");
     CheckFactories.registerCheck<SimplifyBooleanExprCheck>(
         "readability-simplify-boolean-expr");

clang-tidy/readability/UniqueptrDeleteReleaseCheck.h

+//===--- UniqueptrDeleteReleaseCheck.h - clang-tidy--------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UNIQUEPTR_DELETE_RELEASE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UNIQUEPTR_DELETE_RELEASE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// Flag statements of the form: delete <unique_ptr expr>.release()
+/// and replace them with: <unique_ptr expr> = nullptr
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-uniqueptr-delete-release.html
+class UniqueptrDeleteReleaseCheck : public ClangTidyCheck {
+public:
+  UniqueptrDeleteReleaseCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UNIQUEPTR_DELETE_RELEASE_H
+

clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp

+//===--- UniqueptrDeleteReleaseCheck.cpp - clang-tidy----------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UniqueptrDeleteReleaseCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+void UniqueptrDeleteReleaseCheck::registerMatchers(MatchFinder *Finder) {
+  auto UniquePtrWithDefaultDelete = classTemplateSpecializationDecl(
+      hasName("std::unique_ptr"),
+      hasTemplateArgument(1, refersToType(qualType(hasDeclaration(cxxRecordDecl(
+                                 hasName("std::default_delete")))))));
+  Finder->addMatcher(
+      cxxDeleteExpr(
+          has(cxxMemberCallExpr(
+              on(expr(hasType(UniquePtrWithDefaultDelete)).bind("uptr")),
+              callee(cxxMethodDecl(hasName("release"))))))
+          .bind("delete"),
+      this);
+}
+
+void UniqueptrDeleteReleaseCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *PtrExpr = Result.Nodes.getNodeAs<Expr>("uptr");
+  const auto *DeleteExpr = Result.Nodes.getNodeAs<Expr>("delete");
+
+  SourceLocation AfterPtr =
+      Lexer::getLocForEndOfToken(PtrExpr->getLocEnd(), 0, *Result.SourceManager,
+                                 Result.Context->getLangOpts());
+
+  diag(DeleteExpr->getLocStart(),
+       "prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> "
+       "objects")
+      << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+             DeleteExpr->getLocStart(), PtrExpr->getLocStart()))
+      << FixItHint::CreateReplacement(
+             CharSourceRange::getTokenRange(AfterPtr, DeleteExpr->getLocEnd()),
+             " = nullptr");
+}
+
+} // namespace tidy
+} // namespace clang
+

docs/clang-tidy/checks/list.rst

.. toctree::
    readability-else-after-return
    readability-function-size
    readability-identifier-naming
    readability-inconsistent-declaration-parameter-name
    readability-named-parameter
    readability-redundant-smartptr-get
    readability-redundant-string-cstr
    readability-simplify-boolean-expr
+   readability-uniqueptr-delete-release

docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst

+readability-uniqueptr-delete-release
+====================================
+
+Replace ``delete <unique_ptr>.release()`` with ``<unique_ptr> = nullptr``.
+The latter is shorter, simpler and does not require use of raw pointer APIs.

test/clang-tidy/readability-uniqueptr-delete-release.cpp

+// RUN: %python %S/check_clang_tidy.py %s readability-uniqueptr-delete-release %t
+
+namespace std {
+template <typename T>
+struct default_delete {};
+
+template <typename T, typename D = default_delete<T>>
+class unique_ptr {
+ public:
+  unique_ptr();
+  ~unique_ptr();
+  explicit unique_ptr(T*);
+  template <typename U, typename E>
+  unique_ptr(unique_ptr<U, E>&&);
+  T* release();
+};
+}  // namespace std
+
+std::unique_ptr<int>& ReturnsAUnique();
+
+void Positives() {
+  std::unique_ptr<int> P;
+  delete P.release();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> objects [readability-uniqueptr-delete-release]
+  // CHECK-FIXES: P = nullptr;
+
+  std::unique_ptr<int> Array[20];
+  delete Array[4].release();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete
+  // CHECK-FIXES: Array[4] = nullptr;
+
+  delete ReturnsAUnique().release();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete
+  // CHECK-FIXES: ReturnsAUnique() = nullptr;
+}
+
+struct NotDefaultDeleter {};
+
+struct NotUniquePtr {
+  int* release();
+};
+
+void Negatives() {
+  std::unique_ptr<int, NotDefaultDeleter> P;
+  delete P.release();
+
+  NotUniquePtr P2;
+  delete P2.release();
+}
+