Skip to content

Commit

Permalink
SCANMAVEN-224 Log a warning message when the version of the scanner i…
Browse files Browse the repository at this point in the history 
…s not specified (#228)
  • Loading branch information
@alban-auzeill
alban-auzeill authored May 30, 2024
1 parent b98e10c commit 5d0c548
Show file tree
Hide file tree
Showing 5 changed files with 272 additions and 28 deletions.
49 changes: 49 additions & 0 deletions src/main/java/org/sonarsource/scanner/maven/SonarQubeMojo.java
View file
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
View file
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

0 comments on commit 5d0c548

Please sign in to comment.