[clang-tools-extra] r217890 - [clang-tidy] When emitting header guard fixes bundle all fix-its into one
Alexander Kornienko
alexfh at google.com
Tue Sep 16 16:43:24 PDT 2014
On Tue, Sep 16, 2014 at 7:41 PM, Benjamin Kramer <benny.kra at googlemail.com>
wrote:
> Author: d0k
> Date: Tue Sep 16 12:41:19 2014
> New Revision: 217890
>
> URL: http://llvm.org/viewvc/llvm-project?rev=217890&view=rev
> Log:
> [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.
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
> clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp
>
> Modified: clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp?rev=217890&r1=217889&r2=217890&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp Tue Sep 16
> 12:41:19 2014
> @@ -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;
>
> Modified: clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp?rev=217890&r1=217889&r2=217890&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp
> (original)
> +++ clang-tools-extra/trunk/unittests/clang-tidy/LLVMModuleTest.cpp Tue
> Sep 16 12:41:19 2014
> @@ -88,9 +88,12 @@ TEST(NamespaceCommentCheckTest, FixWrong
>
> // 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 LLVMHea
> } // 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)
> {
>
Not sure if you've got this comment
<http://reviews.llvm.org/rL217890#inline-284>. Repeating it here:
I'd remove the default value here and added argument comments
(/*NumWarnings=*/) for this parameter to the tests to make the expectations
more explicit.
> + 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, FixHeader
> 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, FixHeader
> 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, FixHeader
> 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
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
--
Alexander Kornienko | Software Engineer | alexfh at google.com | Google
Germany, Munich
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140917/1e6e5a2f/attachment.html>
More information about the cfe-commits
mailing list