Skip to content

Commit

Permalink
SLLS-219 open issue in IDE without rule description
Browse files Browse the repository at this point in the history
  • Loading branch information
sophio-japharidze-sonarsource committed Mar 6, 2024
1 parent b5df83c commit a4cbb35
Show file tree
Hide file tree
Showing 10 changed files with 165 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import org.sonarsource.sonarlint.ls.util.Utils;

import static java.lang.String.format;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toSet;

/**
Expand Down Expand Up @@ -287,7 +288,20 @@ private void analyseNotIgnoredFiles(List<VersionedOpenFile> files) {
var notIgnoredFiles = files
.stream().filter(it -> notIgnoredFileUris.getFileUris().contains(it.getUri().toString()))
.toList();
analyzeAsync(AnalysisParams.newAnalysisParams(notIgnoredFiles));
var filesByMaybeFolderUri = notIgnoredFiles.stream().collect(groupingBy(f -> workspaceFoldersManager.findFolderForFile(f.getUri())));
for (var entry : filesByMaybeFolderUri.entrySet()) {
var maybeFolderUri = entry.getKey();
var filesToAnalyse = entry.getValue();
if (maybeFolderUri.isPresent()) {
var folderUri = maybeFolderUri.get().getUri();
if (workspaceFoldersManager.isReadyForAnalysis(folderUri.toString())) {
analyzeAsync(AnalysisParams.newAnalysisParams(filesToAnalyse));
}
}
if (bindingManager.getBindingIfExists(filesToAnalyse.get(0).getUri()).isEmpty()) {
analyzeAsync(AnalysisParams.newAnalysisParams(filesToAnalyse));
}
}
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,15 @@ public void showHotspot(String configurationScopeId, HotspotDetailsDto hotspotDe

@Override
public void showIssue(String folderUri, IssueDetailsDto issueDetails) {
var maybeBinding = bindingManager.getBinding(URI.create(folderUri));
maybeBinding.ifPresent(projectBindingWrapper ->
client.showIssue(new ShowAllLocationsCommand.Param(new ShowIssueParams(folderUri, issueDetails), projectBindingWrapper.getConnectionId())));
var maybeBinding = bindingManager.getBindingIfExists(URI.create(folderUri));
if (maybeBinding.isPresent()) {
logOutput.debug("Show issue with description");
var projectBinding = maybeBinding.get();
client.showIssue(new ShowAllLocationsCommand.Param(new ShowIssueParams(folderUri, issueDetails), projectBinding.getConnectionId(), true));
} else {
logOutput.debug("Show issue without description");
client.showIssue(new ShowAllLocationsCommand.Param(new ShowIssueParams(folderUri, issueDetails), null, false));
}
}

@Override
Expand Down Expand Up @@ -252,9 +258,10 @@ private HashMap<String, ServerConnectionSettings> getCurrentConnections(AssistCr

@Override
public AssistBindingResponse assistBinding(AssistBindingParams params, CancelChecker cancelChecker) {
workspaceFoldersManager.updateAnalysisReadiness(Set.of(params.getConfigScopeId()), false);
return client.assistBinding(params)
.thenCompose(response -> bindingManager.getUpdatedBindingForWorkspaceFolder(URI.create(response.getConfigurationScopeId())))
.thenApply(configurationScopeId -> {
.thenApply(response -> {
var configurationScopeId = response.getConfigurationScopeId();
var pathParts = configurationScopeId.split("/");
var projectName = pathParts[pathParts.length - 1];
client.showMessage(new MessageParams(MessageType.Info, "Project '" + projectName + "' was successfully bound to '" + params.getProjectKey() + "'."));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ private ShowAllLocationsCommand() {
public static class Param {
private final URI fileUri;
private final String message;
private final boolean shouldOpenRuleDescription;
private final String severity;
private final String ruleKey;
private final List<Flow> flows;
Expand All @@ -67,6 +68,7 @@ public static class Param {
private final TextRangeDto textRange;
private boolean codeMatches = false;


private Param(Issue issue) {
this.fileUri = nullableUri(issue.getInputFile());
this.message = issue.getMessage();
Expand All @@ -76,11 +78,13 @@ private Param(Issue issue) {
this.textRange = issue.getTextRange();
this.connectionId = null;
this.creationDate = null;
this.shouldOpenRuleDescription = true;
}

public Param(ShowIssueParams showIssueParams, String connectionId) {
public Param(ShowIssueParams showIssueParams, @Nullable String connectionId, boolean shouldOpenRuleDescription) {
this.fileUri = getFullFileUriFromFragments(showIssueParams.getConfigurationScopeId(), showIssueParams.getIssueDetails().getIdeFilePath());
this.message = showIssueParams.getIssueDetails().getMessage();
this.shouldOpenRuleDescription = shouldOpenRuleDescription;
this.severity = "";
this.ruleKey = showIssueParams.getIssueDetails().getRuleKey();
this.flows = showIssueParams.getIssueDetails().getFlows().stream().map(f -> new Flow(f, showIssueParams.getConfigurationScopeId())).toList();
Expand Down Expand Up @@ -118,6 +122,7 @@ public Param(ShowIssueParams showIssueParams, String connectionId) {
this.textRange = textRangeWithHashDtoToTextRangeDto(taint.getTextRange());
this.connectionId = connectionId;
this.creationDate = DateTimeFormatter.ISO_DATE_TIME.format(taint.getIntroductionDate().atOffset(ZoneOffset.UTC));
this.shouldOpenRuleDescription = true;
}

public URI getFileUri() {
Expand Down Expand Up @@ -157,6 +162,11 @@ public TextRangeDto getTextRange() {
public boolean getCodeMatches() {
return codeMatches;
}

public boolean isShouldOpenRuleDescription() {
return shouldOpenRuleDescription;
}

}

static class Flow {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,10 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Predicate;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -137,15 +134,38 @@ public Optional<ProjectBinding> getBinding(WorkspaceFolderWrapper folder) {
* @return empty if the file is unbound
*/
public Optional<ProjectBinding> getBinding(URI fileUri) {
if (!isUriValidAndNotNotebook(fileUri)) return Optional.empty();
var folder = foldersManager.findFolderForFile(fileUri);
var cacheKey = folder.map(WorkspaceFolderWrapper::getUri).orElse(fileUri);
return getBinding(folder, cacheKey);
}

