fix: don't display IaC ignored errors in balloon notification (#546)

* fix: don't display balloon notification for ignored iac error

* fix: only process files in file listener

* fix: tests

* fix: don't output amplitude connection problems as warnings/errors

* fix: improve file update detection and disposal on project close

* fix: add disposable/disposer to Snyk Language Client

* fix: add better disposal to LanguageServerWrapper

* fix: compile error

* chore: improve disposals

* chore: added tests for language server wrapper

* refactor: make disposed property private

* chore: add more tests

* chore: remove test stubs

* fix: actually dispose when dispose() is called

* docs: added javadoc

* fix: comment

Author: bastiandoetsch

CHANGELOG.md

@@ -1,8 +1,13 @@
 # Snyk Security Changelog
 
+## [2.8.5]
+### Fixed
+- don't display balloon warnings if IaC error is ignored (e.g. no IaC files found)
+- don't output amplitude errors as warning, only debug
+
 ## [2.8.4]
 ### Fixed
-- dont use kotlin specific convenience function that may cause errors on non kotlin IDEs
+- don't use kotlin specific convenience function that may cause errors on non kotlin IDEs
 
 ## [2.8.3]
 

src/main/kotlin/io/snyk/plugin/SnykBulkFileListener.kt

@@ -3,6 +3,7 @@ package io.snyk.plugin
 import com.intellij.ide.impl.ProjectUtil
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.vfs.VirtualFile
+import com.intellij.openapi.vfs.isFile
 import com.intellij.openapi.vfs.newvfs.BulkFileListener
 import com.intellij.openapi.vfs.newvfs.events.VFileContentChangeEvent
 import com.intellij.openapi.vfs.newvfs.events.VFileCopyEvent
@@ -121,6 +122,7 @@ abstract class SnykBulkFileListener : BulkFileListener {
             .filter { eventsFilter.invoke(it) }
             .mapNotNull(eventToVirtualFileTransformer)
             .filter(VirtualFile::isValid)
+            .filter(VirtualFile::isFile)
             .toSet()
     }
 

src/main/kotlin/io/snyk/plugin/SnykProjectManagerListener.kt

@@ -7,18 +7,21 @@ import com.intellij.openapi.progress.Task.Backgroundable
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.project.ProjectManagerListener
 import snyk.common.lsp.LanguageServerWrapper
