[clang-tidy] Adding a new modernize use nodiscard checker

Summary: Adds a checker to clang-tidy to warn when a non void const member function, taking only parameters passed by value or const reference could be marked as '[[nodiscard]]'

Patch by MyDeveloperDay.

Reviewers: alexfh, stephenkelly, curdeius, aaron.ballman, hokein, JonasToth

Reviewed By: curdeius, JonasToth

Subscribers: Eugene.Zelenko, lefticus, lebedev.ri, mgorny, xazax.hun, cfe-commits

Tags: #clang-tools-extra

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

7 files changed, 370 insertions, 0 deletions

diff --git a/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-clang-unused.cpp b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-clang-unused.cpp
new file mode 100644
index 00000000000..0951e6118d4
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-clang-unused.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: '[[clang::warn_unused_result]]'}]}" \
+// RUN: -- -std=c++11 
+
+class Foo
+{
+public:
+    bool f1() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked {{\[\[clang::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[clang::warn_unused_result\]\]}} bool f1() const;
+
+    bool f2(int) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked {{\[\[clang::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[clang::warn_unused_result\]\]}} bool f2(int) const;
+
+    bool f3(const int &) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked {{\[\[clang::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[clang::warn_unused_result\]\]}} bool f3(const int &) const;
+
+    bool f4(void) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked {{\[\[clang::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[clang::warn_unused_result\]\]}} bool f4(void) const;
+
+};
+
diff --git a/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-cxx11.cpp b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-cxx11.cpp
new file mode 100644
index 00000000000..1a72bf4936a
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-cxx11.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: '__attribute__((warn_unused_result))'}]}" \
+// RUN: -- -std=c++11 \
+
+class Foo
+{
+public:
+    bool f1() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked __attribute__((warn_unused_result)) [modernize-use-nodiscard]
+    // CHECK-FIXES: __attribute__((warn_unused_result)) bool f1() const;
+
+    bool f2(int) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked __attribute__((warn_unused_result)) [modernize-use-nodiscard]
+    // CHECK-FIXES: __attribute__((warn_unused_result)) bool f2(int) const;
+
+    bool f3(const int &) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked __attribute__((warn_unused_result)) [modernize-use-nodiscard]
+    // CHECK-FIXES: __attribute__((warn_unused_result)) bool f3(const int &) const;
+
+    bool f4(void) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked __attribute__((warn_unused_result)) [modernize-use-nodiscard]
+    // CHECK-FIXES: __attribute__((warn_unused_result)) bool f4(void) const;
+};
+
diff --git a/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-gcc-unused.cpp b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-gcc-unused.cpp
new file mode 100644
index 00000000000..54b808ed726
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-gcc-unused.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: '[[gcc::warn_unused_result]]'}]}" \
+// RUN: -- -std=c++11 
+
+class Foo
+{
+public:
+    bool f1() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked {{\[\[gcc::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[gcc::warn_unused_result\]\]}} bool f1() const;
+
+    bool f2(int) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked {{\[\[gcc::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[gcc::warn_unused_result\]\]}} bool f2(int) const;
+
+    bool f3(const int &) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked {{\[\[gcc::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[gcc::warn_unused_result\]\]}} bool f3(const int &) const;
+
+    bool f4(void) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked {{\[\[gcc::warn_unused_result\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[gcc::warn_unused_result\]\]}} bool f4(void) const;
+
+};
+
diff --git a/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-no-macro-inscope-cxx11.cpp b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-no-macro-inscope-cxx11.cpp
new file mode 100644
index 00000000000..0564a133603
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-no-macro-inscope-cxx11.cpp
@@ -0,0 +1,13 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: 'CUSTOM_NO_DISCARD'}]}" -- -std=c++11 
+
+// As if the macro was not defined.
+// #define CUSTOM_NO_DISCARD __attribute_((warn_unused_result))
+
+class Foo
+{
+public:
+    bool f1() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked CUSTOM_NO_DISCARD [modernize-use-nodiscard]
+};
+
diff --git a/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-no-macro.cpp b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-no-macro.cpp
new file mode 100644
index 00000000000..7898b6ede34
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard-no-macro.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- -- -std=c++17 
+
+class Foo
+{
+public:
+    bool f1() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked {{\[\[nodiscard\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[nodiscard\]\]}} bool f1() const;
+
+    bool f2(int) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked {{\[\[nodiscard\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[nodiscard\]\]}} bool f2(int) const;
+
+    bool f3(const int &) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked {{\[\[nodiscard\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[nodiscard\]\]}} bool f3(const int &) const;
+
+    bool f4(void) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked {{\[\[nodiscard\]\]}} [modernize-use-nodiscard]
+    // CHECK-FIXES: {{\[\[nodiscard\]\]}} bool f4(void) const;
+
+};
diff --git a/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard.cpp b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard.cpp
new file mode 100644
index 00000000000..f4408320154
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard.cpp
@@ -0,0 +1,256 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: 'NO_DISCARD'}]}" \
+// RUN: -- -std=c++17
+
+#include <functional>
+
+namespace boost {
+template <class>
+class function;
+}
+
+#include "modernize-use-nodiscard.h"
+
+#define BOOLEAN_FUNC bool f23() const
+
+typedef unsigned my_unsigned;
+typedef unsigned &my_unsigned_reference;
+typedef const unsigned &my_unsigned_const_reference;
+
+class Foo {
+public:
+    using size_type = unsigned;
+
+    bool f1() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f1() const;
+
+    bool f2(int) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f2(int) const;
+
+    bool f3(const int &) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f3(const int &) const;
+
+    bool f4(void) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f4(void) const;
+    
+    // negative tests
+
+    void f5() const;
+    
+    bool f6();
+    
+    bool f7(int &);
+    
+    bool f8(int &) const;
+    
+    bool f9(int *) const;
+    
+    bool f10(const int &, int &) const;
+    
+    NO_DISCARD bool f12() const;
+    
+    MUST_USE_RESULT bool f13() const;
+    
+    [[nodiscard]] bool f11() const;
+    
+    [[clang::warn_unused_result]] bool f11a() const;
+    
+    [[gnu::warn_unused_result]] bool f11b() const;
+
+    bool _f20() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function '_f20' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool _f20() const;
+    
+    NO_RETURN bool f21() const;
+    
+    ~Foo();
+    
+    bool operator+=(int) const;
+    
+    // extra keywords (virtual,inline,const) on return type
+    
+    virtual bool f14() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f14' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD virtual bool f14() const;
+    
+    const bool f15() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f15' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD const bool f15() const;
+    
+    inline const bool f16() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f16' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD inline const bool f16() const;
+
+    inline const std::string &f45() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f45' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD inline const std::string &f45() const;
+
+    inline virtual const bool f17() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f17' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD inline virtual const bool f17() const;
+
+    // inline with body
+    bool f18() const 
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f18' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f18() const 
+    {
+     return true;
+    }
+
+    bool f19() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f19' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f19() const;
+
+    BOOLEAN_FUNC;
+    
+    bool f24(size_type) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f24' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f24(size_type) const;
+    
+    bool f28(my_unsigned) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f28' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f28(my_unsigned) const;
+
+    bool f29(my_unsigned_reference) const;
+
+    bool f30(my_unsigned_const_reference) const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f30' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool f30(my_unsigned_const_reference) const;
+
+    template <class F>
+    F f37(F a, F b) const;
+
+    template <class F>
+    bool f38(F a) const;
+
+    bool f39(const std::function<bool()> &predicate) const;
+
+    bool f39a(std::function<bool()> predicate) const;
+
+    bool f39b(const std::function<bool()> predicate) const;
+
+    bool f45(const boost::function<bool()> &predicate) const;
+
+    bool f45a(boost::function<bool()> predicate) const;
+
+    bool f45b(const boost::function<bool()> predicate) const;
+
+    // Do not add ``[[nodiscard]]`` to parameter packs.
+    template <class... Args>
+    bool ParameterPack(Args... args) const;
+
+    template <typename... Targs>
+    bool ParameterPack2(Targs... Fargs) const;
+
+    // Do not add ``[[nodiscard]]`` to variadic functions.
+    bool VariadicFunctionTest(const int &, ...) const;
+
+    // Do not add ``[[nodiscard]]`` to non constant static functions.
+    static bool not_empty();
+
+    // Do not add ``[[nodiscard]]`` to conversion functions.
+    // explicit operator bool() const { return true; }
+};
+
+// Do not add ``[[nodiscard]]`` to Lambda.
+const auto nonConstReferenceType = [] {
+  return true;
+};
+
+auto lambda1 = [](int a, int b) { return a < b; };
+auto lambda1a = [](int a) { return a; };
+auto lambda1b = []()  { return true;};
+
+auto get_functor = [](bool check) {
+    return  [&](const std::string& sr)->std::string {
+        if(check){
+            return "some string";
+        }
+        return "another string";
+    };
+};
+
+// Do not add ``[[nodiscard]]`` to function definition.
+bool Foo::f19() const {
+  return true;
+}
+
+template <class T>
+class Bar {
+public:
+    using value_type = T;
+    using reference = value_type &;
+    using const_reference = const value_type &;
+
+    // Do not add ``[[nodiscard]]`` to non explicit conversion functions.
+    operator bool() const { return true; }
+
+    bool empty() const;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'empty' should be marked NO_DISCARD [modernize-use-nodiscard]
+    // CHECK-FIXES: NO_DISCARD bool empty() const;
+
+    // we cannot assume that the template parameter isn't a pointer
+    bool f25(value_type) const;
+
+    bool f27(reference) const;
+
+    typename T::value_type f35() const;
+
+    T f34() const;
+
+    bool f31(T) const;
+
+    bool f33(T &) const;
+
+    bool f26(const_reference) const;
+
+    bool f32(const T &) const;
+};
+
+template <typename _Tp, int cn>
+class Vec {
+public:
+    Vec(_Tp v0, _Tp v1); //!< 2-element vector constructor
+
+    Vec cross(const Vec &v) const;
+
+    template <typename T2>
+    operator Vec<T2, cn>() const;
+};
+    
+template <class T>
+class Bar2 {
+public:
+  typedef T value_type;
+  typedef value_type &reference;
+  typedef const value_type &const_reference;
+
+  // we cannot assume that the template parameter isn't a pointer
+  bool f40(value_type) const;
+
+  bool f41(reference) const;
+
+  value_type f42() const;
+
+  typename T::value_type f43() const;
+
+  bool f44(const_reference) const;
+};
+
+template <class T>
+bool Bar<T>::empty() const {
+  return true;
+}
+
+// don't mark typical ``[[nodiscard]]`` candidates if the class
+// has mutable member variables
+class MutableExample {
+  mutable bool m_isempty;
+
+public:
+  bool empty() const;
+};
diff --git a/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard.h b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard.h
new file mode 100644
index 00000000000..56f3c296023
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/modernize-use-nodiscard.h
@@ -0,0 +1,5 @@
+
+#define MUST_USE_RESULT __attribute__((warn_unused_result))
+#define NO_DISCARD [[nodiscard]]
+#define NO_RETURN [[noreturn]]
+