[clang-tools-extra] r284399 - [clang-tidy] Clean up code after applying replacements.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 17 10:25:02 PDT 2016
Author: alexfh
Date: Mon Oct 17 12:25:02 2016
New Revision: 284399
URL: http://llvm.org/viewvc/llvm-project?rev=284399&view=rev
Log:
[clang-tidy] Clean up code after applying replacements.
Summary:
Remove empty namespaces and initializer list commas / colons in
affected ranges. Initial patch: proper options for enabling the cleanup and
specifying the format style are needed.
Reviewers: hokein, ioeric
Subscribers: beanz, mgorny, cfe-commits
Differential Revision: https://reviews.llvm.org/D24572
Added:
clang-tools-extra/trunk/test/clang-tidy/clean-up-code.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/trunk/test/clang-tidy/fix.cpp
clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp
Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=284399&r1=284398&r2=284399&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Mon Oct 17 12:25:02 2016
@@ -15,6 +15,7 @@ add_clang_library(clangTidy
clangAST
clangASTMatchers
clangBasic
+ clangFormat
clangFrontend
clangLex
clangRewrite
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=284399&r1=284398&r2=284399&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Mon Oct 17 12:25:02 2016
@@ -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 @@ public:
DiagPrinter(new TextDiagnosticPrinter(llvm::outs(), &*DiagOpts)),
Diags(IntrusiveRefCntPtr<DiagnosticIDs>(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,58 @@ public:
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));
+ CanBeApplied = true;
+ ++AppliedFixes;
+ } 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 +193,37 @@ public:
void Finish() {
// FIXME: Run clang-format on changes.
if (ApplyFixes && TotalFixes > 0) {
- llvm::errs() << "clang-tidy applied " << AppliedFixes << " of "
- << TotalFixes << " suggested fixes.\n";
- Rewrite.overwriteChangedFiles();
+ Rewriter Rewrite(SourceMgr, LangOpts);
+ for (const auto &FileAndReplacements : FileReplacements) {
+ StringRef File = FileAndReplacements.first();
+ llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> 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 fixes for other files as well.
+ continue;
+ }
+ StringRef Code = Buffer.get()->getBuffer();
+ // FIXME: Make the style customizable.
+ format::FormatStyle Style = format::getStyle("file", File, "LLVM");
+ llvm::Expected<Replacements> 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";
+ }
+ }
+ if (Rewrite.overwriteChangedFiles()) {
+ llvm::errs() << "clang-tidy failed to apply suggested fixes.\n";
+ } else {
+ llvm::errs() << "clang-tidy applied " << AppliedFixes << " of "
+ << TotalFixes << " suggested fixes.\n";
+ }
}
}
@@ -197,7 +252,7 @@ private:
DiagnosticConsumer *DiagPrinter;
DiagnosticsEngine Diags;
SourceManager SourceMgr;
- Rewriter Rewrite;
+ llvm::StringMap<Replacements> FileReplacements;
bool ApplyFixes;
unsigned TotalFixes;
unsigned AppliedFixes;
@@ -416,7 +471,7 @@ ClangTidyOptions::OptionMap getCheckOpti
ClangTidyStats
runClangTidy(std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
- const tooling::CompilationDatabase &Compilations,
+ const CompilationDatabase &Compilations,
ArrayRef<std::string> InputFiles,
std::vector<ClangTidyError> *Errors, ProfileData *Profile) {
ClangTool Tool(Compilations, InputFiles);
@@ -519,7 +574,7 @@ void handleErrors(const std::vector<Clan
void exportReplacements(const std::vector<ClangTidyError> &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(),
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=284399&r1=284398&r2=284399&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Mon Oct 17 12:25:02 2016
@@ -79,12 +79,13 @@ protected:
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!");
}
}
Added: clang-tools-extra/trunk/test/clang-tidy/clean-up-code.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clean-up-code.cpp?rev=284399&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/clean-up-code.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/clean-up-code.cpp Mon Oct 17 12:25:02 2016
@@ -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.
Modified: clang-tools-extra/trunk/test/clang-tidy/fix.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/fix.cpp?rev=284399&r1=284398&r2=284399&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/fix.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/fix.cpp Mon Oct 17 12:25:02 2016
@@ -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
Modified: clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp?rev=284399&r1=284398&r2=284399&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/google-readability-namespace-comments.cpp Mon Oct 17 12:25:02 2016
@@ -4,7 +4,7 @@ namespace n1 {
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]
More information about the cfe-commits
mailing list