From: Dmitri Gribenko Date: Mon, 6 Aug 2012 17:08:27 +0000 (+0000) Subject: Comment diagnostics: warn on duplicate \brief and \return commands. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9443c57150e870e308406e1e4e6d9d64712b417e;p=clang 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 53fd5dc7b1..114cfc577b 100644 --- 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 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 1594c313d7..235ca79564 100644 --- 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, DefaultIgnore; +def warn_doc_block_command_duplicate : Warning< + "duplicated command '\\%0'">, + InGroup, 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 770d5bba24..57f8fda07c 100644 --- 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(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 19d43ab488..c6ba0151b9 100644 --- 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 void test_attach26(T aaa); // expected-warning@+1 {{empty paragraph passed to '\brief' command}} -/// \brief\brief Aaa +/// \brief\author Aaa /// \tparam T Aaa template 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 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; // expected-warning@+1 {{empty paragraph passed to '\brief' command}} -/// \brief\brief Aaa +/// \brief\author Aaa /// \tparam T Aaa template class test_attach30 { }; // expected-warning@+1 {{empty paragraph passed to '\brief' command}} -/// \brief\brief Aaa +/// \brief\author Aaa /// \tparam T Aaa template class test_attach30 { }; // expected-warning@+1 {{empty paragraph passed to '\brief' command}} -/// \brief\brief Aaa +/// \brief\author Aaa template<> class test_attach30 { }; // expected-warning@+1 {{empty paragraph passed to '\brief' command}} -/// \brief\brief Aaa +/// \brief\author Aaa template using test_attach31 = test_attach30; // expected-warning@+1 {{empty paragraph passed to '\brief' command}} -/// \brief\brief Aaa +/// \brief\author Aaa /// \tparam T Aaa template class test_attach32 { }; // expected-warning@+1 {{empty paragraph passed to '\brief' command}} -/// \brief\brief Aaa +/// \brief\author Aaa /// \tparam T Aaa template class test_attach32 { }; // expected-warning@+1 {{empty paragraph passed to '\brief' command}} -/// \brief\brief Aaa +/// \brief\author Aaa /// \tparam T Aaa template class test_attach32 { }; // 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 { }; // 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 void test_attach34(T aaa, U bbb); @@ -542,7 +600,7 @@ template 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 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::test_attach36(int aaa, int bbb) {} @@ -559,20 +617,20 @@ template 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 void test_attach37::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::test_attach38(int aaa, int bbb) {} diff --git a/test/Sema/warn-documentation.m b/test/Sema/warn-documentation.m index 7b6361c1f4..d6af6edcc8 100644 --- 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