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

SCANMAVEN-224 Log a warning message when the version of the scanner is not specified #228

Merged
merged 7 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
49 changes: 49 additions & 0 deletions src/main/java/org/sonarsource/scanner/maven/SonarQubeMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,16 @@
*/
package org.sonarsource.scanner.maven;

import com.google.common.annotations.VisibleForTesting;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.stream.Stream;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.lifecycle.LifecycleExecutor;
import org.apache.maven.model.Plugin;
import org.apache.maven.model.PluginManagement;
import org.apache.maven.plugin.AbstractMojo;
import org.apache.maven.plugin.MojoExecution;
import org.apache.maven.plugin.MojoExecutionException;
Expand Down Expand Up @@ -88,6 +93,8 @@ public void execute() throws MojoExecutionException, MojoFailureException {
return;
}

warnAboutUnspecifiedSonarPluginVersion();

Properties envProps = Utils.loadEnvironmentProperties(System.getenv());

MavenCompilerResolver mavenCompilerResolver = new MavenCompilerResolver(session, lifecycleExecutor, getLog(), toolchainManager);
Expand All @@ -106,6 +113,44 @@ public void execute() throws MojoExecutionException, MojoFailureException {
new ScannerBootstrapper(getLog(), session, runner, mavenProjectConverter, propertyDecryptor).execute();
}

private void warnAboutUnspecifiedSonarPluginVersion() {
String effectivePluginVersion = mojoExecution.getVersion();
String groupId = mojoExecution.getGroupId();
String artifactId = mojoExecution.getArtifactId();
Plugin plugin = mojoExecution.getPlugin();
MavenProject project = session.getTopLevelProject();
List<String> goals = session.getGoals();
boolean requiredArgumentsAreNotNull = !Arrays.asList(effectivePluginVersion, groupId, artifactId, plugin, project, goals).contains(null);
if (requiredArgumentsAreNotNull) {
String invalidPluginVersion = null;
String configuredPluginVersion = plugin.getVersion();
if ("LATEST".equals(configuredPluginVersion) || "RELEASE".equals(configuredPluginVersion)) {
invalidPluginVersion = configuredPluginVersion;
} else if (!isPluginVersionDefinedInTheProject(project, groupId, artifactId) && isVersionMissingFromSonarGoal(goals, groupId, artifactId)) {
invalidPluginVersion = "an unspecified version";
}
if (invalidPluginVersion != null) {
getLog().warn(String.format("Using %s instead of an explicit plugin version may introduce breaking analysis changes at an unwanted time. " +
"It is highly recommended to use an explicit version, e.g. '%s:%s:%s'.", invalidPluginVersion, groupId, artifactId, effectivePluginVersion));
}
}
}

@VisibleForTesting
static boolean isPluginVersionDefinedInTheProject(MavenProject project, String groupId, String artifactId) {
Stream<Plugin> pluginStream = project.getBuildPlugins().stream();
PluginManagement pluginManagement = project.getPluginManagement();
pluginStream = pluginManagement != null ? Stream.concat(pluginStream, pluginManagement.getPlugins().stream()) : pluginStream;
return pluginStream.anyMatch(plugin -> (plugin.getGroupId() == null || groupId.equals(plugin.getGroupId())) &&
artifactId.equals(plugin.getArtifactId()) &&
(plugin.getVersion() != null && !plugin.getVersion().isBlank()));
}

private static boolean isVersionMissingFromSonarGoal(List<String> goals, String groupId, String artifactId) {
List<String> sonarGoalsWithoutVersion = Arrays.asList("sonar:sonar", groupId + ":" + artifactId + ":sonar");
return goals.stream().anyMatch(sonarGoalsWithoutVersion::contains);
}

