From c4eb1eb3c68144a9fa250d405d8072d92c309dff Mon Sep 17 00:00:00 2001 From: "Andrei.Kuznetsov" Date: Wed, 25 Dec 2024 01:26:39 +0100 Subject: [PATCH] IJPL-174027: use the same project files condition in FileBasedIndexImpl.forceUpdate and FileBasedIndexImpl.indexUnsavedDocuments (cherry picked from commit a3df35169570a3605dd10de57d8479a54ce6f530) IJ-CR-152316 GitOrigin-RevId: 486f82ff62bae94293b5e3db4ca1a211e476d504 --- .../psi/resolve/StubPsiConsistencyTest.kt | 128 ++++++++++++++++++ .../util/indexing/ProjectFilesCondition.java | 35 +++-- .../util/indexing/FileBasedIndexImpl.java | 35 ++--- 3 files changed, 172 insertions(+), 26 deletions(-) create mode 100644 java/java-tests/testSrc/com/intellij/java/psi/resolve/StubPsiConsistencyTest.kt diff --git a/java/java-tests/testSrc/com/intellij/java/psi/resolve/StubPsiConsistencyTest.kt b/java/java-tests/testSrc/com/intellij/java/psi/resolve/StubPsiConsistencyTest.kt new file mode 100644 index 000000000000..8928910b9f92 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/psi/resolve/StubPsiConsistencyTest.kt @@ -0,0 +1,128 @@ +// 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.java.psi.resolve + +import com.intellij.openapi.application.readAction +import com.intellij.openapi.application.writeAction +import com.intellij.openapi.fileEditor.FileDocumentManager +import com.intellij.openapi.project.Project +import com.intellij.openapi.vfs.VirtualFile +import com.intellij.openapi.vfs.VirtualFileWithId +import com.intellij.psi.PsiClass +import com.intellij.psi.PsiDocumentManager +import com.intellij.psi.impl.PsiManagerEx +import com.intellij.psi.impl.java.stubs.index.JavaStubIndexKeys +import com.intellij.psi.impl.source.PsiFileImpl +import com.intellij.psi.search.GlobalSearchScope +import com.intellij.psi.stubs.StubIndex +import com.intellij.testFramework.IndexingTestUtil +import com.intellij.testFramework.common.timeoutRunBlocking +import com.intellij.testFramework.junit5.TestApplication +import com.intellij.testFramework.junit5.fixture.moduleFixture +import com.intellij.testFramework.junit5.fixture.projectFixture +import com.intellij.testFramework.junit5.fixture.psiFileFixture +import com.intellij.testFramework.junit5.fixture.sourceRootFixture +import com.intellij.testFramework.utils.editor.commitToPsi +import com.intellij.util.indexing.FileBasedIndex +import com.intellij.util.indexing.FileBasedIndexImpl +import com.intellij.util.io.write +import kotlinx.coroutines.suspendCancellableCoroutine +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import kotlin.time.Duration.Companion.minutes + +const val originalText1 = "class Test {}" +const val originalText2 = "class F2Test {}" +const val newText = "package x; import java.lang.Object; import java.lang.Integer; class Test {}" + +@TestApplication +internal class StubPsiConsistencyTest { + private val projectFixture = projectFixture(openAfterCreation = true) + private val moduleFixture = projectFixture.moduleFixture("src") + private val sourceRootFixture = moduleFixture.sourceRootFixture() + private val psiFile1Fixture = sourceRootFixture.psiFileFixture("Test.java", originalText1) + private val psiFile2Fixture = sourceRootFixture.psiFileFixture("F2Test.java", originalText2) + + @Test + fun testIJPL174027() = timeoutRunBlocking(timeout = 1.minutes) { + val project = projectFixture.get() + val virtualFile1 = psiFile1Fixture.get().virtualFile + val virtualFile2 = psiFile2Fixture.get().virtualFile + val documentManager = FileDocumentManager.getInstance() + + IndexingTestUtil.suspendUntilIndexesAreReady(project) + + val scope1 = readAction { GlobalSearchScope.fileScope(project, virtualFile1) } + val scope2 = readAction { GlobalSearchScope.fileScope(project, virtualFile2) } + val scopeEverything = readAction { GlobalSearchScope.everythingScope(project) } + + assertTrue(documentManager.unsavedDocuments.isEmpty()) + readAction { + val elements = StubIndex.getElements(JavaStubIndexKeys.CLASS_SHORT_NAMES, "Test", project, scopeEverything, PsiClass::class.java) + assertEquals(1, elements.size) + } + + // update file externally + virtualFile1.toNioPath().write(newText) + + // refresh the file in VFS + refresh(virtualFile1) + + assertFileContentIsNotIndexed(project, virtualFile1) + + // update document to match old content + writeAction { + val doc1 = documentManager.getDocument(virtualFile1)!! + doc1.setText(originalText1) + doc1.commitToPsi(project) + assertTrue(documentManager.unsavedDocuments.contains(doc1)) + } + + // ensure PSI is loaded + readAction { + val doc1 = documentManager.getDocument(virtualFile1)!! + assertEquals(originalText1, PsiDocumentManager.getInstance(project).getLastCommittedText(doc1).toString()) + val psiFile = PsiManagerEx.getInstanceEx(project).getFileManager().getCachedPsiFile(virtualFile1)!! + assertEquals(originalText1, (psiFile as PsiFileImpl).text) + } + + assertFileContentIsNotIndexed(project, virtualFile1) + + readAction { + // This will index doc1 as diff to file1. File1 is not indexed yet => there is no diff => in-memory index will be deleted + val elements = StubIndex.getElements(JavaStubIndexKeys.CLASS_SHORT_NAMES, "F2Test", project, scope2, PsiClass::class.java) + + // FileBasedIndexImpl.ensureUpToDate will invoke getChangedFilesCollector().ensureUpToDate(), which will in turn index VFS files + // synchronously, because we are in RA. But it will index files AFTER it has indexed unsaved documents anyway. + + assertEquals(1, elements.size) + assertEquals("F2Test", elements.first().name) + } + + readAction { + val doc1 = documentManager.getDocument(virtualFile1)!! + assertTrue(documentManager.unsavedDocuments.contains(doc1)) + assertEquals(originalText1, PsiDocumentManager.getInstance(project).getLastCommittedText(doc1).toString()) + val psiFile = PsiManagerEx.getInstanceEx(project).getFileManager().getCachedPsiFile(virtualFile1)!! + assertEquals(originalText1, (psiFile as PsiFileImpl).text) + + val elements = StubIndex.getElements(JavaStubIndexKeys.CLASS_SHORT_NAMES, "Test", project, scopeEverything, PsiClass::class.java) + assertEquals(1, elements.size) + assertEquals("Test", elements.first().name) + } + } + + private fun assertFileContentIsNotIndexed(project: Project, virtualFile1: VirtualFile) { + val fbi = (FileBasedIndex.getInstance() as FileBasedIndexImpl) + val projectDirtyFiles = fbi.changedFilesCollector.dirtyFiles.getProjectDirtyFiles(project)!! + assertTrue(projectDirtyFiles.containsFile((virtualFile1 as VirtualFileWithId).id)) + } + + private suspend fun refresh(file: VirtualFile) { + suspendCancellableCoroutine { continuation -> + file.refresh(true, false, { + continuation.resumeWith(Result.success(true)) + }) + } + } +} \ No newline at end of file diff --git a/platform/indexing-impl/src/com/intellij/util/indexing/ProjectFilesCondition.java b/platform/indexing-impl/src/com/intellij/util/indexing/ProjectFilesCondition.java index c90e97e0175c..019d6e6c0528 100644 --- a/platform/indexing-impl/src/com/intellij/util/indexing/ProjectFilesCondition.java +++ b/platform/indexing-impl/src/com/intellij/util/indexing/ProjectFilesCondition.java @@ -1,14 +1,16 @@ // Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.util.indexing; -import com.intellij.openapi.util.Condition; import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.openapi.vfs.VirtualFileWithId; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.util.indexing.events.FileIndexingRequest; import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; @ApiStatus.Internal -final class ProjectFilesCondition implements Condition { +final class ProjectFilesCondition { private static final int MAX_FILES_TO_UPDATE_FROM_OTHER_PROJECT = 2; private final VirtualFile myRestrictedTo; private final GlobalSearchScope myFilter; @@ -27,14 +29,7 @@ final class ProjectFilesCondition implements Condition { } } - @Override - public boolean value(FileIndexingRequest request) { - if (request.isDeleteRequest()) { - return true; - } - - VirtualFile file = request.getFile(); - int fileId = request.getFileId(); + private boolean acceptsFileAndId(VirtualFile file, int fileId) { if (myIndexableFilesFilter != null && !myIndexableFilesFilter.containsFileId(fileId)) { if (myFilesFromOtherProjects >= MAX_FILES_TO_UPDATE_FROM_OTHER_PROJECT) return false; ++myFilesFromOtherProjects; @@ -49,4 +44,24 @@ final class ProjectFilesCondition implements Condition { } return false; } + + public boolean acceptsFile(@Nullable VirtualFile file) { + if (file == null) { + return false; + } + else if (file instanceof VirtualFileWithId fileWithId) { + return acceptsFileAndId(file, fileWithId.getId()); + } + else { + return acceptsFileAndId(file, 0); + } + } + + public boolean acceptsRequest(@NotNull FileIndexingRequest request) { + if (request.isDeleteRequest()) { + return true; + } + + return acceptsFileAndId(request.getFile(), request.getFileId()); + } } diff --git a/platform/lang-impl/src/com/intellij/util/indexing/FileBasedIndexImpl.java b/platform/lang-impl/src/com/intellij/util/indexing/FileBasedIndexImpl.java index 0a06f0d0917c..2dbd2b805d2b 100644 --- a/platform/lang-impl/src/com/intellij/util/indexing/FileBasedIndexImpl.java +++ b/platform/lang-impl/src/com/intellij/util/indexing/FileBasedIndexImpl.java @@ -850,10 +850,22 @@ public final class FileBasedIndexImpl extends FileBasedIndexEx { } return false; } + + // Make sure that condition for forceUpdate is not more restrictive than condition for indexUnsavedDocuments, + // because indexUnsavedDocuments builds a diff of in-memory/on-the-disc state. + // Also make sure that if forceUpdate is invoked, it is invoked before indexUnsavedDocuments + boolean includeFilesFromOtherProjects = restrictedFile == null && project == null; + ProjectFilesCondition projectFilterCondition = new ProjectFilesCondition( + projectIndexableFiles(project), + // Index everything for performance reasons: unindexed project files will trigger "indexUnsavedDocument" too often + project == null ? GlobalSearchScope.EMPTY_SCOPE : GlobalSearchScope.everythingScope(project), + restrictedFile, + includeFilesFromOtherProjects); + if (!ActionUtil.isDumbMode(project) || getCurrentDumbModeAccessType_NoDumbChecks() == null) { - forceUpdate(project, filter, restrictedFile); + forceUpdate(project, projectFilterCondition, restrictedFile); } - indexUnsavedDocuments(indexId, project, filter, restrictedFile); + indexUnsavedDocuments(indexId, project, projectFilterCondition, restrictedFile); } catch (RuntimeException e) { final Throwable cause = e.getCause(); @@ -986,7 +998,7 @@ public final class FileBasedIndexImpl extends FileBasedIndexEx { private void indexUnsavedDocuments(final @NotNull ID indexId, @Nullable("All projects") Project project, - @Nullable GlobalSearchScope filter, + @NotNull ProjectFilesCondition filter, @Nullable VirtualFile restrictedFile) { if (myUpToDateIndicesForUnsavedOrTransactedDocuments.contains(indexId)) { return; // no need to index unsaved docs // todo: we only index files for a project, but this service is app-wide @@ -1004,13 +1016,8 @@ public final class FileBasedIndexImpl extends FileBasedIndexEx { documents.addAll(transactedDocuments); Collections.addAll(documents, uncommittedDocuments); - LOG.assertTrue(project == null || filter == null || filter.getProject() == null || project.equals(filter.getProject()), - "filter should filter files in provided project. ref: 50cf572587cf"); - Collection documentsToProcessForProject = project == null ? documents : - ContainerUtil.filter(documents, - document -> belongsToScope( - myFileDocumentManager.getFile(document), restrictedFile, - GlobalSearchScope.everythingScope(project))); + Condition docFilter = doc -> filter.acceptsFile(myFileDocumentManager.getFile(doc)); + Collection documentsToProcessForProject = ContainerUtil.filter(documents, docFilter); if (!documentsToProcessForProject.isEmpty()) { UpdateTask task = myRegisteredIndexes.getUnsavedDataUpdateTask(indexId); @@ -1757,15 +1764,11 @@ public final class FileBasedIndexImpl extends FileBasedIndexEx { private final VirtualFileUpdateTask myForceUpdateTask = new VirtualFileUpdateTask(); - private void forceUpdate(@Nullable Project project, final @Nullable GlobalSearchScope filter, final @Nullable VirtualFile restrictedTo) { + private void forceUpdate(@Nullable Project project, ProjectFilesCondition filter, final @Nullable VirtualFile restrictedTo) { Collection allFilesToUpdate = getAllFilesToUpdate(); if (!allFilesToUpdate.isEmpty()) { - boolean includeFilesFromOtherProjects = restrictedTo == null && project == null; - List virtualFilesToBeUpdatedForProject = ContainerUtil.filter( - allFilesToUpdate, - new ProjectFilesCondition(projectIndexableFiles(project), filter, restrictedTo, includeFilesFromOtherProjects) - ); + List virtualFilesToBeUpdatedForProject = ContainerUtil.filter(allFilesToUpdate, filter::acceptsRequest); if (!virtualFilesToBeUpdatedForProject.isEmpty()) { myForceUpdateTask.processAll(virtualFilesToBeUpdatedForProject, project);