Comment diagnostics: warn on duplicate \brief and \return commands.

Doxygen manual claims that multiple \brief or \returns commands will be merged
together, but actual behavior is different (second \brief command becomes a
part of a discussion, second \returns becomes a "Returns: blah" paragraph on
its own).  Anyway, it seems to be a bad idea to use multiple \brief or \returns
commands in a single command.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@161325 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/CommentSema.h b/include/clang/AST/CommentSema.h
index 53fd5dc7b167b43ed448b98ce76f571e53676f41..114cfc577bb7ceb356fdc0360308c56ede58d598 100644 (file)
--- a/include/clang/AST/CommentSema.h
+++ b/include/clang/AST/CommentSema.h
@@ -56,6 +56,12 @@ class Sema {
   /// Contains a valid value if \c DeclInfo->IsFilled is true.
   llvm::StringMap<TParamCommandComment *> TemplateParameterDocs;
 
+  /// AST node for the \\brief command and its aliases.
+  const BlockCommandComment *BriefCommand;
+
+  /// AST node for the \\returns command and its aliases.
+  const BlockCommandComment *ReturnsCommand;
+
   DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID) {
     return Diags.Report(Loc, DiagID);
   }
@@ -183,6 +189,10 @@ public:
 
   void checkReturnsCommand(const BlockCommandComment *Command);
 
+  /// Emit diagnostics about duplicate block commands that should be
+  /// used only once per comment, e.g., \\brief and \\returns.
+  void checkBlockCommandDuplicate(const BlockCommandComment *Command);
+
   bool isFunctionDecl();
   bool isTemplateDecl();
 
@@ -212,6 +222,7 @@ public:
   bool isBlockCommand(StringRef Name);
   bool isParamCommand(StringRef Name);
   bool isTParamCommand(StringRef Name);
+  bool isBriefCommand(StringRef Name);
   bool isReturnsCommand(StringRef Name);
   unsigned getBlockCommandNumArgs(StringRef Name);
 

diff --git a/include/clang/Basic/DiagnosticCommentKinds.td b/include/clang/Basic/DiagnosticCommentKinds.td
index 1594c313d7cb025ba95d81f48eb20d500996cb73..235ca79564d36edd4261d0a5677542fe25133450 100644 (file)
--- a/include/clang/Basic/DiagnosticCommentKinds.td
+++ b/include/clang/Basic/DiagnosticCommentKinds.td
@@ -47,6 +47,16 @@ def warn_doc_block_command_empty_paragraph : Warning<
   "empty paragraph passed to '\\%0' command">,
   InGroup<Documentation>, DefaultIgnore;
 
+def warn_doc_block_command_duplicate : Warning<
+  "duplicated command '\\%0'">,
+  InGroup<Documentation>, DefaultIgnore;
+
+def note_doc_block_command_previous : Note<
+  "previous command '\\%0' here">;
+
+def note_doc_block_command_previous_alias : Note<
+  "previous command '\\%0' (an alias of '\\%1') here">;
+
 // \param command
 
 def warn_doc_param_invalid_direction : Warning<

