Skip to content

Commit eb213cb

Browse files
d0kd0k
committedSep 16, 2014
[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
1 parent 7587744 commit eb213cb

File tree

2 files changed

+48
-28
lines changed

2 files changed

+48
-28
lines changed
 

‎clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp

+31-17
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,7 @@ class HeaderGuardPPCallbacks : public PPCallbacks {
109109
EndIfs[Ifndefs[MacroEntry.first.getIdentifierInfo()].first];
110110

111111
// If the macro Name is not equal to what we can compute, correct it in
112-
// the
113-
// #ifndef and #define.
112+
// the #ifndef and #define.
114113
StringRef CurHeaderGuard =
115114
MacroEntry.first.getIdentifierInfo()->getName();
116115
std::string NewGuard = checkHeaderGuardDefinition(
@@ -119,6 +118,22 @@ class HeaderGuardPPCallbacks : public PPCallbacks {
119118
// Now look at the #endif. We want a comment with the header guard. Fix it
120119
// at the slightest deviation.
121120
checkEndifComment(FileName, EndIf, NewGuard);
121+
122+
// Bundle all fix-its into one warning. The message depends on whether we
123+
// changed the header guard or not.
124+
if (!FixIts.empty()) {
125+
if (CurHeaderGuard != NewGuard) {
126+
auto D = Check->diag(Ifndef,
127+
"header guard does not follow preferred style");
128+
for (const FixItHint Fix : FixIts)
129+
D.AddFixItHint(Fix);
130+
} else {
131+
auto D = Check->diag(EndIf, "#endif for a header guard should "
132+
"reference the guard macro in a comment");
133+
for (const FixItHint Fix : FixIts)
134+
D.AddFixItHint(Fix);
135+
}
136+
}
122137
}
123138

124139
// Emit warnings for headers that are missing guards.
@@ -129,6 +144,7 @@ class HeaderGuardPPCallbacks : public PPCallbacks {
129144
Files.clear();
130145
Ifndefs.clear();
131146
EndIfs.clear();
147+
FixIts.clear();
132148
}
133149

134150
bool wouldFixEndifComment(StringRef FileName, SourceLocation EndIf,
@@ -170,15 +186,14 @@ class HeaderGuardPPCallbacks : public PPCallbacks {
170186
if (Ifndef.isValid() && CurHeaderGuard != CPPVar &&
171187
(CurHeaderGuard != CPPVarUnder ||
172188
wouldFixEndifComment(FileName, EndIf, CurHeaderGuard))) {
173-
Check->diag(Ifndef, "header guard does not follow preferred style")
174-
<< FixItHint::CreateReplacement(
175-
CharSourceRange::getTokenRange(
176-
Ifndef, Ifndef.getLocWithOffset(CurHeaderGuard.size())),
177-
CPPVar)
178-
<< FixItHint::CreateReplacement(
179-
CharSourceRange::getTokenRange(
180-
Define, Define.getLocWithOffset(CurHeaderGuard.size())),
181-
CPPVar);
189+
FixIts.push_back(FixItHint::CreateReplacement(
190+
CharSourceRange::getTokenRange(
191+
Ifndef, Ifndef.getLocWithOffset(CurHeaderGuard.size())),
192+
CPPVar));
193+
FixIts.push_back(FixItHint::CreateReplacement(
194+
CharSourceRange::getTokenRange(
195+
Define, Define.getLocWithOffset(CurHeaderGuard.size())),
196+
CPPVar));
182197
return CPPVar;
183198
}
184199
return CurHeaderGuard;
@@ -191,12 +206,10 @@ class HeaderGuardPPCallbacks : public PPCallbacks {
191206
size_t EndIfLen;
192207
if (wouldFixEndifComment(FileName, EndIf, HeaderGuard, &EndIfLen)) {
193208
std::string Correct = "endif // " + HeaderGuard.str();
194-
Check->diag(EndIf, "#endif for a header guard should reference the "
195-
"guard macro in a comment")
196-
<< FixItHint::CreateReplacement(
197-
CharSourceRange::getCharRange(EndIf,
198-
EndIf.getLocWithOffset(EndIfLen)),
199-
Correct);
209+
FixIts.push_back(FixItHint::CreateReplacement(
210+
CharSourceRange::getCharRange(EndIf,
211+
EndIf.getLocWithOffset(EndIfLen)),
212+
Correct));
200213
}
201214
}
202215

@@ -257,6 +270,7 @@ class HeaderGuardPPCallbacks : public PPCallbacks {
257270
std::map<const IdentifierInfo *, std::pair<SourceLocation, SourceLocation>>
258271
Ifndefs;
259272
std::map<SourceLocation, SourceLocation> EndIfs;
273+
std::vector<FixItHint> FixIts;
260274

261275
Preprocessor *PP;
262276
HeaderGuardCheck *Check;

‎clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

+17-11
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,12 @@ TEST(NamespaceCommentCheckTest, FixWrongComments) {
8888

8989
// FIXME: It seems this might be incompatible to dos path. Investigating.
9090
#if !defined(_WIN32)
91-
static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename) {
92-
return test::runCheckOnCode<LLVMHeaderGuardCheck>(
93-
Code, /*Errors=*/nullptr, Filename, std::string("-xc++-header"));
91+
static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
92+
unsigned NumWarnings = 1) {
93+
std::vector<ClangTidyError> Errors;
94+
std::string Result = test::runCheckOnCode<LLVMHeaderGuardCheck>(
95+
Code, &Errors, Filename, std::string("-xc++-header"));
96+
return Errors.size() == NumWarnings ? Result : "invalid error count";
9497
}
9598

9699
namespace {
@@ -102,9 +105,12 @@ struct WithEndifComment : public LLVMHeaderGuardCheck {
102105
} // namespace
103106

104107
static std::string runHeaderGuardCheckWithEndif(StringRef Code,
105-
const Twine &Filename) {
106-
return test::runCheckOnCode<WithEndifComment>(
107-
Code, /*Errors=*/nullptr, Filename, std::string("-xc++-header"));
108+
const Twine &Filename,
109+
unsigned NumWarnings = 1) {
110+
std::vector<ClangTidyError> Errors;
111+
std::string Result = test::runCheckOnCode<WithEndifComment>(
112+
Code, &Errors, Filename, std::string("-xc++-header"));
113+
return Errors.size() == NumWarnings ? Result : "invalid error count";
108114
}
109115

110116
TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
@@ -116,7 +122,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
116122
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H_\n#define LLVM_ADT_FOO_H_\n#endif\n",
117123
runHeaderGuardCheck(
118124
"#ifndef LLVM_ADT_FOO_H_\n#define LLVM_ADT_FOO_H_\n#endif\n",
119-
"include/llvm/ADT/foo.h"));
125+
"include/llvm/ADT/foo.h", 0));
120126

121127
EXPECT_EQ("#ifndef LLVM_CLANG_C_BAR_H\n#define LLVM_CLANG_C_BAR_H\n\n\n#endif\n",
122128
runHeaderGuardCheck("", "./include/clang-c/bar.h"));
@@ -157,14 +163,14 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
157163
runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n#define "
158164
"LLVM_ADT_FOO_H\n"
159165
"#endif /* LLVM_ADT_FOO_H */\n",
160-
"include/llvm/ADT/foo.h"));
166+
"include/llvm/ADT/foo.h", 0));
161167