private boolean isUriValidAndNotNotebook(URI fileUri) {
if (!uriHasFileScheme(fileUri) || openNotebooksCache.isNotebook(fileUri)) {
if (globalLogOutput != null) {
globalLogOutput.log("Ignoring connected mode settings for unsupported URI: " + fileUri, ClientLogOutput.Level.DEBUG);
}
return Optional.empty();
return false;
}
return true;
}

/**
* Return the binding of the given file.
*
* @return empty if the file is unbound
*/
public Optional<ProjectBinding> getBindingIfExists(URI fileUri) {
if (!isUriValidAndNotNotebook(fileUri)) return Optional.empty();
var folder = foldersManager.findFolderForFile(fileUri);
var cacheKey = folder.map(WorkspaceFolderWrapper::getUri).orElse(fileUri);
return getBinding(folder, cacheKey);
return getBindingIfExists(folder, cacheKey);
}

private Optional<ProjectBinding> getBindingIfExists(Optional<WorkspaceFolderWrapper> folder, URI fileUri) {
return folder.isPresent()
? folderBindingCache.getOrDefault(folder.get().getUri(), Optional.empty())
: fileBindingCache.getOrDefault(fileUri, Optional.empty());
}

private Optional<ProjectBinding> getBinding(Optional<WorkspaceFolderWrapper> folder, URI fileUri) {
Expand All @@ -162,33 +182,6 @@ private Optional<ProjectBinding> getBinding(Optional<WorkspaceFolderWrapper> fol
});
}

