[clang-tidy] Add new cert-dcl21-cpp check

This check flags postfix operator++/-- declarations,
where the return type is not a const object.

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

llvm-svn: 302637

clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp

@@ -20,6 +20,7 @@
 #include "DontModifyStdNamespaceCheck.h"
 #include "FloatLoopCounter.h"
 #include "LimitedRandomnessCheck.h"
+#include "PostfixOperatorCheck.h"
 #include "SetLongJmpCheck.h"
 #include "StaticObjectExceptionCheck.h"
 #include "StrToNumCheck.h"
@@ -35,6 +36,8 @@ public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     // C++ checkers
     // DCL
+    CheckFactories.registerCheck<PostfixOperatorCheck>(
+        "cert-dcl21-cpp");
     CheckFactories.registerCheck<VariadicFunctionDefCheck>("cert-dcl50-cpp");
     CheckFactories.registerCheck<misc::NewDeleteOverloadsCheck>(
         "cert-dcl54-cpp");

clang-tools-extra/clang-tidy/cert/CMakeLists.txt

@@ -6,6 +6,7 @@ add_clang_library(clangTidyCERTModule
   DontModifyStdNamespaceCheck.cpp
   FloatLoopCounter.cpp
   LimitedRandomnessCheck.cpp
+  PostfixOperatorCheck.cpp
   SetLongJmpCheck.cpp
   StaticObjectExceptionCheck.cpp
   StrToNumCheck.cpp

clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.cpp

@@ -0,0 +1,88 @@
+//===--- PostfixOperatorCheck.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 "PostfixOperatorCheck.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 {
+namespace cert {
+
+void PostfixOperatorCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  Finder->addMatcher(functionDecl(anyOf(hasOverloadedOperatorName("++"),
+                                        hasOverloadedOperatorName("--")))
+                         .bind("decl"),
+                     this);
+}
+
+void PostfixOperatorCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("decl");
+
+  bool HasThis = false;
+  if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl))
+    HasThis = MethodDecl->isInstance();
+
+  // Check if the operator is a postfix one.
+  if (FuncDecl->getNumParams() != (HasThis ? 1 : 2))
+    return;
+
+  SourceRange ReturnRange = FuncDecl->getReturnTypeSourceRange();
+  SourceLocation Location = ReturnRange.getBegin();
+  if (!Location.isValid())
+    return;
+
+  QualType ReturnType = FuncDecl->getReturnType();
+
+  // Warn when the operators return a reference.
+  if (const auto *RefType = ReturnType->getAs<ReferenceType>()) {
+    auto Diag = diag(Location, "overloaded %0 returns a reference instead of a "
+                               "constant object type")
+                << FuncDecl;
+
+    if (Location.isMacroID() || ReturnType->getAs<TypedefType>() ||
+        RefType->getPointeeTypeAsWritten()->getAs<TypedefType>())
+      return;
+
+    QualType ReplaceType =
+        ReturnType.getNonReferenceType().getLocalUnqualifiedType();
+    // The getReturnTypeSourceRange omits the qualifiers. We do not want to
+    // duplicate the const.
+    if (!ReturnType->getPointeeType().isConstQualified())
+      ReplaceType.addConst();
+
+    Diag << FixItHint::CreateReplacement(
+        ReturnRange,
+        ReplaceType.getAsString(Result.Context->getPrintingPolicy()) + " ");
+
+    return;
+  }
+
+  if (ReturnType.isConstQualified() || ReturnType->isBuiltinType() ||
+      ReturnType->isPointerType())
+    return;
+
+  auto Diag =
+      diag(Location, "overloaded %0 returns a non-constant object instead of a "
+                     "constant object type")
+      << FuncDecl;
+
+  if (!Location.isMacroID() && !ReturnType->getAs<TypedefType>())
+    Diag << FixItHint::CreateInsertion(Location, "const ");
+}
+
+} // namespace cert
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/cert/PostfixOperatorCheck.h