162168
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H_\n#define LLVM_ADT_FOO_H_\n#endif "
163169
"// LLVM_ADT_FOO_H_\n",
164170
runHeaderGuardCheckWithEndif(
165171
"#ifndef LLVM_ADT_FOO_H_\n#define "
166172
"LLVM_ADT_FOO_H_\n#endif // LLVM_ADT_FOO_H_\n",
167-
"include/llvm/ADT/foo.h"));
173+
"include/llvm/ADT/foo.h", 0));
168174

169175
EXPECT_EQ(
170176
"#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif // "
@@ -178,14 +184,14 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
178184
runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n#define "
179185
"LLVM_ADT_FOO_H\n#endif \\ \n// "
180186
"LLVM_ADT_FOO_H\n",
181-
"include/llvm/ADT/foo.h"));
187+
"include/llvm/ADT/foo.h", 0));
182188

183189
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif /* "
184190
"LLVM_ADT_FOO_H\\ \n FOO */",
185191
runHeaderGuardCheckWithEndif(
186192
"#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif /* "
187193
"LLVM_ADT_FOO_H\\ \n FOO */",
188-
"include/llvm/ADT/foo.h"));
194+
"include/llvm/ADT/foo.h", 0));
189195
}
190196
#endif
191197

0 commit comments

Comments
 (0)