[clang-tools-extra] 2cd2acc - [clang-tidy] Fix false positives involving type aliases in `misc-unconventional-assign-operator` check

[Fabian Wolff via cfe-commits]

Author: Fabian Wolff
Date: 2022-01-17T21:16:17+01:00
New Revision: 2cd2accc61ea0900bde66c79d1d04b29bb9e3ed7

URL: https://github.com/llvm/llvm-project/commit/2cd2accc61ea0900bde66c79d1d04b29bb9e3ed7
DIFF: https://github.com/llvm/llvm-project/commit/2cd2accc61ea0900bde66c79d1d04b29bb9e3ed7.diff

LOG: [clang-tidy] Fix false positives involving type aliases in `misc-unconventional-assign-operator` check

clang-tidy currently reports false positives even for simple cases such as:
```
struct S {
    using X = S;
    X &operator=(const X&) { return *this; }
};
```
This is due to the fact that the `misc-unconventional-assign-operator` check fails to look at the //canonical// types. This patch fixes this behavior.

Reviewed By: aaron.ballman, mizvekov

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

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
    clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst
    clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
index 594c83a30e7ef..12ba5d9997fb1 100644
--- a/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -18,13 +18,14 @@ namespace misc {
 
 void UnconventionalAssignOperatorCheck::registerMatchers(
     ast_matchers::MatchFinder *Finder) {
-  const auto HasGoodReturnType = cxxMethodDecl(returns(lValueReferenceType(
-      pointee(unless(isConstQualified()),
-              anyOf(autoType(), hasDeclaration(equalsBoundNode("class")))))));
+  const auto HasGoodReturnType =
+      cxxMethodDecl(returns(hasCanonicalType(lValueReferenceType(pointee(
+          unless(isConstQualified()),
+          anyOf(autoType(), hasDeclaration(equalsBoundNode("class"))))))));
 
-  const auto IsSelf = qualType(
+  const auto IsSelf = qualType(hasCanonicalType(
       anyOf(hasDeclaration(equalsBoundNode("class")),
-            referenceType(pointee(hasDeclaration(equalsBoundNode("class"))))));
+            referenceType(pointee(hasDeclaration(equalsBoundNode("class")))))));
   const auto IsAssign =
       cxxMethodDecl(unless(anyOf(isDeleted(), isPrivate(), isImplicit())),
                     hasName("operator="), ofClass(recordDecl().bind("class")))
@@ -37,9 +38,9 @@ void UnconventionalAssignOperatorCheck::registerMatchers(
       cxxMethodDecl(IsAssign, unless(HasGoodReturnType)).bind("ReturnType"),
       this);
 
-  const auto BadSelf = referenceType(
+  const auto BadSelf = qualType(hasCanonicalType(referenceType(
       anyOf(lValueReferenceType(pointee(unless(isConstQualified()))),
-            rValueReferenceType(pointee(isConstQualified()))));
+            rValueReferenceType(pointee(isConstQualified()))))));
 
   Finder->addMatcher(
       cxxMethodDecl(IsSelfAssign,

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst
index 8b85332fa0c35..49e3fd5b6ee42 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst
@@ -8,6 +8,8 @@ Finds declarations of assign operators with the wrong return and/or argument
 types and definitions with good return type but wrong ``return`` statements.
 
   * The return type must be ``Class&``.
-  * Works with move-assign and assign by value.
+  * The assignment may be from the class type by value, const lvalue
+    reference, non-const rvalue reference, or from a completely 
diff erent
+    type (e.g. ``int``).
   * Private and deleted operators are ignored.
   * The operator must always return ``*this``.

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp
index 4f0713a01f6f4..b010a8a230bcc 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp
@@ -127,3 +127,39 @@ struct AssignmentCallAtReturn {
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operator=() should always return '*this'
   }
 };
+
+// Check that no false positives are issued when using type aliases.
+struct TypeAlias {
+  using Alias = TypeAlias;
+  // This is correct and should not produce any warnings:
+  Alias &operator=(const Alias &) { return *this; }
+
+  using AliasRef = Alias &;
+  // So is this (assignments from other types are fine):
+  AliasRef operator=(int) { return *this; }
+};
+
+// Same check as above with typedef instead of using
+struct TypeAliasTypedef {
+  typedef TypeAliasTypedef Alias;
+  Alias &operator=(const Alias &) { return *this; }
+
+  typedef Alias &AliasRef;
+  AliasRef operator=(int) { return *this; }
+};
+
+// Same check as above for a template class
+template <typename T>
+struct TemplateTypeAlias {
+  using Alias1 = TemplateTypeAlias &;
+  using Alias2 = TemplateTypeAlias const &;
+  Alias1 operator=(Alias2) { return *this; }
+
+  template <typename U>
+  using Alias3 = TemplateTypeAlias<U>;
+  Alias3<T> &operator=(int) { return *this; }
+
+  // Using a 
diff erent type parameter in the return type should give a warning
+  Alias3<TypeAlias::Alias> &operator=(double) { return *this; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'TemplateTypeAlias&' [misc-unconventional-assign-operator]
+};