From 05dc4c1c481c5b2ed145826d7e6ae4458a837aa0 Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Tue, 1 Mar 2016 13:56:22 +0300 Subject: [PATCH] ResolutionFacade for synthetic files should be invalidated on changes in these synthetic files Fixed EA-77017 --- .../KotlinCodeBlockModificationListener.kt | 114 ++++++------------ .../org/jetbrains/kotlin/utils/addToStdlib.kt | 8 ++ .../caches/resolve/KotlinCacheServiceImpl.kt | 21 ++-- ...OutOfBlockCompletionModificationTracker.kt | 48 -------- .../completion/KotlinCompletionContributor.kt | 11 +- idea/src/META-INF/plugin.xml | 8 +- .../kotlin/idea/j2k/J2kPostProcessor.kt | 4 - .../idea/DummyFileResolveCachingTest.kt | 45 +++++++ 8 files changed, 108 insertions(+), 151 deletions(-) delete mode 100644 idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/KotlinOutOfBlockCompletionModificationTracker.kt create mode 100644 idea/tests/org/jetbrains/kotlin/idea/DummyFileResolveCachingTest.kt diff --git a/compiler/light-classes/src/org/jetbrains/kotlin/asJava/KotlinCodeBlockModificationListener.kt b/compiler/light-classes/src/org/jetbrains/kotlin/asJava/KotlinCodeBlockModificationListener.kt index 90c2e46668d..2aea3755f60 100644 --- a/compiler/light-classes/src/org/jetbrains/kotlin/asJava/KotlinCodeBlockModificationListener.kt +++ b/compiler/light-classes/src/org/jetbrains/kotlin/asJava/KotlinCodeBlockModificationListener.kt @@ -16,15 +16,16 @@ package org.jetbrains.kotlin.asJava -import com.intellij.openapi.diagnostic.Logger -import com.intellij.psi.PsiClass +import com.intellij.openapi.project.Project +import com.intellij.openapi.util.Key +import com.intellij.pom.PomManager +import com.intellij.pom.PomModelAspect +import com.intellij.pom.event.PomModelEvent +import com.intellij.pom.event.PomModelListener +import com.intellij.pom.tree.TreeAspect +import com.intellij.pom.tree.events.TreeChangeEvent import com.intellij.psi.PsiElement -import com.intellij.psi.PsiFileSystemItem -import com.intellij.psi.PsiInvalidElementAccessException import com.intellij.psi.impl.PsiModificationTrackerImpl -import com.intellij.psi.impl.PsiTreeChangeEventImpl -import com.intellij.psi.impl.PsiTreeChangeEventImpl.PsiEventType.* -import com.intellij.psi.impl.PsiTreeChangePreprocessor import com.intellij.psi.util.PsiModificationTracker import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.isAncestor @@ -33,84 +34,40 @@ import org.jetbrains.kotlin.psi.psiUtil.parents /** * Tested in OutOfBlockModificationTestGenerated */ -class KotlinCodeBlockModificationListener(modificationTracker: PsiModificationTracker) : PsiTreeChangePreprocessor { - private val myModificationTracker = modificationTracker as PsiModificationTrackerImpl - - override fun treeChanged(event: PsiTreeChangeEventImpl) { - if (event.file !is KtFile) return - - when (event.code) { - BEFORE_CHILDREN_CHANGE, - BEFORE_PROPERTY_CHANGE, - BEFORE_CHILD_MOVEMENT, - BEFORE_CHILD_REPLACEMENT, - BEFORE_CHILD_ADDITION, - BEFORE_CHILD_REMOVAL -> { - // skip +class KotlinCodeBlockModificationListener( + modificationTracker: PsiModificationTracker, + private val project: Project, + private val treeAspect: TreeAspect +) { + init { + val model = PomManager.getModel(project) + @Suppress("NAME_SHADOWING") + val modificationTracker = modificationTracker as PsiModificationTrackerImpl + model.addModelListener(object: PomModelListener { + override fun isAspectChangeInteresting(aspect: PomModelAspect): Boolean { + return aspect == treeAspect } - CHILD_ADDED, - CHILD_REMOVED, - CHILD_REPLACED -> { - processChange(event.parent, event.oldChild, event.child) - } - - CHILDREN_CHANGED -> { - if (!event.isGenericChange) { - processChange(event.parent, event.parent, null) + override fun modelChanged(event: PomModelEvent) { + val changeSet = event.getChangeSet(treeAspect) as TreeChangeEvent? ?: return + val file = changeSet.rootElement.psi.containingFile as? KtFile ?: return + if (changeSet.changedElements.any { !isInsideCodeBlock(it.psi) }) { + if (file.isPhysical) { + modificationTracker.incCounter() + } + incOutOfBlockModificationCount(file) } } - - CHILD_MOVED, - PROPERTY_CHANGED -> { - myModificationTracker.incCounter() - } - - else -> LOG.error("Unknown code:" + event.code) - } - } - - private fun processChange(parent: PsiElement?, child1: PsiElement?, child2: PsiElement?) { - try { - if (!isInsideCodeBlock(parent)) { - if (parent != null && parent.containingFile is KtFile) { - myModificationTracker.incCounter() - } - else { - myModificationTracker.incOutOfCodeBlockModificationCounter() - } - return - } - - if (containsClassesInside(child1) || (child2 != child1 && containsClassesInside(child2))) { - myModificationTracker.incCounter() - } - } - catch (e: PsiInvalidElementAccessException) { - myModificationTracker.incCounter() // Shall not happen actually, just a pre-release paranoia - } + }) } companion object { - private val LOG = Logger.getInstance("#org.jetbrains.kotlin.asJava.KotlinCodeBlockModificationListener") - - private fun containsClassesInside(element: PsiElement?): Boolean { - if (element == null) return false - if (element is PsiClass) return true - - var child = element.firstChild - while (child != null) { - if (containsClassesInside(child)) return true - child = child.nextSibling - } - - return false + private fun incOutOfBlockModificationCount(file: KtFile) { + val count = file.getUserData(FILE_OUT_OF_BLOCK_MODIFICATION_COUNT) ?: 0 + file.putUserData(FILE_OUT_OF_BLOCK_MODIFICATION_COUNT, count + 1) } - fun isInsideCodeBlock(element: PsiElement?): Boolean { - if (element is PsiFileSystemItem) return false - if (element == null || element.parent == null) return true - + private fun isInsideCodeBlock(element: PsiElement): Boolean { //TODO: other types val blockDeclaration = KtPsiUtil.getTopmostParentOfTypes(element, *BLOCK_DECLARATION_TYPES) ?: return false if (blockDeclaration.parents.any { it !is KtClassBody && it !is KtClassOrObject && it !is KtFile }) return false // should not be local declaration @@ -149,3 +106,8 @@ class KotlinCodeBlockModificationListener(modificationTracker: PsiModificationTr ) } } + +private val FILE_OUT_OF_BLOCK_MODIFICATION_COUNT = Key("FILE_OUT_OF_BLOCK_MODIFICATION_COUNT") + +val KtFile.outOfBlockModificationCount: Long + get() = getUserData(FILE_OUT_OF_BLOCK_MODIFICATION_COUNT) ?: 0 diff --git a/core/util.runtime/src/org/jetbrains/kotlin/utils/addToStdlib.kt b/core/util.runtime/src/org/jetbrains/kotlin/utils/addToStdlib.kt index 9dda07bd9a6..94635284695 100644 --- a/core/util.runtime/src/org/jetbrains/kotlin/utils/addToStdlib.kt +++ b/core/util.runtime/src/org/jetbrains/kotlin/utils/addToStdlib.kt @@ -112,3 +112,11 @@ inline fun Iterable.firstNotNullResult(transform: (T) -> R?): R? } return null } + +inline fun Iterable.sumByLong(selector: (T) -> Long): Long { + var sum: Long = 0 + for (element in this) { + sum += selector(element) + } + return sum +} diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/KotlinCacheServiceImpl.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/KotlinCacheServiceImpl.kt index 921fe815a72..020b10f9e59 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/KotlinCacheServiceImpl.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/KotlinCacheServiceImpl.kt @@ -19,12 +19,14 @@ package org.jetbrains.kotlin.idea.caches.resolve import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.project.Project import com.intellij.openapi.roots.ProjectRootModificationTracker +import com.intellij.openapi.util.ModificationTracker import com.intellij.psi.util.CachedValue import com.intellij.psi.util.CachedValueProvider import com.intellij.psi.util.CachedValuesManager import com.intellij.psi.util.PsiModificationTracker import com.intellij.util.containers.SLRUCache import org.jetbrains.kotlin.analyzer.EmptyResolverForProject +import org.jetbrains.kotlin.asJava.outOfBlockModificationCount import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.caches.resolve.KotlinCacheService import org.jetbrains.kotlin.container.getService @@ -40,6 +42,7 @@ import org.jetbrains.kotlin.resolve.TargetPlatform import org.jetbrains.kotlin.resolve.diagnostics.KotlinSuppressCache import org.jetbrains.kotlin.resolve.jvm.platform.JvmPlatform import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode +import org.jetbrains.kotlin.utils.addToStdlib.sumByLong import org.jetbrains.kotlin.utils.keysToMap internal val LOG = Logger.getInstance(KotlinCacheService::class.java) @@ -92,10 +95,10 @@ class KotlinCacheServiceImpl(val project: Project) : KotlinCacheService { // we assume that all files come from the same module val targetPlatform = files.map { TargetPlatformDetector.getPlatform(it) }.toSet().single() val syntheticFileModule = files.map { it.getModuleInfo() }.toSet().single() - val dependenciesForSyntheticFileCache = listOf( - PsiModificationTracker.OUT_OF_CODE_BLOCK_MODIFICATION_COUNT, - KotlinOutOfBlockCompletionModificationTracker.getInstance(project) - ) + val filesModificationTracker = ModificationTracker { + files.sumByLong { it.outOfBlockModificationCount } + } + val dependenciesForSyntheticFileCache = listOf(PsiModificationTracker.OUT_OF_CODE_BLOCK_MODIFICATION_COUNT, filesModificationTracker) val debugName = "completion/highlighting in $syntheticFileModule for files ${files.joinToString { it.name }} for platform $targetPlatform" return when { syntheticFileModule is ModuleSourceInfo -> { @@ -179,19 +182,17 @@ class KotlinCacheServiceImpl(val project: Project) : KotlinCacheService { private val syntheticFileCachesLock = Any() - private val slruCacheProvider = CachedValueProvider { + private val syntheticFilesCacheProvider = CachedValueProvider { CachedValueProvider.Result(object : SLRUCache, ProjectResolutionFacade>(2, 3) { - override fun createValue(files: Set): ProjectResolutionFacade { - return createFacadeForSyntheticFiles(files) - } + override fun createValue(files: Set) = createFacadeForSyntheticFiles(files) }, LibraryModificationTracker.getInstance(project), ProjectRootModificationTracker.getInstance(project)) } private fun getFacadeForSyntheticFiles(files: Set): ProjectResolutionFacade { - return synchronized(syntheticFileCachesLock) { + synchronized(syntheticFileCachesLock) { //NOTE: computations inside createCacheForSyntheticFiles depend on project root structure // so we additionally drop the whole slru cache on change - CachedValuesManager.getManager(project).getCachedValue(project, slruCacheProvider).get(files) + return CachedValuesManager.getManager(project).getCachedValue(project, syntheticFilesCacheProvider).get(files) } } diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/KotlinOutOfBlockCompletionModificationTracker.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/KotlinOutOfBlockCompletionModificationTracker.kt deleted file mode 100644 index d7b3633345c..00000000000 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/KotlinOutOfBlockCompletionModificationTracker.kt +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright 2010-2015 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.jetbrains.kotlin.idea.caches.resolve - -import com.intellij.openapi.components.ServiceManager -import com.intellij.openapi.project.Project -import com.intellij.openapi.util.SimpleModificationTracker -import com.intellij.psi.PsiElement -import org.jetbrains.kotlin.asJava.KotlinCodeBlockModificationListener - -// NOTE: Sadly we must track out of block completion and drop sessions for synthetic files after the completion happens. -// Synthetic file for completion can be modified without sending tree changed events and sequence of completions can lead to inconsistent -// resolve session being cached for such a file otherwise. -// This code is not tested. See KT-6216 for an example. -class KotlinOutOfBlockCompletionModificationTracker() : SimpleModificationTracker() { - companion object { - fun getInstance(project: Project): KotlinOutOfBlockCompletionModificationTracker - = ServiceManager.getService(project, KotlinOutOfBlockCompletionModificationTracker::class.java)!! - } -} - - -fun performCompletionWithOutOfBlockTracking(completionPosition: PsiElement, body: () -> Unit) { - if (KotlinCodeBlockModificationListener.isInsideCodeBlock(completionPosition)) { - body() - return - } - try { - body() - } - finally { - KotlinOutOfBlockCompletionModificationTracker.getInstance(completionPosition.project).incModificationCount() - } -} \ No newline at end of file diff --git a/idea/idea-completion/src/org/jetbrains/kotlin/idea/completion/KotlinCompletionContributor.kt b/idea/idea-completion/src/org/jetbrains/kotlin/idea/completion/KotlinCompletionContributor.kt index 90962bdc39f..e2711a12604 100644 --- a/idea/idea-completion/src/org/jetbrains/kotlin/idea/completion/KotlinCompletionContributor.kt +++ b/idea/idea-completion/src/org/jetbrains/kotlin/idea/completion/KotlinCompletionContributor.kt @@ -34,7 +34,6 @@ import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.ClassKind import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade -import org.jetbrains.kotlin.idea.caches.resolve.performCompletionWithOutOfBlockTracking import org.jetbrains.kotlin.idea.completion.smart.SmartCompletion import org.jetbrains.kotlin.idea.completion.smart.SmartCompletionSession import org.jetbrains.kotlin.lexer.KtTokens @@ -254,17 +253,13 @@ class KotlinCompletionContributor : CompletionContributor() { val context = position.getUserData(CompletionContext.COMPLETION_CONTEXT_KEY)!! val correctedOffset = context.offsetMap.getOffset(STRING_TEMPLATE_AFTER_DOT_REAL_START_OFFSET) val correctedParameters = parameters.withPosition(correctedPosition, correctedOffset) - performCompletionWithOutOfBlockTracking(position) { - doComplete(correctedParameters, toFromOriginalFileMapper, result, - lookupElementPostProcessor = { wrapLookupElementForStringTemplateAfterDotCompletion(it) }) - } + doComplete(correctedParameters, toFromOriginalFileMapper, result, + lookupElementPostProcessor = { wrapLookupElementForStringTemplateAfterDotCompletion(it) }) return } } - performCompletionWithOutOfBlockTracking(position) { - doComplete(parameters, toFromOriginalFileMapper, result) - } + doComplete(parameters, toFromOriginalFileMapper, result) } private fun doComplete( diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 62235fcb804..9d1406ee8b8 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -31,6 +31,9 @@ org.jetbrains.kotlin.idea.completion.LookupCancelWatcher + + org.jetbrains.kotlin.asJava.KotlinCodeBlockModificationListener + @@ -264,9 +267,6 @@ - - @@ -583,8 +583,6 @@ - - diff --git a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt index 5c61020f80d..77830d28874 100644 --- a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt @@ -22,7 +22,6 @@ import com.intellij.psi.PsiElement import com.intellij.psi.PsiRecursiveElementVisitor import com.intellij.psi.codeStyle.CodeStyleManager import com.intellij.util.SmartList -import org.jetbrains.kotlin.idea.caches.resolve.KotlinOutOfBlockCompletionModificationTracker import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade import org.jetbrains.kotlin.idea.caches.resolve.resolveImportReference import org.jetbrains.kotlin.idea.conversion.copy.range @@ -66,9 +65,6 @@ class J2kPostProcessor(private val formatCode: Boolean) : PostProcessor { if (modificationStamp == file.modificationStamp) break - //TODO: it's a hack! - KotlinOutOfBlockCompletionModificationTracker.getInstance(file.project).incModificationCount() - elementToActions = collectAvailableActions(file, rangeMarker) } diff --git a/idea/tests/org/jetbrains/kotlin/idea/DummyFileResolveCachingTest.kt b/idea/tests/org/jetbrains/kotlin/idea/DummyFileResolveCachingTest.kt new file mode 100644 index 00000000000..7023559ff83 --- /dev/null +++ b/idea/tests/org/jetbrains/kotlin/idea/DummyFileResolveCachingTest.kt @@ -0,0 +1,45 @@ +/* + * Copyright 2010-2016 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.idea + +import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase +import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade +import org.jetbrains.kotlin.idea.test.KotlinWithJdkAndRuntimeLightProjectDescriptor +import org.jetbrains.kotlin.idea.util.getFileResolutionScope +import org.jetbrains.kotlin.incremental.components.NoLookupLocation +import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.psi.KtPsiFactory +import org.jetbrains.kotlin.resolve.scopes.utils.findClassifier + +class DummyFileResolveCachingTest : LightCodeInsightFixtureTestCase() { + override fun getProjectDescriptor() = KotlinWithJdkAndRuntimeLightProjectDescriptor.INSTANCE + + fun test() { + myFixture.configureByText(KotlinFileType.INSTANCE, "") + + val dummyFileText = "import java.util.ArrayList" + val dummyFile = KtPsiFactory(project).createAnalyzableFile("Dummy.kt", dummyFileText, file) + + dummyFile.getResolutionFacade().getFileResolutionScope(dummyFile) + + dummyFile.importDirectives.single().delete() + + val resolutionScope = dummyFile.getResolutionFacade().getFileResolutionScope(dummyFile) + val classifier = resolutionScope.findClassifier(Name.identifier("ArrayList"), NoLookupLocation.FROM_IDE) + assertNull(classifier) + } +} \ No newline at end of file