From e1b7b959012bd7d5f526d53653ab6b4c6582c811 Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Mon, 16 Oct 2017 17:31:16 +0000 Subject: [PATCH] Recommit r315738 "[clang-refactor] Apply source replacements" The fixed commit ensures that ParsedSourceRange works correctly with Windows paths. Original message: This commit actually brings clang-refactor to a usable state as it can now apply the refactoring changes to source files. The -selection option is now also fully supported. Differential Revision: https://reviews.llvm.org/D38402 llvm-svn: 315918 --- .../clang/Frontend/CommandLineSourceLoc.h | 46 +++++++++ .../test/Refactor/tool-apply-replacements.cpp | 11 +++ clang/test/Refactor/tool-selection-option.c | 15 +++ clang/tools/clang-refactor/ClangRefactor.cpp | 94 +++++++++++++++++-- clang/unittests/Frontend/CMakeLists.txt | 1 + .../Frontend/ParsedSourceLocationTest.cpp | 37 ++++++++ 6 files changed, 198 insertions(+), 6 deletions(-) create mode 100644 clang/test/Refactor/tool-apply-replacements.cpp create mode 100644 clang/test/Refactor/tool-selection-option.c create mode 100644 clang/unittests/Frontend/ParsedSourceLocationTest.cpp diff --git a/clang/include/clang/Frontend/CommandLineSourceLoc.h b/clang/include/clang/Frontend/CommandLineSourceLoc.h index a78c96d23afa..f5c1e1a8a67d 100644 --- a/clang/include/clang/Frontend/CommandLineSourceLoc.h +++ b/clang/include/clang/Frontend/CommandLineSourceLoc.h @@ -51,6 +51,52 @@ public: } }; +/// 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 Begin; + /// The ending location of the range. The first element is the line and the + /// second element is the column. + std::pair 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 fromString(StringRef Str) { + std::pair RangeSplit = Str.rsplit('-'); + unsigned EndLine, EndColumn; + bool HasEndLoc = false; + if (!RangeSplit.second.empty()) { + std::pair Split = RangeSplit.second.rsplit(':'); + if (Split.first.getAsInteger(10, EndLine) || + Split.second.getAsInteger(10, EndColumn)) { + // The string does not end in end_line:end_column, so the '-' + // probably belongs to the filename which menas the whole + // string should be parsed. + RangeSplit.first = Str; + } else + HasEndLoc = true; + } + auto Begin = ParsedSourceLocation::FromString(RangeSplit.first); + if (Begin.FileName.empty()) + return None; + if (!HasEndLoc) { + EndLine = Begin.Line; + EndColumn = Begin.Column; + } + return ParsedSourceRange{std::move(Begin.FileName), + {Begin.Line, Begin.Column}, + {EndLine, EndColumn}}; + } +}; } namespace llvm { diff --git a/clang/test/Refactor/tool-apply-replacements.cpp b/clang/test/Refactor/tool-apply-replacements.cpp new file mode 100644 index 000000000000..9f4595089cac --- /dev/null +++ b/clang/test/Refactor/tool-apply-replacements.cpp @@ -0,0 +1,11 @@ +// 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 { +}; diff --git a/clang/test/Refactor/tool-selection-option.c b/clang/test/Refactor/tool-selection-option.c new file mode 100644 index 000000000000..f80457a0678d --- /dev/null +++ b/clang/test/Refactor/tool-selection-option.c @@ -0,0 +1,15 @@ +// 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 diff --git a/clang/tools/clang-refactor/ClangRefactor.cpp b/clang/tools/clang-refactor/ClangRefactor.cpp index b1a1ebdf6845..800729db8619 100644 --- a/clang/tools/clang-refactor/ClangRefactor.cpp +++ b/clang/tools/clang-refactor/ClangRefactor.cpp @@ -14,6 +14,7 @@ //===----------------------------------------------------------------------===// #include "TestSupport.h" +#include "clang/Frontend/CommandLineSourceLoc.h" #include "clang/Rewrite/Core/Rewriter.h" #include "clang/Tooling/CommonOptionsParser.h" #include "clang/Tooling/Refactoring.h" @@ -54,7 +55,7 @@ public: /// 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. @@ -99,6 +100,41 @@ 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 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::fromString(StringRef Value) { if (Value.startswith("test:")) { @@ -110,10 +146,12 @@ SourceSelectionArgument::fromString(StringRef Value) { return llvm::make_unique( std::move(*ParsedTestSelection)); } - // FIXME: Support true selection ranges. + Optional Range = ParsedSourceRange::fromString(Value); + if (Range) + return llvm::make_unique(std::move(*Range)); llvm::errs() << "error: '-selection' option must be specified using " ":: or " - "::-: format"; + "::-: format\n"; return nullptr; } @@ -268,11 +306,22 @@ private: 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()); + } + + void handle(SymbolOccurrences Occurrences) override { + RefactoringResultConsumer::handle(std::move(Occurrences)); + } + + const AtomicChanges &getSourceChanges() const { return SourceChanges; } + +private: + AtomicChanges SourceChanges; }; class ClangRefactorTool { @@ -352,6 +401,39 @@ public: } } + bool applySourceChanges(const AtomicChanges &Replacements) { + std::set 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> 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 Sources) { @@ -423,7 +505,7 @@ public: // FIXME (Alex L): Implement non-selection based invocation path. })) return true; - return HasFailed; + return HasFailed || applySourceChanges(Consumer.getSourceChanges()); } }; diff --git a/clang/unittests/Frontend/CMakeLists.txt b/clang/unittests/Frontend/CMakeLists.txt index e3a21f57bab6..c1f4f186354b 100644 --- a/clang/unittests/Frontend/CMakeLists.txt +++ b/clang/unittests/Frontend/CMakeLists.txt @@ -7,6 +7,7 @@ add_clang_unittest(FrontendTests CompilerInstanceTest.cpp FrontendActionTest.cpp CodeGenActionTest.cpp + ParsedSourceLocationTest.cpp PCHPreambleTest.cpp ) target_link_libraries(FrontendTests diff --git a/clang/unittests/Frontend/ParsedSourceLocationTest.cpp b/clang/unittests/Frontend/ParsedSourceLocationTest.cpp new file mode 100644 index 000000000000..0cbdc7e1d5a2 --- /dev/null +++ b/clang/unittests/Frontend/ParsedSourceLocationTest.cpp @@ -0,0 +1,37 @@ +//===- unittests/Frontend/ParsedSourceLocationTest.cpp - ------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "clang/Frontend/CommandLineSourceLoc.h" +#include "gtest/gtest.h" + +using namespace llvm; +using namespace clang; + +namespace { + +TEST(ParsedSourceRange, ParseTest) { + auto Check = [](StringRef Value, StringRef Filename, unsigned BeginLine, + unsigned BeginColumn, unsigned EndLine, unsigned EndColumn) { + Optional PSR = ParsedSourceRange::fromString(Value); + ASSERT_TRUE(PSR); + EXPECT_EQ(PSR->FileName, Filename); + EXPECT_EQ(PSR->Begin.first, BeginLine); + EXPECT_EQ(PSR->Begin.second, BeginColumn); + EXPECT_EQ(PSR->End.first, EndLine); + EXPECT_EQ(PSR->End.second, EndColumn); + }; + + Check("/Users/test/a-b.cpp:1:2", "/Users/test/a-b.cpp", 1, 2, 1, 2); + Check("/Users/test/a-b.cpp:1:2-3:4", "/Users/test/a-b.cpp", 1, 2, 3, 4); + + Check("C:/Users/bob/a-b.cpp:1:2", "C:/Users/bob/a-b.cpp", 1, 2, 1, 2); + Check("C:/Users/bob/a-b.cpp:1:2-3:4", "C:/Users/bob/a-b.cpp", 1, 2, 3, 4); +} + +} // anonymous namespace