Hi djasper,
This patch makes the check work better for LLVM code:
* always fix existing #endif comments
* use one space before the comment (+allow customization for other styles)
http://reviews.llvm.org/D5795
Files:
clang-tidy/utils/HeaderGuard.cpp
clang-tidy/utils/HeaderGuard.h
unittests/clang-tidy/LLVMModuleTest.cpp
Index: clang-tidy/utils/HeaderGuard.cpp
===================================================================
--- clang-tidy/utils/HeaderGuard.cpp
+++ clang-tidy/utils/HeaderGuard.cpp
@@ -150,24 +150,29 @@
bool wouldFixEndifComment(StringRef FileName, SourceLocation EndIf,
StringRef HeaderGuard,
size_t *EndIfLenPtr = nullptr) {
- if (!Check->shouldSuggestEndifComment(FileName))
+ if (!EndIf.isValid())
return false;
-
const char *EndIfData = PP->getSourceManager().getCharacterData(EndIf);
size_t EndIfLen = std::strcspn(EndIfData, "\r\n");
if (EndIfLenPtr)
*EndIfLenPtr = EndIfLen;
StringRef EndIfStr(EndIfData, EndIfLen);
+ EndIfStr = EndIfStr.substr(EndIfStr.find_first_not_of("#endif \t"));
// Give up if there's an escaped newline.
size_t FindEscapedNewline = EndIfStr.find_last_not_of(' ');
if (FindEscapedNewline != StringRef::npos &&
EndIfStr[FindEscapedNewline] == '\\')
return false;
- return (EndIf.isValid() && !EndIfStr.endswith("// " + HeaderGuard.str()) &&
- !EndIfStr.endswith("/* " + HeaderGuard.str() + " */"));
+ if (!Check->shouldSuggestEndifComment(FileName) &&
+ !(EndIfStr.startswith("//") ||
+ (EndIfStr.startswith("/*") && EndIfStr.endswith("*/"))))
+ return false;
+
+ return (EndIfStr != "// " + HeaderGuard.str()) &&
+ (EndIfStr != "/* " + HeaderGuard.str() + " */");
}
/// \brief Look for header guards that don't match the preferred style. Emit
@@ -207,11 +212,10 @@
std::vector<FixItHint> &FixIts) {
size_t EndIfLen;
if (wouldFixEndifComment(FileName, EndIf, HeaderGuard, &EndIfLen)) {
- std::string Correct = "endif // " + HeaderGuard.str();
FixIts.push_back(FixItHint::CreateReplacement(
CharSourceRange::getCharRange(EndIf,
EndIf.getLocWithOffset(EndIfLen)),
- Correct));
+ Check->formatEndIf(HeaderGuard)));
}
}
@@ -261,7 +265,7 @@
<< FixItHint::CreateInsertion(
SM.getLocForEndOfFile(FID),
Check->shouldSuggestEndifComment(FileName)
- ? "\n#endif // " + CPPVar + "\n"
+ ? "\n#" + Check->formatEndIf(CPPVar) + "\n"
: "\n#endif\n");
}
}
@@ -294,5 +298,9 @@
return FileName.endswith(".h");
}
+std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) {
+ return "endif // " + HeaderGuard.str();
+}
+
} // namespace tidy
} // namespace clang
Index: clang-tidy/utils/HeaderGuard.h
===================================================================
--- clang-tidy/utils/HeaderGuard.h
+++ clang-tidy/utils/HeaderGuard.h
@@ -32,6 +32,9 @@
/// \brief Returns true if the checker should add a header guard to the file
/// if it has none.
virtual bool shouldSuggestToAddHeaderGuard(StringRef Filename);
+ /// \brief Returns a replacement for endif line with a comment mentioning
+ /// \p HeaderGuard. The replacement should start with "endif".
+ virtual std::string formatEndIf(StringRef HeaderGuard);
/// \brief Get the canonical header guard for a file.
virtual std::string getHeaderGuard(StringRef Filename,
StringRef OldGuard = StringRef()) = 0;
Index: unittests/clang-tidy/LLVMModuleTest.cpp
===================================================================
--- unittests/clang-tidy/LLVMModuleTest.cpp
+++ unittests/clang-tidy/LLVMModuleTest.cpp
@@ -103,18 +103,28 @@
"#endif\n",
"include/clang/bar.h", /*ExpectedWarnings=*/1));
+ // Fix incorrect #endif comments even if we shouldn't add new ones.
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
- "#endif // LLVM_ADT_FOO_H\n",
+ "#endif // LLVM_ADT_FOO_H\n",
+ runHeaderGuardCheck("#ifndef FOO\n"
+ "#define FOO\n"
+ "#endif // FOO\n",
+ "include/llvm/ADT/foo.h",
+ /*ExpectedWarnings=*/1));
+
+ EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
+ "#define LLVM_ADT_FOO_H\n"
+ "#endif // LLVM_ADT_FOO_H\n",
runHeaderGuardCheckWithEndif("#ifndef FOO\n"
"#define FOO\n"
"#endif\n",
"include/llvm/ADT/foo.h",
/*ExpectedWarnings=*/1));
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
- "#endif // LLVM_ADT_FOO_H\n",
+ "#endif // LLVM_ADT_FOO_H\n",
runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
"#endif // LLVM_H\n",
@@ -141,7 +151,7 @@
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
- "#endif // LLVM_ADT_FOO_H\n",
+ "#endif // LLVM_ADT_FOO_H\n",
runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H_\n"
"#define LLVM_ADT_FOO_H_\n"
"#endif // LLVM\n",
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits