Index: include/clang/Format/Format.h =================================================================== --- include/clang/Format/Format.h +++ include/clang/Format/Format.h @@ -828,6 +828,15 @@ ArrayRef Ranges, StringRef FileName = ""); +/// \brief Fix namespace end comments in the given \p Ranges in \p Code. +/// +/// Returns the ``Replacements`` that fix the namespace comments in all +/// \p Ranges in \p Code. +tooling::Replacements fixNamespaceEndComments(const FormatStyle &Style, + StringRef Code, + ArrayRef Ranges, + StringRef FileName = ""); + /// \brief Returns the ``LangOpts`` that the formatter expects you to set. /// /// \param Style determines specific settings for lexing mode. Index: lib/Format/CMakeLists.txt =================================================================== --- lib/Format/CMakeLists.txt +++ lib/Format/CMakeLists.txt @@ -7,6 +7,7 @@ Format.cpp FormatToken.cpp FormatTokenLexer.cpp + NamespaceEndCommentsFixer.cpp SortJavaScriptImports.cpp TokenAnalyzer.cpp TokenAnnotator.cpp Index: lib/Format/Format.cpp =================================================================== --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -17,6 +17,7 @@ #include "AffectedRangeManager.h" #include "ContinuationIndenter.h" #include "FormatTokenLexer.h" +#include "NamespaceEndCommentsFixer.h" #include "SortJavaScriptImports.h" #include "TokenAnalyzer.h" #include "TokenAnnotator.h" @@ -1858,6 +1859,16 @@ return Clean.process(); } +tooling::Replacements fixNamespaceEndComments(const FormatStyle &Style, + StringRef Code, + ArrayRef Ranges, + StringRef FileName) { + std::unique_ptr Env = + Environment::CreateVirtualEnvironment(Code, FileName, Ranges); + NamespaceEndCommentsFixer Fix(*Env, Style); + return Fix.process(); +} + LangOptions getFormattingLangOpts(const FormatStyle &Style) { LangOptions LangOpts; LangOpts.CPlusPlus = 1; Index: lib/Format/NamespaceEndCommentsFixer.h =================================================================== --- /dev/null +++ lib/Format/NamespaceEndCommentsFixer.h @@ -0,0 +1,37 @@ +//===--- NamespaceEndCommentsFixer.h ----------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// \brief This file declares NamespaceEndCommentsFixer, a TokenAnalyzer that +/// fixes namespace end comments. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_FORMAT_NAMESPACEENDCOMMENTSFIXER_H +#define LLVM_CLANG_LIB_FORMAT_NAMESPACEENDCOMMENTSFIXER_H + +#include "TokenAnalyzer.h" + +namespace clang { +namespace format { + +class NamespaceEndCommentsFixer : public TokenAnalyzer { +public: + NamespaceEndCommentsFixer(const Environment &Env, const FormatStyle &Style); + + tooling::Replacements + analyze(TokenAnnotator &Annotator, + SmallVectorImpl &AnnotatedLines, + FormatTokenLexer &Tokens) override; +}; + +} // end namespace format +} // end namespace clang + +#endif Index: lib/Format/NamespaceEndCommentsFixer.cpp =================================================================== --- /dev/null +++ lib/Format/NamespaceEndCommentsFixer.cpp @@ -0,0 +1,166 @@ +//===--- NamespaceEndCommentsFixer.cpp --------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// \brief This file implements NamespaceEndCommentsFixer, a TokenAnalyzer that +/// fixes namespace end comments. +/// +//===----------------------------------------------------------------------===// + +#include "NamespaceEndCommentsFixer.h" +#include "llvm/Support/Debug.h" +#include "llvm/Support/Regex.h" + +#define DEBUG_TYPE "namespace-end-comments-fixer" + +namespace clang { +namespace format { + +namespace { +// The maximal number of lines that a short namespace spans. +// Short namespaces don't need an end comment. +static const int kShortNamespaceMaxLines = 1; + +// Matches a valid namespace end comment. +// Valid namespace end comments don't need to be edited. +static llvm::Regex kNamespaceCommentPattern = + llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *" + "namespace( +([a-zA-Z0-9:_]+))?\\.? *(\\*/)?$", + llvm::Regex::IgnoreCase); + +// Computes the name of a namespace given the namespace token. +// Returns "" for anonymous namespace. +std::string computeName(const FormatToken *NamespaceTok) { + assert(NamespaceTok && NamespaceTok->is(tok::kw_namespace) && + "expecting a namespace token"); + std::string name = ""; + // Collects all the non-comment tokens between 'namespace' and '{'. + const FormatToken *Tok = NamespaceTok->getNextNonComment(); + while (Tok && !Tok->is(tok::l_brace)) { + name += Tok->TokenText; + ; + Tok = Tok->getNextNonComment(); + } + return name; +} + +std::string computeEndCommentText(StringRef NamespaceName, bool AddNewline) { + std::string text = "// namespace"; + if (!NamespaceName.empty()) { + text += ' '; + text += NamespaceName; + } + if (AddNewline) + text += '\n'; + return text; +} + +bool isShort(const FormatToken *NamespaceTok, const FormatToken *RBraceTok, + const SourceManager &SourceMgr) { + int StartLine = + SourceMgr.getSpellingLineNumber(NamespaceTok->Tok.getLocation()); + int EndLine = SourceMgr.getSpellingLineNumber(RBraceTok->Tok.getLocation()); + return EndLine - StartLine + 1 <= kShortNamespaceMaxLines; +} + +bool hasEndComment(const FormatToken *RBraceTok) { + return RBraceTok->Next && RBraceTok->Next->is(tok::comment); +} + +bool validEndComment(const FormatToken *RBraceTok, StringRef NamespaceName) { + assert(hasEndComment(RBraceTok)); + const FormatToken *Comment = RBraceTok->Next; + SmallVector Groups; + if (kNamespaceCommentPattern.match(Comment->TokenText, &Groups)) { + StringRef NamespaceNameInComment = Groups.size() > 5 ? Groups[5] : ""; + // Anonymous namespace comments must not mention a namespace name. + if (NamespaceName.empty() && !NamespaceNameInComment.empty()) + return false; + StringRef AnonymousInComment = Groups.size() > 3 ? Groups[3] : ""; + // Named namespace comments must not mention anonymous namespace. + if (!NamespaceName.empty() && !AnonymousInComment.empty()) + return false; + return NamespaceNameInComment == NamespaceName; + } + return false; +} + +void addEndComment(const FormatToken *RBraceTok, StringRef EndCommentText, + const SourceManager &SourceMgr, + tooling::Replacements *Fixes) { + auto EndLoc = RBraceTok->Tok.getEndLoc(); + auto Range = CharSourceRange::getCharRange(EndLoc, EndLoc); + auto Err = Fixes->add(tooling::Replacement(SourceMgr, Range, EndCommentText)); + if (Err) { + llvm::errs() << "Error while adding namespace end comment: " + << llvm::toString(std::move(Err)) << "\n"; + } +} + +void updateEndComment(const FormatToken *RBraceTok, StringRef EndCommentText, + const SourceManager &SourceMgr, + tooling::Replacements *Fixes) { + assert(hasEndComment(RBraceTok)); + const FormatToken *Comment = RBraceTok->Next; + auto Range = CharSourceRange::getCharRange(Comment->getStartOfNonWhitespace(), + Comment->Tok.getEndLoc()); + auto Err = Fixes->add(tooling::Replacement(SourceMgr, Range, EndCommentText)); + if (Err) { + llvm::errs() << "Error while updating namespace end comment: " + << llvm::toString(std::move(Err)) << "\n"; + } +} +} // namespace + +NamespaceEndCommentsFixer::NamespaceEndCommentsFixer(const Environment &Env, + const FormatStyle &Style) + : TokenAnalyzer(Env, Style) {} + +tooling::Replacements NamespaceEndCommentsFixer::analyze( + TokenAnnotator &Annotator, SmallVectorImpl &AnnotatedLines, + FormatTokenLexer &Tokens) { + const SourceManager &SourceMgr = Env.getSourceManager(); + AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(), + AnnotatedLines.end()); + tooling::Replacements Fixes; + for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) { + if (!AnnotatedLines[I]->Affected || AnnotatedLines[I]->InPPDirective || + !AnnotatedLines[I]->startsWith(tok::r_brace)) + continue; + const AnnotatedLine *EndLine = AnnotatedLines[I]; + size_t StartLineIndex = EndLine->MatchingOpeningBlockLineIndex; + if (StartLineIndex == UnwrappedLine::kInvalidIndex) + continue; + assert(StartLineIndex < E); + const FormatToken *NamespaceTok = AnnotatedLines[StartLineIndex]->First; + // Detect "(inline)? namespace" in the beginning of a line. + if (NamespaceTok->is(tok::kw_inline)) + NamespaceTok = NamespaceTok->getNextNonComment(); + if (NamespaceTok->isNot(tok::kw_namespace)) + continue; + const FormatToken *RBraceTok = EndLine->First; + const std::string NamespaceName = computeName(NamespaceTok); + bool AddNewline = (I + 1 < E) && + AnnotatedLines[I + 1]->First->NewlinesBefore == 0 && + AnnotatedLines[I + 1]->First->isNot(tok::eof); + const std::string EndCommentText = + computeEndCommentText(NamespaceName, AddNewline); + if (!hasEndComment(RBraceTok)) { + if (!isShort(NamespaceTok, RBraceTok, SourceMgr)) + addEndComment(RBraceTok, EndCommentText, SourceMgr, &Fixes); + continue; + } + if (!validEndComment(RBraceTok, NamespaceName)) + updateEndComment(RBraceTok, EndCommentText, SourceMgr, &Fixes); + } + return Fixes; +} + +} // namespace format +} // namespace clang Index: lib/Format/TokenAnnotator.h =================================================================== --- lib/Format/TokenAnnotator.h +++ lib/Format/TokenAnnotator.h @@ -39,6 +39,7 @@ public: AnnotatedLine(const UnwrappedLine &Line) : First(Line.Tokens.front().Tok), Level(Line.Level), + MatchingOpeningBlockLineIndex(Line.MatchingOpeningBlockLineIndex), InPPDirective(Line.InPPDirective), MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false), IsMultiVariableDeclStmt(false), Affected(false), @@ -109,6 +110,7 @@ LineType Type; unsigned Level; + size_t MatchingOpeningBlockLineIndex; bool InPPDirective; bool MustBeDeclaration; bool MightBeFunctionDecl; Index: lib/Format/UnwrappedLineParser.h =================================================================== --- lib/Format/UnwrappedLineParser.h +++ lib/Format/UnwrappedLineParser.h @@ -48,6 +48,14 @@ bool InPPDirective; bool MustBeDeclaration; + + /// \brief If this \c UnwrappedLine closes a block in a sequence of lines, + /// \c MatchingOpeningBlockLineIndex stores the index of the corresponding + /// opening line. Otherwise, \c MatchingOpeningBlockLineIndex must be + /// \c kInvalidIndex. + size_t MatchingOpeningBlockLineIndex; + + static const size_t kInvalidIndex = -1; }; class UnwrappedLineConsumer { @@ -234,8 +242,8 @@ SmallVector Children; }; -inline UnwrappedLine::UnwrappedLine() - : Level(0), InPPDirective(false), MustBeDeclaration(false) {} +inline UnwrappedLine::UnwrappedLine() : Level(0), InPPDirective(false), + MustBeDeclaration(false), MatchingOpeningBlockLineIndex(kInvalidIndex) {} } // end namespace format } // end namespace clang Index: lib/Format/UnwrappedLineParser.cpp =================================================================== --- lib/Format/UnwrappedLineParser.cpp +++ lib/Format/UnwrappedLineParser.cpp @@ -428,6 +428,8 @@ parseParens(); addUnwrappedLine(); + size_t OpeningLineIndex = + Lines.empty() ? (UnwrappedLine::kInvalidIndex) : (Lines.size() - 1); ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack, MustBeDeclaration); @@ -453,6 +455,7 @@ if (MunchSemi && FormatTok->Tok.is(tok::semi)) nextToken(); Line->Level = InitialLevel; + Line->MatchingOpeningBlockLineIndex = OpeningLineIndex; } static bool isGoogScope(const UnwrappedLine &Line) { Index: unittests/Format/CMakeLists.txt =================================================================== --- unittests/Format/CMakeLists.txt +++ unittests/Format/CMakeLists.txt @@ -6,11 +6,12 @@ CleanupTest.cpp FormatTest.cpp FormatTestComments.cpp - FormatTestJava.cpp FormatTestJS.cpp + FormatTestJava.cpp FormatTestObjC.cpp FormatTestProto.cpp FormatTestSelective.cpp + NamespaceEndCommentsFixerTest.cpp SortImportsTestJS.cpp SortIncludesTest.cpp ) Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp =================================================================== --- /dev/null +++ unittests/Format/NamespaceEndCommentsFixerTest.cpp @@ -0,0 +1,350 @@ +//===- NamespaceEndCommentsFixerTest.cpp - Formatting unit tests ----------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "clang/Format/Format.h" + +#include "clang/Frontend/TextDiagnosticPrinter.h" +#include "llvm/Support/Debug.h" +#include "gtest/gtest.h" + +#define DEBUG_TYPE "namespace-end-comments-fixer-test" + +namespace clang { +namespace format { +namespace { + +class NamespaceEndCommentsFixerTest : public ::testing::Test { +protected: + std::string + fixNamespaceEndComments(llvm::StringRef Code, + std::vector Ranges, + const FormatStyle &Style = getLLVMStyle()) { + DEBUG(llvm::errs() << "---\n"); + DEBUG(llvm::errs() << Code << "\n\n"); + tooling::Replacements Replaces = + clang::format::fixNamespaceEndComments(Style, Code, Ranges, ""); + auto Result = applyAllReplacements(Code, Replaces); + EXPECT_TRUE(static_cast(Result)); + DEBUG(llvm::errs() << "\n" << *Result << "\n\n"); + return *Result; + } + + std::string + fixNamespaceEndComments(llvm::StringRef Code, + const FormatStyle &Style = getLLVMStyle()) { + return fixNamespaceEndComments( + Code, + /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style); + } +}; + +TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) { + EXPECT_EQ("namespace {\n" + " int i;\n" + "}// namespace", + fixNamespaceEndComments("namespace {\n" + " int i;\n" + "}")); + EXPECT_EQ("namespace {\n" + " int i;\n" + "}// namespace\n", + fixNamespaceEndComments("namespace {\n" + " int i;\n" + "}\n")); + EXPECT_EQ("namespace A {\n" + " int i;\n" + "}// namespace A", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "}")); + EXPECT_EQ("inline namespace A {\n" + " int i;\n" + "}// namespace A", + fixNamespaceEndComments("inline namespace A {\n" + " int i;\n" + "}")); + EXPECT_EQ("namespace ::A {\n" + " int i;\n" + "}// namespace ::A", + fixNamespaceEndComments("namespace ::A {\n" + " int i;\n" + "}")); + EXPECT_EQ("namespace ::A::B {\n" + " int i;\n" + "}// namespace ::A::B", + fixNamespaceEndComments("namespace ::A::B {\n" + " int i;\n" + "}")); + EXPECT_EQ("namespace /**/::/**/A/**/::/**/B/**/ {\n" + " int i;\n" + "}// namespace ::A::B", + fixNamespaceEndComments("namespace /**/::/**/A/**/::/**/B/**/ {\n" + " int i;\n" + "}")); + EXPECT_EQ("namespace A {\n" + "namespace B {\n" + " int i;\n" + "}// namespace B\n" + "}// namespace A", + fixNamespaceEndComments("namespace A {\n" + "namespace B {\n" + " int i;\n" + "}\n" + "}")); + EXPECT_EQ("namespace A {\n" + " int a;\n" + "}// namespace A\n" + "namespace B {\n" + " int b;\n" + "}// namespace B", + fixNamespaceEndComments("namespace A {\n" + " int a;\n" + "}\n" + "namespace B {\n" + " int b;\n" + "}")); + EXPECT_EQ("namespace A {\n" + " int a1;\n" + "}// namespace A\n" + "namespace A {\n" + " int a2;\n" + "}// namespace A", + fixNamespaceEndComments("namespace A {\n" + " int a1;\n" + "}\n" + "namespace A {\n" + " int a2;\n" + "}")); + EXPECT_EQ("namespace A {\n" + " int a;\n" + "}// namespace A\n" + "// comment about b\n" + "int b;", + fixNamespaceEndComments("namespace A {\n" + " int a;\n" + "}\n" + "// comment about b\n" + "int b;")); + + EXPECT_EQ("namespace A {\n" + "namespace B {\n" + "namespace C {\n" + "namespace D {\n" + "}// namespace D\n" + "}// namespace C\n" + "}// namespace B\n" + "}// namespace A", + fixNamespaceEndComments("namespace A {\n" + "namespace B {\n" + "namespace C {\n" + "namespace D {\n" + "}\n" + "}\n" + "}\n" + "}")); +} + +TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) { + EXPECT_EQ("namespace A {\n" + " int i;\n" + "}// namespace A\n" + " int j;", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "} int j;")); + EXPECT_EQ("namespace {\n" + " int i;\n" + "}// namespace\n" + " int j;", + fixNamespaceEndComments("namespace {\n" + " int i;\n" + "} int j;")); + EXPECT_EQ("namespace A {\n" + " int i;\n" + "}// namespace A\n" + " namespace B {\n" + " int j;\n" + "}// namespace B", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "} namespace B {\n" + " int j;\n" + "}")); +} + +TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddEndCommentForShortNamespace) { + EXPECT_EQ("namespace {}", fixNamespaceEndComments("namespace {}")); + EXPECT_EQ("namespace A {}", fixNamespaceEndComments("namespace A {}")); + EXPECT_EQ("namespace A { int i; }", + fixNamespaceEndComments("namespace A { int i; }")); +} + +TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddCommentAfterUnaffectedRBrace) { + EXPECT_EQ("namespace A {\n" + " int i;\n" + "}", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "}", + // The range (16, 3) spans the 'int' above. + /*Ranges=*/{1, tooling::Range(16, 3)})); +} + +TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddCommentAfterRBraceInPPDirective) { + EXPECT_EQ("#define SAD \\\n" + "namespace A { \\\n" + " int i; \\\n" + "}", + fixNamespaceEndComments("#define SAD \\\n" + "namespace A { \\\n" + " int i; \\\n" + "}")); +} + +TEST_F(NamespaceEndCommentsFixerTest, KeepsValidEndComment) { + EXPECT_EQ("namespace {\n" + " int i;\n" + "} // end anonymous namespace", + fixNamespaceEndComments("namespace {\n" + " int i;\n" + "} // end anonymous namespace")); + EXPECT_EQ("namespace A {\n" + " int i;\n" + "} /* end of namespace A */", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "} /* end of namespace A */")); + EXPECT_EQ("namespace A {\n" + " int i;\n" + "} // namespace A", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "} // namespace A")); + EXPECT_EQ("namespace A::B {\n" + " int i;\n" + "} // end namespace A::B", + fixNamespaceEndComments("namespace A::B {\n" + " int i;\n" + "} // end namespace A::B")); +} + +TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndLineComment) { + EXPECT_EQ("namespace {\n" + " int i;\n" + "} // namespace", + fixNamespaceEndComments("namespace {\n" + " int i;\n" + "} // namespace A")); + EXPECT_EQ("namespace A {\n" + " int i;\n" + "} // namespace A", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "} // namespace")); + EXPECT_EQ("namespace A {\n" + " int i;\n" + "} // namespace A", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "} //")); + EXPECT_EQ("namespace A {\n" + " int i;\n" + "} // namespace A", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "} //")); + EXPECT_EQ("namespace A {\n" + " int i;\n" + "} // namespace A", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "} // banamespace A")); + + // Updates invalid line comments even for short namespaces. + EXPECT_EQ("namespace A {} // namespace A", + fixNamespaceEndComments("namespace A {} // namespace")); +} + +TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndBlockComment) { + EXPECT_EQ("namespace {\n" + " int i;\n" + "} // namespace", + fixNamespaceEndComments("namespace {\n" + " int i;\n" + "} /* namespace A */")); + EXPECT_EQ("namespace A {\n" + " int i;\n" + "} // namespace A", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "} /* end namespace */")); + EXPECT_EQ("namespace A {\n" + " int i;\n" + "} // namespace A", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "} /**/")); + EXPECT_EQ("namespace A {\n" + " int i;\n" + "} // namespace A", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "} /* end unnamed namespace */")); + EXPECT_EQ("namespace A {\n" + " int i;\n" + "} // namespace A", + fixNamespaceEndComments("namespace A {\n" + " int i;\n" + "} /* banamespace A */")); + EXPECT_EQ("namespace A {} // namespace A", + fixNamespaceEndComments("namespace A {} /**/")); +} + +TEST_F(NamespaceEndCommentsFixerTest, + DoesNotAddEndCommentForNamespacesControlledByMacros) { + EXPECT_EQ("#ifdef 1\n" + "namespace A {\n" + "#elseif\n" + "namespace B {\n" + "#endif\n" + " int i;\n" + "}\n" + "}\n", + fixNamespaceEndComments("#ifdef 1\n" + "namespace A {\n" + "#elseif\n" + "namespace B {\n" + "#endif\n" + " int i;\n" + "}\n" + "}\n")); +} + +TEST_F(NamespaceEndCommentsFixerTest, + DoesNotAddEndCommentForNamespacesInMacroDeclarations) { + EXPECT_EQ("#ifdef 1\n" + "namespace A {\n" + "#elseif\n" + "namespace B {\n" + "#endif\n" + " int i;\n" + "}\n" + "}\n", + fixNamespaceEndComments("#ifdef 1\n" + "namespace A {\n" + "#elseif\n" + "namespace B {\n" + "#endif\n" + " int i;\n" + "}\n" + "}\n")); +} +} // end namespace +} // end namespace format +} // end namespace clang