[clang-apply-replacements] Add command line option to overwrite readonly files.

[zturner]

Summary

Some source code control systems attempt to prevent you from editing files unless you explicitly check them out. This makes it impossible to use certain refactoring tools such as this, since only the tool itself is able to determine the set of files that need to be modified. This patch adds a --force option which clears the read-only bit of the file so that it can be modified.

[aaron.ballman]

Can you add a test case for this functionality?

clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp

52

Missing full stop at the end of the string.

Also, it looks like this is doing something subtly different than what the help text says. It looks like this is overwriting the *permissions* of the file when applying replacements. Nothing seems to reset the permissions back to what they once were when the file is closed. Is that accurate?

161

Rather than spell out the type in the declaration name but use auto, how about: ErrorOr<perms> EP or something?

[zturner]

In D70553#1757862, @aaron.ballman wrote:

Can you add a test case for this functionality?

The reason there's no test case is because it seems like that wouldn't really be any different than testing the functionality of llvm::sys::fs, which should already be handled at the llvm/Support layer. There's not really any interesting logic here. I can still do one if you think it's important but that was my reasoning anyway.

[aaron.ballman]

In D70553#1773046, @zturner wrote:

In D70553#1757862, @aaron.ballman wrote:

Can you add a test case for this functionality?

The reason there's no test case is because it seems like that wouldn't really be any different than testing the functionality of llvm::sys::fs, which should already be handled at the llvm/Support layer. There's not really any interesting logic here. I can still do one if you think it's important but that was my reasoning anyway.

I have a slight preference for a test because it's a user-facing feature and we want to be alerted if we ever regress it. The unit tests in llvm/Support will catch if we break the underlying get/set permissions, but not the user feature (though it would be hard to break this user feature except through get/set permissions, lol). If it's hard to test for some reason, I don't insist on it.

clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp

static cl::opt<bool> RemoveTUReplacementFiles(
     cl::init(false), cl::cat(ReplacementCategory));
 
 static cl::opt<bool> DoFormat(
     "format",
     cl::desc("Enable formatting of code changed by applying replacements.\n"
              "Use -style to choose formatting style.\n"),
     cl::cat(FormattingCategory));
 
+static cl::opt<bool> ForceOverwriteReadOnly(
+    "force", cl::desc("Overwrite read-only files when applying replacements\n"),
+    cl::init(false), cl::cat(ReplacementCategory));
+
 // FIXME: Consider making the default behaviour for finding a style
 // configuration file to start the search anew for every file being changed to
 // handle situations where the style is different for different parts of a
 // project.
 
 static cl::opt<std::string> FormatStyleConfig(
     "style-config",
     cl::desc("Path to a directory containing a .clang-format file\n"
llvm::Expected<std::string> NewFileData =
         applyChanges(FileName, FileChange.second, Spec, Diagnostics);
     if (!NewFileData) {
       errs() << llvm::toString(NewFileData.takeError()) << "\n";
       continue;
     }
 
     // Write new file to disk
     std::error_code EC;
+    if (ForceOverwriteReadOnly) {
+      using namespace llvm::sys::fs;
+      if (auto ErrorOrPerms = getPermissions(FileName)) {
+        perms P = ErrorOrPerms.get();
+        setPermissions(FileName, P | all_write);
+      }
+    }
     llvm::raw_fd_ostream FileStream(FileName, EC, llvm::sys::fs::OF_None);
     if (EC) {
       llvm::errs() << "Could not open " << FileName << " for writing\n";
       continue;
     }
     FileStream << *NewFileData;
   }
 
   return 0;
 }