@@ -0,0 +1,36 @@
+//===--- PostfixOperatorCheck.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_CERT_POSTFIX_OPERATOR_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_POSTFIX_OPERATOR_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+/// Checks if the overloaded postfix ++ and -- operator return a constant
+/// object.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cert-postfix-operator.html
+class PostfixOperatorCheck : public ClangTidyCheck {
+public:
+  PostfixOperatorCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cert
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_POSTFIX_OPERATOR_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -57,6 +57,11 @@ The improvements are...
 Improvements to clang-tidy
 --------------------------
 
+- New `cert-dcl21-cpp
+  <http://clang.llvm.org/extra/clang-tidy/checks/cert-dcl21-cpp.html>`_ check
+
+  Checks if the overloaded postfix ``operator++/--`` returns a constant object.
+
 - New `cert-dcl58-cpp
   <http://clang.llvm.org/extra/clang-tidy/checks/cert-dcl58-cpp.html>`_ check
 

clang-tools-extra/docs/clang-tidy/checks/cert-dcl21-cpp.rst

@@ -0,0 +1,12 @@
+.. title:: clang-tidy - cert-dcl21-cpp
+
+cert-dcl21-cpp
+==============
+
+This check flags postfix ``operator++`` and ``operator--`` declarations
+if the return type is not a const object. This also warns if the return type
+is a reference type.
+
+This check corresponds to the CERT C++ Coding Standard recommendation
+`DCL21-CPP. Overloaded postfix increment and decrement operators should return a const object
+<https://www.securecoding.cert.org/confluence/display/cplusplus/DCL21-CPP.+Overloaded+postfix+increment+and+decrement+operators+should+return+a+const+object>`_.

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -6,6 +6,7 @@ Clang-Tidy Checks
 .. toctree::
    boost-use-to-string
    cert-dcl03-c (redirects to misc-static-assert) <cert-dcl03-c>
+   cert-dcl21-cpp
    cert-dcl50-cpp
    cert-dcl54-cpp (redirects to misc-new-delete-overloads) <cert-dcl54-cpp>
    cert-dcl58-cpp

clang-tools-extra/test/clang-tidy/cert-dcl21-cpp.cpp

@@ -0,0 +1,134 @@
+// RUN: %check_clang_tidy %s cert-dcl21-cpp %t
+
+class A {};
+
+A operator++(A &, int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator++' returns a non-constant object instead of a constant object type [cert-dcl21-cpp]
+// CHECK-FIXES: {{^}}const A operator++(A &, int);
+
+A operator--(A &, int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator--' returns a no
+// CHECK-FIXES: {{^}}const A operator--(A &, int);
+
+class B {};
+
+B &operator++(B &);
+const B operator++(B &, int);
+
+B &operator--(B &);
+const B operator--(B &, int);
+
+
+class D {
+D &operator++();
+const D operator++(int);
+
+D &operator--();
+const D operator--(int);
+};
+
+class C {
+C operator++(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator++' returns a no
+// CHECK-FIXES: {{^}}const C operator++(int);
+
+C operator--(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator--' returns a no
+// CHECK-FIXES: {{^}}const C operator--(int);
+};
+
+class E {};
+
+E &operator++(E &, int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator++' returns a reference instead of a constant object type [cert-dcl21-cpp]
+// CHECK-FIXES: {{^}}const E operator++(E &, int);
+
+E &operator--(E &, int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator--' returns a re
+// CHECK-FIXES: {{^}}const E operator--(E &, int);
+
+class G {
+G &operator++(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator++' returns a re
+// CHECK-FIXES: {{^}}const G operator++(int);
+
+G &operator--(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator--' returns a re
+// CHECK-FIXES: {{^}}const G operator--(int);
+};
+
+class F {};
+
+const F &operator++(F &, int);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator++' returns a re
+// CHECK-FIXES: {{^}}const F operator++(F &, int);
+
+const F &operator--(F &, int);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator--' returns a re
+// CHECK-FIXES: {{^}}const F operator--(F &, int);
+
+class H {
+const H &operator++(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator++' returns a re
+// CHECK-FIXES: {{^}}const H operator++(int);
+
+const H &operator--(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator--' returns a re
+// CHECK-FIXES: {{^}}const H operator--(int);
+};
+
+
+#define FROM_MACRO P&
+class P {
+const FROM_MACRO operator++(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator++' returns a re
+// CHECK-FIXES: {{^}}const FROM_MACRO operator++(int);
+};
+
+
+template<typename T>
+class Q {
+const Q &operator++(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator++' returns a re
+// CHECK-FIXES: {{^}}const Q<T> operator++(int);
+
+const Q &operator--(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: overloaded 'operator--' returns a re
+// CHECK-FIXES: {{^}}const Q<T> operator--(int);
+};
+
+void foobar() {
+  Q<int> a;
+  Q<float> b;
+  (void)a;
+  (void)b;
+}
+
+struct S {};
+typedef S& SRef;
+
+SRef operator++(SRef, int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: overloaded 'operator++' returns a re
+// CHECK-FIXES: {{^}}SRef operator++(SRef, int);
+
+struct T {
+  typedef T& TRef;
+  
+  TRef operator++(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: overloaded 'operator++' returns a re
+// CHECK-FIXES: {{^}}  TRef operator++(int);
+};
+
+struct U {
+  typedef const U& ConstURef;
+  
+  ConstURef& operator++(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: overloaded 'operator++' returns a re
+// CHECK-FIXES: {{^}}  ConstURef& operator++(int);
+};
+
+struct V {
+  V *operator++(int);
+  V *const operator--(int);
+};
+