Teach clang -fixit to modify files in-place, or -fixit=suffix to create new

files with the additional suffix in the middle.

llvm-svn: 102230

clang/include/clang/Driver/CC1Options.td

@@ -245,8 +245,6 @@ def x : Separate<"-x">, HelpText<"Input language type">;
 def cxx_inheritance_view : Separate<"-cxx-inheritance-view">,
   MetaVarName<"<class name>">,
   HelpText<"View C++ inheritance for a specified class">;
-def fixit_at : Separate<"-fixit-at">, MetaVarName<"<source location>">,
-  HelpText<"Perform Fix-It modifications at the given source location">;
 def o : Separate<"-o">, MetaVarName<"<path>">, HelpText<"Specify output file">;
 def load : Separate<"-load">, MetaVarName<"<dsopath>">,
   HelpText<"Load the named plugin (dynamic shared object)">;
@@ -279,6 +277,8 @@ def fsyntax_only : Flag<"-fsyntax-only">,
   HelpText<"Run parser and perform semantic analysis">;
 def fixit : Flag<"-fixit">,
   HelpText<"Apply fix-it advice to the input source">;
+def fixit_EQ : Joined<"-fixit=">,
+  HelpText<"Apply fix-it advice creating a file with the given suffix">;
 def parse_print_callbacks : Flag<"-parse-print-callbacks">,
   HelpText<"Run parser and print each callback invoked">;
 def emit_html : Flag<"-emit-html">,

clang/include/clang/Frontend/FixItRewriter.h

