Comment diagnostics: warn if \returns is used in a non-function comment or if

the function returns void.

llvm-svn: 161261

clang/include/clang/AST/Comment.h

@@ -15,6 +15,7 @@
 #define LLVM_CLANG_AST_COMMENT_H
 
 #include "clang/Basic/SourceLocation.h"
+#include "clang/AST/Type.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -919,6 +920,10 @@ struct DeclInfo {
   /// that we consider a "function".
   ArrayRef<const ParmVarDecl *> ParamVars;
 
+  /// Function result type if \c ThisDecl is something that we consider
+  /// a "function".
+  QualType ResultType;
+
   /// Template parameters that can be referenced by \\tparam if \c ThisDecl is
   /// a template.
   const TemplateParameterList *TemplateParameters;
@@ -926,6 +931,9 @@ struct DeclInfo {
   /// A simplified description of \c ThisDecl kind that should be good enough
   /// for documentation rendering purposes.
   enum DeclKind {
+    /// Everything else not explicitly mentioned below.
+    OtherKind,
+
     /// Something that we consider a "function":
     /// \li function,
     /// \li function template,

clang/include/clang/AST/CommentSema.h

@@ -181,6 +181,8 @@ public:
 
   void checkBlockCommandEmptyParagraph(BlockCommandComment *Command);
 
+  void checkReturnsCommand(const BlockCommandComment *Command);
+
   bool isFunctionDecl();
   bool isTemplateDecl();
 
@@ -210,6 +212,7 @@ public:
   bool isBlockCommand(StringRef Name);
   bool isParamCommand(StringRef Name);
   bool isTParamCommand(StringRef Name);
+  bool isReturnsCommand(StringRef Name);
   unsigned getBlockCommandNumArgs(StringRef Name);
 
   bool isInlineCommand(StringRef Name) const;

clang/include/clang/Basic/DiagnosticCommentKinds.td

@@ -98,5 +98,17 @@ def warn_doc_tparam_not_found : Warning<
 def note_doc_tparam_name_suggestion : Note<
   "did you mean '%0'?">;
 
+// \returns command
+
+def warn_doc_returns_not_attached_to_a_function_decl : Warning<
+  "'\\%0' command used in a comment that is not attached to "
+  "a function declaration">,
+  InGroup<Documentation>, DefaultIgnore;
+
+def warn_doc_returns_attached_to_a_void_function : Warning<
+  "'\\%0' command used in a comment that is attached to a "
+  "%select{void function|constructor|destructor}1">,
+  InGroup<Documentation>, DefaultIgnore;
+
 } // end of documentation issue category
 } // end of AST component

clang/lib/AST/Comment.cpp

