From 4e8b1634e6b22375ea6c2cd0495fe98a8d5907a4 Mon Sep 17 00:00:00 2001 From: Sergey Rostov Date: Wed, 6 May 2020 23:24:33 +0300 Subject: [PATCH] UnusedSymbolInspection: load script configurations from cache --- .../configuration/DefaultScriptingSupport.kt | 131 ++++++++++-------- .../utils/ScriptClassRootsUpdater.kt | 29 ++-- .../inspections/UnusedSymbolInspection.kt | 15 +- .../script/ScriptConfigurationLoadingTest.kt | 8 +- 4 files changed, 108 insertions(+), 75 deletions(-) diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/configuration/DefaultScriptingSupport.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/configuration/DefaultScriptingSupport.kt index e06ceb7c066..1f052f9ad32 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/configuration/DefaultScriptingSupport.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/configuration/DefaultScriptingSupport.kt @@ -51,10 +51,8 @@ import kotlin.script.experimental.api.ScriptDiagnostic * cached configurations. So, for now we are indexing roots that loaded from FS with * default [reloadOutOfDateConfiguration] mechanics. * - * Also, [ensureConfigurationUpToDate] may be called from [UnusedSymbolInspection] + * Also, [ensureLoadedFromCache] may be called from [UnusedSymbolInspection] * to ensure that configuration of all scripts containing some symbol are up-to-date or try load it in sync. - * Note: it makes sense only in case of "auto apply" mode and sync loader, in other cases all symbols just - * will be treated as used. * * ## Loading * @@ -123,7 +121,7 @@ class DefaultScriptingSupport(manager: CompositeScriptConfigurationManager) : De * * Each files may be in on of the states described below: * - scriptDefinition is not ready. `ScriptDefinitionsManager.getInstance(project).isReady() == false`. - * [clearConfigurationCachesAndRehighlight] will be called when [ScriptDefinitionsManager] will be ready + * [updateScriptDefinitions] will be called when [ScriptDefinitionsManager] will be ready * which will call [reloadOutOfDateConfiguration] for opened editors. * - unknown. When [isFirstLoad] true (`cache[file] == null`). * - up-to-date. `cache[file]?.upToDate == true`. @@ -157,36 +155,39 @@ class DefaultScriptingSupport(manager: CompositeScriptConfigurationManager) : De override fun reloadOutOfDateConfiguration( file: KtFile, isFirstLoad: Boolean, - loadEvenWillNotBeApplied: Boolean, forceSync: Boolean, - isPostponedLoad: Boolean - ) { - val virtualFile = file.originalFile.virtualFile ?: return + isPostponedLoad: Boolean, + fromCacheOnly: Boolean + ): Boolean { + val virtualFile = file.originalFile.virtualFile ?: return false - val autoReloadEnabled = KotlinScriptingSettings.getInstance(project).isAutoReloadEnabled - val shouldLoad = isFirstLoad || loadEvenWillNotBeApplied || autoReloadEnabled - if (!shouldLoad) return - - val postponeLoading = isPostponedLoad && !autoReloadEnabled && !isFirstLoad - - if (!ScriptDefinitionsManager.getInstance(project).isReady()) return - val scriptDefinition = file.findScriptDefinition() ?: return + if (!ScriptDefinitionsManager.getInstance(project).isReady()) return false + val scriptDefinition = file.findScriptDefinition() ?: return false val (async, sync) = loaders.partition { it.shouldRunInBackground(scriptDefinition) } val syncLoader = sync.firstOrNull { it.loadDependencies(isFirstLoad, file, scriptDefinition, loadingContext) } if (syncLoader == null) { - if (forceSync) { - loaders.firstOrNull { it.loadDependencies(isFirstLoad, file, scriptDefinition, loadingContext) } - } else { - if (postponeLoading) { - LoadScriptConfigurationNotificationFactory.showNotification(virtualFile, project) { - runAsyncLoaders(file, virtualFile, scriptDefinition, async, isLoadingPostponed = true) - } + if (!fromCacheOnly) { + if (forceSync) { + loaders.firstOrNull { it.loadDependencies(isFirstLoad, file, scriptDefinition, loadingContext) } } else { - runAsyncLoaders(file, virtualFile, scriptDefinition, async, isLoadingPostponed = false) + val autoReloadEnabled = KotlinScriptingSettings.getInstance(project).isAutoReloadEnabled + val postponeLoading = isPostponedLoad && !autoReloadEnabled + + if (postponeLoading) { + LoadScriptConfigurationNotificationFactory.showNotification(virtualFile, project) { + runAsyncLoaders(file, virtualFile, scriptDefinition, async, isLoadingPostponed = true) + } + } else { + runAsyncLoaders(file, virtualFile, scriptDefinition, async, isLoadingPostponed = false) + } } } + + return false + } else { + return true } } @@ -203,7 +204,7 @@ class DefaultScriptingSupport(manager: CompositeScriptConfigurationManager) : De val applied = cached?.applied if (applied != null && applied.inputs.isUpToDate(project, virtualFile)) { // in case user reverted to applied configuration - suggestOrSaveConfiguration(virtualFile, applied, isLoadingPostponed) + suggestOrSaveConfiguration(virtualFile, applied, skipNotification = isLoadingPostponed) } else if (cached == null || !cached.isUpToDate(project, virtualFile)) { // don't start loading if nothing was changed // (in case we checking for up-to-date and loading concurrently) @@ -368,19 +369,18 @@ abstract class DefaultScriptingSupportBase(val manager: CompositeScriptConfigura * (e.g. after user will click "apply context" or/and configuration will be calculated by some background thread). * * @param isFirstLoad may be set explicitly for optimization reasons (to avoid expensive fs cache access) - * @param loadEvenWillNotBeApplied may should be set to false only on requests from particular editor, when - * user can see potential notification and accept new configuration. In other cases this should be `false` since - * loaded configuration will be just leaved in hidden user notification and cannot be used in any way. * @param forceSync should be used in tests only * @param isPostponedLoad is used to postspone loading: show a notification for out of date script and start loading when user request + * @param fromCacheOnly load only when builtin fast synchronous loaders are available + * @return true, if configuration loaded in sync */ protected abstract fun reloadOutOfDateConfiguration( file: KtFile, isFirstLoad: Boolean = getAppliedConfiguration(file.originalFile.virtualFile) == null, - loadEvenWillNotBeApplied: Boolean = false, forceSync: Boolean = false, - isPostponedLoad: Boolean = false - ) + isPostponedLoad: Boolean = false, + fromCacheOnly: Boolean = false + ): Boolean fun getCachedConfigurationState(file: VirtualFile?): ScriptConfigurationState? { if (file == null) return null @@ -390,7 +390,7 @@ abstract class DefaultScriptingSupportBase(val manager: CompositeScriptConfigura fun getAppliedConfiguration(file: VirtualFile?): ScriptConfigurationSnapshot? = getCachedConfigurationState(file)?.applied - fun hasCachedConfiguration(file: KtFile): Boolean = + private fun hasCachedConfiguration(file: KtFile): Boolean = getAppliedConfiguration(file.originalFile.virtualFile) != null fun isConfigurationLoadingInProgress(file: KtFile): Boolean { @@ -412,41 +412,65 @@ abstract class DefaultScriptingSupportBase(val manager: CompositeScriptConfigura return getAppliedConfiguration(virtualFile)?.configuration } + /** + * Load new configuration and suggest to apply it (only if it is changed) + */ fun ensureUpToDatedConfigurationSuggested(file: KtFile) { - reloadIfOutOfDate(listOf(file), loadEvenWillNotBeApplied = true, isPostponedLoad = false) - } - - fun ensureConfigurationUpToDate(files: List): Boolean { - return reloadIfOutOfDate(files, loadEvenWillNotBeApplied = false, isPostponedLoad = false) + reloadIfOutOfDate(file) } + /** + * Show notification about changed script configuration with action to start loading it + */ fun suggestToUpdateConfigurationIfOutOfDate(file: KtFile) { - reloadIfOutOfDate(listOf(file), loadEvenWillNotBeApplied = true, isPostponedLoad = true) + reloadIfOutOfDate(file, isPostponedLoad = true) } - private fun reloadIfOutOfDate(files: List, loadEvenWillNotBeApplied: Boolean, isPostponedLoad: Boolean): Boolean { + private fun reloadIfOutOfDate(file: KtFile, isPostponedLoad: Boolean = false) { + if (!ScriptDefinitionsManager.getInstance(project).isReady()) return + + manager.updater.update { + val virtualFile = file.originalFile.virtualFile + if (virtualFile != null) { + val state = cache[virtualFile] + if (state == null || !state.isUpToDate(project, virtualFile, file)) { + reloadOutOfDateConfiguration( + file, + isFirstLoad = state == null, + isPostponedLoad = isPostponedLoad + ) + } + } + } + } + + /** + * Ensure that any configuration for [files] is loaded from cache + */ + fun ensureLoadedFromCache(files: List): Boolean { if (!ScriptDefinitionsManager.getInstance(project).isReady()) return false - var upToDate = true + var allLoaded = true manager.updater.update { files.forEach { file -> val virtualFile = file.originalFile.virtualFile if (virtualFile != null) { val state = cache[virtualFile] - if (state == null || !state.isUpToDate(project, virtualFile, file)) { - upToDate = false - reloadOutOfDateConfiguration( - file, - isFirstLoad = state == null, - loadEvenWillNotBeApplied = loadEvenWillNotBeApplied, - isPostponedLoad = isPostponedLoad - ) + if (state == null) { + if (!reloadOutOfDateConfiguration( + file, + isFirstLoad = true, + fromCacheOnly = true + ) + ) { + allLoaded = false + } } } } } - return upToDate + return allLoaded } protected open fun setAppliedConfiguration( @@ -460,12 +484,7 @@ abstract class DefaultScriptingSupportBase(val manager: CompositeScriptConfigura if (newConfiguration != null) { cache.setApplied(file, newConfigurationSnapshot) - - if (syncUpdate) { - manager.updater.updateSynchronously() - } else { - manager.updater.invalidate(file) - } + manager.updater.invalidate(file, synchronous = syncUpdate) } } @@ -486,7 +505,7 @@ abstract class DefaultScriptingSupportBase(val manager: CompositeScriptConfigura if (cache[virtualFile]?.isUpToDate(project, virtualFile, file) == true) return manager.updater.update { - reloadOutOfDateConfiguration(file, forceSync = true, loadEvenWillNotBeApplied = true) + reloadOutOfDateConfiguration(file, forceSync = true) } } diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/configuration/utils/ScriptClassRootsUpdater.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/configuration/utils/ScriptClassRootsUpdater.kt index 32abfdef22f..4e55bfcbbd2 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/configuration/utils/ScriptClassRootsUpdater.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/configuration/utils/ScriptClassRootsUpdater.kt @@ -39,19 +39,29 @@ class ScriptClassRootsUpdater( val manager: CompositeScriptConfigurationManager ) { private var invalidated: Boolean = false + private var syncUpdateRequired: Boolean = false private val concurrentUpdates = AtomicInteger() + /** + * @param synchronous Used from legacy FS cache only, don't use + */ @Synchronized @Suppress("UNUSED_PARAMETER") - fun invalidate(file: VirtualFile) { + fun invalidate(file: VirtualFile, synchronous: Boolean = false) { // todo: record invalided files for some optimisations in update - invalidate() + invalidate(synchronous) } + /** + * @param synchronous Used from legacy FS cache only, don't use + */ @Synchronized - fun invalidate() { + fun invalidate(synchronous: Boolean = false) { checkInTransaction() invalidated = true + if (synchronous) { + syncUpdateRequired = true + } } fun checkInTransaction() { @@ -79,11 +89,17 @@ class ScriptClassRootsUpdater( scheduleUpdateIfInvalid() } + @Synchronized private fun scheduleUpdateIfInvalid() { if (!invalidated) return invalidated = false - ensureUpdateScheduled() + if (syncUpdateRequired) { + syncUpdateRequired = false + updateSynchronously() + } else { + ensureUpdateScheduled() + } } private val syncLock = ReentrantLock() @@ -97,11 +113,8 @@ class ScriptClassRootsUpdater( } } - /** - * Used from legacy FS cache only, don't use - */ @Synchronized - fun updateSynchronously() { + private fun updateSynchronously() { syncLock.withLock { scheduledUpdate?.cancel() doUpdate(false) diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/UnusedSymbolInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/UnusedSymbolInspection.kt index 0b7d848d69b..dfea43cdcf4 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/UnusedSymbolInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/UnusedSymbolInspection.kt @@ -45,7 +45,7 @@ import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.caches.resolve.findModuleDescriptor import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny import org.jetbrains.kotlin.idea.core.isInheritable -import org.jetbrains.kotlin.idea.core.script.ScriptConfigurationManager +import org.jetbrains.kotlin.idea.core.script.configuration.DefaultScriptingSupport import org.jetbrains.kotlin.idea.core.toDescriptor import org.jetbrains.kotlin.idea.findUsages.KotlinFindUsagesHandlerFactory import org.jetbrains.kotlin.idea.findUsages.handlers.KotlinFindClassUsagesHandler @@ -145,12 +145,13 @@ class UnusedSymbolInspection : AbstractKotlinInspection() { val project = declaration.project val psiSearchHelper = PsiSearchHelper.getInstance(project) -// val usedScripts = findScriptsWithUsages(declaration) -// if (usedScripts.isNotEmpty()) { -// if (!ScriptConfigurationManager.getInstance(declaration.project).updater.ensureConfigurationUpToDate(usedScripts)) { -// return TOO_MANY_OCCURRENCES -// } -// } + val usedScripts = findScriptsWithUsages(declaration) + if (usedScripts.isNotEmpty()) { + if (!DefaultScriptingSupport.getInstance(declaration.project).ensureLoadedFromCache(usedScripts)) { + // Not all script configuration are loaded; behave like it is used + return TOO_MANY_OCCURRENCES + } + } val useScope = psiSearchHelper.getUseScope(declaration) if (useScope is GlobalSearchScope) { diff --git a/idea/tests/org/jetbrains/kotlin/idea/script/ScriptConfigurationLoadingTest.kt b/idea/tests/org/jetbrains/kotlin/idea/script/ScriptConfigurationLoadingTest.kt index f4ee66413b9..e2d0dffc3d5 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/script/ScriptConfigurationLoadingTest.kt +++ b/idea/tests/org/jetbrains/kotlin/idea/script/ScriptConfigurationLoadingTest.kt @@ -6,10 +6,10 @@ package org.jetbrains.kotlin.idea.script import org.jetbrains.kotlin.psi.KtFile -import org.jetbrains.kotlin.test.JUnit3WithIdeaConfigurationRunner +import org.jetbrains.kotlin.test.JUnit3RunnerWithInners import org.junit.runner.RunWith -@RunWith(JUnit3WithIdeaConfigurationRunner::class) +@RunWith(JUnit3RunnerWithInners::class) class ScriptConfigurationLoadingTest : AbstractScriptConfigurationLoadingTest() { fun testSimple() { assertAndLoadInitialConfiguration() @@ -163,10 +163,10 @@ class ScriptConfigurationLoadingTest : AbstractScriptConfigurationLoadingTest() fun testLoadingForUsagesSearch() { assertAndLoadInitialConfiguration() - assertTrue(scriptConfigurationManager.default.ensureConfigurationUpToDate(listOf(myFile as KtFile))) + assertTrue(scriptConfigurationManager.default.ensureLoadedFromCache(listOf(myFile as KtFile))) changeContents("A") - assertFalse(scriptConfigurationManager.default.ensureConfigurationUpToDate(listOf(myFile as KtFile))) + assertTrue(scriptConfigurationManager.default.ensureLoadedFromCache(listOf(myFile as KtFile))) } fun testReportsOnAutoApply() {