diff --git a/src/main/java/org/sonarsource/scanner/maven/SonarQubeMojo.java b/src/main/java/org/sonarsource/scanner/maven/SonarQubeMojo.java index 95d7bf91..37dd7322 100644 --- a/src/main/java/org/sonarsource/scanner/maven/SonarQubeMojo.java +++ b/src/main/java/org/sonarsource/scanner/maven/SonarQubeMojo.java @@ -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; @@ -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); @@ -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 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 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 goals, String groupId, String artifactId) { + List 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 @@ -163,4 +208,8 @@ private boolean isSkip(Map properties) { MavenSession getSession() { return session; } + + MojoExecution getMojoExecution() { + return mojoExecution; + } } diff --git a/src/test/java/org/sonarsource/scanner/maven/SonarQubeMojoTest.java b/src/test/java/org/sonarsource/scanner/maven/SonarQubeMojoTest.java index e17a3f52..997e2322 100644 --- a/src/test/java/org/sonarsource/scanner/maven/SonarQubeMojoTest.java +++ b/src/test/java/org/sonarsource/scanner/maven/SonarQubeMojoTest.java @@ -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"); @@ -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(); } @@ -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 actualListOfSources = extractSonarSources("target/dump.properties", baseDir.toPath()); assertThat(actualListOfSources).containsExactlyInAnyOrder( @@ -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"); @@ -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 actualListOfSources = extractSonarSources("target/dump.properties", baseDir.toPath()); @@ -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 actualListOfSources = extractSonarSources("target/dump.properties", baseDir.toPath()); @@ -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 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 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) { @@ -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 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... entries) throws IOException { + private void assertPropsContains(MapEntry... entries) throws IOException { assertThat(readProps("target/dump.properties")).contains(entries); } diff --git a/src/test/java/org/sonarsource/scanner/maven/TimestampLoggerTest.java b/src/test/java/org/sonarsource/scanner/maven/TimestampLoggerTest.java index dbefd6d1..53f91911 100644 --- a/src/test/java/org/sonarsource/scanner/maven/TimestampLoggerTest.java +++ b/src/test/java/org/sonarsource/scanner/maven/TimestampLoggerTest.java @@ -22,7 +22,6 @@ import java.time.LocalTime; import java.util.ArrayList; import java.util.List; -import java.util.regex.Pattern; import org.apache.maven.plugin.logging.Log; import org.junit.jupiter.api.Test; import org.mockito.MockedStatic; @@ -101,21 +100,25 @@ void log_level_matches_underlying_maven_log_level() { assertThat(new TimestampLogger(errorLevelLog).isErrorEnabled()).isTrue(); } - private static class TestLog implements Log { - private enum LogLevel { + public static class TestLog implements Log { + public enum LogLevel { DEBUG, INFO, WARN, ERROR, } - private final LogLevel logLevel; - private final List logs = new ArrayList<>(); + public LogLevel logLevel; + public final List logs = new ArrayList<>(); public TestLog(LogLevel logLevel) { this.logLevel = logLevel; } + public void setLogLevel(LogLevel logLevel) { + this.logLevel = logLevel; + } + @Override public boolean isDebugEnabled() { return logLevel == LogLevel.DEBUG; @@ -196,4 +199,4 @@ public void error(Throwable error) { error(error.getMessage()); } } -} \ No newline at end of file +} diff --git a/src/test/projects/project-with-sonar-plugin-configuration/pom.xml b/src/test/projects/project-with-sonar-plugin-configuration/pom.xml new file mode 100644 index 00000000..3f635c3c --- /dev/null +++ b/src/test/projects/project-with-sonar-plugin-configuration/pom.xml @@ -0,0 +1,25 @@ + + 4.0.0 + + org.codehaus.sonar + sample-project + 1.0-SNAPSHOT + Test project + + + target/dump.properties + + + + + + org.sonarsource.scanner.maven + sonar-maven-plugin + 1.2.3.4 + + + + + diff --git a/src/test/projects/project-with-sonar-plugin-management-configuration/pom.xml b/src/test/projects/project-with-sonar-plugin-management-configuration/pom.xml new file mode 100644 index 00000000..c32c148e --- /dev/null +++ b/src/test/projects/project-with-sonar-plugin-management-configuration/pom.xml @@ -0,0 +1,39 @@ + + 4.0.0 + + org.codehaus.sonar + sample-project + 1.0-SNAPSHOT + Test project + + + target/dump.properties + + + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + unknow-group-id + sonar-maven-plugin + + + org.sonarsource.scanner.maven + unknow-artifact-id + + + org.sonarsource.scanner.maven + sonar-maven-plugin + 1.2.3.4 + + + + + +