Author: alexfh Date: Thu Jan 11 05:00:28 2018 New Revision: 322274 URL: http://llvm.org/viewvc/llvm-project?rev=322274&view=rev Log: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces
Summary: Fixes bug 34701 When we encounter a namespace find the location of the left bracket. Then if the text between the name and the left bracket contains a ':' then it's a C++17 nested namespace. Reviewers: #clang-tools-extra, alexfh, aaron.ballman Reviewed By: aaron.ballman Subscribers: curdeius, cfe-commits, krasimir, JonasToth, JDevlieghere, xazax.hun Tags: #clang-tools-extra Patch by Alexandru Octavian Buțiu! Differential Revision: https://reviews.llvm.org/D38284 Added: clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp Modified: clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h Modified: clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp?rev=322274&r1=322273&r2=322274&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp Thu Jan 11 05:00:28 2018 @@ -23,7 +23,7 @@ NamespaceCommentCheck::NamespaceCommentC ClangTidyContext *Context) : ClangTidyCheck(Name, Context), NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *" - "namespace( +([a-zA-Z0-9_]+))?\\.? *(\\*/)?$", + "namespace( +([a-zA-Z0-9_:]+))?\\.? *(\\*/)?$", llvm::Regex::IgnoreCase), ShortNamespaceLines(Options.get("ShortNamespaceLines", 1u)), SpacesBeforeComments(Options.get("SpacesBeforeComments", 1u)) {} @@ -56,6 +56,15 @@ static std::string getNamespaceComment(c return Fix; } +static std::string getNamespaceComment(const std::string &NameSpaceName, + bool InsertLineBreak) { + std::string Fix = "// namespace "; + Fix.append(NameSpaceName); + if (InsertLineBreak) + Fix.append("\n"); + return Fix; +} + void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) { const auto *ND = Result.Nodes.getNodeAs<NamespaceDecl>("namespace"); const SourceManager &Sources = *Result.SourceManager; @@ -74,11 +83,44 @@ void NamespaceCommentCheck::check(const SourceLocation AfterRBrace = ND->getRBraceLoc().getLocWithOffset(1); SourceLocation Loc = AfterRBrace; Token Tok; + SourceLocation LBracketLocation = ND->getLocation(); + SourceLocation NestedNamespaceBegin = LBracketLocation; + + // Currently for nested namepsace (n1::n2::...) the AST matcher will match foo + // then bar instead of a single match. So if we got a nested namespace we have + // to skip the next ones. + for (const auto &EndOfNameLocation : Ends) { + if (Sources.isBeforeInTranslationUnit(NestedNamespaceBegin, + EndOfNameLocation)) + return; + } + + // Ignore macros + if (!ND->getLocation().isMacroID()) { + while (Lexer::getRawToken(LBracketLocation, Tok, Sources, getLangOpts()) || + !Tok.is(tok::l_brace)) { + LBracketLocation = LBracketLocation.getLocWithOffset(1); + } + } + + auto TextRange = + Lexer::getAsCharRange(SourceRange(NestedNamespaceBegin, LBracketLocation), + Sources, getLangOpts()); + StringRef NestedNamespaceName = + Lexer::getSourceText(TextRange, Sources, getLangOpts()).rtrim(); + bool IsNested = NestedNamespaceName.contains(':'); + + if (IsNested) + Ends.push_back(LBracketLocation); + else + NestedNamespaceName = ND->getName(); + // Skip whitespace until we find the next token. while (Lexer::getRawToken(Loc, Tok, Sources, getLangOpts()) || Tok.is(tok::semi)) { Loc = Loc.getLocWithOffset(1); } + if (!locationsInSameFile(Sources, ND->getRBraceLoc(), Loc)) return; @@ -98,10 +140,14 @@ void NamespaceCommentCheck::check(const StringRef NamespaceNameInComment = Groups.size() > 5 ? Groups[5] : ""; StringRef Anonymous = Groups.size() > 3 ? Groups[3] : ""; - // Check if the namespace in the comment is the same. - if ((ND->isAnonymousNamespace() && NamespaceNameInComment.empty()) || - (ND->getNameAsString() == NamespaceNameInComment && - Anonymous.empty())) { + if (IsNested && NestedNamespaceName == NamespaceNameInComment) { + // C++17 nested namespace. + return; + } else if ((ND->isAnonymousNamespace() && + NamespaceNameInComment.empty()) || + (ND->getNameAsString() == NamespaceNameInComment && + Anonymous.empty())) { + // Check if the namespace in the comment is the same. // FIXME: Maybe we need a strict mode, where we always fix namespace // comments with different format. return; @@ -131,13 +177,16 @@ void NamespaceCommentCheck::check(const std::string NamespaceName = ND->isAnonymousNamespace() ? "anonymous namespace" - : ("namespace '" + ND->getNameAsString() + "'"); + : ("namespace '" + NestedNamespaceName.str() + "'"); diag(AfterRBrace, Message) - << NamespaceName << FixItHint::CreateReplacement( - CharSourceRange::getCharRange(OldCommentRange), - std::string(SpacesBeforeComments, ' ') + - getNamespaceComment(ND, NeedLineBreak)); + << NamespaceName + << FixItHint::CreateReplacement( + CharSourceRange::getCharRange(OldCommentRange), + std::string(SpacesBeforeComments, ' ') + + (IsNested + ? getNamespaceComment(NestedNamespaceName, NeedLineBreak) + : getNamespaceComment(ND, NeedLineBreak))); diag(ND->getLocation(), "%0 starts here", DiagnosticIDs::Note) << NamespaceName; } Modified: clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h?rev=322274&r1=322273&r2=322274&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h Thu Jan 11 05:00:28 2018 @@ -34,6 +34,7 @@ private: llvm::Regex NamespaceCommentPattern; const unsigned ShortNamespaceLines; const unsigned SpacesBeforeComments; + llvm::SmallVector<SourceLocation, 4> Ends; }; } // namespace readability Added: clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp?rev=322274&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp (added) +++ clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp Thu Jan 11 05:00:28 2018 @@ -0,0 +1,17 @@ +// RUN: %check_clang_tidy %s google-readability-namespace-comments %t + +namespace n1::n2 { +namespace n3 { + +// So that namespace is not empty. +void f(); + + +// CHECK-MESSAGES: :[[@LINE+4]]:2: warning: namespace 'n3' not terminated with +// CHECK-MESSAGES: :[[@LINE-7]]:11: note: namespace 'n3' starts here +// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: namespace 'n1::n2' not terminated with a closing comment [google-readability-namespace-comments] +// CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'n1::n2' starts here +}} +// CHECK-FIXES: } // namespace n3 +// CHECK-FIXES: } // namespace n1::n2 + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
