[clang-tidy] Fix modernize-use-nodiscard check for classes marked as [[nodiscard]]

[sedykh.eugene]

Summary

Current implementation suggests to add [[nodiscard]] to methods even if the return type is marked already as [[nodiscard]]:

Try this:

struct [[nodiscard]] S{};

class C{
  S method() const; --> suggests adding [[nodiscard]]
};

This small diff fixes this incorrect behaviour.

This is my first timid try to contribute to open source, so please help me with this piece of code. Maybe there are better ways.

[MyDeveloperDay]

Thank you for the patch, I wrote this checker originally and this LGTM.

I'm not sure if others have any objections to using "using clang::attr::WarnUnusedResult" in the body of the function, I couldn't see this pattern used elsewhere in clang-tidy, all I've seen is it being used in ClangTidyOptions.cpp globally.

I'd wait for some more feedback from the main owners, but thank you and welcome.

[aaron.ballman]

Thank you for the patch, and welcome! LGTM aside from a minor nit. Do you need someone to commit this on your behalf?

clang-tidy/modernize/UseNodiscardCheck.cpp

97

I'd just sink this down into the matcher itself rather than use a using declaration for it.

[sedykh.eugene]

I removed using. Not a big deal. Thank you for the reviews!

[sedykh.eugene]

In D69388#1723589, @aaron.ballman wrote:

Thank you for the patch, and welcome! LGTM aside from a minor nit. Do you need someone to commit this on your behalf?

Thank you. You mean I don't rights? Then, yes, I need someone to commit.

[aaron.ballman]

Thank you for the patch, I've commit on your behalf in 661d2ce619e05dc47a9a232333c01dcba445cd13

clang-tidy/modernize/UseNodiscardCheck.cpp

       cxxMethodDecl(
           allOf(isConst(), isDefinitionOrInline(),
                 unless(anyOf(
-                    returns(voidType()), isNoReturn(), isOverloadedOperator(),
+                    returns(voidType()),
+                    returns(hasDeclaration(decl(hasAttr(clang::attr::WarnUnusedResult)))),
+                    isNoReturn(), isOverloadedOperator(),
                     isVariadic(), hasTemplateReturnType(),
                     hasClassMutableFields(), isConversionOperator(),
                     hasAttr(clang::attr::WarnUnusedResult),

test/clang-tidy/checkers/modernize-use-nodiscard.cpp

 typedef unsigned &my_unsigned_reference;
 typedef const unsigned &my_unsigned_const_reference;
 
+struct NO_DISCARD NoDiscardStruct{};
+
 class Foo {
 public:
     using size_type = unsigned;

 
     // Do not add ``[[nodiscard]]`` to conversion functions.
     // explicit operator bool() const { return true; }
+
+    // Do not add ``[[nodiscard]]`` to functions returning types marked [[nodiscard]].
+    NoDiscardStruct f50() const;
 };
 
 // Do not add ``[[nodiscard]]`` to Lambda.