/**
* Should scanner be delayed?
* @return true if goal is attached to phase and not last in a multi-module project
Expand Down Expand Up @@ -163,4 +208,8 @@ private boolean isSkip(Map<String, String> properties) {
MavenSession getSession() {
return session;
}

MojoExecution getMojoExecution() {
return mojoExecution;
}
}
172 changes: 150 additions & 22 deletions src/test/java/org/sonarsource/scanner/maven/SonarQubeMojoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,45 +23,49 @@
import java.io.FileInputStream;
import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.apache.commons.io.IOUtils;
import org.apache.maven.execution.ProjectDependencyGraph;
import org.apache.maven.graph.GraphBuilder;
import org.apache.maven.model.Plugin;
import org.apache.maven.model.PluginManagement;
import org.apache.maven.model.building.Result;
import org.apache.maven.plugin.logging.Log;
import org.apache.maven.plugin.descriptor.PluginDescriptor;
import org.apache.maven.plugin.testing.MojoRule;
import org.apache.maven.project.MavenProject;
import org.assertj.core.data.MapEntry;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.sonarsource.scanner.maven.TimestampLoggerTest.TestLog;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.entry;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class SonarQubeMojoTest {

private static final String UNSPECIFIED_VERSION_WARNING_SUFFIX = "instead of an explicit plugin version may introduce breaking analysis changes at an unwanted time. " +
"It is highly recommended to use an explicit version, e.g. 'org.sonarsource.scanner.maven:sonar-maven-plugin:";

private static final String DEFAULT_GOAL = "org.sonarsource.scanner.maven:sonar-maven-plugin:sonar";

@Rule
public MojoRule mojoRule = new MojoRule();

private Log mockedLogger;
private TestLog logger = new TestLog(TestLog.LogLevel.WARN);

private SonarQubeMojo getMojo(File baseDir) throws Exception {
return (SonarQubeMojo) mojoRule.lookupConfiguredMojo(baseDir, "sonar");
}

@Before
public void setUpMocks() {
mockedLogger = mock(Log.class);
}

@Test
public void executeMojo() throws Exception {
executeProject("sample-project");
Expand All @@ -75,7 +79,7 @@ public void executeMojo() throws Exception {
public void should_skip() throws Exception {
File propsFile = new File("target/dump.properties");
propsFile.delete();
executeProject("sample-project", "sonar.scanner.skip", "true");
executeProject("sample-project", DEFAULT_GOAL, "sonar.scanner.skip", "true");
assertThat(propsFile).doesNotExist();
}

Expand Down Expand Up @@ -108,6 +112,7 @@ public void shouldExportOverridenWarWebSource() throws Exception {
public void project_with_java_files_not_in_src_should_not_be_collected() throws Exception {
File baseDir = executeProject(
"project-with-java-files-not-in-src",
DEFAULT_GOAL,
"sonar.maven.scanAll", "true");
Set<String> actualListOfSources = extractSonarSources("target/dump.properties", baseDir.toPath());
assertThat(actualListOfSources).containsExactlyInAnyOrder(
Expand All @@ -118,6 +123,7 @@ public void project_with_java_files_not_in_src_should_not_be_collected() throws
public void project_with_java_files_not_in_src_should_be_collected_when_user_define_binaries_and_libraries() throws Exception {
File baseDir = executeProject(
"project-with-java-files-not-in-src",
DEFAULT_GOAL,
"sonar.maven.scanAll", "true",
"sonar.java.binaries", "target/classes",
"sonar.java.libraries", "target/lib/logger.jar");
Expand All @@ -130,6 +136,7 @@ public void project_with_java_files_not_in_src_should_be_collected_when_user_def
public void project_with_java_files_not_in_src_should_not_be_collected_when_user_define_only_binaries() throws Exception {
File baseDir = executeProject(
"project-with-java-files-not-in-src",
DEFAULT_GOAL,
"sonar.maven.scanAll", "true",
"sonar.java.binaries", "target/classes");
Set<String> actualListOfSources = extractSonarSources("target/dump.properties", baseDir.toPath());
Expand All @@ -141,6 +148,7 @@ public void project_with_java_files_not_in_src_should_not_be_collected_when_user
public void project_with_java_files_not_in_src_should_not_be_collected_when_user_define_only_libraries() throws Exception {
File baseDir = executeProject(
"project-with-java-files-not-in-src",
DEFAULT_GOAL,
"sonar.maven.scanAll", "true",
"sonar.java.libraries", "target/lib/logger.jar");
Set<String> actualListOfSources = extractSonarSources("target/dump.properties", baseDir.toPath());
Expand All @@ -166,49 +174,135 @@ public void shouldExportSurefireCustomReportsPath() throws Exception {
}

@Test
public void reuse_findbugs_exclusions_from_reporting() throws IOException, Exception {
public void reuse_findbugs_exclusions_from_reporting() throws Exception {
File baseDir = executeProject("project-with-findbugs-reporting");
assertPropsContains(entry("sonar.findbugs.excludeFilters", new File(baseDir, "findbugs-exclude.xml").getAbsolutePath()));
}

@Test
public void exclude_report_paths_from_scanAll() throws Exception {
File projectBarDir = executeProject("project-with-external-reports", "sonar.maven.scanAll", "true");
File projectBarDir = executeProject("project-with-external-reports", DEFAULT_GOAL, "sonar.maven.scanAll", "true");
Set<String> actualListOfSources = extractSonarSources("target/dump.properties", projectBarDir.toPath());
assertThat(actualListOfSources).containsExactlyInAnyOrder("/other.xml", "/pom.xml");
}

@Test
public void reuse_findbugs_exclusions_from_plugin() throws IOException, Exception {
public void reuse_findbugs_exclusions_from_plugin() throws Exception {
File baseDir = executeProject("project-with-findbugs-build");
assertPropsContains(entry("sonar.findbugs.excludeFilters", new File(baseDir, "findbugs-exclude.xml").getAbsolutePath()));
}

@Test
public void reuse_findbugs_exclusions_from_plugin_management() throws IOException, Exception {
public void reuse_findbugs_exclusions_from_plugin_management() throws Exception {
File baseDir = executeProject("project-with-findbugs-plugin-management");
assertPropsContains(entry("sonar.findbugs.excludeFilters", new File(baseDir, "findbugs-exclude.xml").getAbsolutePath()));
}

@Test
public void sonar_maven_plugin_version_not_set_should_display_a_warning() throws Exception {
executeProject("sample-project", "sonar:sonar");
assertThat(logger.logs).anyMatch(log -> log.contains("Using an unspecified version " + UNSPECIFIED_VERSION_WARNING_SUFFIX));
}

@Test
public void sonar_maven_plugin_version_LATEST_in_goal_should_display_a_warning() throws Exception {
executeProject("sample-project", "sonar:LATEST:sonar");
assertThat(logger.logs).anyMatch(log -> log.contains("Using LATEST " + UNSPECIFIED_VERSION_WARNING_SUFFIX));
}

@Test
public void sonar_maven_plugin_version_RELEASE_in_goal_should_display_a_warning() throws Exception {
executeProject("sample-project", "org.sonarsource.scanner.maven:sonar-maven-plugin:RELEASE:sonar");
assertThat(logger.logs).anyMatch(log -> log.contains("Using RELEASE " + UNSPECIFIED_VERSION_WARNING_SUFFIX));
}

@Test
public void sonar_maven_plugin_version_set_in_goal_should_not_display_a_warning() throws Exception {
executeProject("sample-project", "org.sonarsource.scanner.maven:sonar-maven-plugin:1.2.3.4:sonar");
assertThat(logger.logs).noneMatch(log -> log.contains(UNSPECIFIED_VERSION_WARNING_SUFFIX));
}

@Test
public void sonar_maven_plugin_version_set_in_sonar_sonar_goal_should_not_display_a_warning() throws Exception {
executeProject("sample-project", "sonar:1.2.3.4:sonar");
assertThat(logger.logs).noneMatch(log -> log.contains(UNSPECIFIED_VERSION_WARNING_SUFFIX));
}

@Test
public void sonar_maven_plugin_version_set_in_plugin_management_should_not_display_a_warning() throws Exception {
executeProject("project-with-sonar-plugin-management-configuration", "org.sonarsource.scanner.maven:sonar-maven-plugin:sonar");
assertThat(logger.logs).noneMatch(log -> log.contains(UNSPECIFIED_VERSION_WARNING_SUFFIX));
}

@Test
public void sonar_maven_plugin_version_set_in_build_plugins_should_not_display_a_warning() throws Exception {
executeProject("project-with-sonar-plugin-configuration", "org.sonarsource.scanner.maven:sonar-maven-plugin:sonar");
assertThat(logger.logs).noneMatch(log -> log.contains(UNSPECIFIED_VERSION_WARNING_SUFFIX));
}

@Test
public void cover_corner_cases_for_isPluginVersionDefinedInTheProject() {
MavenProject project = new MavenProject();
assertThat(SonarQubeMojo.isPluginVersionDefinedInTheProject(project, "org.sonarsource.scanner.maven", "sonar-maven-plugin")).isFalse();

Plugin pluginDefinition = new Plugin();
project.getBuild().getPlugins().add(pluginDefinition);

pluginDefinition.setGroupId(null);
pluginDefinition.setArtifactId("sonar-maven-plugin");
pluginDefinition.setVersion(null);
assertThat(SonarQubeMojo.isPluginVersionDefinedInTheProject(project, "org.sonarsource.scanner.maven", "sonar-maven-plugin")).isFalse();

pluginDefinition.setGroupId("org.sonarsource.scanner.maven");
pluginDefinition.setArtifactId("sonar-maven-plugin");
pluginDefinition.setVersion(null);
assertThat(SonarQubeMojo.isPluginVersionDefinedInTheProject(project, "org.sonarsource.scanner.maven", "sonar-maven-plugin")).isFalse();

pluginDefinition.setGroupId("org.sonarsource.scanner.maven");
pluginDefinition.setArtifactId("sonar-maven-plugin");
pluginDefinition.setVersion(" ");
assertThat(SonarQubeMojo.isPluginVersionDefinedInTheProject(project, "org.sonarsource.scanner.maven", "sonar-maven-plugin")).isFalse();

pluginDefinition.setGroupId("org.sonarsource.scanner.maven");
pluginDefinition.setArtifactId("sonar-maven-plugin");
pluginDefinition.setVersion("1.2.3.4");
assertThat(SonarQubeMojo.isPluginVersionDefinedInTheProject(project, "org.sonarsource.scanner.maven", "sonar-maven-plugin")).isTrue();

pluginDefinition.setGroupId("org.other");
pluginDefinition.setArtifactId("sonar-maven-plugin");
pluginDefinition.setVersion("1.2.3.4");
assertThat(SonarQubeMojo.isPluginVersionDefinedInTheProject(project, "org.sonarsource.scanner.maven", "sonar-maven-plugin")).isFalse();

pluginDefinition.setGroupId("org.sonarsource.scanner.maven");
pluginDefinition.setArtifactId("other-plugin");
pluginDefinition.setVersion("1.2.3.4");
assertThat(SonarQubeMojo.isPluginVersionDefinedInTheProject(project, "org.sonarsource.scanner.maven", "sonar-maven-plugin")).isFalse();
}

@Test
public void verbose() throws Exception {
when(mockedLogger.isDebugEnabled()).thenReturn(true);
logger.setLogLevel(TestLog.LogLevel.DEBUG);
executeProject("project-with-findbugs-reporting");
verify(mockedLogger, atLeastOnce()).isDebugEnabled();
assertThat(readProps("target/dump.properties")).contains((entry("sonar.verbose", "true")));
}

private File executeProject(String projectName, String... properties) throws Exception {
private File executeProject(String projectName) throws Exception {
return executeProject(projectName, DEFAULT_GOAL);
}

private File executeProject(String projectName, String goal, String... properties) throws Exception {
File baseDir = new File("src/test/projects/" + projectName).getAbsoluteFile();
SonarQubeMojo mojo = getMojo(baseDir);
mojo.getSession().getRequest().setGoals(Collections.singletonList(goal));
mojo.getSession().getProjects().get(0).setExecutionRoot(true);
mojo.getSession().setAllProjects(mojo.getSession().getProjects());
PluginDescriptor pluginDescriptor = mojo.getMojoExecution().getMojoDescriptor().getPluginDescriptor();
pluginDescriptor.setPlugin(createSonarPluginFrom(pluginDescriptor, goal, mojo.getSession().getTopLevelProject()));

Result<? extends ProjectDependencyGraph> result = mojoRule.lookup(GraphBuilder.class).build(mojo.getSession());
mojo.getSession().setProjectDependencyGraph(result.get()); // done by maven in a normal execution

mojo.setLog(mockedLogger);
mojo.setLog(logger);

Properties userProperties = mojo.getSession().getUserProperties();
if ((properties.length % 2) != 0) {
Expand All @@ -224,8 +318,42 @@ private File executeProject(String projectName, String... properties) throws Exc
return baseDir;
}

private Plugin createSonarPluginFrom(PluginDescriptor pluginDescriptor, String goal, MavenProject project) {
String version = null;
Pattern versionPattern = Pattern.compile("(?:" +
"sonar|" +
"org\\.codehaus\\.mojo:sonar-maven-plugin|" +
"org\\.sonarsource\\.scanner\\.maven:sonar-maven-plugin" +
"):([^:]+):sonar");
Matcher matcher = versionPattern.matcher(goal);
if (matcher.matches()) {
version = matcher.group(1);
}
if (version == null) {
List<Plugin> plugins = new ArrayList<>(project.getBuildPlugins());
PluginManagement pluginManagement = project.getPluginManagement();
if (pluginManagement != null) {
plugins.addAll(pluginManagement.getPlugins());
}
for (Plugin plugin : plugins) {
String pluginGroupId = plugin.getGroupId();
if ((pluginGroupId == null || pluginGroupId.equals(pluginDescriptor.getGroupId())) &&
pluginDescriptor.getArtifactId().equals(plugin.getArtifactId()) &&
plugin.getVersion() != null){
version = plugin.getVersion();
break;
}
}
}
Plugin plugin = new Plugin();
plugin.setGroupId(pluginDescriptor.getGroupId());
plugin.setArtifactId(pluginDescriptor.getArtifactId());
plugin.setVersion(version);
return plugin;
}

@SafeVarargs
private final void assertPropsContains(MapEntry<String, String>... entries) throws IOException {
private void assertPropsContains(MapEntry<String, String>... entries) throws IOException {
assertThat(readProps("target/dump.properties")).contains(entries);
}

Expand Down
Loading