diff --git a/clang-tools-extra/clang-tidy/utils/HeaderGuard.h b/clang-tools-extra/clang-tidy/utils/HeaderGuard.h --- a/clang-tools-extra/clang-tidy/utils/HeaderGuard.h +++ b/clang-tools-extra/clang-tidy/utils/HeaderGuard.h @@ -48,6 +48,9 @@ /// Returns ``true`` if the check should add a header guard to the file /// if it has none. virtual bool shouldSuggestToAddHeaderGuard(StringRef Filename); + /// Returns ``true`` if the check should skip past a block comment at the + /// start of a file when trying to add a header guard. + virtual bool shouldSkipLicenseComments(StringRef Filename); /// Returns a replacement for the ``#endif`` line with a comment mentioning /// \p HeaderGuard. The replacement should start with ``endif``. virtual std::string formatEndIf(StringRef HeaderGuard); diff --git a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp --- a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp +++ b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp @@ -203,12 +203,72 @@ } } + enum class NewLineInsertions { None, One, Two }; + + /// Gets the first SourceLocation in a file after stripping the first block of + /// comments if its not attached to code, as this is likely a license. The + /// second item returned is the number of new lines that will need to be + /// inserted when creating the header guard. + std::pair + getLocAfterLicense(FileID File, const Preprocessor *PP) { + const SourceManager &SM = PP->getSourceManager(); + SourceLocation StartLoc = SM.getLocForStartOfFile(File); + bool Invalid = false; + StringRef Buffer = SM.getBufferData(File, &Invalid); + if (Invalid || Buffer.empty() || isWhitespace(Buffer.front())) + return std::make_pair(StartLoc, NewLineInsertions::None); + + // Create a lexer starting at the beginning of this token. + Lexer TheLexer(StartLoc, PP->getLangOpts(), Buffer.begin(), Buffer.data(), + Buffer.end()); + TheLexer.SetCommentRetentionState(true); + Token Cur; + if (TheLexer.LexFromRawLexer(Cur)) { + // Only one token in stream. If its a comment, presume its a license and + // return the location following the comment, otherwise return start of + // file. + if (Cur.is(tok::comment)) + return std::make_pair(TheLexer.getSourceLocation(), + NewLineInsertions::Two); + return std::make_pair(StartLoc, NewLineInsertions::None); + } + auto HasTwoTrailingNewLines = [&] { + StringRef Next = Buffer.drop_front(TheLexer.getCurrentBufferOffset()); + StringRef WS = Next.take_while(isWhitespace); + return WS.count('\n') > 1; + }; + if (HasTwoTrailingNewLines()) { + // Return the start of the next token. + TheLexer.LexFromRawLexer(Cur); + return std::make_pair(Cur.getLocation(), NewLineInsertions::None); + } + while (true) { + if (TheLexer.LexFromRawLexer(Cur)) { + if (Cur.is(tok::comment)) + return std::make_pair(TheLexer.getSourceLocation(), + NewLineInsertions::Two); + if (Cur.is(tok::eof)) + return std::make_pair(TheLexer.getSourceLocation(), + NewLineInsertions::One); + return std::make_pair(StartLoc, NewLineInsertions::None); + } + if (HasTwoTrailingNewLines()) { + TheLexer.LexFromRawLexer(Cur); + return std::make_pair(Cur.getLocation(), NewLineInsertions::None); + } + if (Cur.is(tok::eof)) + return std::make_pair(TheLexer.getSourceLocation(), + NewLineInsertions::One); + if (Cur.isNot(tok::comment)) + return std::make_pair(StartLoc, NewLineInsertions::None); + } + } + /// Looks for files that were visited but didn't have a header guard. /// Emits a warning with fixits suggesting adding one. void checkGuardlessHeaders() { // Look for header files that didn't have a header guard. Emit a warning and // fix-its to add the guard. - // TODO: Insert the guard after top comments. for (const auto &FE : Files) { StringRef FileName = FE.getKey(); if (!Check->shouldSuggestToAddHeaderGuard(FileName)) @@ -216,7 +276,15 @@ SourceManager &SM = PP->getSourceManager(); FileID FID = SM.translateFile(FE.getValue()); - SourceLocation StartLoc = SM.getLocForStartOfFile(FID); + SourceLocation StartLoc; + NewLineInsertions AddNewLine; + if (Check->shouldSkipLicenseComments(FileName)) { + std::tie(StartLoc, AddNewLine) = getLocAfterLicense(FID, PP); + } else { + StartLoc = SM.getLocForStartOfFile(FID); + AddNewLine = NewLineInsertions::None; + } + if (StartLoc.isInvalid()) continue; @@ -242,10 +310,14 @@ if (SeenMacro) continue; - + StringRef Opening = AddNewLine == NewLineInsertions::None ? "#ifndef " + : AddNewLine == NewLineInsertions::One + ? "\n#ifndef " + : "\n\n#ifndef "; Check->diag(StartLoc, "header is missing header guard") << FixItHint::CreateInsertion( - StartLoc, "#ifndef " + CPPVar + "\n#define " + CPPVar + "\n\n") + StartLoc, + (Opening + CPPVar + "\n#define " + CPPVar + "\n\n").str()) << FixItHint::CreateInsertion( SM.getLocForEndOfFile(FID), Check->shouldSuggestEndifComment(FileName) @@ -286,6 +358,10 @@ return utils::isFileExtension(FileName, HeaderFileExtensions); } +bool HeaderGuardCheck::shouldSkipLicenseComments(StringRef FileName) { + return true; +} + std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) { return "endif // " + HeaderGuard.str(); } diff --git a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp --- a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp @@ -221,6 +221,171 @@ "", "/llvm-project/clang-tools-extra/clangd/foo.h", StringRef("header is missing header guard"))); } + +TEST(LLVMHeaderGuardCheckTest, FixHeaderGuardsWithComments) { + EXPECT_EQ("// LicenseText //\n" + "\n" + "#ifndef LLVM_CLANG_BAR_H\n" + "#define LLVM_CLANG_BAR_H\n" + "\n" + "\n" + "#endif\n", + runHeaderGuardCheck("// LicenseText //", "include/clang/bar.h", + StringRef("header is missing header guard"))); + + EXPECT_EQ("// LicenseText //\n" + "\n" + "#ifndef LLVM_CLANG_BAR_H\n" + "#define LLVM_CLANG_BAR_H\n" + "\n" + "\n" + "#endif\n", + runHeaderGuardCheck("// LicenseText //\n", "include/clang/bar.h", + StringRef("header is missing header guard"))); + EXPECT_EQ("// LicenseText //\n" + "// SecondLine //\n" + "// ThirdLine //\n" + "\n" + "#ifndef LLVM_CLANG_BAR_H\n" + "#define LLVM_CLANG_BAR_H\n" + "\n" + "\n" + "#endif\n", + runHeaderGuardCheck("// LicenseText //\n" + "// SecondLine //\n" + "// ThirdLine //\n", + "include/clang/bar.h", + StringRef("header is missing header guard"))); + EXPECT_EQ("// LicenseText //\n" + "// SecondLine //\n" + "// ThirdLine //\n" + "\n" + "#ifndef LLVM_CLANG_BAR_H\n" + "#define LLVM_CLANG_BAR_H\n" + "\n" + "\n" + "#endif\n", + runHeaderGuardCheck("// LicenseText //\n" + "// SecondLine //\n" + "// ThirdLine //", + "include/clang/bar.h", + StringRef("header is missing header guard"))); + + EXPECT_EQ("// LicenseText //\n" + "// SecondLine //\n" + "// ThirdLine //\n" + "\n" + "#ifndef LLVM_CLANG_BAR_H\n" + "#define LLVM_CLANG_BAR_H\n" + "\n" + "// FunctionDoc\n" + "void Foo();\n" + "\n" + "#endif\n", + runHeaderGuardCheck("// LicenseText //\n" + "// SecondLine //\n" + "// ThirdLine //\n" + "\n" + "// FunctionDoc\n" + "void Foo();\n", + "include/clang/bar.h", + StringRef("header is missing header guard"))); + + EXPECT_EQ("#ifndef LLVM_CLANG_BAR_H\n" + "#define LLVM_CLANG_BAR_H\n" + "\n" + "// Function Doc\n" + "void foo();\n" + "\n" + "#endif\n", + runHeaderGuardCheck("// Function Doc\n" + "void foo();\n", + "include/clang/bar.h", + StringRef("header is missing header guard"))); + + EXPECT_EQ("/* LicenseText \n" + " SecondLine \n" + " ThirdLine */\n" + "\n" + "#ifndef LLVM_CLANG_BAR_H\n" + "#define LLVM_CLANG_BAR_H\n" + "\n" + "\n" + "#endif\n", + runHeaderGuardCheck("/* LicenseText \n" + " SecondLine \n" + " ThirdLine */\n", + "include/clang/bar.h", + StringRef("header is missing header guard"))); + + EXPECT_EQ("/* LicenseText \n" + " SecondLine \n" + " ThirdLine */\n" + "\n" + "#ifndef LLVM_CLANG_BAR_H\n" + "#define LLVM_CLANG_BAR_H\n" + "\n" + "// FunctionDoc\n" + "void Foo();\n" + "\n" + "#endif\n", + runHeaderGuardCheck("/* LicenseText \n" + " SecondLine \n" + " ThirdLine */\n" + "\n" + "// FunctionDoc\n" + "void Foo();\n", + "include/clang/bar.h", + StringRef("header is missing header guard"))); + + EXPECT_EQ("// LicenseText //\n" + "// SecondLine //\n" + "// ThirdLine //\n" + "\n" + "#ifndef LLVM_CLANG_BAR_H\n" + "#define LLVM_CLANG_BAR_H\n" + "\n" + "// FunctionDoc\n" + "void Foo();\n" + "\n" + "#endif\n", + runHeaderGuardCheck("// LicenseText //\n" + "// SecondLine //\n" + "// ThirdLine //\n" + "\n" + "#ifndef LLVM_CLANG_BAR_H\n" + "#define LLVM_CLANG_BAR_H\n" + "\n" + "// FunctionDoc\n" + "void Foo();\n" + "\n" + "#endif\n", + "include/clang/bar.h", None)); + + EXPECT_EQ("/* LicenseText \n" + " SecondLine \n" + " ThirdLine */\n" + "\n" + "#ifndef LLVM_CLANG_BAR_H\n" + "#define LLVM_CLANG_BAR_H\n" + "\n" + "// FunctionDoc\n" + "void Foo();\n" + "\n" + "#endif\n", + runHeaderGuardCheck("/* LicenseText \n" + " SecondLine \n" + " ThirdLine */\n" + "\n" + "#ifndef LLVM_CLANG_BAR_H\n" + "#define LLVM_CLANG_BAR_H\n" + "\n" + "// FunctionDoc\n" + "void Foo();\n" + "\n" + "#endif\n", + "include/clang/bar.h", None)); +} #endif } // namespace test