[vfs] IJPL-247444: prohibit VirtualFile.rename() for already deleted VirtualFiles

+ `VirtualFile.rename()` should throw an exception if the file is already deleted because of general rule "accessing !valid files is illegal"
+ `VirtualFile.delete()` is unnatural to behave this way, so it just log warning


(cherry picked from commit bcf565ddf9d2014d1e95fe6451c6a9629d7b0ad3)

IJ-CR-211437

GitOrigin-RevId: 529bec2d3da89e0c707bd8e813694c4e95e1979f

platform/core-api/src/com/intellij/openapi/vfs/InvalidVirtualFileAccessException.java

@@ -23,7 +23,7 @@ public class InvalidVirtualFileAccessException extends RuntimeException {
 
   private static @NonNls String composeMessage(@NotNull VirtualFile file) {
     String url = file.getUrl();
-    @NonNls String message = "Accessing invalid virtual file: " + url;
+    @NonNls String message = "Accessing invalid (=!isValid) virtual file: " + url;
     String reason = getInvalidationReason(file);
     if (reason != null) {
       message += "; reason: " + reason;

platform/core-api/src/com/intellij/openapi/vfs/VirtualFile.java

@@ -260,10 +260,10 @@ public abstract class VirtualFile extends UserDataHolderBase implements Modifica
 
   /// Checks whether this `VirtualFile` is valid. File can be invalidated either by deleting it or one of its
   /// parents with [#delete] method or by an external change.
-  /// If file is not valid only [#equals], [#hashCode],
+  /// If the file is not valid only [#equals], [#hashCode],
   /// [#getName()], [#getPath()], [#getUrl()], [#getPresentableUrl()] and methods from
   /// [UserDataHolder] can be called for it. Using any other methods for an invalid [VirtualFile] instance
-  /// produce unpredictable results.
+  /// produces unpredictable results, usually an exception.
   ///
   /// @return `true` if this is a valid file, `false` otherwise
   public abstract boolean isValid();
@@ -419,11 +419,14 @@ public abstract class VirtualFile extends UserDataHolderBase implements Modifica
   /// @param requestor any object to control who called this method. Note that
   ///                  it is considered to be an external change if `requestor` is `null`.
   ///                  See [VirtualFileEvent#getRequestor]
-  /// @throws IOException if file failed to be deleted
+  /// @throws IOException if the file failed to be deleted
   @RequiresWriteLock
   public void delete(Object requestor) throws IOException {
     ApplicationManager.getApplication().assertWriteAccessAllowed();
-    LOG.assertTrue(isValid(), "Deleting invalid file");
+    if (!isValid()) {
+      LOG.warn("Deleting invalid (already deleted?) file: " + this + " -> nothing to delete, skip");
+      return;
+    }
     getFileSystem().deleteFile(requestor, this);
   }
 

platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/impl/VirtualFileSystemEntry.java

@@ -497,6 +497,28 @@ public abstract class VirtualFileSystemEntry extends NewVirtualFile {
   @Override
   public void delete(Object requestor) throws IOException {
     ApplicationManager.getApplication().assertWriteAccessAllowed();
+
+    if (!isValid()) {
+      //We have a general rule "Accessing !valid VirtualFile is incorrect, except for a limited set of methods needed
+      // for identifying a VirtualFile, like id/path/toString" -- see VirtualFile.isValid jdocs.
+      // According to this rule, an attempt to _delete_ VirtualFile which is !valid -- should be an error.
+      // But it is a bit of unnatural behavior for .delete(): .delete()-like methods are _usually_ idempotent, i.e.,
+      // safe to be called repeatedly -- and people expect them to behave that way.
+      // And historically .delete() didn't throw an exception in this case -- and as a result, we have a number of
+      // usages in our codebase (and, probably, in plugins too) that don't expect .delete() to throw an exception
+      // if called on an already deleted file.
+      // So, for the sake of backward compatibility, we keep supporting that legacy behavior here: log warning (which
+      // should contain a stacktrace of an original deleter), and return unharmed.
+      // Luckily, the rule in VirtualFile.isValid() does specify that accessing an invalid file is "incorrect", but does
+      // NOT specify that kind of bad things will follow -- so just "warn" is pretty legit.
+
+      Logger.getInstance(VirtualFileSystemEntry.class).warn(
+        "Deleting invalid (already deleted?) file -> nothing to delete, skip",
+        new InvalidVirtualFileAccessException(this)
+      );
+      return;
+    }
+
     owningPersistentFS().deleteFile(requestor, this);
   }
 
@@ -504,6 +526,8 @@ public abstract class VirtualFileSystemEntry extends NewVirtualFile {
   @Override
   public void rename(Object requestor, @NotNull @NonNls String newName) throws IOException {
     ApplicationManager.getApplication().assertWriteAccessAllowed();
+    failIfFileIsInvalid();
+
     if (getName().equals(newName)) return;
     validateName(newName);
     owningPersistentFS().renameFile(requestor, this, newName);
@@ -512,10 +536,17 @@ public abstract class VirtualFileSystemEntry extends NewVirtualFile {
   @RequiresWriteLock
   @Override
   public @NotNull VirtualFile createChildData(Object requestor, @NotNull String name) throws IOException {
+    failIfFileIsInvalid();
     validateName(name);
     return owningPersistentFS().createChildFile(requestor, this, name);
   }
 
+  private void failIfFileIsInvalid() {
+    if (!isValid()) {
+      throw new InvalidVirtualFileAccessException(this);
+    }
+  }
+
   @Override
   public boolean isWritable() {
     return getFlagInt(VfsDataFlags.IS_WRITABLE_FLAG);

platform/testFramework/junit5/src/fixture/fixtures.kt

@@ -376,7 +376,9 @@ fun TestFixture<PsiDirectory>.virtualFileFixture(
   }
   initialized(file) {
     edtWriteAction {
-      file.delete(dirFixture)
+      if (file.isValid) {
+        file.delete(dirFixture)
+      }
     }
   }
 }

platform/testFramework/src/com/intellij/testFramework/VfsTestUtil.java

@@ -130,7 +130,9 @@ public final class VfsTestUtil {
   public static void deleteFile(@NotNull VirtualFile file) {
     try {
       // requestor must be notnull (for GlobalUndoTest)
-      WriteAction.runAndWait(() -> file.delete(file));
+      WriteAction.runAndWait(() -> {
+        if(file.isValid()) file.delete(file);
+      });
     }
     catch (Throwable throwable) {
       ExceptionUtil.rethrow(throwable);

platform/testFramework/src/com/intellij/testFramework/fixtures/impl/TempDirTestFixtureImpl.java

@@ -140,7 +140,9 @@ public class TempDirTestFixtureImpl extends BaseFixture implements TempDirTestFi
         VirtualFile virtualFile = LocalFileSystem.getInstance().findFileByPath(FileUtil.toSystemIndependentName(myTempDir.toString()));
         if (virtualFile != null) {
           WriteAction.runAndWait(() -> {
-            virtualFile.delete(this);
+            if(virtualFile.isValid()) {
+              virtualFile.delete(this);
+            }
           });
         }
       }