On Tue, Sep 16, 2014 at 7:41 PM, Benjamin Kramer <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > -- Alexander Kornienko | Software Engineer | [email protected] | Google Germany, Munich
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
