[cfe-commits] [PATCH] Add OverwriteChangedFiles to the Rewriter

[Manuel Klimek]

Ping :)

On Fri, May 11, 2012 at 1:33 PM, Manuel Klimek <klimek at google.com> wrote:
> On Tue, May 8, 2012 at 11:49 AM, Manuel Klimek <klimek at google.com> wrote:
>> On Thu, May 3, 2012 at 6:40 PM, Manuel Klimek <klimek at google.com> wrote:
>>> as requested by Doug in the clangRefactoring review... in addition,
>>> this patch adds a unit test for the Rewriter (into Tooling/ so we
>>> don't link yet another unit test), and pulls out a RewriterTestContext
>>> which I need for clangRefactoring when it's checked in - but it's also
>>> really nice to unit test Rewriter functionality...
>
> Inlining the patch in the hope that that speeds up review :)
>
> First, the changed code:
>
> Index: include/clang/Rewrite/Rewriter.h
> ===================================================================
> --- include/clang/Rewrite/Rewriter.h    (revision 156059)
> +++ include/clang/Rewrite/Rewriter.h    (working copy)
> @@ -279,6 +279,13 @@
>   buffer_iterator buffer_begin() { return RewriteBuffers.begin(); }
>   buffer_iterator buffer_end() { return RewriteBuffers.end(); }
>
> +  /// SaveFiles - Save all changed files to disk.
> +  ///
> +  /// Returns whether all changes were saves successfully.
> +  /// Outputs diagnostics via the source manager's diagnostic engine
> +  /// in case of an error.
> +  bool OverwriteChangedFiles();
> +
>  private:
>   unsigned getLocationOffsetAndFileID(SourceLocation Loc, FileID &FID) const;
>  };
> Index: lib/Rewrite/Rewriter.cpp
> ===================================================================
> --- lib/Rewrite/Rewriter.cpp    (revision 156059)
> +++ lib/Rewrite/Rewriter.cpp    (working copy)
> @@ -15,8 +15,10 @@
>  #include "clang/Rewrite/Rewriter.h"
>  #include "clang/AST/Stmt.h"
>  #include "clang/AST/Decl.h"
> +#include "clang/Basic/FileManager.h"
> +#include "clang/Basic/SourceManager.h"
>  #include "clang/Lex/Lexer.h"
> -#include "clang/Basic/SourceManager.h"
> +#include "clang/Frontend/FrontendDiagnostic.h"
>  #include "llvm/ADT/SmallString.h"
>  using namespace clang;
>
> @@ -412,3 +414,23 @@
>
>   return false;
>  }
> +
> +bool Rewriter::OverwriteChangedFiles() {
> +  bool AllWritten = true;
> +  for (buffer_iterator I = buffer_begin(), E = buffer_end(); I != E; ++I) {
> +    const FileEntry *Entry =
> +        getSourceMgr().getFileEntryForID(I->first);
> +    std::string ErrorInfo;
> +    llvm::raw_fd_ostream FileStream(
> +        Entry->getName(), ErrorInfo, llvm::raw_fd_ostream::F_Binary);
> +    if (!ErrorInfo.empty()) {
> +      getSourceMgr().getDiagnostics().Report(clang::diag::err_fe_unable_to_open_output)
> +        << Entry->getName() << ErrorInfo;
> +      AllWritten = false;
> +      continue;
> +    }
> +    I->second.write(FileStream);
> +    FileStream.flush();
> +  }
> +  return AllWritten;
> +}
>
> Second, the testing overhead; note that the RewriterTestContext will
> also be used in a subsequent CL which is currently under review, which
> is why I pulled out a class.
>
>
> Index: unittests/Tooling/RewriterTest.cpp
> ===================================================================
> --- unittests/Tooling/RewriterTest.cpp  (revision 0)
> +++ unittests/Tooling/RewriterTest.cpp  (revision 0)
> @@ -0,0 +1,37 @@
> +//===- unittest/Tooling/RewriterTest.cpp
> ----------------------------------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "RewriterTestContext.h"
> +#include "gtest/gtest.h"
> +
> +namespace clang {
> +
> +TEST(Rewriter, OverwritesChangedFiles) {
> +  RewriterTestContext Context;
> +  FileID ID = Context.createOnDiskFile("t.cpp", "line1\nline2\nline3\nline4");
> +  Context.Rewrite.ReplaceText(Context.getLocation(ID, 2, 1), 5, "replaced");
> +  EXPECT_TRUE(Context.Rewrite.OverwriteChangedFiles());
> +  EXPECT_EQ("line1\nreplaced\nline3\nline4",
> +            Context.getFileContentFromDisk("t.cpp"));
> +}
> +
> +TEST(Rewriter, ContinuesOverwritingFilesOnError) {
> +  RewriterTestContext Context;
> +  FileID FailingID = Context.createInMemoryFile("invalid/failing.cpp", "test");
> +  Context.Rewrite.ReplaceText(Context.getLocation(FailingID, 1, 2),
> 1, "other");
> +  FileID WorkingID = Context.createOnDiskFile(
> +    "working.cpp", "line1\nline2\nline3\nline4");
> +  Context.Rewrite.ReplaceText(Context.getLocation(WorkingID, 2, 1), 5,
> +                              "replaced");
> +  EXPECT_FALSE(Context.Rewrite.OverwriteChangedFiles());
> +  EXPECT_EQ("line1\nreplaced\nline3\nline4",
> +            Context.getFileContentFromDisk("working.cpp"));
> +}
> +
> +} // end namespace clang
>
> Index: unittests/Tooling/RewriterTestContext.h
> ===================================================================
> --- unittests/Tooling/RewriterTestContext.h     (revision 0)
> +++ unittests/Tooling/RewriterTestContext.h     (revision 0)
> @@ -0,0 +1,120 @@
> +//===--- RewriterTestContext.h ----------------------------------*-
> C++ -*-===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +//  This file defines a utility class for Rewriter related tests.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef LLVM_CLANG_REWRITER_TEST_CONTEXT_H
> +#define LLVM_CLANG_REWRITER_TEST_CONTEXT_H
> +
> +#include "clang/Basic/Diagnostic.h"
> +#include "clang/Basic/FileManager.h"
> +#include "clang/Basic/LangOptions.h"
> +#include "clang/Basic/SourceManager.h"
> +#include "clang/Frontend/DiagnosticOptions.h"
> +#include "clang/Frontend/TextDiagnosticPrinter.h"
> +#include "clang/Rewrite/Rewriter.h"
> +#include "llvm/Support/Path.h"
> +#include "llvm/Support/raw_ostream.h"
> +
> +namespace clang {
> +
> +/// \brief A class that sets up a ready to use Rewriter.
> +///
> +/// Useful in unit tests that need a Rewriter. Creates all dependencies
> +/// of a Rewriter with default values for testing and provides convenience
> +/// methods, which help with writing tests that change files.
> +class RewriterTestContext {
> + public:
> +  RewriterTestContext()
> +      : Diagnostics(llvm::IntrusiveRefCntPtr<DiagnosticIDs>()),
> +        DiagnosticPrinter(llvm::outs(), DiagnosticOptions()),
> +        Files((FileSystemOptions())),
> +        Sources(Diagnostics, Files),
> +        Rewrite(Sources, Options) {
> +    Diagnostics.setClient(&DiagnosticPrinter, false);
> +  }
> +
> +  ~RewriterTestContext() {
> +    if (TemporaryDirectory.isValid()) {
> +      std::string ErrorInfo;
> +      TemporaryDirectory.eraseFromDisk(true, &ErrorInfo);
> +      assert(ErrorInfo.empty());
> +    }
> +  }
> +
> +  FileID createInMemoryFile(StringRef Name, StringRef Content) {
> +    const llvm::MemoryBuffer *Source =
> +      llvm::MemoryBuffer::getMemBuffer(Content);
> +    const FileEntry *Entry =
> +      Files.getVirtualFile(Name, Source->getBufferSize(), 0);
> +    Sources.overrideFileContents(Entry, Source, true);
> +    assert(Entry != NULL);
> +    return Sources.createFileID(Entry, SourceLocation(), SrcMgr::C_User);
> +  }
> +
> +  FileID createOnDiskFile(StringRef Name, StringRef Content) {
> +    if (!TemporaryDirectory.isValid()) {
> +      std::string ErrorInfo;
> +      TemporaryDirectory = llvm::sys::Path::GetTemporaryDirectory(&ErrorInfo);
> +      assert(ErrorInfo.empty());
> +    }
> +    llvm::SmallString<1024> Path(TemporaryDirectory.str());
> +    llvm::sys::path::append(Path, Name);
> +    std::string ErrorInfo;
> +    llvm::raw_fd_ostream OutStream(Path.c_str(),
> +                                   ErrorInfo, llvm::raw_fd_ostream::F_Binary);
> +    assert(ErrorInfo.empty());
> +    OutStream << Content;
> +    OutStream.close();
> +    const FileEntry *File = Files.getFile(Path);
> +    assert(File != NULL);
> +    return Sources.createFileID(File, SourceLocation(), SrcMgr::C_User);
> +  }
> +
> +  SourceLocation getLocation(FileID ID, unsigned Line, unsigned Column) {
> +    SourceLocation Result = Sources.translateFileLineCol(
> +        Sources.getFileEntryForID(ID), Line, Column);
> +    assert(Result.isValid());
> +    return Result;
> +  }
> +
> +  std::string getRewrittenText(FileID ID) {
> +    std::string Result;
> +    llvm::raw_string_ostream OS(Result);
> +    Rewrite.getEditBuffer(ID).write(OS);
> +    return Result;
> +  }
> +
> +  std::string getFileContentFromDisk(StringRef Name) {
> +    llvm::SmallString<1024> Path(TemporaryDirectory.str());
> +    llvm::sys::path::append(Path, Name);
> +    // We need to read directly from the FileManager without relaying through
> +    // a FileEntry, as otherwise we'd read through an already opened file
> +    // descriptor, which might not see the changes made.
> +    // FIXME: Figure out whether there is a way to get the SourceManger to
> +    // reopen the file.
> +    return Files.getBufferForFile(Path, NULL)->getBuffer();
> +  }
> +
> +  DiagnosticsEngine Diagnostics;
> +  TextDiagnosticPrinter DiagnosticPrinter;
> +  FileManager Files;
> +  SourceManager Sources;
> +  LangOptions Options;
> +  Rewriter Rewrite;
> +
> +  // Will be set once on disk files are generated.
> +  llvm::sys::Path TemporaryDirectory;
> +};
> +
> +} // end namespace clang
> +
> +#endif
> Index: unittests/CMakeLists.txt
> ===================================================================
> --- unittests/CMakeLists.txt    (revision 156059)
> +++ unittests/CMakeLists.txt    (working copy)
> @@ -70,5 +70,6 @@
>   Tooling/CompilationDatabaseTest.cpp
>   Tooling/ToolingTest.cpp
>   Tooling/RecursiveASTVisitorTest.cpp
> -  USED_LIBS gtest gtest_main clangAST clangTooling
> +  Tooling/RewriterTest.cpp
> +  USED_LIBS gtest gtest_main clangAST clangTooling clangRewrite
>  )