njames93 updated this revision to Diff 277348.
njames93 added a comment.
Added /**/ style license comment test cases
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83223/new/
https://reviews.llvm.org/D83223
Files:
clang-tools-extra/clang-tidy/ClangTidyCheck.h
clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
clang-tools-extra/clang-tidy/utils/HeaderGuard.h
clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -221,6 +221,123 @@
"", "/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")));
+}
#endif
} // namespace test
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ 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);
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
+++ 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<SourceLocation, NewLineInsertions>
+ 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();
}
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -45,7 +45,7 @@
class MissingOptionError : public OptionError<MissingOptionError> {
public:
explicit MissingOptionError(std::string OptionName)
- : OptionName(OptionName) {}
+ : OptionName(std::move(OptionName)) {}
std::string message() const override;
static char ID;
@@ -58,12 +58,13 @@
public:
explicit UnparseableEnumOptionError(std::string LookupName,
std::string LookupValue)
- : LookupName(LookupName), LookupValue(LookupValue) {}
+ : LookupName(std::move(LookupName)), LookupValue(std::move(LookupValue)) {
+ }
explicit UnparseableEnumOptionError(std::string LookupName,
std::string LookupValue,
std::string SuggestedValue)
- : LookupName(LookupName), LookupValue(LookupValue),
- SuggestedValue(SuggestedValue) {}
+ : LookupName(std::move(LookupName)), LookupValue(std::move(LookupValue)),
+ SuggestedValue(std::move(SuggestedValue)) {}
std::string message() const override;
static char ID;
@@ -80,8 +81,8 @@
explicit UnparseableIntegerOptionError(std::string LookupName,
std::string LookupValue,
bool IsBoolean = false)
- : LookupName(LookupName), LookupValue(LookupValue), IsBoolean(IsBoolean) {
- }
+ : LookupName(std::move(LookupName)), LookupValue(std::move(LookupValue)),
+ IsBoolean(IsBoolean) {}
std::string message() const override;
static char ID;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