@@ -141,7 +141,7 @@ void DeclInfo::fill() {
   assert(!IsFilled);
 
   // Set defaults.
-  Kind = FunctionKind;
+  Kind = OtherKind;
   IsTemplateDecl = false;
   IsTemplateSpecialization = false;
   IsTemplatePartialSpecialization = false;
@@ -170,6 +170,7 @@ void DeclInfo::fill() {
     Kind = FunctionKind;
     ParamVars = ArrayRef<const ParmVarDecl *>(FD->param_begin(),
                                               FD->getNumParams());
+    ResultType = FD->getResultType();
     unsigned NumLists = FD->getNumTemplateParameterLists();
     if (NumLists != 0) {
       IsTemplateDecl = true;
@@ -178,7 +179,8 @@ void DeclInfo::fill() {
           FD->getTemplateParameterList(NumLists - 1);
     }
 
-    if (K == Decl::CXXMethod) {
+    if (K == Decl::CXXMethod || K == Decl::CXXConstructor ||
+        K == Decl::CXXDestructor || K == Decl::CXXConversion) {
       const CXXMethodDecl *MD = cast<CXXMethodDecl>(ThisDecl);
       IsInstanceMethod = MD->isInstance();
       IsClassMethod = !IsInstanceMethod;
@@ -190,6 +192,7 @@ void DeclInfo::fill() {
     Kind = FunctionKind;
     ParamVars = ArrayRef<const ParmVarDecl *>(MD->param_begin(),
                                               MD->param_size());
+    ResultType = MD->getResultType();
     IsInstanceMethod = MD->isInstanceMethod();
     IsClassMethod = !IsInstanceMethod;
     break;
@@ -201,6 +204,7 @@ void DeclInfo::fill() {
     const FunctionDecl *FD = FTD->getTemplatedDecl();
     ParamVars = ArrayRef<const ParmVarDecl *>(FD->param_begin(),
                                               FD->getNumParams());
+    ResultType = FD->getResultType();
     TemplateParameters = FTD->getTemplateParameters();
     break;
   }
@@ -226,6 +230,7 @@ void DeclInfo::fill() {
     IsTemplateSpecialization = true;
     break;
   case Decl::Record:
+  case Decl::CXXRecord:
     Kind = ClassKind;
     break;
   case Decl::Var:

clang/lib/AST/CommentSema.cpp

@@ -55,6 +55,7 @@ BlockCommandComment *Sema::actOnBlockCommandFinish(
                               ParagraphComment *Paragraph) {
   Command->setParagraph(Paragraph);
   checkBlockCommandEmptyParagraph(Command);
+  checkReturnsCommand(Command);
   return Command;
 }
 
@@ -472,6 +473,37 @@ void Sema::checkBlockCommandEmptyParagraph(BlockCommandComment *Command) {
   }
 }
 
+void Sema::checkReturnsCommand(const BlockCommandComment *Command) {
+  if (!isReturnsCommand(Command->getCommandName()))
+    return;
+  if (isFunctionDecl()) {
+    if (ThisDeclInfo->ResultType->isVoidType()) {
+      unsigned DiagKind;
+      switch (ThisDeclInfo->ThisDecl->getKind()) {
+      default:
+        DiagKind = 0;
+        break;
+      case Decl::CXXConstructor:
+        DiagKind = 1;
+        break;
+      case Decl::CXXDestructor:
+        DiagKind = 2;
+        break;
+      }
+      Diag(Command->getLocation(),
+           diag::warn_doc_returns_attached_to_a_void_function)
+        << Command->getCommandName()
+        << DiagKind
+        << Command->getSourceRange();
+    }
+    return;
+  }
+  Diag(Command->getLocation(),
+       diag::warn_doc_returns_not_attached_to_a_function_decl)
+    << Command->getCommandName()
+    << Command->getSourceRange();
+}
+
 bool Sema::isFunctionDecl() {
   if (!ThisDeclInfo)
     return false;
@@ -643,16 +675,15 @@ StringRef Sema::correctTypoInTParamReference(
 
 // TODO: tablegen
 bool Sema::isBlockCommand(StringRef Name) {
-  return llvm::StringSwitch<bool>(Name)
+  return isReturnsCommand(Name) ||
+      isParamCommand(Name) || isTParamCommand(Name) ||
+      llvm::StringSwitch<bool>(Name)
       .Cases("brief", "short", true)
-      .Case("result", true)
-      .Case("return", true)
-      .Case("returns", true)
       .Case("author", true)
       .Case("authors", true)
       .Case("pre", true)
       .Case("post", true)
-      .Default(false) || isParamCommand(Name) || isTParamCommand(Name);
+      .Default(false);
 }
 
 bool Sema::isParamCommand(StringRef Name) {
@@ -666,6 +697,10 @@ bool Sema::isTParamCommand(StringRef Name) {
   return Name == "tparam";
 }
 
+bool Sema::isReturnsCommand(StringRef Name) {
+  return Name == "returns" || Name == "return" || Name == "result";
+}
+
 unsigned Sema::getBlockCommandNumArgs(StringRef Name) {
   return llvm::StringSwitch<unsigned>(Name)
       .Cases("brief", "short", 0)

clang/test/Sema/warn-documentation.cpp

@@ -262,6 +262,84 @@ using test_tparam14 = test_tparam13<T, int>;
 template<typename T>
 using test_tparam15 = test_tparam13<T, int>;
 
+// no-warning
+/// \returns Aaa
+int test_returns_right_decl_1(int);
+
+class test_returns_right_decl_2 {
+  // no-warning
+  /// \returns Aaa
+  int test_returns_right_decl_3(int);
+};
+
+// no-warning
+/// \returns Aaa
+template<typename T>
+int test_returns_right_decl_4(T aaa);
+
+// no-warning
+/// \returns Aaa
+template<>
+int test_returns_right_decl_4(int aaa);
+
+/// \returns Aaa
+template<typename T>
+T test_returns_right_decl_5(T aaa);
+
+// expected-warning@+1 {{'\returns' command used in a comment that is not attached to a function declaration}}
+/// \returns Aaa
+int test_returns_wrong_decl_1;
+
+// expected-warning@+1 {{'\return' command used in a comment that is not attached to a function declaration}}
+/// \return Aaa
+int test_returns_wrong_decl_2;
+
+// expected-warning@+1 {{'\result' command used in a comment that is not attached to a function declaration}}
+/// \result Aaa
+int test_returns_wrong_decl_3;
+
+// expected-warning@+1 {{'\returns' command used in a comment that is attached to a void function}}
+/// \returns Aaa
+void test_returns_wrong_decl_4(int);
+
+// expected-warning@+1 {{'\returns' command used in a comment that is attached to a void function}}
+/// \returns Aaa
+template<typename T>
+void test_returns_wrong_decl_5(T aaa);
+
+// expected-warning@+1 {{'\returns' command used in a comment that is attached to a void function}}
+/// \returns Aaa
+template<>
+void test_returns_wrong_decl_5(int aaa);
+
+// expected-warning@+1 {{'\returns' command used in a comment that is not attached to a function declaration}}
+/// \returns Aaa
+struct test_returns_wrong_decl_6 { };
+
+// expected-warning@+1 {{'\returns' command used in a comment that is not attached to a function declaration}}
+/// \returns Aaa
+class test_returns_wrong_decl_7 {
+  // expected-warning@+1 {{'\returns' command used in a comment that is attached to a constructor}}
+  /// \returns Aaa
+  test_returns_wrong_decl_7();
+
+  // expected-warning@+1 {{'\returns' command used in a comment that is attached to a destructor}}
+  /// \returns Aaa
+  ~test_returns_wrong_decl_7();
+};
+
+// expected-warning@+1 {{'\returns' command used in a comment that is not attached to a function declaration}}
+/// \returns Aaa
+enum test_returns_wrong_decl_8 {
+  // expected-warning@+1 {{'\returns' command used in a comment that is not attached to a function declaration}}
+  /// \returns Aaa
+  test_returns_wrong_decl_9
+};
+
+// expected-warning@+1 {{'\returns' command used in a comment that is not attached to a function declaration}}
+/// \returns Aaa
+namespace test_returns_wrong_decl_10 { };
+
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
 int test1; ///< \brief\brief Aaa