Skip to content

Commit

Permalink
SLI-1633 Cancel previous redundant analysis when triggering a new one
Browse files Browse the repository at this point in the history
  • Loading branch information
nquinquenel committed Sep 30, 2024
1 parent c6c5880 commit afde9b1
Show file tree
Hide file tree
Showing 20 changed files with 104 additions and 75 deletions.
4 changes: 2 additions & 2 deletions git/src/main/kotlin/org/sonarlint/intellij/git/GitRepo.kt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class GitRepo(private val repo: GitRepository, private val project: Project) : V
override fun electBestMatchingServerBranchForCurrentHead(mainBranchName: String, allBranchNames: Set<String>): String? {
return try {
val currentBranch = repo.currentBranchName
if (currentBranch != null && allBranchNames.contains(currentBranch)) {
if (currentBranch != null && currentBranch in allBranchNames) {
return currentBranch
}
val head = repo.currentRevision ?: return null // Could be the case if no commit has been made in the repo
Expand All @@ -46,7 +46,7 @@ class GitRepo(private val repo: GitRepository, private val project: Project) : V
branchesPerDistance.computeIfAbsent(distance) { HashSet() }.add(serverBranchName)
}
val bestCandidates = branchesPerDistance.minByOrNull { it.key }?.value ?: return null
if (bestCandidates.contains(mainBranchName)) {
if (mainBranchName in bestCandidates) {
// Favor the main branch when there are multiple candidates with the same distance
mainBranchName
} else bestCandidates.first()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ object SonarLintIntelliJClient : SonarLintRpcClientDelegate {

val matchedFile = tryFindFile(project, filePath)

if (matchedFile != null && openFiles.contains(matchedFile)) {
if (matchedFile != null && matchedFile in openFiles) {
impactedFiles.add(matchedFile)
}

Expand Down Expand Up @@ -821,7 +821,7 @@ object SonarLintIntelliJClient : SonarLintRpcClientDelegate {
}

private fun findProjects(projectKey: String?) = ProjectManager.getInstance().openProjects.filter { project ->
getService(project, ProjectBindingManager::class.java).uniqueProjectKeys.contains(projectKey)
projectKey in getService(project, ProjectBindingManager::class.java).uniqueProjectKeys
}.toSet()

override fun raiseIssues(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class ExcludeFileAction : AbstractSonarAction {
override fun isEnabled(e: AnActionEvent, project: Project, status: AnalysisStatus): Boolean {
val files = e.getData(CommonDataKeys.VIRTUAL_FILE_ARRAY)?.asSequence() ?: return false
val exclusions = Settings.getSettingsFor(project).fileExclusions.toSet()
return toExclusionPatterns(project, files).any { !exclusions.contains(it) }
return toExclusionPatterns(project, files).any { it !in exclusions }
}

override fun actionPerformed(e: AnActionEvent) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ private Summary analyzePerModule(AnalysisScope scope, ProgressIndicator indicato
getService(BackendService.class).updateFileSystem(Map.of(module, filesEvent));
}

var analysisState = new AnalysisState(analysisId, callback, entry.getValue(), module, trigger);
var analysisState = new AnalysisState(analysisId, callback, entry.getValue(), module, trigger, indicator);
results.put(module, analyzer.analyzeModule(module, entry.getValue(), analysisState, indicator, scope.shouldFetchServerIssues()));
checkCanceled(indicator);
}
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/org/sonarlint/intellij/analysis/AnalysisState.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package org.sonarlint.intellij.analysis
import com.intellij.openapi.fileEditor.FileDocumentManager
import com.intellij.openapi.module.Module
import com.intellij.openapi.progress.ProcessCanceledException
import com.intellij.openapi.progress.ProgressIndicator
import com.intellij.openapi.vfs.VirtualFile
import java.net.URI
import java.time.Instant
Expand All @@ -35,6 +36,7 @@ import org.sonarlint.intellij.finding.RawIssueAdapter
import org.sonarlint.intellij.finding.hotspot.LiveSecurityHotspot
import org.sonarlint.intellij.finding.issue.LiveIssue
import org.sonarlint.intellij.trigger.TriggerType
import org.sonarlint.intellij.trigger.TriggerType.Companion.forcedAnalysisSnapshot
import org.sonarlint.intellij.util.VirtualFileUtils.uriToVirtualFile
import org.sonarsource.sonarlint.core.rpc.protocol.client.hotspot.RaisedHotspotDto
import org.sonarsource.sonarlint.core.rpc.protocol.client.issue.RaisedIssueDto
Expand All @@ -46,6 +48,7 @@ class AnalysisState(
private val filesToAnalyze: MutableCollection<VirtualFile>,
private val module: Module,
private val triggerType: TriggerType,
private val progress: ProgressIndicator
) {
private val modificationStampByFile = ConcurrentHashMap<VirtualFile, Long>()
private val analysisDate: Instant = Instant.now()
Expand All @@ -68,6 +71,10 @@ class AnalysisState(
}
}

fun cancel() {
progress.cancel()
}

fun addRawHotspots(analysisId: UUID, hotspotsByFile: Map<URI, List<RaisedHotspotDto>>, isIntermediate: Boolean) {
hasReceivedFinalHotspots = !isIntermediate

Expand Down Expand Up @@ -164,4 +171,14 @@ class AnalysisState(
return hasReceivedFinalIssues && (!shouldReceiveHotspot || hasReceivedFinalHotspots)
}

// Analysis are redundant if both are forced or both are not forced, otherwise they should not cancel each other
// For security reason, pre commit check should never be cancelled
// All the files of the redundant analysis should be contained in the new one, otherwise information might be lost
fun isRedundant(analysisState: AnalysisState): Boolean {
val bothForced = triggerType in forcedAnalysisSnapshot && analysisState.triggerType in forcedAnalysisSnapshot
val bothNotForced = triggerType !in forcedAnalysisSnapshot && analysisState.triggerType !in forcedAnalysisSnapshot
return triggerType != TriggerType.CHECK_IN && analysisState.triggerType != TriggerType.CHECK_IN
&& (bothForced || bothNotForced) && analysisState.filesToAnalyze.containsAll(filesToAnalyze)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class OnTheFlyFindingsHolder(private val project: Project) : FileEditorManagerLi
RawIssueAdapter.toLiveSecurityHotspot(module, it, virtualFile, null)
}
virtualFile to liveIssues
}.toMap().filterKeys { openFiles.contains(it) }
}.toMap().filterKeys { it in openFiles }
currentSecurityHotspotsPerOpenFile.putAll(securityHotspots)
if (selectedFile == null) {
runOnUiThread(project) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package org.sonarlint.intellij.analysis
import com.intellij.openapi.components.Service
import java.util.UUID
import java.util.concurrent.ConcurrentHashMap
import org.sonarlint.intellij.common.ui.SonarLintConsole

@Service(Service.Level.PROJECT)
class RunningAnalysesTracker {
Expand All @@ -44,6 +45,22 @@ class RunningAnalysesTracker {
return analysisStateById[analysisId]
}

fun cancel(analysisId: UUID) {
analysisStateById[analysisId]?.let {
it.cancel()
finish(it)
}
}

fun cancelSimilarAnalysis(analysisState: AnalysisState, console: SonarLintConsole) {
for (analysis in analysisStateById.values) {
if (analysis.isRedundant(analysisState)) {
console.info("Cancelling analysis ${analysis.id}")
analysis.cancel()
}
}
}

fun isAnalysisRunning(): Boolean {
return analysisStateById.isNotEmpty()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public ModuleAnalysisResult analyzeModule(Module module, Collection<VirtualFile>

// Analyze
try {
getService(myProject, RunningAnalysesTracker.class).cancelSimilarAnalysis(analysisState, console);
getService(myProject, RunningAnalysesTracker.class).track(analysisState);

var what = filesToAnalyze.size() == 1 ? String.format("'%s'", filesToAnalyze.iterator().next().getName()) : String.format("%d files", filesToAnalyze.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ class AutomaticServerConnectionCreator(private val serverOrOrg: String, private
private fun findFirstUniqueConnectionName(connectionNames: Set<String>, newConnectionName: String): String {
var suffix = 1
var uniqueName = newConnectionName
while (connectionNames.contains(uniqueName)) {
while (uniqueName in connectionNames) {
uniqueName = "$newConnectionName-$suffix"
suffix++
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,7 @@ class ModuleBindingPanel(private val project: Project, currentConnectionSupplier
val existingModuleNames = collectionListModel.items.map { it.module.name }
val modulesToPickFrom =
ModuleManager.getInstance(project).modules.filter {
!existingModuleNames.contains(it.name) &&
getService(it, ModuleBindingManager::class.java).isBindingOverrideAllowed
it.name !in existingModuleNames && getService(it, ModuleBindingManager::class.java).isBindingOverrideAllowed
}
val dialog = if (ProjectAttachProcessor.canAttachToProject())
ChooseModulesDialog(modulesList, modulesToPickFrom.toMutableList(), "Select attached project", "Select the attached project for which you want to override the binding")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class CodeAnalyzerRestarter @NonInjectable internal constructor(private val myPr
val openFiles = fileEditorManager.openFiles
runReadActionSafely(myProject) {
openFiles
.filter { changedFiles.contains(it) }
.filter { it in changedFiles }
.mapNotNull { getPsi(it) }
.forEach { codeAnalyzer.restart(it) }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ import com.intellij.openapi.editor.markup.TextAttributes
import com.intellij.openapi.project.Project
import com.intellij.openapi.util.Disposer
import com.intellij.ui.JBColor
import java.awt.Font
import java.util.function.Consumer
import org.sonarlint.intellij.common.ui.ReadActionUtils.Companion.computeReadActionSafely
import org.sonarlint.intellij.config.SonarLintTextAttributes
import org.sonarlint.intellij.finding.Flow
import org.sonarlint.intellij.finding.LiveFinding
import org.sonarlint.intellij.finding.Location
import org.sonarlint.intellij.finding.issue.vulnerabilities.LocalTaintVulnerability
import org.sonarlint.intellij.ui.UiUtils.Companion.runOnUiThread
import java.awt.Font
import java.util.function.Consumer

private const val HIGHLIGHT_GROUP_ID = 1001

Expand Down Expand Up @@ -171,7 +171,7 @@ class EditorDecorator(private val project: Project) {
}

fun isActiveInEditor(editor: Editor): Boolean {
return currentHighlightedDoc.contains(editor.document)
return editor.document in currentHighlightedDoc
}

private fun createHighlights(locations: List<Location>): MutableList<Highlight> {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/sonarlint/intellij/finding/LiveFindings.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ class LiveFindings(

fun onlyFor(files: Set<VirtualFile>): LiveFindings {
return LiveFindings(
issuesPerFile.filterKeys { files.contains(it) },
securityHotspotsPerFile.filterKeys { files.contains(it) })
issuesPerFile.filterKeys { it in files },
securityHotspotsPerFile.filterKeys { it in files })
}

fun merge(other: LiveFindings?): LiveFindings {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/sonarlint/intellij/finding/QuickFix.kt
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fun convert(
modificationStamp: Long?,
): QuickFix? {
val virtualFileEdits = coreQuickFix.fileEdits().map { convert(it, modificationStamp) }
if (virtualFileEdits.contains(null)) {
if (null in virtualFileEdits) {
SonarLintConsole.get(project).debug("Quick fix won't be proposed as it is invalid")
return null
}
Expand All @@ -61,7 +61,7 @@ private fun convert(fileEdit: FileEditDto, modificationStamp: Long?): VirtualFil
return null
}
val virtualFileEdits = fileEdit.textEdits().map { convert(document, it) }
if (virtualFileEdits.contains(null)) {
if (null in virtualFileEdits) {
return null
}
return VirtualFileEdit(virtualFile, virtualFileEdits.mapNotNull { it })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ class TaintVulnerabilitiesCache(val project: Project) {

fun update(taintVulnerabilityIdsToRemove: Set<UUID>, taintVulnerabilitiesToAdd: List<LocalTaintVulnerability>, taintVulnerabilitiesToUpdate: List<LocalTaintVulnerability>) {
val currentTaintVulnerabilities = taintVulnerabilities.toMutableList()
currentTaintVulnerabilities.removeAll { taintVulnerabilityIdsToRemove.contains(it.getId()) }
currentTaintVulnerabilities.removeAll { it.getId() in taintVulnerabilityIdsToRemove }
currentTaintVulnerabilities.addAll(taintVulnerabilitiesToAdd)
val updatedTaintVulnerabilityKeys = taintVulnerabilitiesToUpdate.map { updatedTaint -> updatedTaint.getServerKey() }
currentTaintVulnerabilities.removeAll { updatedTaintVulnerabilityKeys.contains(it.getServerKey()) }
currentTaintVulnerabilities.removeAll { it.getServerKey() in updatedTaintVulnerabilityKeys }
currentTaintVulnerabilities.addAll(taintVulnerabilitiesToUpdate)
taintVulnerabilities = currentTaintVulnerabilities
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ class PromotionProvider(private val project: Project) {
notifications: SonarLintProjectNotifications,
files: Collection<VirtualFile>,
) {
if (nonReportAnalysisTriggers.contains(triggerType)) {
if (triggerType in nonReportAnalysisTriggers) {
processAutoAnalysisTriggers(wasAutoAnalyzed, notifications)
}

if (reportAnalysisTriggers.contains(triggerType)) {
if (triggerType in reportAnalysisTriggers) {
processReportAnalysisTriggers(files, notifications)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ class AutomaticSharedConfigCreator(
private fun findFirstUniqueConnectionName(connectionNames: Set<String>, newConnectionName: String): String {
var suffix = 1
var uniqueName = newConnectionName
while (connectionNames.contains(uniqueName)) {
while (uniqueName in connectionNames) {
uniqueName = "$newConnectionName-$suffix"
suffix++
}
Expand Down
51 changes: 0 additions & 51 deletions src/main/java/org/sonarlint/intellij/trigger/TriggerType.java

This file was deleted.

46 changes: 46 additions & 0 deletions src/main/java/org/sonarlint/intellij/trigger/TriggerType.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* SonarLint for IntelliJ IDEA
* Copyright (C) 2015-2024 SonarSource
* [email protected]
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
*/
package org.sonarlint.intellij.trigger

enum class TriggerType(private val displayName: String, private val shouldUpdateServerIssues: Boolean) {

EDITOR_OPEN("Editor open", true),
CURRENT_FILE_ACTION("Current file action", true),
RIGHT_CLICK("Right click", true),
ALL("All files", false),
CHANGED_FILES("Changed files", false),
COMPILATION("Compilation", false),
EDITOR_CHANGE("Editor change", false),
CHECK_IN("Pre-commit check", true),
CONFIG_CHANGE("Config change", true),
BINDING_UPDATE("Binding update", true),
OPEN_FINDING("Open finding", true),
SERVER_SENT_EVENT("Server-sent event", false);

companion object {
// Forced analysis that will appear in the report tab
val forcedAnalysisSnapshot = listOf(RIGHT_CLICK, ALL, CHANGED_FILES, OPEN_FINDING)
}

fun getName() = displayName

fun isShouldUpdateServerIssues() = shouldUpdateServerIssues

}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class SonarLintDashboardPopup(private val editor: Editor) {

override fun mouseExited(event: MouseEvent) {
val point = event.point
if (!dashboard.panel.bounds.contains(point) || point.x == 0 || point.y == 0) {
if (point !in dashboard.panel.bounds || point.x == 0 || point.y == 0) {
insidePopup = false
if (canClose()) {
hidePopup()
Expand Down

0 comments on commit afde9b1

Please sign in to comment.