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 authored Oct 1, 2024
1 parent 280afb6 commit 980369b
Show file tree
Hide file tree
Showing 24 changed files with 220 additions and 93 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: 16 additions & 1 deletion 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,17 +36,19 @@ 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.analysisSnapshot
import org.sonarlint.intellij.trigger.TriggerType.Companion.nonAnalysisSnapshot
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


class AnalysisState(
val id: UUID,
private val analysisCallback: AnalysisCallback,
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,12 @@ class AnalysisState(
return hasReceivedFinalIssues && (!shouldReceiveHotspot || hasReceivedFinalHotspots)
}

// Analysis are redundant if both are snapshots (report tab) or both are not snapshots, otherwise they should not cancel each other
// 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 bothSnapshot = triggerType in analysisSnapshot && analysisState.triggerType in analysisSnapshot
val bothNonSnapshot = triggerType in nonAnalysisSnapshot && analysisState.triggerType in nonAnalysisSnapshot
return (bothSnapshot || bothNonSnapshot) && analysisState.filesToAnalyze.containsAll(filesToAnalyze)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

import static java.util.stream.Collectors.toSet;

class ModuleAnalysisResult {
public class ModuleAnalysisResult {

private final Set<VirtualFile> failedFiles;

Expand Down
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 @@ -36,6 +36,8 @@ import org.sonarlint.intellij.core.EnabledLanguages.extraEnabledLanguagesInConne
import org.sonarlint.intellij.messages.AnalysisListener
import org.sonarlint.intellij.notifications.SonarLintProjectNotifications
import org.sonarlint.intellij.trigger.TriggerType
import org.sonarlint.intellij.trigger.TriggerType.Companion.analysisSnapshot
import org.sonarlint.intellij.trigger.TriggerType.Companion.nonAnalysisSnapshot
import org.sonarsource.sonarlint.core.commons.api.SonarLanguage
import org.sonarsource.sonarlint.core.rpc.protocol.common.Language

Expand All @@ -50,16 +52,6 @@ class PromotionProvider(private val project: Project) {
Language.JAVA, Language.PYTHON, Language.PHP, Language.JS, Language.TS, Language.CS
)

private val reportAnalysisTriggers: Set<TriggerType> = EnumSet.of(
TriggerType.RIGHT_CLICK, TriggerType.CHANGED_FILES, TriggerType.ALL
)

private val nonReportAnalysisTriggers: Set<TriggerType> = EnumSet.of(
TriggerType.EDITOR_OPEN, TriggerType.BINDING_UPDATE,
TriggerType.SERVER_SENT_EVENT, TriggerType.CONFIG_CHANGE,
TriggerType.EDITOR_CHANGE, TriggerType.COMPILATION, TriggerType.CURRENT_FILE_ACTION
)

fun subscribeToTriggeringEvents() {
val busConnection = project.messageBus.connect()
with(busConnection) {
Expand Down Expand Up @@ -93,11 +85,11 @@ class PromotionProvider(private val project: Project) {
notifications: SonarLintProjectNotifications,
files: Collection<VirtualFile>,
) {
if (nonReportAnalysisTriggers.contains(triggerType)) {
if (triggerType in nonAnalysisSnapshot) {
processAutoAnalysisTriggers(wasAutoAnalyzed, notifications)
}

if (reportAnalysisTriggers.contains(triggerType)) {
if (triggerType in analysisSnapshot) {
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.

Loading

0 comments on commit 980369b

Please sign in to comment.