Index: clang-tidy/ClangTidy.cpp =================================================================== --- clang-tidy/ClangTidy.cpp +++ clang-tidy/ClangTidy.cpp @@ -22,6 +22,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Format/Format.h" #include "clang/Frontend/ASTConsumers.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendActions.h" @@ -101,9 +102,8 @@ DiagPrinter(new TextDiagnosticPrinter(llvm::outs(), &*DiagOpts)), Diags(IntrusiveRefCntPtr(new DiagnosticIDs), &*DiagOpts, DiagPrinter), - SourceMgr(Diags, Files), Rewrite(SourceMgr, LangOpts), - ApplyFixes(ApplyFixes), TotalFixes(0), AppliedFixes(0), - WarningsAsErrors(0) { + SourceMgr(Diags, Files), ApplyFixes(ApplyFixes), TotalFixes(0), + AppliedFixes(0), WarningsAsErrors(0) { DiagOpts->ShowColors = llvm::sys::Process::StandardOutHasColors(); DiagPrinter->BeginSourceFile(LangOpts); } @@ -127,31 +127,56 @@ auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]")) << Message.Message << Name; for (const auto &FileAndReplacements : Error.Fix) { - for (const auto &Replacement : FileAndReplacements.second) { + for (const auto &Repl : FileAndReplacements.second) { // Retrieve the source range for applicable fixes. Macro definitions // on the command line have locations in a virtual buffer and don't // have valid file paths and are therefore not applicable. SourceRange Range; SourceLocation FixLoc; - if (Replacement.isApplicable()) { - SmallString<128> FixAbsoluteFilePath = Replacement.getFilePath(); + ++TotalFixes; + bool CanBeApplied = false; + if (Repl.isApplicable()) { + SmallString<128> FixAbsoluteFilePath = Repl.getFilePath(); Files.makeAbsolutePath(FixAbsoluteFilePath); - FixLoc = getLocation(FixAbsoluteFilePath, Replacement.getOffset()); + if (ApplyFixes) { + tooling::Replacement R(FixAbsoluteFilePath, Repl.getOffset(), + Repl.getLength(), + Repl.getReplacementText()); + Replacements &Replacements = FileReplacements[R.getFilePath()]; + llvm::Error Err = Replacements.add(R); + if (Err) { + // FIXME: Implement better conflict handling. + llvm::errs() << "Trying to resolve conflict: " + << llvm::toString(std::move(Err)) << "\n"; + unsigned NewOffset = + Replacements.getShiftedCodePosition(R.getOffset()); + unsigned NewLength = Replacements.getShiftedCodePosition( + R.getOffset() + R.getLength()) - + NewOffset; + if (NewLength == R.getLength()) { + R = Replacement(R.getFilePath(), NewOffset, NewLength, + R.getReplacementText()); + Replacements = Replacements.merge(tooling::Replacements(R)); + } else { + llvm::errs() + << "Can't resolve conflict, skipping the replacement.\n"; + } + + } else { + CanBeApplied = true; + ++AppliedFixes; + } + } + FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset()); SourceLocation FixEndLoc = - FixLoc.getLocWithOffset(Replacement.getLength()); + FixLoc.getLocWithOffset(Repl.getLength()); Range = SourceRange(FixLoc, FixEndLoc); - Diag << FixItHint::CreateReplacement( - Range, Replacement.getReplacementText()); + Diag << FixItHint::CreateReplacement(Range, + Repl.getReplacementText()); } - ++TotalFixes; - if (ApplyFixes) { - bool Success = - Replacement.isApplicable() && Replacement.apply(Rewrite); - if (Success) - ++AppliedFixes; - FixLocations.push_back(std::make_pair(FixLoc, Success)); - } + if (ApplyFixes) + FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied)); } } } @@ -166,9 +191,33 @@ void Finish() { // FIXME: Run clang-format on changes. if (ApplyFixes && TotalFixes > 0) { + Rewriter Rewrite(SourceMgr, LangOpts); + for (const auto &FileAndReplacements : FileReplacements) { + StringRef File = FileAndReplacements.first(); + llvm::ErrorOr> Buffer = + SourceMgr.getFileManager().getBufferForFile(File); + if (!Buffer) { + llvm::errs() << "Can't get buffer for file " << File << ": " + << Buffer.getError().message() << "\n"; + // FIXME: Maybe don't apply anything. + continue; + } + StringRef Code = Buffer.get()->getBuffer(); + format::FormatStyle Style = format::getLLVMStyle(); + llvm::Expected CleanReplacements = + format::cleanupAroundReplacements(Code, FileAndReplacements.second, + Style); + if (!CleanReplacements) { + llvm::errs() << llvm::toString(CleanReplacements.takeError()) << "\n"; + continue; + } + if (!tooling::applyAllReplacements(CleanReplacements.get(), Rewrite)) { + llvm::errs() << "Can't apply replacements for file " << File << "\n"; + } + } + Rewrite.overwriteChangedFiles(); llvm::errs() << "clang-tidy applied " << AppliedFixes << " of " << TotalFixes << " suggested fixes.\n"; - Rewrite.overwriteChangedFiles(); } } @@ -197,7 +246,7 @@ DiagnosticConsumer *DiagPrinter; DiagnosticsEngine Diags; SourceManager SourceMgr; - Rewriter Rewrite; + llvm::StringMap FileReplacements; bool ApplyFixes; unsigned TotalFixes; unsigned AppliedFixes; @@ -416,7 +465,7 @@ ClangTidyStats runClangTidy(std::unique_ptr OptionsProvider, - const tooling::CompilationDatabase &Compilations, + const CompilationDatabase &Compilations, ArrayRef InputFiles, std::vector *Errors, ProfileData *Profile) { ClangTool Tool(Compilations, InputFiles); @@ -519,7 +568,7 @@ void exportReplacements(const std::vector &Errors, raw_ostream &OS) { - tooling::TranslationUnitReplacements TUR; + TranslationUnitReplacements TUR; for (const ClangTidyError &Error : Errors) { for (const auto &FileAndFixes : Error.Fix) TUR.Replacements.insert(TUR.Replacements.end(), Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp =================================================================== --- clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -79,12 +79,13 @@ tooling::Replacement Replacement(SM, Range, FixIt.CodeToInsert); llvm::Error Err = Error.Fix[Replacement.getFilePath()].add(Replacement); - // FIXME: better error handling. + // FIXME: better error handling (at least, don't let other replacements be + // applied). if (Err) { llvm::errs() << "Fix conflicts with existing fix! " << llvm::toString(std::move(Err)) << "\n"; + assert(false && "Fix conflicts with existing fix!"); } - assert(!Err && "Fix conflicts with existing fix!"); } } Index: test/clang-tidy/clean-up-code.cpp =================================================================== --- /dev/null +++ test/clang-tidy/clean-up-code.cpp @@ -0,0 +1,12 @@ +// RUN: %check_clang_tidy %s misc-unused-using-decls %t +namespace a { class A {}; } +namespace b { +using a::A; +} +namespace c {} +// CHECK-MESSAGES: :[[@LINE-3]]:10: warning: using decl 'A' is unused [misc-unused-using-decls] +// CHECK-FIXES: {{^namespace a { class A {}; }$}} +// CHECK-FIXES-NOT: namespace +// CHECK-FIXES: {{^namespace c {}$}} +// FIXME: cleanupAroundReplacements leaves whitespace. Otherwise we could just +// check the next line. Index: test/clang-tidy/fix.cpp =================================================================== --- test/clang-tidy/fix.cpp +++ test/clang-tidy/fix.cpp @@ -5,6 +5,7 @@ // RUN: FileCheck -input-file=%t.yaml -check-prefix=CHECK-YAML %s namespace i { +void f(); // So that the namespace isn't empty. } // CHECK: } // namespace i // CHECK-MESSAGES: note: FIX-IT applied suggested code changes Index: test/clang-tidy/google-readability-namespace-comments.cpp =================================================================== --- test/clang-tidy/google-readability-namespace-comments.cpp +++ test/clang-tidy/google-readability-namespace-comments.cpp @@ -4,7 +4,7 @@ namespace n2 { - +void f(); // So that the namespace isn't empty. // CHECK-MESSAGES: :[[@LINE+4]]:2: warning: namespace 'n2' not terminated with a closing comment [google-readability-namespace-comments]