Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SLE-1000: Embed CFamily analyzer #781

Merged
merged 4 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.Set;
Expand All @@ -47,6 +47,7 @@
import org.sonarlint.eclipse.core.analysis.IPreAnalysisContext;
import org.sonarlint.eclipse.core.analysis.SonarLintLanguage;
import org.sonarlint.eclipse.core.internal.jobs.DefaultPreAnalysisContext;
import org.sonarlint.eclipse.core.internal.utils.SonarLintUtils;
import org.sonarlint.eclipse.core.resource.ISonarLintFile;
import org.sonarlint.eclipse.core.resource.ISonarLintProject;

Expand Down Expand Up @@ -90,8 +91,34 @@ public void configure(IPreAnalysisContext context, IProgressMonitor monitor) {
.filter(f -> f.getResource() instanceof IFile && fileValidator.test((IFile) f.getResource()))
.collect(Collectors.toList());

// This contains the files that will be later removed from the analysis because of the information not being
// available from CDT but being crucial for the CFamily analyzer to NOT fail the whole analysis.
var removedFilesFromAnalysis = new ArrayList<ISonarLintFile>();

try {
var configuredFiles = configureCProject(context, context.getProject(), filesToAnalyze);
var configuredFiles = configureCProject(context, context.getProject(), filesToAnalyze, removedFilesFromAnalysis);

// We have to provide this information back to the analysis when there are files that were removed by the CDT
// integration. Due to this relying on extension points, this can only be done via an internal property.
if (!removedFilesFromAnalysis.isEmpty()) {
var excludedFilesPerUri = removedFilesFromAnalysis.stream()
.map(slFile -> slFile.getResource().getLocationURI().toString())
.collect(Collectors.toList());

context.setAnalysisProperty(SonarLintUtils.SONARLINT_ANALYSIS_CDT_EXCLUSION_PROPERY, excludedFilesPerUri);

// The actual user facing notification is sent from the CORE bundle as this sub-plug-in has no access to the
// notification framework.
logger.debug("The following C/C++ files were excluded by the CDT integration as no information could be "
+ "accessed from the Eclipse CDT plug-in: " + String.join(",", excludedFilesPerUri));
}

// When there is no configured file, we should inform the user that the build info was not written to any file
if (configuredFiles.isEmpty()) {
logger.debug("Wrote no build info as there is no file information available for the build wrapper.");
return;
}

var jsonPath = writeJson(context, context.getProject(), configuredFiles);
logger.debug("Wrote build info to: " + jsonPath.toString());
context.setAnalysisProperty(CFAMILY_USE_CACHE, Boolean.FALSE.toString());
Expand All @@ -101,7 +128,8 @@ public void configure(IPreAnalysisContext context, IProgressMonitor monitor) {
}
}

private Collection<ConfiguredFile> configureCProject(IPreAnalysisContext context, ISonarLintProject project, Collection<ISonarLintFile> filesToAnalyze) {
private Collection<ConfiguredFile> configureCProject(IPreAnalysisContext context, ISonarLintProject project,
Collection<ISonarLintFile> filesToAnalyze, Collection<ISonarLintFile> removedFilesFromAnalysis) {
var files = new LinkedList<ConfiguredFile>();
var infoProvider = cCorePlugin.getScannerInfoProvider((IProject) project.getResource());

Expand All @@ -111,14 +139,26 @@ private Collection<ConfiguredFile> configureCProject(IPreAnalysisContext context
var path = ((DefaultPreAnalysisContext) context).getLocalPath(file);
var fileInfo = infoProvider.getScannerInformation(file.getResource());

builder.includes(fileInfo.getIncludePaths() != null ? fileInfo.getIncludePaths() : new String[0])
.symbols(fileInfo.getDefinedSymbols() != null ? fileInfo.getDefinedSymbols() : Collections.emptyMap())
// We cannot work on this file when:
// - fileInfo is null which means there was no information (yet) available to CDT about the file
// - fileInfo.getDefinedSymbols() is null or empty as the requirement is to have at least one define directive
// - this can happen when the project is not configured correctly or CDT is not yet ready
// In this case we have to remove it as otherwise the whole analysis will fail!
if (fileInfo == null
|| fileInfo.getDefinedSymbols() == null
|| fileInfo.getDefinedSymbols().isEmpty()) {
removedFilesFromAnalysis.add(file);
continue;
}

builder.includes(fileInfo.getIncludePaths() == null ? new String[0] : fileInfo.getIncludePaths())
.symbols(fileInfo.getDefinedSymbols())
.path(path);

files.add(builder.build());
}
return files;

return files;
}

private Path writeJson(IPreAnalysisContext context, ISonarLintProject project, Collection<ConfiguredFile> files) throws IOException {
Expand Down Expand Up @@ -163,6 +203,11 @@ private SonarLintLanguage getFileLanguage(IProject project, IFile file) {
private static Path createJsonFile(Path workDir, String content) throws IOException {
var jsonFilePath = workDir.resolve(BUILD_WRAPPER_OUTPUT_FILENAME);
Files.createDirectories(workDir);

// In order to be able to trace the build wrapper output content for
SonarLintLogger.get().debug("CDT generated build wrapper output '" + jsonFilePath
+ "' for C/C++ analysis written containing: " + content);

Files.write(jsonFilePath, content.getBytes(BUILD_WRAPPER_OUTPUT_CHARSET));
return jsonFilePath;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,8 @@
import org.sonarlint.eclipse.core.internal.resources.DefaultSonarLintProjectAdapter;
import org.sonarlint.eclipse.core.resource.ISonarLintFile;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.anyCollection;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
Expand All @@ -69,7 +67,7 @@ public void setUp() {
}

@Test
public void should_configure() throws Exception {
public void should_configure_no_build_wrapper_output() throws Exception {
var projectBaseDir = temp.newFolder().toPath();
var project = mock(IProject.class);
var file = mock(IFile.class);
Expand All @@ -94,15 +92,12 @@ public void should_configure() throws Exception {

configurator.configure(context, monitor);

// json created
verify(jsonFactory).create(anyCollection(), eq(projectBaseDir.toRealPath().toString()));
// build-wrapper-dump.json was not created as no useful file was provided to the CDT integraton
verify(jsonFactory, never()).create(ArgumentMatchers.any(), ArgumentMatchers.any());

// json written
assertThat(temp.getRoot().toPath().resolve("build-wrapper-dump.json")).hasContent("json");

// property created
verify(context).setAnalysisProperty("sonar.cfamily.build-wrapper-output", temp.getRoot().toPath().toString());
verify(context).setAnalysisProperty("sonar.cfamily.useCache", "false");
// properties were not set
verify(context, never()).setAnalysisProperty("sonar.cfamily.build-wrapper-output", temp.getRoot().toPath().toString());
verify(context, never()).setAnalysisProperty("sonar.cfamily.useCache", "false");

// no errors
verify(logger, never()).error(ArgumentMatchers.any(), ArgumentMatchers.any());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,17 @@ public static class Notification {
private final String title;
private final String shortMsg;
private final String longMsg;
private final String learnMoreUrl;

public Notification(String title, String shortMsg, @Nullable String longMsg) {
this(title, shortMsg, longMsg, null);
}

public Notification(String title, String shortMsg, @Nullable String longMsg, @Nullable String learnMoreUrl) {
this.title = title;
this.shortMsg = shortMsg;
this.longMsg = longMsg;
this.learnMoreUrl = learnMoreUrl;
}

public String getTitle() {
Expand All @@ -89,6 +95,11 @@ public String getLongMsg() {
return longMsg;
}

@Nullable
public String getLearnMoreUrl() {
return learnMoreUrl;
}

@Override
public String toString() {
return String.format("Notification{title=%s,shortMsg=%s,longMsg=%s}", title, shortMsg, longMsg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ public class SonarLintDocumentation {
public static final String ECLIPSE_M2E = "https://projects.eclipse.org/projects/technology.m2e";
public static final String ECLIPSE_BUILDSHIP = "https://projects.eclipse.org/projects/tools.buildship";

// Eclipse CDT documentation
public static final String ECLIPSE_CDT_DOCS = "https://help.eclipse.org/latest/index.jsp?topic=%2Forg.eclipse.cdt.doc.user%2Fconcepts%2Fcdt_o_home.htm";

private SonarLintDocumentation() {
// utility class
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,12 @@
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jface.text.IDocument;
import org.sonarlint.eclipse.core.SonarLintLogger;
import org.sonarlint.eclipse.core.SonarLintNotifications;
import org.sonarlint.eclipse.core.analysis.IAnalysisConfigurator;
import org.sonarlint.eclipse.core.analysis.IPostAnalysisContext;
import org.sonarlint.eclipse.core.configurator.ProjectConfigurationRequest;
import org.sonarlint.eclipse.core.configurator.ProjectConfigurator;
import org.sonarlint.eclipse.core.documentation.SonarLintDocumentation;
import org.sonarlint.eclipse.core.internal.SonarLintCorePlugin;
import org.sonarlint.eclipse.core.internal.TriggerType;
import org.sonarlint.eclipse.core.internal.backend.ConfigScopeSynchronizer;
Expand All @@ -58,6 +60,8 @@
import org.sonarlint.eclipse.core.internal.utils.FileExclusionsChecker;
import org.sonarlint.eclipse.core.internal.utils.FileUtils;
import org.sonarlint.eclipse.core.internal.utils.JobUtils;
import org.sonarlint.eclipse.core.internal.utils.SonarLintUtils;
import org.sonarlint.eclipse.core.internal.utils.StringUtils;
import org.sonarlint.eclipse.core.resource.ISonarLintFile;
import org.sonarlint.eclipse.core.resource.ISonarLintProject;
import org.sonarsource.sonarlint.core.commons.api.progress.CanceledException;
Expand Down Expand Up @@ -150,6 +154,29 @@ protected IStatus doRun(final IProgressMonitor monitor) {
ResourcesPlugin.getWorkspace().run(m -> SonarLintMarkerUpdater.deleteAllMarkersFromReport(), monitor);
}

// This is for working with the CDT integration and the CFamily analysis: It can be that files have to be
// removed from the analysis because no necessary information could be gathered and otherwise the analysis will
// fail as a whole.
var removedFilesByCdt = mergedExtraProps.get(SonarLintUtils.SONARLINT_ANALYSIS_CDT_EXCLUSION_PROPERY);
if (removedFilesByCdt != null) {
SonarLintNotifications.get()
.showNotification(new SonarLintNotifications.Notification(
"C/C++ analysis not available (yet)",
"Some C/C++ files were removed from the analysis by the CDT integration as no information was available "
+ "from the Eclipse CDT plug-in. This might happen when the project was not yet built or the project is "
+ "not configured correctly.",
"The following files were removed from the analysis: '" + removedFilesByCdt + "'. The information "
+ "provided by Eclipse CDT is crucial for the analysis to work correctly, please consult the offcial"
thahnen marked this conversation as resolved.
Show resolved Hide resolved
+ "documentation at: " + SonarLintDocumentation.ECLIPSE_CDT_DOCS,
SonarLintDocumentation.ECLIPSE_CDT_DOCS));

for (var uri : StringUtils.splitFromCommaString(removedFilesByCdt)) {
inputFiles = inputFiles.stream()
.filter(eclipseFile -> !eclipseFile.getFile().getResource().getLocationURI().toString().equals(uri))
.collect(Collectors.toList());
}
}

if (!inputFiles.isEmpty()) {
var start = System.currentTimeMillis();
var result = run(filesToAnalyzeMap.keySet(), mergedExtraProps, start, monitor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,16 @@
import org.sonarsource.sonarlint.core.rpc.protocol.common.Language;

public class SonarLintUtils {
/**
* This internal property is used between the CDT integration sub-plug-in and the CORE bundle as the communication
* is only one-way and happening via the extension point:
*
* When CDT ain't yet ready for a file, it cannot provide values for the `build-wrapper-dump.json` that is crucial
* for the CFamily analysis. This can happen in corner cases like creating a project from within the IDE with a
* source code file already present -> then the CDT integration for that project is not yet ready.
*/
public static final String SONARLINT_ANALYSIS_CDT_EXCLUSION_PROPERY = "sonarlint.internal.analysis.cdt.exclusion";

/**
* Enabled languages should be consistent with https://www.sonarsource.com/products/sonarlint/features/eclipse!
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collection;
import java.util.Objects;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -88,6 +89,10 @@ public static String joinWithCommaSkipNull(Collection<String> values) {
.collect(joining(COMMA_SEPARATOR));
}

public static Collection<String> splitFromCommaString(String list) {
return Arrays.asList(list.split(COMMA_SEPARATOR));
}

private static String csvEscape(String string) {
// escape only when needed
return string.contains(",") ? ("\"" + string + "\"") : string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ private static class PopupNotification implements NotificationListener {
@Override
public void showNotification(Notification notif) {
Display.getDefault().asyncExec(() -> {
var popup = new GenericNotificationPopup(notif.getTitle(), notif.getShortMsg(), notif.getLongMsg());
var popup = new GenericNotificationPopup(notif.getTitle(), notif.getShortMsg(), notif.getLongMsg(),
notif.getLearnMoreUrl());
popup.open();
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,14 @@ public class GenericNotificationPopup extends AbstractSonarLintPopup {
private final String title;
private final String shortMsg;
private final String longMsg;
private final String learnMoreUrl;

public GenericNotificationPopup(String title, String shortMsg, @Nullable String longMsg) {
public GenericNotificationPopup(String title, String shortMsg, @Nullable String longMsg,
@Nullable String learnMoreUrl) {
this.title = title;
this.shortMsg = shortMsg;
this.longMsg = longMsg;
this.learnMoreUrl = learnMoreUrl;
}

@Override
Expand All @@ -60,12 +63,16 @@ protected void createContentArea(Composite composite) {

addLink("Dismiss", e -> close());

if (learnMoreUrl != null) {
addLink("Learn more",
e -> BrowserUtils.openExternalBrowser(learnMoreUrl, PlatformUI.getWorkbench().getDisplay()));
}

if (longMsg != null) {
addLink("More details...", e -> {
var dialog = new DialogWithLink(PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(),
"SonarQube - " + title, longMsg);
dialog.open();
close();
});
}
}
Expand Down
Loading