+import java.util.concurrent.ExecutorService
 import java.util.concurrent.Executors
 import java.util.concurrent.TimeUnit
 
 private const val TIMEOUT = 1L
 
 class SnykProjectManagerListener : ProjectManagerListener {
+    val threadPool: ExecutorService = Executors.newWorkStealingPool()
+
     override fun projectClosing(project: Project) {
         val closingTask = object : Backgroundable(project, "Project closing ${project.name}") {
             override fun run(indicator: ProgressIndicator) {
-                // limit clean up to 5s
+                // limit clean up to TIMEOUT
                 try {
-                    Executors.newCachedThreadPool().submit {
+                    threadPool.submit {
                         // lets all running ProgressIndicators release MUTEX first
                         val ls = LanguageServerWrapper.getInstance()
                         if (ls.isInitialized) {

src/main/kotlin/io/snyk/plugin/Utils.kt

@@ -315,15 +315,16 @@ fun findPsiFileIgnoringExceptions(virtualFile: VirtualFile, project: Project): P
     }
 
 fun refreshAnnotationsForOpenFiles(project: Project) {
-    if (project.isDisposed) return
+    if (project.isDisposed || ApplicationManager.getApplication().isDisposed) return
     VirtualFileManager.getInstance().asyncRefresh()
 
     val openFiles = FileEditorManager.getInstance(project).openFiles
 
     ApplicationManager.getApplication().invokeLater {
-        project.service<CodeVisionHost>().invalidateProvider(CodeVisionHost.LensInvalidateSignal(null))
+        if (!project.isDisposed) {
+            project.service<CodeVisionHost>().invalidateProvider(CodeVisionHost.LensInvalidateSignal(null))
+        }
     }
-
     openFiles.forEach {
         val psiFile = findPsiFileIgnoringExceptions(it, project)
         if (psiFile != null) {

src/main/kotlin/io/snyk/plugin/services/SnykTaskQueueService.kt

@@ -263,6 +263,7 @@ class SnykTaskQueueService(val project: Project) {
 
     fun downloadLatestRelease(force: Boolean = false) {
         // abort even before submitting a task
+        if (project.isDisposed || ApplicationManager.getApplication().isDisposed) return
         val cliDownloader = getSnykCliDownloaderService()
         if (!pluginSettings().manageBinariesAutomatically) {
             if (!isCliInstalled()) {

src/main/kotlin/io/snyk/plugin/services/download/CliDownloaderService.kt

@@ -32,7 +32,7 @@ class SnykCliDownloaderService {
 
     private var currentProgressIndicator: ProgressIndicator? = null
 
-    fun isCliDownloading() = currentProgressIndicator != null
+    fun isCliDownloading() = currentProgressIndicator != null && !ApplicationManager.getApplication().isDisposed
 
     fun stopCliDownload() = currentProgressIndicator?.let {
         it.cancel()

src/main/kotlin/io/snyk/plugin/snykcode/SnykCodeBulkFileListener.kt

@@ -2,12 +2,11 @@ package io.snyk.plugin.snykcode
 
 import com.google.common.cache.CacheBuilder
 import com.intellij.codeInsight.daemon.DaemonCodeAnalyzer
-import com.intellij.openapi.application.runInEdt
+import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.diagnostic.logger
-import com.intellij.openapi.progress.ProgressIndicator
-import com.intellij.openapi.progress.ProgressManager
-import com.intellij.openapi.progress.Task.Backgroundable
+import com.intellij.openapi.project.DumbService
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.roots.ProjectFileIndex
 import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.openapi.vfs.VirtualFileManager
 import com.intellij.openapi.vfs.newvfs.events.VFileEvent
@@ -18,7 +17,9 @@ import io.snyk.plugin.toLanguageServerURL
 import io.snyk.plugin.toSnykFileSet
 import org.eclipse.lsp4j.DidSaveTextDocumentParams
 import org.eclipse.lsp4j.TextDocumentIdentifier
+import org.jetbrains.concurrency.runAsync
 import snyk.common.lsp.LanguageServerWrapper
+import java.io.File
 import java.time.Duration
 
 class SnykCodeBulkFileListener : SnykBulkFileListener() {
@@ -28,50 +29,71 @@ class SnykCodeBulkFileListener : SnykBulkFileListener() {
         CacheBuilder.newBuilder()
             .expireAfterWrite(Duration.ofMillis(1000)).build<String, Boolean>()
 
+    private val blackListedDirectories =
+        setOf(".idea", ".git", ".hg", ".svn")
+
     override fun before(project: Project, virtualFilesAffected: Set<VirtualFile>) = Unit
 
     override fun after(project: Project, virtualFilesAffected: Set<VirtualFile>) {
         if (virtualFilesAffected.isEmpty()) return
-        ProgressManager.getInstance().run(object : Backgroundable(
-            project,
-            "Snyk: forwarding save event to Language Server"
-        ) {
-            override fun run(indicator: ProgressIndicator) {
-                val languageServerWrapper = LanguageServerWrapper.getInstance()
-                if (!languageServerWrapper.isInitialized) return
-                val languageServer = languageServerWrapper.languageServer
-                val cache = getSnykCachedResults(project)?.currentSnykCodeResultsLS ?: return
-                val filesAffected = toSnykFileSet(project, virtualFilesAffected)
-                for (file in filesAffected) {
-                    val virtualFile = file.virtualFile
-                    if (!shouldProcess(virtualFile)) continue
-                    cache.remove(file)
-                    val param =
-                        DidSaveTextDocumentParams(
-                            TextDocumentIdentifier(virtualFile.toLanguageServerURL()),
-                            virtualFile.readText()
-                        )
-                    languageServer.textDocumentService.didSave(param)
-                }
 
-                VirtualFileManager.getInstance().asyncRefresh()
-                runInEdt { DaemonCodeAnalyzer.getInstance(project).restart() }
-            }
-        })
+        runAsync {
+            if (ApplicationManager.getApplication().isDisposed) return@runAsync
+            if (project.isDisposed) return@runAsync
+            if (DumbService.getInstance(project).isDumb) return@runAsync
 
+            val languageServerWrapper = LanguageServerWrapper.getInstance()
+            if (languageServerWrapper.isDisposed() || !languageServerWrapper.isInitialized) return@runAsync
+
+            val languageServer = languageServerWrapper.languageServer
+            val cache = getSnykCachedResults(project)?.currentSnykCodeResultsLS
+            val filesAffected = toSnykFileSet(project, virtualFilesAffected)
+            val index = ProjectFileIndex.getInstance(project)
+
+            for (file in filesAffected) {
+                val virtualFile = file.virtualFile
+                if (!shouldProcess(virtualFile, index, project)) continue
+                cache?.remove(file)
+                val param =
+                    DidSaveTextDocumentParams(
+                        TextDocumentIdentifier(virtualFile.toLanguageServerURL()),
+                        virtualFile.readText()
+                    )
+                languageServer.textDocumentService.didSave(param)
+            }
+            VirtualFileManager.getInstance().asyncRefresh()
+            DaemonCodeAnalyzer.getInstance(project).restart()
+        }
     }
 
-    private fun shouldProcess(file: VirtualFile): Boolean {
-        val inCache = debounceFileCache.getIfPresent(file.path)
+    private fun shouldProcess(file: VirtualFile, index: ProjectFileIndex, project: Project): Boolean {
+        var shouldProcess = false
+        val application = ApplicationManager.getApplication()
+        if (application.isDisposed) return false
+        if (project.isDisposed) return false
+        if (DumbService.getInstance(project).isDumb) shouldProcess = false
 
-        return if (inCache != null) {
-            logger<SnykCodeBulkFileListener>().info("not forwarding file event to ls, debouncing")
-            false
-        } else {
-            debounceFileCache.put(file.path, true)
-            logger<SnykCodeBulkFileListener>().info("forwarding file event to ls, not debouncing")
-            true
+        application.runReadAction {
+            val inCache = debounceFileCache.getIfPresent(file.path)
+            if (inCache != null) {
+                shouldProcess = false
+            } else {
+                debounceFileCache.put(file.path, true)
+                if (index.isInContent(file) && !isInBlacklistedParentDir(file)) {
+                    shouldProcess = true
+                } else {
+                    shouldProcess = false
+                    return@runReadAction
+                }
+            }
         }
+        return shouldProcess
+    }
+
+    private fun isInBlacklistedParentDir(file: VirtualFile): Boolean {
+        val path = file.path.split(File.separatorChar)
+            .filter { blackListedDirectories.contains(it.trimEnd(File.separatorChar)) }
+        return path.isNotEmpty()
     }
 
     override fun forwardEvents(events: MutableList<out VFileEvent>) = Unit

src/main/kotlin/io/snyk/plugin/ui/toolwindow/SnykToolWindowPanel.kt

@@ -85,6 +85,7 @@ import snyk.container.ui.ContainerIssueTreeNode
 import snyk.iac.IacError
 import snyk.iac.IacIssue
 import snyk.iac.IacResult
+import snyk.iac.ignorableErrorCodes
 import snyk.iac.ui.toolwindow.IacFileTreeNode
 import snyk.iac.ui.toolwindow.IacIssueTreeNode
 import snyk.oss.OssResult
@@ -247,7 +248,7 @@ class SnykToolWindowPanel(val project: Project) : JPanel(), Disposable {
                     override fun scanningIacError(snykError: SnykError) {
                         var iacResultsCount: Int? = null
                         ApplicationManager.getApplication().invokeLater {
-                            if (snykError.code == IacError.NO_IAC_FILES_CODE) {
+                            if (snykError.code != null && ignorableErrorCodes.contains(snykError.code)) {
                                 iacResultsCount = NODE_NOT_SUPPORTED_STATE
                             } else {
                                 SnykBalloonNotificationHelper.showError(snykError.message, project)

src/main/kotlin/io/snyk/plugin/ui/toolwindow/SnykToolWindowSnykScanListenerLS.kt

@@ -1,7 +1,9 @@
 package io.snyk.plugin.ui.toolwindow
 
+import com.intellij.openapi.Disposable
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.util.Disposer
 import com.intellij.openapi.util.TextRange
 import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.util.ui.tree.TreeUtil
@@ -35,8 +37,19 @@ class SnykToolWindowSnykScanListenerLS(
     private val rootSecurityIssuesTreeNode: DefaultMutableTreeNode,
     private val rootQualityIssuesTreeNode: DefaultMutableTreeNode,
     private val rootOssIssuesTreeNode: DefaultMutableTreeNode,
-) : SnykScanListenerLS {
+) : SnykScanListenerLS, Disposable {
+    private var disposed = false; get() { return project.isDisposed || ApplicationManager.getApplication().isDisposed || field }
+    init {
+        Disposer.register(SnykPluginDisposable.getInstance(project), this)
+    }
+
+    override fun dispose() {
+        disposed = true
+    }
+    fun isDisposed() = disposed
+
     override fun scanningStarted(snykScan: SnykScanParams) {
+        if (disposed) return
         ApplicationManager.getApplication().invokeLater {
             rootSecurityIssuesTreeNode.userObject = "$CODE_SECURITY_ROOT_TEXT (scanning...)"
             rootQualityIssuesTreeNode.userObject = "$CODE_QUALITY_ROOT_TEXT (scanning...)"
@@ -45,6 +58,7 @@ class SnykToolWindowSnykScanListenerLS(
     }
 
     override fun scanningSnykCodeFinished(snykResults: Map<SnykFile, List<ScanIssue>>) {
+        if (disposed) return
         ApplicationManager.getApplication().invokeLater {
             this.snykToolWindowPanel.navigateToSourceEnabled = false
             displaySnykCodeResults(snykResults)
@@ -54,6 +68,7 @@ class SnykToolWindowSnykScanListenerLS(
     }
 
     override fun scanningOssFinished(snykResults: Map<SnykFile, List<ScanIssue>>) {
+        if (disposed) return
         ApplicationManager.getApplication().invokeLater {
             this.snykToolWindowPanel.navigateToSourceEnabled = false
             displayOssResults(snykResults)
@@ -65,6 +80,7 @@ class SnykToolWindowSnykScanListenerLS(
     override fun scanningError(snykScan: SnykScanParams) = Unit
 
     fun displaySnykCodeResults(snykResults: Map<SnykFile, List<ScanIssue>>) {
+        if (disposed) return
         if (getSnykCachedResults(project)?.currentSnykCodeError != null) return
 
         val settings = pluginSettings()
@@ -101,6 +117,7 @@ class SnykToolWindowSnykScanListenerLS(
     }
 
     fun displayOssResults(snykResults: Map<SnykFile, List<ScanIssue>>) {
+        if (disposed) return
         if (getSnykCachedResults(project)?.currentOssError != null) return
 
         val settings = pluginSettings()
@@ -180,6 +197,8 @@ class SnykToolWindowSnykScanListenerLS(
         securityIssuesCount: Int? = null,
         fixableIssuesCount: Int? = null,
     ) {
+        if (disposed) return
+
         // only add these info tree nodes to Snyk Code security vulnerabilities for now
         if (securityIssuesCount == null) {
             return

src/main/kotlin/snyk/amplitude/api/AmplitudeExperimentApiClient.kt

@@ -28,18 +28,19 @@ class AmplitudeExperimentApiClient private constructor(
         return try {
             val response = variantService().sdkVardata(user).execute()
             if (!response.isSuccessful) {
-                log.warn("Error response: $response")
+                // we don't really care
+                log.debug("Error response: $response")
                 return emptyMap()
             }
 
             val variants = response.body()
             log.debug("Received variants: $variants")
             variants ?: emptyMap()
         } catch (e: IOException) {
-            log.warn("Could not fetch variants because of network communication error with amplitude server", e)
+            log.debug("Could not fetch variants because of network communication error with amplitude server. Continuing without", e)
             emptyMap()
         } catch (e: Exception) {
-            log.warn("Could not fetch variants because of unexpected error", e)
+            log.info("Could not fetch variants because of unexpected error. Continuing without.", e)
             emptyMap()
         }
     }