UnusedSymbolInspection: load script configurations from cache

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
+        isPostponedLoad: Boolean,
-    ) {
+        fromCacheOnly: Boolean
-        val virtualFile = file.originalFile.virtualFile ?: return
+    ): Boolean {
+        val virtualFile = file.originalFile.virtualFile ?: return false
 
-        val autoReloadEnabled = KotlinScriptingSettings.getInstance(project).isAutoReloadEnabled
+        if (!ScriptDefinitionsManager.getInstance(project).isReady()) return false
-        val shouldLoad = isFirstLoad || loadEvenWillNotBeApplied || autoReloadEnabled
+        val scriptDefinition = file.findScriptDefinition() ?: return false
-        if (!shouldLoad) return
-
-        val postponeLoading = isPostponedLoad && !autoReloadEnabled && !isFirstLoad
-
-        if (!ScriptDefinitionsManager.getInstance(project).isReady()) return
-        val scriptDefinition = file.findScriptDefinition() ?: return
 
         val (async, sync) = loaders.partition { it.shouldRunInBackground(scriptDefinition) }
 
         val syncLoader = sync.firstOrNull { it.loadDependencies(isFirstLoad, file, scriptDefinition, loadingContext) }
         if (syncLoader == null) {
-            if (forceSync) {
+            if (!fromCacheOnly) {
-                loaders.firstOrNull { it.loadDependencies(isFirstLoad, file, scriptDefinition, loadingContext) }
+                if (forceSync) {
-            } else {
+                    loaders.firstOrNull { it.loadDependencies(isFirstLoad, file, scriptDefinition, loadingContext) }
-                if (postponeLoading) {
-                    LoadScriptConfigurationNotificationFactory.showNotification(virtualFile, project) {
-                        runAsyncLoaders(file, virtualFile, scriptDefinition, async, isLoadingPostponed = true)
-                    }
                 } 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)
+        reloadIfOutOfDate(file)
-    }
-
-    fun ensureConfigurationUpToDate(files: List<KtFile>): Boolean {
-        return reloadIfOutOfDate(files, loadEvenWillNotBeApplied = false, isPostponedLoad = false)
     }
 
+    /**
+     * 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<KtFile>, 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<KtFile>): 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)) {
+                    if (state == null) {
-                        upToDate = false
+                        if (!reloadOutOfDateConfiguration(
-                        reloadOutOfDateConfiguration(
+                                file,
-                            file,
+                                isFirstLoad = true,
-                            isFirstLoad = state == null,
+                                fromCacheOnly = true
-                            loadEvenWillNotBeApplied = loadEvenWillNotBeApplied,
+                            )
-                            isPostponedLoad = isPostponedLoad
+                        ) {
-                        )
+                            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)
-
+            manager.updater.invalidate(file, synchronous = syncUpdate)
-            if (syncUpdate) {
-                manager.updater.updateSynchronously()
-            } else {
-                manager.updater.invalidate(file)
-            }
         }
     }
 
@@ -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)
         }
     }
 

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)

idea/src/org/jetbrains/kotlin/idea/inspections/UnusedSymbolInspection.kt

@@ -7,7 +7,6 @@ package org.jetbrains.kotlin.idea.inspections
 
 import com.intellij.codeInsight.FileModificationService
 import com.intellij.codeInsight.daemon.QuickFixBundle
-import com.intellij.codeInsight.daemon.impl.HighlightInfoType
 import com.intellij.codeInsight.daemon.impl.analysis.HighlightUtil
 import com.intellij.codeInsight.daemon.impl.analysis.JavaHighlightUtil
 import com.intellij.codeInspection.*
@@ -45,7 +44,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
@@ -136,12 +135,13 @@ class UnusedSymbolInspection : AbstractKotlinInspection() {
             val project = declaration.project
             val psiSearchHelper = PsiSearchHelper.getInstance(project)
 
-//            val usedScripts = findScriptsWithUsages(declaration)
+            val usedScripts = findScriptsWithUsages(declaration)
-//            if (usedScripts.isNotEmpty()) {
+            if (usedScripts.isNotEmpty()) {
-//                if (!ScriptConfigurationManager.getInstance(declaration.project).updater.ensureConfigurationUpToDate(usedScripts)) {
+                if (!DefaultScriptingSupport.getInstance(declaration.project).ensureLoadedFromCache(usedScripts)) {
-//                    return TOO_MANY_OCCURRENCES
+                    // Not all script configuration are loaded; behave like it is used
-//                }
+                    return TOO_MANY_OCCURRENCES
-//            }
+                }
+            }
 
             val useScope = psiSearchHelper.getUseScope(declaration)
             if (useScope is GlobalSearchScope) {

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() {