public CompletableFuture<String> getUpdatedBindingForWorkspaceFolder(URI folderUri) {
var bindingUpdatedLatch = new CountDownLatch(1);
var updatedBinding = new CompletableFuture<String>();
bindingUpdateQueue.put(folderUri, bindingUpdatedLatch);
Executors.newSingleThreadExecutor().submit(() -> {
var actuallyUpdated = false;
try {
actuallyUpdated = bindingUpdatedLatch.await(10, TimeUnit.SECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new IllegalStateException("Interrupted", e);
}
if (actuallyUpdated) {
getBinding(folderUri);
updatedBinding.complete(folderUri.toString());
} else {
updatedBinding.completeExceptionally(new IllegalStateException(String.format("Expected binding update for %s did not happen", folderUri)));
}
});
return updatedBinding;
}


private Optional<ProjectBinding> getBindingAndRepublishTaints(Optional<WorkspaceFolderWrapper> folder, URI fileUri) {
return getBinding(folder, fileUri);
}

@CheckForNull
private ProjectBinding computeProjectBinding(WorkspaceFolderSettings settings, Path folderRoot) {
var connectionId = requireNonNull(settings.getConnectionId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,12 @@ public TaintIssuesUpdater(ProjectBindingManager bindingManager, TaintVulnerabili
}

public void updateTaintIssuesAsync(URI fileUri) {
asyncExecutor.submit(() -> updateTaintIssues(fileUri));
workspaceFoldersManager.findFolderForFile(fileUri)
.ifPresent(workspaceFolderWrapper -> {
if (workspaceFoldersManager.isReadyForAnalysis(workspaceFolderWrapper.getUri().toString())) {
asyncExecutor.submit(() -> updateTaintIssues(fileUri));
}
});
}

private void updateTaintIssues(URI fileUri) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* SonarLint Language Server
* Copyright (C) 2009-2024 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* 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 02110-1301, USA.
*/
@ParametersAreNonnullByDefault
package org.sonarsource.sonarlint.ls.domain;

import javax.annotation.ParametersAreNonnullByDefault;
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ public void shutdown() {
}

public void updateAnalysisReadiness(Set<String> configurationScopeIds, boolean areReadyForAnalysis) {
configurationScopeIds.forEach(s -> logOutput.debug("Analysis readiness changed for config scope " + s + " to " + areReadyForAnalysis));
configurationScopeIds.forEach(folderUri -> analysisReadiness.put(folderUri, areReadyForAnalysis));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
Expand Down Expand Up @@ -390,7 +392,9 @@ void assistBindingShouldCallClientMethod() {
var assistBindingParams = new AssistBindingParams("connectionId", projectKey, configScopeId);
when(client.assistBinding(any())).thenReturn(
CompletableFuture.completedFuture(new SonarLintExtendedLanguageClient.AssistBindingResponse("folderUri")));
when(bindingManager.getUpdatedBindingForWorkspaceFolder(URI.create(configScopeId))).thenReturn(CompletableFuture.completedFuture(configScopeId));
var workspaceFoldersManager = mock(WorkspaceFoldersManager.class);
underTest.setWorkspaceFoldersManager(workspaceFoldersManager);

underTest.assistBinding(assistBindingParams, null);

verify(client).showMessage(new MessageParams(MessageType.Info, "Project '" + configScopeId + "' was successfully bound to '" + projectKey + "'."));
Expand Down Expand Up @@ -489,20 +493,42 @@ void shouldForwardOpenIssueRequest() {
}

@Test
void shouldNotForwardOpenIssueRequestWhenBindingDoesNotExist() {
void shouldForwardOpenIssueRequestWithoutRuleDescriptionWhenBindingDoesNotExist() {
var fileUri = fileInAWorkspaceFolderPath.toUri();
var textRangeDto = new TextRangeDto(1, 2, 3, 4);
var issueDetailsDto = new IssueDetailsDto(textRangeDto, "rule:S1234",
"issueKey", FILE_PYTHON, "bb", null, "this is wrong", "29.09.2023", "print('ddd')",
false, List.of());

when(bindingManager.getBinding(fileUri))
when(bindingManager.getBindingIfExists(fileUri))
.thenReturn(Optional.empty());

underTest.showIssue(fileUri.toString(), issueDetailsDto);
verify(client, never()).showIssue(any(ShowAllLocationsCommand.Param.class));
verify(client).showIssue(paramCaptor.capture());

var showAllLocationParams = paramCaptor.getValue();

assertFalse(showAllLocationParams.isShouldOpenRuleDescription());
}

@Test
void shouldForwardOpenIssueRequestWithRuleDescriptionWhenBindingDoesExist() {
var fileUri = fileInAWorkspaceFolderPath.toUri();
var textRangeDto = new TextRangeDto(1, 2, 3, 4);
var issueDetailsDto = new IssueDetailsDto(textRangeDto, "rule:S1234",
"issueKey", FILE_PYTHON, "bb", null, "this is wrong", "29.09.2023", "print('ddd')",
false, List.of());
when(bindingManager.getBindingIfExists(fileUri))
.thenReturn(Optional.of(new ProjectBinding("connectionId", "projectKey", null, null)));

underTest.showIssue(fileUri.toString(), issueDetailsDto);
verify(client).showIssue(paramCaptor.capture());

var showAllLocationParams = paramCaptor.getValue();

assertTrue(showAllLocationParams.isShouldOpenRuleDescription());
}


@Test
void shouldLogMessagesWithLogLevel() {
underTest.log(new LogParams(LogLevel.ERROR, null, null, null, Instant.now()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ void shouldBuildCommandParamsFromShowIssueParams() {
"issueKey", Path.of("myFile.py"), "branch", "pr", "this is wrong",
"29.09.2023", "print('1234')", false, flows));

var result = new ShowAllLocationsCommand.Param(showIssueParams, "connectionId");
var result = new ShowAllLocationsCommand.Param(showIssueParams, "connectionId", true);

assertTrue(result.getCodeMatches());
assertThat(result.getFileUri()).hasToString(workspaceFolderPath.toUri() + "myFile.py");
Expand All @@ -162,7 +162,7 @@ void shouldBuildCommandParamsFromShowIssueParamsForFileLevelIssue() {
print('b')
print('kkkk')""", false, List.of()));

var result = new ShowAllLocationsCommand.Param(showIssueParams, "connectionId");
var result = new ShowAllLocationsCommand.Param(showIssueParams, "connectionId", true);

assertTrue(result.getCodeMatches());
}
Expand All @@ -174,7 +174,7 @@ void shouldBuildCommandParamsFromShowIssueParamsForInvalidTextRange() {
"issueKey", Path.of("myFile.py"), "bb", "1234", "this is wrong",
"29.09.2023", "print('1234')", false, List.of()));

var result = new ShowAllLocationsCommand.Param(showIssueParams, "connectionId");
var result = new ShowAllLocationsCommand.Param(showIssueParams, "connectionId", true);

assertFalse(result.getCodeMatches());
}
Expand Down
Loading

0 comments on commit a4cbb35

Please sign in to comment.