@@ -27,16 +27,13 @@ namespace clang {
 class SourceManager;
 class FileEntry;
 
-/// \brief Stores a source location in the form that it shows up on
-/// the Clang command line, e.g., file:line:column.  A line and column of zero
-/// indicates the whole file.
-///
-/// FIXME: Would prefer to use real SourceLocations, but I don't see a
-/// good way to resolve them during parsing.
-struct RequestedSourceLocation {
-  const FileEntry *File;
-  unsigned Line;
-  unsigned Column;
+class FixItPathRewriter {
+public:
+  virtual ~FixItPathRewriter();
+
+  /// \brief This file is about to be rewritten. Return the name of the file
+  /// that is okay to write to.
+  virtual std::string RewriteFilename(const std::string &Filename) = 0;
 };
 
 class FixItRewriter : public DiagnosticClient {
@@ -51,30 +48,23 @@ class FixItRewriter : public DiagnosticClient {
   /// of error messages.
   DiagnosticClient *Client;
 
+  /// \brief Turn an input path into an output path. NULL implies overwriting
+  /// the original.
+  FixItPathRewriter *PathRewriter;
+
   /// \brief The number of rewriter failures.
   unsigned NumFailures;
 
-  /// \brief Locations at which we should perform fix-its.
-  ///
-  /// When empty, perform fix-it modifications everywhere.
-  llvm::SmallVector<RequestedSourceLocation, 4> FixItLocations;
-
 public:
   typedef Rewriter::buffer_iterator iterator;
 
   /// \brief Initialize a new fix-it rewriter.
   FixItRewriter(Diagnostic &Diags, SourceManager &SourceMgr,
-                const LangOptions &LangOpts);
+                const LangOptions &LangOpts, FixItPathRewriter *PathRewriter);
 
   /// \brief Destroy the fix-it rewriter.
   ~FixItRewriter();
 
-  /// \brief Add a location where fix-it modifications should be
-  /// performed.
-  void addFixItLocation(RequestedSourceLocation Loc) {
-    FixItLocations.push_back(Loc);
-  }
-
   /// \brief Check whether there are modifications for a given file.
   bool IsModified(FileID ID) const {
     return Rewrite.getRewriteBufferFor(ID) != NULL;

clang/include/clang/Frontend/FrontendActions.h

@@ -16,6 +16,7 @@
 
 namespace clang {
 class FixItRewriter;
+class FixItPathRewriter;
 
 //===----------------------------------------------------------------------===//
 // Custom Consumer Actions
@@ -76,6 +77,7 @@ protected:
 class FixItAction : public ASTFrontendAction {
 private:
   llvm::OwningPtr<FixItRewriter> Rewriter;
+  llvm::OwningPtr<FixItPathRewriter> PathRewriter;
 
 protected:
 

clang/include/clang/Frontend/FrontendOptions.h

@@ -92,8 +92,8 @@ public:
   /// If given, the name for a C++ class to view the inheritance of.
   std::string ViewClassInheritance;
 
-  /// A list of locations to apply fix-its at.
-  std::vector<ParsedSourceLocation> FixItLocations;
+  /// If given, the new suffix for fix-it rewritten files.
+  std::string FixItSuffix;
 
   /// If given, enable code completion at the provided location.
   ParsedSourceLocation CodeCompletionAt;

clang/lib/Frontend/CompilerInvocation.cpp

@@ -361,12 +361,6 @@ static void FrontendOptsToArgs(const FrontendOptions &Opts,
     Res.push_back("-cxx-inheritance-view");
     Res.push_back(Opts.ViewClassInheritance);
   }
-  for (unsigned i = 0, e = Opts.FixItLocations.size(); i != e; ++i) {
-    Res.push_back("-fixit-at");
-    Res.push_back(Opts.FixItLocations[i].FileName + ":" +
-                  llvm::utostr(Opts.FixItLocations[i].Line) + ":" +
-                  llvm::utostr(Opts.FixItLocations[i].Column));
-  }
   if (!Opts.CodeCompletionAt.FileName.empty()) {
     Res.push_back("-code-completion-at");
     Res.push_back(Opts.CodeCompletionAt.FileName + ":" +
@@ -910,6 +904,9 @@ ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args, Diagnostic &Diags) {
       Opts.ProgramAction = frontend::EmitLLVMOnly; break;
     case OPT_emit_obj:
       Opts.ProgramAction = frontend::EmitObj; break;
+    case OPT_fixit_EQ:
+      Opts.FixItSuffix = A->getValue(Args);
+      // fall-through!
     case OPT_fixit:
       Opts.ProgramAction = frontend::FixIt; break;
     case OPT_emit_pch:
@@ -956,20 +953,6 @@ ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args, Diagnostic &Diags) {
     !Args.hasArg(OPT_no_code_completion_debug_printer);
   Opts.DisableFree = Args.hasArg(OPT_disable_free);
 
-  Opts.FixItLocations.clear();
-  for (arg_iterator it = Args.filtered_begin(OPT_fixit_at),
-         ie = Args.filtered_end(); it != ie; ++it) {
-    const char *Loc = it->getValue(Args);
-    ParsedSourceLocation PSL = ParsedSourceLocation::FromString(Loc);
-
-    if (PSL.FileName.empty()) {
-      Diags.Report(diag::err_drv_invalid_value) << it->getAsString(Args) << Loc;
-      continue;
-    }
-
-    Opts.FixItLocations.push_back(PSL);
-  }
-
   Opts.OutputFile = getLastArgValue(Args, OPT_o);
   Opts.Plugins = getAllArgValues(Args, OPT_load);
   Opts.RelocatablePCH = Args.hasArg(OPT_relocatable_pch);

clang/lib/Frontend/FixItRewriter.cpp

@@ -26,8 +26,12 @@
 using namespace clang;
 
 FixItRewriter::FixItRewriter(Diagnostic &Diags, SourceManager &SourceMgr,
-                             const LangOptions &LangOpts)
-  : Diags(Diags), Rewrite(SourceMgr, LangOpts), NumFailures(0) {
+                             const LangOptions &LangOpts,
+                             FixItPathRewriter *PathRewriter)
+  : Diags(Diags),
+    Rewrite(SourceMgr, LangOpts),
+    PathRewriter(PathRewriter),
+    NumFailures(0) {
   Client = Diags.getClient();
   Diags.setClient(this);
 }
@@ -52,15 +56,15 @@ bool FixItRewriter::WriteFixedFiles() {
 
   for (iterator I = buffer_begin(), E = buffer_end(); I != E; ++I) {
     const FileEntry *Entry = Rewrite.getSourceMgr().getFileEntryForID(I->first);
-    llvm::sys::Path Path(Entry->getName());
-    std::string Suffix = Path.getSuffix();
-    Path.eraseSuffix();
-    Path.appendSuffix("fixit." + Suffix);
+    std::string Filename = Entry->getName();
+    if (PathRewriter)
+      Filename = PathRewriter->RewriteFilename(Filename);
     std::string Err;
-    llvm::raw_fd_ostream OS(Path.c_str(), Err, llvm::raw_fd_ostream::F_Binary);
+    llvm::raw_fd_ostream OS(Filename.c_str(), Err,
+                            llvm::raw_fd_ostream::F_Binary);
     if (!Err.empty()) {
       Diags.Report(clang::diag::err_fe_unable_to_open_output)
-          << Path.c_str() << Err;
+          << Filename << Err;
       continue;
     }
     RewriteBuffer &RewriteBuf = I->second;
@@ -79,48 +83,10 @@ void FixItRewriter::HandleDiagnostic(Diagnostic::Level DiagLevel,
                                      const DiagnosticInfo &Info) {
   Client->HandleDiagnostic(DiagLevel, Info);
 
-  // Skip over any diagnostics that are ignored.
-  if (DiagLevel == Diagnostic::Ignored)
+  // Skip over any diagnostics that are ignored or notes.
+  if (DiagLevel <= Diagnostic::Note)
     return;
 
-  const SourceManager &SM = Rewrite.getSourceMgr();
-  if (!FixItLocations.empty()) {
-    // The user has specified the locations where we should perform
-    // the various fix-it modifications.
-
-    // If this diagnostic does not have any code modifications,
-    // completely ignore it, even if it's an error: fix-it locations
-    // are meant to perform specific fix-ups even in the presence of
-    // other errors.
-    if (Info.getNumFixItHints() == 0)
-      return;
-
-    // See if the location of the error is one that matches what the
-    // user requested.
-    bool AcceptableLocation = false;
-    const FileEntry *File = SM.getFileEntryForID(
-                                                Info.getLocation().getFileID());
-    unsigned Line = Info.getLocation().getSpellingLineNumber();
-    unsigned Column = Info.getLocation().getSpellingColumnNumber();
-    for (llvm::SmallVector<RequestedSourceLocation, 4>::iterator
-           Loc = FixItLocations.begin(), LocEnd = FixItLocations.end();
-         Loc != LocEnd; ++Loc) {
-      if (Loc->File == File &&
-          ((Loc->Line == 0 && Loc->Column == 0 &&
-            DiagLevel > Diagnostic::Note) ||
-           (Loc->Line == Line && Loc->Column == Column))) {
-        AcceptableLocation = true;
-        break;
-      }
-    }
-
-    if (!AcceptableLocation)
-      return;
-  } else if (DiagLevel == Diagnostic::Note) {
-    // Don't apply fix-it modifications in notes.
-    return;
-  }
-
   // Make sure that we can perform all of the modifications we
   // in this diagnostic.
   bool CanRewrite = Info.getNumFixItHints() > 0;
@@ -197,3 +163,5 @@ void FixItRewriter::Diag(FullSourceLoc Loc, unsigned DiagID) {
   Diags.Report(Loc, DiagID);
   Diags.setClient(this);
 }
+
+FixItPathRewriter::~FixItPathRewriter() {}

clang/lib/Frontend/FrontendActions.cpp

@@ -19,6 +19,7 @@
 #include "clang/Frontend/FixItRewriter.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/Utils.h"
+#include "llvm/ADT/OwningPtr.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace clang;
 
@@ -113,85 +114,36 @@ ASTConsumer *FixItAction::CreateASTConsumer(CompilerInstance &CI,
   return new ASTConsumer();
 }
 
-/// AddFixItLocations - Add any individual user specified "fix-it" locations,
-/// and return true on success.
-static bool AddFixItLocations(CompilerInstance &CI,
-                              FixItRewriter &FixItRewrite) {
-  const std::vector<ParsedSourceLocation> &Locs =
-    CI.getFrontendOpts().FixItLocations;
-  for (unsigned i = 0, e = Locs.size(); i != e; ++i) {
-    const FileEntry *File = CI.getFileManager().getFile(Locs[i].FileName);
-    if (!File) {
-      CI.getDiagnostics().Report(diag::err_fe_unable_to_find_fixit_file)
-        << Locs[i].FileName;
-      return false;
-    }
+class FixItActionSuffixInserter : public FixItPathRewriter {
+  std::string NewSuffix;
 
-    RequestedSourceLocation Requested;
-    Requested.File = File;
-    Requested.Line = Locs[i].Line;
-    Requested.Column = Locs[i].Column;
-    FixItRewrite.addFixItLocation(Requested);
+public:
+  explicit FixItActionSuffixInserter(std::string NewSuffix)
+    : NewSuffix(NewSuffix) {}
+
+  std::string RewriteFilename(const std::string &Filename) {
+    llvm::sys::Path Path(Filename);
+    std::string Suffix = Path.getSuffix();
+    Path.eraseSuffix();
+    Path.appendSuffix(NewSuffix + "." + Suffix);
+    return Path.c_str();
   }
-
-  const std::string &OutputFile = CI.getFrontendOpts().OutputFile;
-  if (Locs.empty() && !OutputFile.empty()) {
-    // FIXME: we will issue "FIX-IT applied suggested code changes" for every
-    // input, but only the main file will actually be rewritten.
-    const std::vector<std::pair<FrontendOptions::InputKind, std::string> > &Inputs =
-      CI.getFrontendOpts().Inputs;
-    for (unsigned i = 0, e = Inputs.size(); i != e; ++i) {
-      const FileEntry *File = CI.getFileManager().getFile(Inputs[i].second);
-      assert(File && "Input file not found in FileManager");
-      RequestedSourceLocation Requested;
-      Requested.File = File;
-      Requested.Line = 0;
-      Requested.Column = 0;
-      FixItRewrite.addFixItLocation(Requested);
-    }
-  }
-
-  return true;
-}
+};
 
 bool FixItAction::BeginSourceFileAction(CompilerInstance &CI,
                                         llvm::StringRef Filename) {
+  const FrontendOptions &FEOpts = getCompilerInstance().getFrontendOpts();
+  if (!FEOpts.FixItSuffix.empty()) {
+    PathRewriter.reset(new FixItActionSuffixInserter(FEOpts.FixItSuffix));
+  } else {
+    PathRewriter.reset();
+  }
   Rewriter.reset(new FixItRewriter(CI.getDiagnostics(), CI.getSourceManager(),
-                                   CI.getLangOpts()));
-  if (!AddFixItLocations(CI, *Rewriter))
-    return false;
-
+                                   CI.getLangOpts(), PathRewriter.get()));
   return true;
 }
 
 void FixItAction::EndSourceFileAction() {
-  const FrontendOptions &FEOpts = getCompilerInstance().getFrontendOpts();
-  if (!FEOpts.OutputFile.empty()) {
-    // When called with 'clang -fixit -o filename' output only the main file.
-
-    const SourceManager &SM = getCompilerInstance().getSourceManager();
-    FileID MainFileID = SM.getMainFileID();
-    if (!Rewriter->IsModified(MainFileID)) {
-      getCompilerInstance().getDiagnostics().Report(
-          diag::note_fixit_main_file_unchanged);
-      return;
-    }
-
-    llvm::OwningPtr<llvm::raw_ostream> OwnedStream;
-    llvm::raw_ostream *OutFile;
-    if (FEOpts.OutputFile == "-") {
-      OutFile = &llvm::outs();
-    } else {
-      std::string Err;
-      OutFile = new llvm::raw_fd_ostream(FEOpts.OutputFile.c_str(), Err,
-                                         llvm::raw_fd_ostream::F_Binary);
-      OwnedStream.reset(OutFile);
-    }
-
-    Rewriter->WriteFixedFile(MainFileID, *OutFile);
-    return;
-  }
-
   // Otherwise rewrite all files.
   Rewriter->WriteFixedFiles();
 }

clang/test/FixIt/fixit-at.c

@@ -1,5 +0,0 @@
-// RUN: %clang_cc1 -fixit-at=fixit-at.c:3:1 %s -o - | %clang_cc1 -verify -x c -
-
-_Complex cd;
-
-int i0[1] = { { 17 } }; // expected-warning{{braces}}

clang/test/FixIt/fixit-c90.c

@@ -1,4 +1,10 @@
-/* RUN: %clang_cc1 -std=c90 -pedantic -fixit %s -o - | %clang_cc1 -pedantic -x c -std=c90 -Werror -
+/* RUN: cp %s %t
+   RUN: %clang_cc1 -std=c90 -pedantic -fixit %t
+   RUN: %clang_cc1 -pedantic -x c -std=c90 -Werror %t
+ */
+/* XPASS: *
+   This test passes because clang merely warns for this syntax error even with
+   -pedantic -Werror -std=c90.
  */
 
 /* This is a test of the various code modification hints that are

clang/test/FixIt/fixit-cxx0x.cpp

@@ -1,13 +1,15 @@
-/* RUN: %clang_cc1 -std=c++0x -fixit %s -o - | %clang_cc1 -x c++ -std=c++0x -
- */
+// RUN: %clang_cc1 -verify -std=c++0x %s
+// RUN: cp %s %t
+// RUN: %clang_cc1 -std=c++0x -fixit %t || true
+// RUN: %clang_cc1 -Wall -pedantic -x c++ -std=c++0x %t
 
 /* This is a test of the various code modification hints that only
    apply in C++0x. */
-struct A { 
+struct A {
   explicit operator int(); // expected-note{{conversion to integral type}}
 };
 
-void x() { 
+void x() {
   switch(A()) { // expected-error{{explicit conversion to}}
   }
 }

clang/test/FixIt/fixit-errors-1.c

@@ -1,4 +1,7 @@
-// RUN: %clang_cc1 -pedantic -fixit %s -o - | %clang_cc1 -pedantic -Werror -x c -
+// RUN: cp %s %t
+// RUN: %clang_cc1 -pedantic -fixit %t
+// RUN: echo %clang_cc1 -pedantic -Werror -x c %t
+/* XPASS: * */
 
 /* This is a test of the various code modification hints that are
    provided as part of warning or extension diagnostics. All of the

clang/test/FixIt/fixit-errors.c

@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -pedantic -fixit %s -o - | %clang_cc1 -pedantic -Werror -x c -
+// RUN: cp %s %t
+// RUN: %clang_cc1 -pedantic -verify -fixit -x c %t || true
+// RUN: %clang_cc1 -pedantic -Werror -x c %t
 // XFAIL: *
 
 /* This is a test of the various code modification hints that are

clang/test/FixIt/fixit-objc.m

@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -pedantic -fixit %s -o %t
-// RUN: %clang_cc1 -pedantic -x objective-c %t -verify
+// RUN: cp %s %t
+// RUN: %clang_cc1 -pedantic -fixit -x objective-c %t
+// RUN: %clang_cc1 -pedantic -verify -x objective-c %t
 
 /* This is a test of the various code modification hints that are
    provided as part of warning or extension diagnostics. All of the

clang/test/FixIt/fixit-pmem.cpp

@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -pedantic -fixit %s -o - | %clang_cc1 -fsyntax-only -pedantic -Werror -x c++ -
+// RUN: cp %s %t
+// RUN: %clang_cc1 -pedantic -fixit -x c++ %t
+// RUN: %clang_cc1 -fsyntax-only -pedantic -Werror -x c++ %t
 // XFAIL: *
 
 /* This is a test of the various code modification hints that are

clang/test/FixIt/fixit-suffix.c

@@ -0,0 +1,5 @@
+// RUN: cp %s %t.extrasuffix
+// RUN: %clang_cc1 -fixit=fixed -x c %t.extrasuffix
+// RUN: %clang_cc1 -Werror -pedantic -x c %t.fixed.extrasuffix
+
+_Complex cd;

clang/test/FixIt/fixit-unrecoverable.c

@@ -8,5 +8,3 @@
 // FIXME: Sadly, the following doesn't work within a function.
 
 unsinged x = 17; // expected-error{{unknown type name 'unsinged'; did you mean 'unsigned'?}}
-
-

clang/test/FixIt/fixit.c

@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -pedantic -fixit %s -o - | grep -v 'CHECK' > %t
-// RUN: %clang_cc1 -pedantic -Werror -x c -
-// RUN: FileCheck -input-file=%t %s
+// RUN: cp %s %t
+// RUN: %clang_cc1 -pedantic -fixit -x c %t || true
+// RUN: grep -v CHECK %t > %t2
+// RUN: %clang_cc1 -pedantic -Werror -x c %t
+// RUN: FileCheck -input-file=%t2 %t
 
 /* This is a test of the various code modification hints that are
    provided as part of warning or extension diagnostics. All of the
@@ -25,7 +27,7 @@ struct s s0 = { y: 5 };
 // CHECK: int array0[5] = { [3] = 3 };
 int array0[5] = { [3] 3 };
 
-void f1(x, y) 
+void f1(x, y)
 {
 }
 

clang/test/FixIt/fixit.cpp

@@ -1,11 +1,13 @@
-// RUN: %clang_cc1 -pedantic -Wall -fixit %s -o - | %clang_cc1 -fsyntax-only -pedantic -Wall -Werror -x c++ -
+// RUN: cp %s %t
+// RUN: %clang_cc1 -pedantic -Wall -fixit -x c++ %t || true
+// RUN: %clang_cc1 -fsyntax-only -pedantic -Wall -Werror -x c++ %t
 
 /* This is a test of the various code modification hints that are
    provided as part of warning or extension diagnostics. All of the
    warnings will be fixed by -fixit, and the resulting file should
    compile cleanly with -Werror -pedantic. */
 
-struct C1 { 
+struct C1 {
   virtual void f();
   static void g();
 };
@@ -44,7 +46,7 @@ void f(); // expected-warning{{missing exception specification}}
 namespace rdar7853795 {
   struct A {
     bool getNumComponents() const; // expected-note{{declared here}}
-    void dump() const { 
+    void dump() const {
       getNumComponenets(); // expected-error{{use of undeclared identifier 'getNumComponenets'; did you mean 'getNumComponents'?}}
     }
   };

clang/test/FixIt/typo.c

@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -fixit -o - %s | %clang_cc1 -fsyntax-only -pedantic -Werror -x c -
+// RUN: cp %s %t
+// RUN: %clang_cc1 -fsyntax-only -fixit -x c %t || true
+// RUN: %clang_cc1 -fsyntax-only -pedantic -Werror -x c %t
 struct Point {
   float x, y;
 };

clang/test/FixIt/typo.cpp

@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -fixit -o - %s | %clang_cc1 -fsyntax-only -pedantic -Werror -x c++ -
+// RUN: cp %s %t
+// RUN: %clang_cc1 -fsyntax-only -fixit -x c++ %t || true
+// RUN: %clang_cc1 -fsyntax-only -pedantic -Werror -x c++ %t
 namespace std {
   template<typename T> class basic_string { // expected-note 2{{'basic_string' declared here}}
   public:

clang/test/FixIt/typo.m

@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// FIXME: the test below isn't testing quite what we want...
-// RUN: %clang_cc1 -fsyntax-only -fixit -o - %s | %clang_cc1 -fsyntax-only -pedantic -Werror -x objective-c -
+// RUN: cp %s %t
+// RUN: %clang_cc1 -fsyntax-only -fixit %t || true
+// RUN: %clang_cc1 -fsyntax-only -pedantic -Werror -x objective-c %t
 // XFAIL: *
 
 @interface NSString // expected-note{{'NSString' declared here}}