[clang-tidy] When emitting header guard fixes bundle all fix-its into one

warning.

Before we would emit two warnings if the header guard was wrong and the comment
on a trailing #endif also needed fixing.

llvm-svn: 217890
This commit is contained in:
Benjamin Kramer 2014-09-16 17:41:19 +00:00
parent 7587744359
commit eb213cb1cd
2 changed files with 48 additions and 28 deletions

View File

@ -109,8 +109,7 @@ public:
EndIfs[Ifndefs[MacroEntry.first.getIdentifierInfo()].first];
// If the macro Name is not equal to what we can compute, correct it in
// the
// #ifndef and #define.
// the #ifndef and #define.
StringRef CurHeaderGuard =
MacroEntry.first.getIdentifierInfo()->getName();
std::string NewGuard = checkHeaderGuardDefinition(
@ -119,6 +118,22 @@ public:
// Now look at the #endif. We want a comment with the header guard. Fix it
// at the slightest deviation.
checkEndifComment(FileName, EndIf, NewGuard);
// Bundle all fix-its into one warning. The message depends on whether we
// changed the header guard or not.
if (!FixIts.empty()) {
if (CurHeaderGuard != NewGuard) {
auto D = Check->diag(Ifndef,
"header guard does not follow preferred style");
for (const FixItHint Fix : FixIts)
D.AddFixItHint(Fix);
} else {
auto D = Check->diag(EndIf, "#endif for a header guard should "
"reference the guard macro in a comment");
for (const FixItHint Fix : FixIts)
D.AddFixItHint(Fix);
}
}
}
// Emit warnings for headers that are missing guards.
@ -129,6 +144,7 @@ public:
Files.clear();
Ifndefs.clear();
EndIfs.clear();
FixIts.clear();
}
bool wouldFixEndifComment(StringRef FileName, SourceLocation EndIf,
@ -170,15 +186,14 @@ public:
if (Ifndef.isValid() && CurHeaderGuard != CPPVar &&
(CurHeaderGuard != CPPVarUnder ||
wouldFixEndifComment(FileName, EndIf, CurHeaderGuard))) {
Check->diag(Ifndef, "header guard does not follow preferred style")
<< FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(
Ifndef, Ifndef.getLocWithOffset(CurHeaderGuard.size())),
CPPVar)
<< FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(
Define, Define.getLocWithOffset(CurHeaderGuard.size())),
CPPVar);
FixIts.push_back(FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(
Ifndef, Ifndef.getLocWithOffset(CurHeaderGuard.size())),
CPPVar));
FixIts.push_back(FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(
Define, Define.getLocWithOffset(CurHeaderGuard.size())),
CPPVar));
return CPPVar;
}
return CurHeaderGuard;
@ -191,12 +206,10 @@ public:
size_t EndIfLen;
if (wouldFixEndifComment(FileName, EndIf, HeaderGuard, &EndIfLen)) {
std::string Correct = "endif // " + HeaderGuard.str();
Check->diag(EndIf, "#endif for a header guard should reference the "
"guard macro in a comment")
<< FixItHint::CreateReplacement(
CharSourceRange::getCharRange(EndIf,
EndIf.getLocWithOffset(EndIfLen)),
Correct);
FixIts.push_back(FixItHint::CreateReplacement(
CharSourceRange::getCharRange(EndIf,
EndIf.getLocWithOffset(EndIfLen)),
Correct));
}
}
@ -257,6 +270,7 @@ private:
std::map<const IdentifierInfo *, std::pair<SourceLocation, SourceLocation>>
Ifndefs;
std::map<SourceLocation, SourceLocation> EndIfs;
std::vector<FixItHint> FixIts;
Preprocessor *PP;
HeaderGuardCheck *Check;

View File

@ -88,9 +88,12 @@ TEST(NamespaceCommentCheckTest, FixWrongComments) {
// FIXME: It seems this might be incompatible to dos path. Investigating.
#if !defined(_WIN32)
static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename) {
return test::runCheckOnCode<LLVMHeaderGuardCheck>(
Code, /*Errors=*/nullptr, Filename, std::string("-xc++-header"));
static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
unsigned NumWarnings = 1) {
std::vector<ClangTidyError> Errors;
std::string Result = test::runCheckOnCode<LLVMHeaderGuardCheck>(
Code, &Errors, Filename, std::string("-xc++-header"));
return Errors.size() == NumWarnings ? Result : "invalid error count";
}
namespace {
@ -102,9 +105,12 @@ struct WithEndifComment : public LLVMHeaderGuardCheck {
} // namespace
static std::string runHeaderGuardCheckWithEndif(StringRef Code,
const Twine &Filename) {
return test::runCheckOnCode<WithEndifComment>(
Code, /*Errors=*/nullptr, Filename, std::string("-xc++-header"));
const Twine &Filename,
unsigned NumWarnings = 1) {
std::vector<ClangTidyError> Errors;
std::string Result = test::runCheckOnCode<WithEndifComment>(
Code, &Errors, Filename, std::string("-xc++-header"));
return Errors.size() == NumWarnings ? Result : "invalid error count";
}
TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
@ -116,7 +122,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H_\n#define LLVM_ADT_FOO_H_\n#endif\n",
runHeaderGuardCheck(
"#ifndef LLVM_ADT_FOO_H_\n#define LLVM_ADT_FOO_H_\n#endif\n",
"include/llvm/ADT/foo.h"));
"include/llvm/ADT/foo.h", 0));
EXPECT_EQ("#ifndef LLVM_CLANG_C_BAR_H\n#define LLVM_CLANG_C_BAR_H\n\n\n#endif\n",
runHeaderGuardCheck("", "./include/clang-c/bar.h"));
@ -157,14 +163,14 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n#define "
"LLVM_ADT_FOO_H\n"
"#endif /* LLVM_ADT_FOO_H */\n",
"include/llvm/ADT/foo.h"));
"include/llvm/ADT/foo.h", 0));
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H_\n#define 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_ADT_FOO_H_\n",
"include/llvm/ADT/foo.h"));
"include/llvm/ADT/foo.h", 0));
EXPECT_EQ(
"#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif // "
@ -178,14 +184,14 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n#define "
"LLVM_ADT_FOO_H\n#endif \\ \n// "
"LLVM_ADT_FOO_H\n",
"include/llvm/ADT/foo.h"));
"include/llvm/ADT/foo.h", 0));
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif /* "
"LLVM_ADT_FOO_H\\ \n FOO */",
runHeaderGuardCheckWithEndif(
"#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif /* "
"LLVM_ADT_FOO_H\\ \n FOO */",
"include/llvm/ADT/foo.h"));
"include/llvm/ADT/foo.h", 0));
}
#endif