diff --git a/lib/AST/CommentSema.cpp b/lib/AST/CommentSema.cpp
index 770d5bba244f8a7f92266b5f1aabf84a891f3e78..57f8fda07c8f278ce819ed165a144d48421eb504 100644 (file)
--- a/lib/AST/CommentSema.cpp
+++ b/lib/AST/CommentSema.cpp
@@ -20,7 +20,7 @@ namespace comments {
 Sema::Sema(llvm::BumpPtrAllocator &Allocator, const SourceManager &SourceMgr,
            DiagnosticsEngine &Diags) :
     Allocator(Allocator), SourceMgr(SourceMgr), Diags(Diags),
-    ThisDeclInfo(NULL) {
+    ThisDeclInfo(NULL), BriefCommand(NULL), ReturnsCommand(NULL) {
 }
 
 void Sema::setDecl(const Decl *D) {
@@ -55,6 +55,7 @@ BlockCommandComment *Sema::actOnBlockCommandFinish(
                               ParagraphComment *Paragraph) {
   Command->setParagraph(Paragraph);
   checkBlockCommandEmptyParagraph(Command);
+  checkBlockCommandDuplicate(Command);
   checkReturnsCommand(Command);
   return Command;
 }
@@ -507,6 +508,39 @@ void Sema::checkReturnsCommand(const BlockCommandComment *Command) {
     << Command->getSourceRange();
 }
 
+void Sema::checkBlockCommandDuplicate(const BlockCommandComment *Command) {
+  StringRef Name = Command->getCommandName();
+  const BlockCommandComment *PrevCommand = NULL;
+  if (isBriefCommand(Name)) {
+    if (!BriefCommand) {
+      BriefCommand = Command;
+      return;
+    }
+    PrevCommand = BriefCommand;
+  } else if (isReturnsCommand(Name)) {
+    if (!ReturnsCommand) {
+      ReturnsCommand = Command;
+      return;
+    }
+    PrevCommand = ReturnsCommand;
+  } else {
+    // We don't want to check this command for duplicates.
+    return;
+  }
+  Diag(Command->getLocation(), diag::warn_doc_block_command_duplicate)
+      << Name
+      << Command->getSourceRange();
+  if (Name == PrevCommand->getCommandName())
+    Diag(PrevCommand->getLocation(), diag::note_doc_block_command_previous)
+        << PrevCommand->getCommandName()
+        << Command->getSourceRange();
+  else
+    Diag(PrevCommand->getLocation(),
+         diag::note_doc_block_command_previous_alias)
+        << PrevCommand->getCommandName()
+        << Name;
+}
+
 bool Sema::isFunctionDecl() {
   if (!ThisDeclInfo)
     return false;
@@ -678,10 +712,9 @@ StringRef Sema::correctTypoInTParamReference(
 
 // TODO: tablegen
 bool Sema::isBlockCommand(StringRef Name) {
-  return isReturnsCommand(Name) ||
+  return isBriefCommand(Name) || isReturnsCommand(Name) ||
       isParamCommand(Name) || isTParamCommand(Name) ||
       llvm::StringSwitch<bool>(Name)
-      .Cases("brief", "short", true)
       .Case("author", true)
       .Case("authors", true)
       .Case("pre", true)
@@ -700,6 +733,10 @@ bool Sema::isTParamCommand(StringRef Name) {
   return Name == "tparam";
 }
 
+bool Sema::isBriefCommand(StringRef Name) {
+  return Name == "brief" || Name == "short";
+}
+
 bool Sema::isReturnsCommand(StringRef Name) {
   return Name == "returns" || Name == "return" || Name == "result";
 }

diff --git a/test/Sema/warn-documentation.cpp b/test/Sema/warn-documentation.cpp
index 19d43ab488fe27668a07ab408492b203bd2606f8..c6ba0151b917a19ad1ad7a3ba969f8d4d3b43fe6 100644 (file)
--- a/test/Sema/warn-documentation.cpp
+++ b/test/Sema/warn-documentation.cpp
@@ -79,34 +79,92 @@ int test_html_nesting7(int);
 
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\returns Aaa
 int test_block_command1(int);
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief \brief Aaa
+/// \brief \returns Aaa
 int test_block_command2(int);
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
 /// \brief
-/// \brief Aaa
+/// \returns Aaa
 int test_block_command3(int);
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
 /// \brief
 ///
-/// \brief Aaa
+/// \returns Aaa
 int test_block_command4(int);
 
 // There is trailing whitespace on one of the following lines, don't remove it!
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
 /// \brief
 /// 
-/// \brief Aaa
+/// \returns Aaa
 int test_block_command5(int);
 
 /// \brief \c Aaa
 int test_block_command6(int);
 
+// expected-warning@+5 {{duplicated command '\brief'}} expected-note@+1 {{previous command '\brief' here}}
+/// \brief Aaa
+///
+/// Bbb
+///
+/// \brief Ccc
+int test_duplicate_brief1(int);
+
+// expected-warning@+5 {{duplicated command '\short'}} expected-note@+1 {{previous command '\short' here}}
+/// \short Aaa
+///
+/// Bbb
+///
+/// \short Ccc
+int test_duplicate_brief2(int);
+
+// expected-warning@+5 {{duplicated command '\brief'}} expected-note@+1 {{previous command '\short' (an alias of '\brief') here}}
+/// \short Aaa
+///
+/// Bbb
+///
+/// \brief Ccc
+int test_duplicate_brief3(int);
+
+
+// expected-warning@+5 {{duplicated command '\return'}} expected-note@+1 {{previous command '\return' here}}
+/// \return Aaa
+///
+/// Bbb
+///
+/// \return Ccc
+int test_duplicate_returns1(int);
+
+// expected-warning@+5 {{duplicated command '\returns'}} expected-note@+1 {{previous command '\returns' here}}
+/// \returns Aaa
+///
+/// Bbb
+///
+/// \returns Ccc
+int test_duplicate_returns2(int);
+
+// expected-warning@+5 {{duplicated command '\result'}} expected-note@+1 {{previous command '\result' here}}
+/// \result Aaa
+///
+/// Bbb
+///
+/// \result Ccc
+int test_duplicate_returns3(int);
+
+// expected-warning@+5 {{duplicated command '\return'}} expected-note@+1 {{previous command '\returns' (an alias of '\return') here}}
+/// \returns Aaa
+///
+/// Bbb
+///
+/// \return Ccc
+int test_duplicate_returns4(int);
+
+
 // expected-warning@+1 {{'\param' command used in a comment that is not attached to a function declaration}}
 /// \param a Blah blah.
 int test_param1;
@@ -342,16 +400,16 @@ namespace test_returns_wrong_decl_10 { };
 
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-int test1; ///< \brief\brief Aaa
+int test1; ///< \brief\author Aaa
 
 // expected-warning@+2 {{empty paragraph passed to '\brief' command}}
 // expected-warning@+2 {{empty paragraph passed to '\brief' command}}
-int test2, ///< \brief\brief Aaa
-    test3; ///< \brief\brief Aaa
+int test2, ///< \brief\author Aaa
+    test3; ///< \brief\author Aaa
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
 int test4; ///< \brief
-           ///< \brief Aaa
+           ///< \author Aaa
 
 
 // Check that we attach the comment to the declaration during parsing in the
@@ -359,118 +417,118 @@ int test4; ///< \brief
 // documentation comments that are not attached to anything.
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 int test_attach1;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 int test_attach2(int);
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 struct test_attach3 {
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-  /// \brief\brief Aaa
+  /// \brief\author Aaa
   int test_attach4;
 
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-  int test_attach5; ///< \brief\brief Aaa
+  int test_attach5; ///< \brief\author Aaa
 
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-  /// \brief\brief Aaa
+  /// \brief\author Aaa
   int test_attach6(int);
 };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 class test_attach7 {
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-  /// \brief\brief Aaa
+  /// \brief\author Aaa
   int test_attach8;
 
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-  int test_attach9; ///< \brief\brief Aaa
+  int test_attach9; ///< \brief\author Aaa
 
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-  /// \brief\brief Aaa
+  /// \brief\author Aaa
   int test_attach10(int);
 };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 enum test_attach9 {
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-  /// \brief\brief Aaa
+  /// \brief\author Aaa
   test_attach10,
 
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-  test_attach11 ///< \brief\brief Aaa
+  test_attach11 ///< \brief\author Aaa
 };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 struct test_noattach12 *test_attach13;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 typedef struct test_noattach14 *test_attach15;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 typedef struct test_attach16 { int a; } test_attach17;
 
 struct S { int a; };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 struct S *test_attach18;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 typedef struct S *test_attach19;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 struct test_attach20;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 typedef struct test_attach21 {
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-  /// \brief\brief Aaa
+  /// \brief\author Aaa
   int test_attach22;
 } test_attach23;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 namespace test_attach24 {
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-  /// \brief\brief Aaa
+  /// \brief\author Aaa
   namespace test_attach25 {
   }
 }
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<typename T>
 void test_attach26(T aaa);
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<typename T, typename U>
 void test_attach27(T aaa, U bbb);
 
 // expected-warning@+2 {{empty paragraph passed to '\brief' command}}
 // expected-warning@+2 {{template parameter 'T' not found in the template declaration}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<>
 void test_attach27(int aaa, int bbb);
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<typename T>
 class test_attach28 {
@@ -478,61 +536,61 @@ class test_attach28 {
 };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 using test_attach29 = test_attach28<int>;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<typename T, typename U>
 class test_attach30 { };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<typename T>
 class test_attach30<T, int> { };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 template<>
 class test_attach30<int, int> { };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 template<typename T>
 using test_attach31 = test_attach30<T, int>;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<typename T, typename U, typename V>
 class test_attach32 { };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<typename T, typename U>
 class test_attach32<T, U, int> { };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<typename T>
 class test_attach32<T, int, int> { };
 
 // expected-warning@+2 {{empty paragraph passed to '\brief' command}}
 // expected-warning@+2 {{template parameter 'T' not found in the template declaration}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<>
 class test_attach32<int, int, int> { };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 class test_attach33 {
   // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-  /// \brief\brief Aaa
+  /// \brief\author Aaa
   /// \tparam T Aaa
   template<typename T, typename U>
   void test_attach34(T aaa, U bbb);
@@ -542,7 +600,7 @@ template<typename T>
 class test_attach35 {
   // expected-warning@+2 {{empty paragraph passed to '\brief' command}}
   // expected-warning@+2 {{template parameter 'T' not found in the template declaration}}
-  /// \brief\brief Aaa
+  /// \brief\author Aaa
   /// \tparam T Aaa
   template<typename TT, typename UU>
   void test_attach36(TT aaa, UU bbb);
@@ -550,7 +608,7 @@ class test_attach35 {
 
 // expected-warning@+2 {{empty paragraph passed to '\brief' command}}
 // expected-warning@+2 {{template parameter 'T' not found in the template declaration}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<> template<>
 void test_attach35<int>::test_attach36(int aaa, int bbb) {}
@@ -559,20 +617,20 @@ template<typename T>
 class test_attach37 {
   // expected-warning@+2 {{empty paragraph passed to '\brief' command}}
   // expected-warning@+2 {{'\tparam' command used in a comment that is not attached to a template declaration}}
-  /// \brief\brief Aaa
+  /// \brief\author Aaa
   /// \tparam T Aaa
   void test_attach38(int aaa, int bbb);
 };
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<typename T>
 void test_attach37<T>::test_attach38(int aaa, int bbb) {}
 
 // expected-warning@+2 {{empty paragraph passed to '\brief' command}}
 // expected-warning@+2 {{template parameter 'T' not found in the template declaration}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 /// \tparam T Aaa
 template<>
 void test_attach37<int>::test_attach38(int aaa, int bbb) {}

diff --git a/test/Sema/warn-documentation.m b/test/Sema/warn-documentation.m
index 7b6361c1f4008d6709a7a8dc4810c10cee57fda5..d6af6edcc85d9c068e5324f58266f391cba44e22 100644 (file)
--- a/test/Sema/warn-documentation.m
+++ b/test/Sema/warn-documentation.m
@@ -3,11 +3,11 @@
 @class NSString;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 @interface Test1
 // expected-warning@+2 {{empty paragraph passed to '\brief' command}}
 /**
- * \brief\brief Aaa
+ * \brief\author Aaa
  * \param aaa Aaa
  * \param bbb Bbb
  */
@@ -20,21 +20,21 @@
 + (NSString *)test2:(NSString *)aaa;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 @property int test3; // a property: ObjCPropertyDecl
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 @property int test4; // a property: ObjCPropertyDecl
 @end
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 @interface Test1()
 @end
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 @implementation Test1 // a class implementation : ObjCImplementationDecl
 + (NSString *)test1:(NSString *)aaa suffix:(NSString *)bbb {
   return 0;
@@ -48,20 +48,20 @@
 @dynamic test4; // a property implementation: ObjCPropertyImplDecl
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 NSString *_test5;
 @end
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 @interface Test1(Test1Category) // a category: ObjCCategoryDecl
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 + (NSString *)test3:(NSString *)aaa;
 @end
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 @implementation Test1(Test1Category) // a category implementation: ObjCCategoryImplDecl
 + (NSString *)test3:(NSString *)aaa {
   return 0;
@@ -69,14 +69,14 @@ NSString *_test5;
 @end
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 @protocol TestProto1 // a protocol: ObjCProtocolDecl
 @end
 
 int a;
 
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
-/// \brief\brief Aaa
+/// \brief\author Aaa
 @interface Test4
 @end