Fix daedalean-structs-and-classes check

clang-tools-extra/clang-tidy/daedalean/StructsAndClassesCheck.cpp

@@ -16,6 +16,51 @@ namespace clang {
 namespace tidy {
 namespace daedalean {
 
+StructsAndClassesCheck::ShouldBeAClass
+StructsAndClassesCheck::shouldBeAClass(const CXXRecordDecl *record) {
+  if (record->hasUserDeclaredCopyConstructor()) {
+    return ShouldBeAClass::YesHasUserDeclaredCopyConstructor;
+  }
+
+  if (record->hasUserDeclaredMoveConstructor()) {
+    return ShouldBeAClass::YesHasUserDeclaredMoveConstructor;
+  }
+
+  if (record->hasUserDeclaredCopyAssignment()) {
+    return ShouldBeAClass::YesHasUserDeclaredCopyAssignment;
+  }
+
+  if (record->hasUserDeclaredMoveAssignment()) {
+    return ShouldBeAClass::YesHasUserDeclaredCopyAssignment;
+  }
+
+  if (record->hasUserDeclaredDestructor()) {
+    return ShouldBeAClass::YesHasUserDeclaredDestructor;
+  }
+
+  if (!record->isAggregate()) {
+    return ShouldBeAClass::YesNotAnAggregate;
+  }
+
+  for (const CXXMethodDecl *method : record->methods()) {
+    const bool isConstructor = llvm::isa<CXXConstructorDecl>(method);
+    if (isConstructor) {
+      // Constructors are ignored
+      continue;
+    }
+
+    if (method->isStatic()) {
+      continue;
+    }
+
+    if (!method->isConst()) {
+      return ShouldBeAClass::YesHasNonConstMethod;
+    }
+  }
+
+  return ShouldBeAClass::No;
+}
+
 void StructsAndClassesCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(cxxRecordDecl(hasDefinition()).bind("x"), this);
 }
@@ -28,29 +73,73 @@ void StructsAndClassesCheck::check(const MatchFinder::MatchResult &Result) {
     return;
   }
 
-  if (MatchedDecl->isPOD()) {
+  if (const ShouldBeAClass shouldBeClass = shouldBeAClass(MatchedDecl);
+      shouldBeClass != ShouldBeAClass::No) {
+    if (MatchedDecl->isStruct()) {
+      switch (shouldBeClass) {
+      case ShouldBeAClass::YesNotAnAggregate:
+        diag(MatchedDecl->getBeginLoc(),
+             "Non-Aggregate type %0 must be declared as a class")
+            << MatchedDecl;
+        break;
+      case ShouldBeAClass::YesHasUserDeclaredCopyConstructor:
+        diag(MatchedDecl->getBeginLoc(),
+             "Type %0 with a user-provided copy constructor must be declared "
+             "as a class")
+            << MatchedDecl;
+        break;
+      case ShouldBeAClass::YesHasUserDeclaredMoveConstructor:
+        diag(MatchedDecl->getBeginLoc(),
+             "Type %0 with a user-provided move constructor must be declared "
+             "as a class")
+            << MatchedDecl;
+        break;
+      case ShouldBeAClass::YesHasUserDeclaredCopyAssignment:
+        diag(MatchedDecl->getBeginLoc(),
+             "Type %0 with a user-provided move copy assignment operator must "
+             "be declared as a class")
+            << MatchedDecl;
+        break;
+      case ShouldBeAClass::YesHasUserDeclaredMoveAssignment:
+        diag(MatchedDecl->getBeginLoc(),
+             "Type %0 with a a user-provided move move assignment operator "
+             "must be declared as a class")
+            << MatchedDecl;
+        break;
+      case ShouldBeAClass::YesHasUserDeclaredDestructor:
+        diag(MatchedDecl->getBeginLoc(),
+             "Type %0 with a user-provided destructor must be declared as a "
+             "class")
+            << MatchedDecl;
+        break;
+      case ShouldBeAClass::YesHasNonConstMethod:
+        diag(MatchedDecl->getBeginLoc(),
+             "Type %0 with non-const methods must be declared as a class")
+            << MatchedDecl;
+        break;
+      default:
+        break;
+      }
+      diag(MatchedDecl->getBeginLoc(), "use class", DiagnosticIDs::Note)
+          << FixItHint::CreateInsertion(MatchedDecl->getBeginLoc(), "class");
+    }
+  } else {
     if (!MatchedDecl->isStruct()) {
-      diag(MatchedDecl->getBeginLoc(), "POD type %0 must be declared as struct")
+      diag(MatchedDecl->getBeginLoc(), "Type %0 must be declared as a struct")
           << MatchedDecl;
       diag(MatchedDecl->getBeginLoc(), "use struct", DiagnosticIDs::Note)
           << FixItHint::CreateReplacement(MatchedDecl->getBeginLoc(), "struct");
     }
-  } else {
-    if (!MatchedDecl->isClass()) {
-      diag(MatchedDecl->getBeginLoc(), "Non-POD type %0 must be declared as class")
-          << MatchedDecl;
-      diag(MatchedDecl->getBeginLoc(), "use class", DiagnosticIDs::Note)
-          << FixItHint::CreateReplacement(MatchedDecl->getBeginLoc(), "class");
-    }
   }
 
   if (MatchedDecl->isClass()) {
-    for (const auto * field: MatchedDecl->fields()) {
+    for (const auto *field : MatchedDecl->fields()) {
       if (field->getType().isConstQualified()) {
         continue;
       }
       if (field->getAccess() != AS_private && field->getAccess() != AS_none) {
-        diag(field->getBeginLoc(), "All non-const data members of class MUST be private")
+        diag(field->getBeginLoc(),
+             "All non-const data members of class MUST be private")
             << MatchedDecl;
         diag(field->getBeginLoc(), "Make field private", DiagnosticIDs::Note)
             << FixItHint::CreateInsertion(field->getBeginLoc(), "private:");

clang-tools-extra/clang-tidy/daedalean/StructsAndClassesCheck.h

@@ -25,6 +25,19 @@ class StructsAndClassesCheck : public ClangTidyCheck {
       : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  enum class ShouldBeAClass {
+    No,
+    YesNotAnAggregate,
+    YesHasUserDeclaredCopyConstructor,
+    YesHasUserDeclaredMoveConstructor,
+    YesHasUserDeclaredCopyAssignment,
+    YesHasUserDeclaredMoveAssignment,
+    YesHasUserDeclaredDestructor,
+    YesHasNonConstMethod,
+  };
+  ShouldBeAClass shouldBeAClass(const CXXRecordDecl *record);
 };
 
 } // namespace daedalean

clang-tools-extra/test/clang-tidy/checkers/daedalean-structs-and-classes.cpp

@@ -5,31 +5,93 @@ struct S1 {
 };
 
 struct S2 {
-  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Non-POD type must be declared as class [daedalean-structs-and-classes]
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Type 'S2' with a user-provided copy constructor must be declared as a class [daedalean-structs-and-classes]
   // CHECK-FIXES: class
-  S2(int a) : b(a) {}
+  S2(const S2 &) = default;
+  int b;
+};
+
+struct S3 {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Type 'S3' with a user-provided move constructor must be declared as a class [daedalean-structs-and-classes]
+  // CHECK-FIXES: class
+  S3(S3 &&) = default;
+  int b;
+};
+
+struct S4 {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Type 'S4' with a user-provided move copy assignment operator must be declared as a class [daedalean-structs-and-classes]
+  // CHECK-FIXES: class
+  S4 &operator=(const S4 &) = default;
+  int b;
+};
+
+struct S5 {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Type 'S5' with a user-provided move copy assignment operator must be declared as a class [daedalean-structs-and-classes]
+  // CHECK-FIXES: class
+  S5 &operator=(S5 &&) = default;
+  int b;
+};
+
+struct S6 {
+  bool isFive() const { return b == 5; }
+  int b;
+};
+
+struct S7 {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Type 'S7' with non-const methods must be declared as a class [daedalean-structs-and-classes]
+  // CHECK-FIXES: class
+  void set(int c) { b = c; }
+  int b;
+};
+
+struct S8 {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Type 'S8' with a user-provided destructor must be declared as a class [daedalean-structs-and-classes]
+  // CHECK-FIXES: class
+  ~S8() {}
+  int b;
+};
+
+struct S9 {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Non-Aggregate type 'S9' must be declared as a class [daedalean-structs-and-classes]
+  S9(int val) : b(val) {}
   int b;
 };
 
 class C1 {
-  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: POD type must be declared as struct [daedalean-structs-and-classes]
-  // CHECK-FIXES: struct
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Type 'C1' must be declared as a struct [daedalean-structs-and-classes]
 public:
   const int a;
 };
 
 class C2 {
 public:
-  C2(int a) : b(a) {}
+  C2(const C2 &) = default;
+
 private:
   int b;
 };
 
 class C3 {
 public:
-  C3(int) {}
+  C3(C3 &&) = default;
 
   int a;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: All non-const data members of class MUST be private [daedalean-structs-and-classes]
   // CHECK-FIXES: private:
 };
+
+class C4 {
+public:
+  C4 &operator=(const C4 &) = default;
+
+private:
+  int a;
+};
+
+class C5 {
+public:
+  C5 &operator=(C5 &&) = default;
+
+private:
+  int a;
+};