[PATCH] D35194: [clang-tidy] clang-apply-replacements: Don't insert null entry

[Kevin Funk via Phabricator via cfe-commits]

kfunk updated this revision to Diff 108061.
kfunk added a comment.

Addressed concerns


https://reviews.llvm.org/D35194

Files:
  clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
  test/clang-apply-replacements/invalid-files.cpp


Index: test/clang-apply-replacements/invalid-files.cpp
===================================================================
--- /dev/null
+++ test/clang-apply-replacements/invalid-files.cpp
@@ -0,0 +1,5 @@
+// RUN: mkdir -p %T/invalid-files
+// RUN: clang-apply-replacements %T/invalid-files
+//
+// Check that the yaml files are *not* deleted after running clang-apply-replacements without remove-change-desc-files.
+// RUN: ls %T/Inputs/invalid-files/invalid-files.yaml
Index: test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
===================================================================
--- /dev/null
+++ test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
@@ -0,0 +1,12 @@
+---
+MainSourceFile:  ''
+Replacements:
+  - FilePath:        idontexist.h
+    Offset:          2669
+    Length:          0
+    ReplacementText: ' override'
+  - FilePath:        idontexist.h
+    Offset:          2669
+    Length:          0
+    ReplacementText: ' override'
+...
Index: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===================================================================
--- clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -288,13 +288,12 @@
     for (const tooling::Replacement &R : TU.Replacements) {
       // Use the file manager to deduplicate paths. FileEntries are
       // automatically canonicalized.
-      const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath());
-      if (!Entry && Warned.insert(R.getFilePath()).second) {
-        errs() << "Described file '" << R.getFilePath()
-               << "' doesn't exist. Ignoring...\n";
-        continue;
+      if (const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath())) {
+        GroupedReplacements[Entry].push_back(R);
+      } else if (Warned.insert(R.getFilePath()).second) {
+          errs() << "Described file '" << R.getFilePath()
+                 << "' doesn't exist. Ignoring...\n";
       }
-      GroupedReplacements[Entry].push_back(R);
     }
   }
 
@@ -314,13 +313,12 @@
         for (const tooling::Replacement &R : Fix.second) {
           // Use the file manager to deduplicate paths. FileEntries are
           // automatically canonicalized.
-          const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath());
-          if (!Entry && Warned.insert(R.getFilePath()).second) {
-            errs() << "Described file '" << R.getFilePath()
-                   << "' doesn't exist. Ignoring...\n";
-            continue;
+          if (const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath())) {
+            GroupedReplacements[Entry].push_back(R);
+          } else if (Warned.insert(R.getFilePath()).second) {
+              errs() << "Described file '" << R.getFilePath()
+                     << "' doesn't exist. Ignoring...\n";
           }
-          GroupedReplacements[Entry].push_back(R);
         }
       }
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35194.108061.patch
Type: text/x-patch
Size: 3011 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170725/6b2f8552/attachment-0001.bin>