[clang-refactor] Apply source replacements

[arphaman]

Summary

This patch actually brings clang-refactor to a usable state as it can now apply the refactoring changes to the source files.
The -selection option is now also fully supported.

[ioeric]

test/Refactor/tool-apply-replacements.cpp

6 ↗

(On Diff #117133)

Why is the result class {?

tools/clang-refactor/ClangRefactor.cpp

319 ↗

(On Diff #117133)

I think Changes or SourceChanges would be a clearer variable name.

330 ↗

(On Diff #117133)

s/AllSourceReplacements/Changes or SourceChanges for clarity.

410 ↗

(On Diff #117133)

s/Replacements/Changes/ ?

416 ↗

(On Diff #117133)

I think we would also want to clean up by default in the future. Maybe add this to FIXME as well?

[arphaman]

Address review comments

test/Refactor/tool-apply-replacements.cpp

6 ↗

(On Diff #117133)

Sorry, the test was broken. Fixed!

[hokein]

include/clang/Frontend/CommandLineSourceLoc.h

57 ↗

(On Diff #118042)

Add a comment documenting what the first element and second element of the pair represent for (<Line, Column>? If I understand correctly).

60 ↗

(On Diff #118042)

Would be clearer to document the valid format of the Str.

test/Refactor/tool-apply-replacements.cpp

3 ↗

(On Diff #118042)

Maybe add one more case for testing -selection=%t.cp.cpp:6:7-6:15.

tools/clang-refactor/ClangRefactor.cpp

309 ↗

(On Diff #118042)

Add override.

313 ↗

(On Diff #118042)

The same, override.

412 ↗

(On Diff #118042)

nit: missing a blank after open.

[arphaman]

Address review comments

[ioeric]

lgtm

[hokein]

LGTM, let's check in it.

cfe/trunk/include/clang/Frontend/CommandLineSourceLoc.h

if (!ColSplit.second.getAsInteger(10, PSL.Column) &&
       if (PSL.FileName == "-")
         PSL.FileName = "<stdin>";
     }
 
     return PSL;
   }
 };
 
+/// A source range that has been parsed on the command line.
+struct ParsedSourceRange {
+  std::string FileName;
+  /// The starting location of the range. The first element is the line and
+  /// the second element is the column.
+  std::pair<unsigned, unsigned> Begin;
+  /// The ending location of the range. The first element is the line and the
+  /// second element is the column.
+  std::pair<unsigned, unsigned> End;
+
+  /// Returns a parsed source range from a string or None if the string is
+  /// invalid.
+  ///
+  /// These source string has the following format:
+  ///
+  /// file:start_line:start_column[-end_line:end_column]
+  ///
+  /// If the end line and column are omitted, the starting line and columns
+  /// are used as the end values.
+  static Optional<ParsedSourceRange> fromString(StringRef Str) {
+    std::pair<StringRef, StringRef> RangeSplit;
+    // Avoid splitting '-' when there's no end line & column as '-' might be
+    // part of the filename.
+    if (Str.count(':') > 2)
+      RangeSplit = Str.rsplit('-');
+    else
+      RangeSplit = {Str, ""};
+    auto Begin = ParsedSourceLocation::FromString(RangeSplit.first);
+    if (Begin.FileName.empty())
+      return None;
+    unsigned EndLine, EndColumn;
+    if (RangeSplit.second.empty()) {
+      EndLine = Begin.Line;
+      EndColumn = Begin.Column;
+    } else {
+      std::pair<StringRef, StringRef> Split = RangeSplit.second.rsplit(':');
+      if (Split.first.getAsInteger(10, EndLine) ||
+          Split.second.getAsInteger(10, EndColumn))
+        return None;
+    }
+    return ParsedSourceRange{std::move(Begin.FileName),
+                             {Begin.Line, Begin.Column},
+                             {EndLine, EndColumn}};
+  }
+};
 }
 
 namespace llvm {
   namespace cl {
     /// \brief Command-line option parser that parses source locations.
     ///
     /// Source locations are of the form filename:line:column.
     template<>

cfe/trunk/test/Refactor/tool-apply-replacements.cpp

+// RUN: rm -f %t.cp.cpp
+// RUN: cp %s %t.cp.cpp
+// RUN: clang-refactor local-rename -selection=%t.cp.cpp:9:7 -new-name=test %t.cp.cpp --
+// RUN: grep -v CHECK %t.cp.cpp | FileCheck %t.cp.cpp
+// RUN: cp %s %t.cp.cpp
+// RUN: clang-refactor local-rename -selection=%t.cp.cpp:9:7-9:15 -new-name=test %t.cp.cpp --
+// RUN: grep -v CHECK %t.cp.cpp | FileCheck %t.cp.cpp
+
+class RenameMe {
+// CHECK: class test {
+};

cfe/trunk/test/Refactor/tool-selection-option.c

+// RUN: rm -f %t.cp.c
+// RUN: cp %s %t.cp.c
+// RUN: clang-refactor local-rename -selection=%t.cp.c:6:5 -new-name=test -v %t.cp.c -- | FileCheck --check-prefix=CHECK1 %s
+// RUN: clang-refactor local-rename -selection=%t.cp.c:6:5-6:9 -new-name=test -v %t.cp.c -- | FileCheck --check-prefix=CHECK2 %s
+
+int test;
+
+// CHECK1: invoking action 'local-rename':
+// CHECK1-NEXT: -selection={{.*}}.cp.c:6:5 -> {{.*}}.cp.c:6:5
+
+// CHECK2: invoking action 'local-rename':
+// CHECK2-NEXT: -selection={{.*}}.cp.c:6:5 -> {{.*}}.cp.c:6:9
+
+// RUN: not clang-refactor local-rename -selection=%s:6:5 -new-name=test -v %t.cp.c -- 2>&1 | FileCheck --check-prefix=CHECK-FILE-ERR %s
+// CHECK-FILE-ERR: given file is not in the target TU

cfe/trunk/tools/clang-refactor/ClangRefactor.cpp

 //===--- ClangRefactor.cpp - Clang-based refactoring tool -----------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// \brief This file implements a clang-refactor tool that performs various
 /// source transformations.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "TestSupport.h"
+#include "clang/Frontend/CommandLineSourceLoc.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Refactoring/RefactoringAction.h"
 #include "clang/Tooling/Refactoring/RefactoringOptions.h"
 #include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
public:
 
   /// Parse the `-selection` argument.
   ///
   /// \returns A valid argument when the parse succedeed, null otherwise.
   static std::unique_ptr<SourceSelectionArgument> fromString(StringRef Value);
 
   /// Prints any additional state associated with the selection argument to
   /// the given output stream.
-  virtual void print(raw_ostream &OS) = 0;
+  virtual void print(raw_ostream &OS) {}
 
   /// Returns a replacement refactoring result consumer (if any) that should
   /// consume the results of a refactoring operation.
   ///
   /// The replacement refactoring result consumer is used by \c
   /// TestSourceSelectionArgument to inject a test-specific result handling
   /// logic into the refactoring operation. The test-specific consumer
   /// ensures that the individual results in a particular test group are
bool forAllRanges(const SourceManager &SM,
                     llvm::function_ref<void(SourceRange R)> Callback) override {
     return TestSelections.foreachRange(SM, Callback);
   }
 
 private:
   TestSelectionRangesInFile TestSelections;
 };
 
+/// Stores the parsed -selection=filename:line:column[-line:column] option.
+class SourceRangeSelectionArgument final : public SourceSelectionArgument {
+public:
+  SourceRangeSelectionArgument(ParsedSourceRange Range)
+      : Range(std::move(Range)) {}
+
+  bool forAllRanges(const SourceManager &SM,
+                    llvm::function_ref<void(SourceRange R)> Callback) override {
+    const FileEntry *FE = SM.getFileManager().getFile(Range.FileName);
+    FileID FID = FE ? SM.translateFile(FE) : FileID();
+    if (!FE || FID.isInvalid()) {
+      llvm::errs() << "error: -selection=" << Range.FileName
+                   << ":... : given file is not in the target TU\n";
+      return true;
+    }
+
+    SourceLocation Start = SM.getMacroArgExpandedLocation(
+        SM.translateLineCol(FID, Range.Begin.first, Range.Begin.second));
+    SourceLocation End = SM.getMacroArgExpandedLocation(
+        SM.translateLineCol(FID, Range.End.first, Range.End.second));
+    if (Start.isInvalid() || End.isInvalid()) {
+      llvm::errs() << "error: -selection=" << Range.FileName << ':'
+                   << Range.Begin.first << ':' << Range.Begin.second << '-'
+                   << Range.End.first << ':' << Range.End.second
+                   << " : invalid source location\n";
+      return true;
+    }
+    Callback(SourceRange(Start, End));
+    return false;
+  }
+
+private:
+  ParsedSourceRange Range;
+};
+
 std::unique_ptr<SourceSelectionArgument>
 SourceSelectionArgument::fromString(StringRef Value) {
   if (Value.startswith("test:")) {
     StringRef Filename = Value.drop_front(strlen("test:"));
     Optional<TestSelectionRangesInFile> ParsedTestSelection =
         findTestSelectionRanges(Filename);
     if (!ParsedTestSelection)
       return nullptr; // A parsing error was already reported.
     return llvm::make_unique<TestSourceSelectionArgument>(
         std::move(*ParsedTestSelection));
   }
-  // FIXME: Support true selection ranges.
+  Optional<ParsedSourceRange> Range = ParsedSourceRange::fromString(Value);
+  if (Range)
+    return llvm::make_unique<SourceRangeSelectionArgument>(std::move(*Range));
   llvm::errs() << "error: '-selection' option must be specified using "
                   "<file>:<line>:<column> or "
-                  "<file>:<line>:<column>-<line>:<column> format";
+                  "<file>:<line>:<column>-<line>:<column> format\n";
   return nullptr;
 }
 
 /// A container that stores the command-line options used by a single
 /// refactoring option.
 class RefactoringActionCommandLineOptions {
 public:
   void addStringOption(const RefactoringOption &Option,
private:
   RefactoringActionRules ActionRules;
   std::unique_ptr<cl::opt<std::string>> Selection;
   std::unique_ptr<SourceSelectionArgument> ParsedSelection;
   RefactoringActionCommandLineOptions Options;
 };
 
 class ClangRefactorConsumer : public RefactoringResultConsumer {
 public:
-  void handleError(llvm::Error Err) {
+  void handleError(llvm::Error Err) override {
     llvm::errs() << llvm::toString(std::move(Err)) << "\n";
   }
 
-  // FIXME: Consume atomic changes and apply them to files.
+  void handle(AtomicChanges Changes) override {
+    SourceChanges.insert(SourceChanges.begin(), Changes.begin(), Changes.end());
+  }
+
+  const AtomicChanges &getSourceChanges() const { return SourceChanges; }
+
+private:
+  AtomicChanges SourceChanges;
 };
 
 class ClangRefactorTool {
 public:
   std::vector<std::unique_ptr<RefactoringActionSubcommand>> SubCommands;
 
   ClangRefactorTool() {
     std::vector<std::unique_ptr<RefactoringAction>> Actions =
if (Context.getSelectionRange().isValid()) {
       llvm::outs() << "  -selection=";
       R.getBegin().print(llvm::outs(), Context.getSources());
       llvm::outs() << " -> ";
       R.getEnd().print(llvm::outs(), Context.getSources());
       llvm::outs() << "\n";
     }
   }
 
+  bool applySourceChanges(const AtomicChanges &Replacements) {
+    std::set<std::string> Files;
+    for (const auto &Change : Replacements)
+      Files.insert(Change.getFilePath());
+    // FIXME: Add automatic formatting support as well.
+    tooling::ApplyChangesSpec Spec;
+    // FIXME: We should probably cleanup the result by default as well.
+    Spec.Cleanup = false;
+    for (const auto &File : Files) {
+      llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> BufferErr =
+          llvm::MemoryBuffer::getFile(File);
+      if (!BufferErr) {
+        llvm::errs() << "error: failed to open " << File << " for rewriting\n";
+        return true;
+      }
+      auto Result = tooling::applyAtomicChanges(File, (*BufferErr)->getBuffer(),
+                                                Replacements, Spec);
+      if (!Result) {
+        llvm::errs() << toString(Result.takeError());
+        return true;
+      }
+
+      std::error_code EC;
+      llvm::raw_fd_ostream OS(File, EC, llvm::sys::fs::F_Text);
+      if (EC) {
+        llvm::errs() << EC.message() << "\n";
+        return true;
+      }
+      OS << *Result;
+    }
+    return false;
+  }
+
   bool invokeAction(RefactoringActionSubcommand &Subcommand,
                     const CompilationDatabase &DB,
                     ArrayRef<std::string> Sources) {
     // Find a set of matching rules.
     SmallVector<RefactoringActionRule *, 4> MatchingRules;
     llvm::StringSet<> MissingOptions;
 
     bool HasSelection = false;
if (foreachTranslationUnit(DB, Sources, [&](ASTContext &AST) {
                       InvokeRule(CustomConsumer ? *CustomConsumer : Consumer);
                     }))
               HasFailed = true;
             return;
           }
           // FIXME (Alex L): Implement non-selection based invocation path.
         }))
       return true;
-    return HasFailed;
+    return HasFailed || applySourceChanges(Consumer.getSourceChanges());
   }
 };
 
 } // end anonymous namespace
 
 int main(int argc, const char **argv) {
   ClangRefactorTool Tool;