From cd3ddb7e5ea90f9dfe40e1614409dc148cafc87e Mon Sep 17 00:00:00 2001 From: Max Medvedev Date: Thu, 13 Nov 2025 15:59:19 +0100 Subject: [PATCH] IJPL-218636 [psi] send delete events for all psi trees on file deletion GitOrigin-RevId: 7448042e20cfe044cb87f5238e3b8021a9c34d0f --- platform/core-impl/api-dump-experimental.txt | 1 + .../psi/impl/file/impl/FileManager.java | 9 ++ .../psi/impl/file/impl/FileManagerEx.java | 9 +- .../psi/impl/file/impl/FileManagerImpl.java | 39 +++--- .../com/intellij/mock/MockFileManager.java | 5 + .../psi/impl/file/impl/PsiVFSListener.kt | 89 ++++++++------ .../impl/file/impl/MultiversePsiEventTest.kt | 112 ++++++++++++++++++ 7 files changed, 206 insertions(+), 58 deletions(-) create mode 100644 platform/lang-impl/testSources/com/intellij/psi/impl/file/impl/MultiversePsiEventTest.kt diff --git a/platform/core-impl/api-dump-experimental.txt b/platform/core-impl/api-dump-experimental.txt index df96ba8a6315..d10a96b5816e 100644 --- a/platform/core-impl/api-dump-experimental.txt +++ b/platform/core-impl/api-dump-experimental.txt @@ -108,6 +108,7 @@ com.intellij.psi.impl.file.impl.FileManager - *a:findFile(com.intellij.openapi.vfs.VirtualFile,com.intellij.codeInsight.multiverse.CodeInsightContext):com.intellij.psi.PsiFile - *a:findViewProvider(com.intellij.openapi.vfs.VirtualFile,com.intellij.codeInsight.multiverse.CodeInsightContext):com.intellij.psi.FileViewProvider - *a:getCachedPsiFile(com.intellij.openapi.vfs.VirtualFile,com.intellij.codeInsight.multiverse.CodeInsightContext):com.intellij.psi.PsiFile +- *a:getCachedPsiFiles(com.intellij.openapi.vfs.VirtualFile):java.util.List a:com.intellij.psi.impl.source.PsiFileImpl - com.intellij.psi.impl.ElementBase - com.intellij.openapi.ui.Queryable diff --git a/platform/core-impl/src/com/intellij/psi/impl/file/impl/FileManager.java b/platform/core-impl/src/com/intellij/psi/impl/file/impl/FileManager.java index c15c1da03f03..111f92be9d18 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/file/impl/FileManager.java +++ b/platform/core-impl/src/com/intellij/psi/impl/file/impl/FileManager.java @@ -39,6 +39,15 @@ public interface FileManager { @Nullable PsiFile getCachedPsiFile(@NotNull VirtualFile vFile); + /** + * @return list of cached PSI files. Note that the list can be shorter than {@link #findCachedViewProviders(VirtualFile)} because + * not all view providers have cached PSI files. + */ + @ApiStatus.Experimental + @RequiresReadLock + @NotNull @Unmodifiable + List<@NotNull PsiFile> getCachedPsiFiles(@NotNull VirtualFile vFile); + @ApiStatus.Experimental @Nullable PsiFile getCachedPsiFile(@NotNull VirtualFile vFile, @NotNull CodeInsightContext context); diff --git a/platform/core-impl/src/com/intellij/psi/impl/file/impl/FileManagerEx.java b/platform/core-impl/src/com/intellij/psi/impl/file/impl/FileManagerEx.java index be507fc7d0b2..25bdb5e248e8 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/file/impl/FileManagerEx.java +++ b/platform/core-impl/src/com/intellij/psi/impl/file/impl/FileManagerEx.java @@ -9,11 +9,9 @@ import com.intellij.psi.PsiDirectory; import com.intellij.psi.PsiFile; import com.intellij.util.concurrency.annotations.RequiresReadLock; import com.intellij.util.concurrency.annotations.RequiresWriteLock; -import org.jetbrains.annotations.ApiStatus; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; -import org.jetbrains.annotations.TestOnly; +import org.jetbrains.annotations.*; +import java.util.List; import java.util.function.Consumer; @ApiStatus.Internal @@ -36,6 +34,9 @@ public interface FileManagerEx extends FileManager { @Nullable PsiFile getCachedPsiFileInner(@NotNull VirtualFile file, @NotNull CodeInsightContext context); + @NotNull @Unmodifiable + List getCachedPsiFilesInner(@NotNull VirtualFile file); + /** * Removes invalid files and directories from the cache. * diff --git a/platform/core-impl/src/com/intellij/psi/impl/file/impl/FileManagerImpl.java b/platform/core-impl/src/com/intellij/psi/impl/file/impl/FileManagerImpl.java index dbf2eae31dbc..91ab65bfa6f6 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/file/impl/FileManagerImpl.java +++ b/platform/core-impl/src/com/intellij/psi/impl/file/impl/FileManagerImpl.java @@ -533,7 +533,19 @@ public final class FileManagerImpl implements FileManagerEx { } @Override - public @Nullable PsiFile getCachedPsiFile(@NotNull VirtualFile vFile, @NotNull CodeInsightContext context) { + public @NotNull @Unmodifiable List getCachedPsiFiles(@NotNull VirtualFile vFile) { + ensureValidAndDispatchPendingEvents(vFile); + + return getCachedPsiFilesInner(vFile); + } + + @Override + public @NotNull List<@NotNull PsiFile> getCachedPsiFilesInner(@NotNull VirtualFile vFile) { + List viewProviders = findCachedViewProviders(vFile); + return ContainerUtil.mapNotNull(viewProviders, p -> ((AbstractFileViewProvider)p).getCachedPsi(p.getBaseLanguage())); + } + + private void ensureValidAndDispatchPendingEvents(@NotNull VirtualFile vFile) { if (!vFile.isValid()) { throw new InvalidVirtualFileAccessException(vFile); } @@ -544,6 +556,11 @@ public final class FileManagerImpl implements FileManagerEx { } dispatchPendingEvents(); + } + + @Override + public @Nullable PsiFile getCachedPsiFile(@NotNull VirtualFile vFile, @NotNull CodeInsightContext context) { + ensureValidAndDispatchPendingEvents(vFile); return getCachedPsiFileInner(vFile, context); } @@ -551,20 +568,11 @@ public final class FileManagerImpl implements FileManagerEx { @RequiresReadLock @Override public @Nullable PsiDirectory findDirectory(@NotNull VirtualFile vFile) { - Project project = myManager.getProject(); - if (project.isDisposed()) { - LOG.error("Access to psi files should not be performed after project disposal: " + project); - } - - if (!vFile.isValid()) { - LOG.error(new InvalidVirtualFileAccessException(vFile)); - return null; - } + ensureValidAndDispatchPendingEvents(vFile); if (!vFile.isDirectory()) { return null; } - dispatchPendingEvents(); return findDirectoryImpl(vFile, getVFileToPsiDirMap()); } @@ -916,14 +924,7 @@ public final class FileManagerImpl implements FileManagerEx { @RequiresReadLock @Override public PsiFile getFastCachedPsiFile(@NotNull VirtualFile vFile, @NotNull CodeInsightContext context) { - if (!vFile.isValid()) { - throw new InvalidVirtualFileAccessException(vFile); - } - Project project = myManager.getProject(); - if (project.isDisposed()) { - LOG.error("Project is already disposed: " + project); - } - dispatchPendingEvents(); + ensureValidAndDispatchPendingEvents(vFile); FileViewProvider viewProvider = getRawCachedViewProvider(vFile, context); if (viewProvider == null || viewProvider.getUserData(IN_COMA) != null) { diff --git a/platform/lang-impl/src/com/intellij/mock/MockFileManager.java b/platform/lang-impl/src/com/intellij/mock/MockFileManager.java index a59a5318e1f5..b7c2e9f8bff7 100644 --- a/platform/lang-impl/src/com/intellij/mock/MockFileManager.java +++ b/platform/lang-impl/src/com/intellij/mock/MockFileManager.java @@ -75,6 +75,11 @@ public final class MockFileManager implements FileManager { return provider.getPsi(provider.getBaseLanguage()); } + @Override + public @NotNull @Unmodifiable List getCachedPsiFiles(@NotNull VirtualFile vFile) { + return ContainerUtil.createMaybeSingletonList(getCachedPsiFile(vFile)); + } + @ApiStatus.Internal @Override public @Nullable PsiFile getCachedPsiFile(@NotNull VirtualFile vFile, @NotNull CodeInsightContext context) { diff --git a/platform/lang-impl/src/com/intellij/psi/impl/file/impl/PsiVFSListener.kt b/platform/lang-impl/src/com/intellij/psi/impl/file/impl/PsiVFSListener.kt index 4efe18e2fae8..302791f03a1c 100644 --- a/platform/lang-impl/src/com/intellij/psi/impl/file/impl/PsiVFSListener.kt +++ b/platform/lang-impl/src/com/intellij/psi/impl/file/impl/PsiVFSListener.kt @@ -86,54 +86,67 @@ private class PsiVFSListener(private val project: Project) { val parentDir = getCachedDirectory(parent) ?: return ApplicationManager.getApplication().runWriteAction(ExternalChangeActionUtil.externalChangeAction { - val item = (if (vFile.isDirectory) fileManager.findDirectory(vFile) else fileManager.getCachedPsiFile(vFile)) - ?: return@externalChangeAction - val treeEvent = PsiTreeChangeEventImpl(manager) - treeEvent.parent = parentDir - treeEvent.child = item - manager.beforeChildRemoval(treeEvent) + val items = if (vFile.isDirectory) listOfNotNull(fileManager.findDirectory(vFile)) else fileManager.getCachedPsiFiles(vFile) + for (item in items) { + val treeEvent = PsiTreeChangeEventImpl(manager) + treeEvent.parent = parentDir + treeEvent.child = item + manager.beforeChildRemoval(treeEvent) + } }) } // optimization: call myFileManager.removeInvalidFilesAndDirs() once for a group of deletion events, instead of once for each event private fun filesDeleted(events: List) { var needToRemoveInvalidFilesAndDirs = false + + fun fireChildRemoved(element: PsiElement, parentDir: PsiDirectory?) { + if (parentDir == null) return + ApplicationManager.getApplication().runWriteAction(ExternalChangeActionUtil.externalChangeAction { + val treeEvent = PsiTreeChangeEventImpl(manager) + treeEvent.parent = parentDir + treeEvent.child = element + manager.childRemoved(treeEvent) + }) + } + + fun dirDeleted(dir: VirtualFile, parent: VirtualFile?) { + val psiDir = fileManager.getCachedDirectory(dir) ?: run { + handleVfsChangeWithoutPsi(parent) + return + } + + val parentDir = getCachedDirectory(parent) + fireChildRemoved(psiDir, parentDir) + needToRemoveInvalidFilesAndDirs = true + } + + fun fileDeleted(vFile: VirtualFile, parent: VirtualFile?) { + val cachedPsiFiles = fileManager.getCachedPsiFilesInner(vFile).ifEmpty { + handleVfsChangeWithoutPsi(parent) + return + } + + val parentDir = getCachedDirectory(parent) + fileManager.setViewProvider(vFile, null) + for (psiFile in cachedPsiFiles) { + fireChildRemoved(psiFile, parentDir) + } + } + for (event in events) { val de = event as VFileDeleteEvent val vFile = de.file val parent = vFile.parent - // todo IJPL-339 implement proper event for multiple files - val psiFile = fileManager.getCachedPsiFileInner(vFile, anyContext()) - var element: PsiElement? - if (psiFile != null) { - fileManager.setViewProvider(vFile, null) - element = psiFile + if (vFile.isDirectory) { + dirDeleted(vFile, parent) } else { - val psiDir = fileManager.getCachedDirectory(vFile) - if (psiDir != null) { - needToRemoveInvalidFilesAndDirs = true - element = psiDir - } - else if (parent != null) { - handleVfsChangeWithoutPsi(parent) - return - } - else { - element = null - } - } - val parentDir = getCachedDirectory(parent) - if (element != null && parentDir != null) { - ApplicationManager.getApplication().runWriteAction(ExternalChangeActionUtil.externalChangeAction { - val treeEvent = PsiTreeChangeEventImpl(manager) - treeEvent.parent = parentDir - treeEvent.child = element - manager.childRemoved(treeEvent) - }) + fileDeleted(vFile, parent) } } + if (needToRemoveInvalidFilesAndDirs) { fileManager.removeInvalidFilesAndDirs(false) } @@ -462,7 +475,9 @@ private class PsiVFSListener(private val project: Project) { ApplicationManager.getApplication().runWriteAction(ExternalChangeActionUtil.externalChangeAction { val treeEvent = PsiTreeChangeEventImpl(manager) if (oldElement == null) { - fileManager.setViewProvider(vFile, newViewProvider) + if (newViewProvider != null) { + fileManager.setViewProvider(vFile, newViewProvider) + } treeEvent.parent = newParentDir treeEvent.child = newElement manager.childAdded(treeEvent) @@ -518,7 +533,11 @@ private class PsiVFSListener(private val project: Project) { ) } - fun handleVfsChangeWithoutPsi(vFile: VirtualFile) { + fun handleVfsChangeWithoutPsi(vFile: VirtualFile?) { + if (vFile == null) { + return + } + if (!reportedUnloadedPsiChange && isInRootModel(vFile)) { fileManager.firePropertyChangedForUnloadedPsi() reportedUnloadedPsiChange = true diff --git a/platform/lang-impl/testSources/com/intellij/psi/impl/file/impl/MultiversePsiEventTest.kt b/platform/lang-impl/testSources/com/intellij/psi/impl/file/impl/MultiversePsiEventTest.kt new file mode 100644 index 000000000000..3737d4e4c1b6 --- /dev/null +++ b/platform/lang-impl/testSources/com/intellij/psi/impl/file/impl/MultiversePsiEventTest.kt @@ -0,0 +1,112 @@ +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.psi.impl.file.impl + +import com.intellij.codeInsight.multiverse.CodeInsightContext +import com.intellij.codeInsight.multiverse.ProjectModelContextBridge +import com.intellij.openapi.application.readAction +import com.intellij.openapi.application.writeAction +import com.intellij.openapi.module.ModuleManager +import com.intellij.openapi.vfs.VirtualFile +import com.intellij.platform.testFramework.junit5.projectStructure.fixture.withSharedSourceEnabled +import com.intellij.psi.PsiFile +import com.intellij.psi.PsiTreeChangeAdapter +import com.intellij.psi.PsiTreeChangeEvent +import com.intellij.psi.PsiTreeChangeListener +import com.intellij.psi.impl.PsiManagerEx +import com.intellij.testFramework.IndexingTestUtil +import com.intellij.testFramework.common.timeoutRunBlocking +import com.intellij.testFramework.junit5.TestApplication +import com.intellij.testFramework.junit5.fixture.disposableFixture +import com.intellij.testFramework.junit5.fixture.moduleFixture +import com.intellij.testFramework.junit5.fixture.projectFixture +import com.intellij.testFramework.junit5.fixture.virtualFileFixture +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import java.lang.ref.Reference +import java.util.concurrent.atomic.AtomicInteger + +@TestApplication +internal class MultiversePsiEventTest { + companion object { + val projectFixture = projectFixture(openAfterCreation = true).withSharedSourceEnabled() + + private val module1 = projectFixture.moduleFixture("m1") + private val module2 = projectFixture.moduleFixture("m2") + + private val sourceRoot = sharedSourceRootFixture(module1, module2) + } + + private val testDisposable by disposableFixture() + + private val virtualFile by sourceRoot.virtualFileFixture("Foo.java", "class Foo {}") + private val project by projectFixture + + private val m1 by lazy { ModuleManager.getInstance(project).findModuleByName("m1")!! } + private val m2 by lazy { ModuleManager.getInstance(project).findModuleByName("m2")!! } + + private val psiManager by lazy { PsiManagerEx.getInstanceEx(project) } + + private suspend fun findPsiFile(context: CodeInsightContext): PsiFile = requireNotNull(readAction { psiManager.findFile(virtualFile, context) }) + + private val c1 = ProjectModelContextBridge.getInstance(project).getContext(m1)!! + private val c2 = ProjectModelContextBridge.getInstance(project).getContext(m2)!! + + + @Test + fun `test we receive 2 before-delete events on deleting file with 2 psi-files`() = doChangeTest( + listenerFactory = { counter -> + object : PsiTreeChangeAdapter() { + override fun beforeChildRemoval(event: PsiTreeChangeEvent) { + if (event.child?.containingFile?.virtualFile == virtualFile) { + counter.incrementAndGet() + } + } + } + }, + updateBlock = { file -> + file.delete(this) + }, + expectedEventNumber = 2 + ) + + @Test + fun `test we receive 2 delete events on deleting file with 2 psi files`() = doChangeTest( + listenerFactory = { counter -> + object : PsiTreeChangeAdapter() { + override fun childRemoved(event: PsiTreeChangeEvent) { + counter.incrementAndGet() + } + } + }, + updateBlock = { file -> file.delete(this) }, + expectedEventNumber = 2 + ) + + private fun doChangeTest( + listenerFactory: (AtomicInteger) -> PsiTreeChangeListener, + updateBlock: (file: VirtualFile) -> Unit, + @Suppress("SameParameterValue") expectedEventNumber: Int + ) = runTest { + val counter = AtomicInteger(0) + val listener = listenerFactory(counter) + + psiManager.addPsiTreeChangeListenerBackgroundable(listener, testDisposable) + + val f1 = findPsiFile(c1) + val f2 = findPsiFile(c2) + + writeAction { + updateBlock(virtualFile) + } + + Reference.reachabilityFence(f1) + Reference.reachabilityFence(f2) + + assertThat(counter.get()).isEqualTo(expectedEventNumber) + } + + private fun runTest(block: suspend () -> Unit) = timeoutRunBlocking { + IndexingTestUtil.waitUntilIndexesAreReady(project) + block() + } +} \ No newline at end of file