Skip to content

Commit

Permalink
Fix from review
Browse files Browse the repository at this point in the history
  • Loading branch information
@alban-auzeill
alban-auzeill committed May 17, 2024
1 parent ab51950 commit df150e4
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 22 deletions.
8 changes: 4 additions & 4 deletions src/main/java/org/sonarsource/scanner/maven/bootstrap/ScannerBootstrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,15 @@ Map<String, String> collectProperties()
Properties userProperties = session.getUserProperties();
Map<String, String> props = mavenProjectConverter.configure(sortedProjects, topLevelProject, userProperties);
props.putAll(propertyDecryptor.decryptProperties(props));

if (shouldCollectAllSources(userProperties)) {
log.info("Parameter " + MavenScannerProperties.PROJECT_SCAN_ALL_SOURCES + " is enabled. The scanner will attempt to collect additional sources.");
if (mavenProjectConverter.isSourceDirsOverridden()) {
log.warn(notCollectingAdditionalSourcesBecauseOf(ScanProperties.PROJECT_SOURCE_DIRS));
} else if (mavenProjectConverter.isTestDirsOverridden()) {
log.warn(notCollectingAdditionalSourcesBecauseOf(ScanProperties.PROJECT_TEST_DIRS));
} else {
collectAllSources(props, userProperties);
boolean shouldCollectJavaAndKotlinSources = isUserDefinedJavaBinaries(userProperties);
collectAllSources(props, shouldCollectJavaAndKotlinSources);
}
}

Expand Down Expand Up @@ -162,7 +162,7 @@ private static Set<Path> excludedReportFiles(Map<String, String> props) {
}

@VisibleForTesting
void collectAllSources(Map<String, String> props, Properties userProperties) {
void collectAllSources(Map<String, String> props, boolean shouldCollectJavaAndKotlinSources) {
String projectBasedir = props.get(ScanProperties.PROJECT_BASEDIR);
// Exclude the files and folders covered by sonar.sources and sonar.tests (and sonar.exclusions) as computed by the MavenConverter
// Combine all the sonar.sources at the top-level and by module
Expand All @@ -179,7 +179,7 @@ void collectAllSources(Map<String, String> props, Properties userProperties) {
.map(Paths::get)
.collect(Collectors.toSet());
SourceCollector visitor = new SourceCollector(existingSources, mavenProjectConverter.getSkippedBasedDirs(),
excludedReportFiles(props), isUserDefinedJavaBinaries(userProperties));
excludedReportFiles(props), shouldCollectJavaAndKotlinSources);
Files.walkFileTree(Paths.get(projectBasedir), visitor);
collectedSources = visitor.getCollectedSources().stream()
.map(file -> file.toAbsolutePath().toString())
Expand Down
11 changes: 5 additions & 6 deletions src/main/java/org/sonarsource/scanner/maven/bootstrap/SourceCollector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ public class SourceCollector implements FileVisitor<Path> {
)
);

private static final Set<String> EXCLUDED_EXTENSIONS_WITH_BINARIES = Stream.of(
private static final Set<String> EXCLUDED_EXTENSIONS_WITH_JAVA_AND_KOTLIN = Stream.of(
".jar",
".war",
".class",
".ear",
".nar",
// Archives
"..DS_Store",
".DS_Store",
".zip",
".7z",
".rar",
Expand All @@ -75,7 +75,7 @@ public class SourceCollector implements FileVisitor<Path> {
.map(ext -> ext.toLowerCase(Locale.ROOT))
.collect(Collectors.toSet());

private static final Set<String> EXCLUDED_EXTENSIONS_WITHOUT_BINARIES = Stream.concat(EXCLUDED_EXTENSIONS_WITH_BINARIES.stream(), Stream.of(
private static final Set<String> EXCLUDED_EXTENSIONS_WITHOUT_JAVA_AND_KOTLIN = Stream.concat(EXCLUDED_EXTENSIONS_WITH_JAVA_AND_KOTLIN.stream(), Stream.of(
".java",
".jav",
".kt")).map(ext -> ext.toLowerCase(Locale.ROOT))
Expand All @@ -92,12 +92,11 @@ public Set<Path> getCollectedSources() {

private final Set<Path> collectedSources = new HashSet<>();

public SourceCollector(Set<Path> existingSources, Set<Path> directoriesToIgnore, Set<Path> excludedFiles,
boolean isUserDefinedJavaBinaries) {
public SourceCollector(Set<Path> existingSources, Set<Path> directoriesToIgnore, Set<Path> excludedFiles, boolean shouldCollectJavaAndKotlinSources) {
this.existingSources = existingSources;
this.directoriesToIgnore = directoriesToIgnore;
this.excludedFiles = excludedFiles;
this.excludedExtensions = isUserDefinedJavaBinaries ? EXCLUDED_EXTENSIONS_WITH_BINARIES : EXCLUDED_EXTENSIONS_WITHOUT_BINARIES;
this.excludedExtensions = shouldCollectJavaAndKotlinSources ? EXCLUDED_EXTENSIONS_WITH_JAVA_AND_KOTLIN : EXCLUDED_EXTENSIONS_WITHOUT_JAVA_AND_KOTLIN;
}

@Override
Expand Down
10 changes: 5 additions & 5 deletions src/test/java/org/sonarsource/scanner/maven/SonarQubeMojoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public void shouldExportOverridenWarWebSource() throws Exception {
}

@Test
public void project_with_java_files_not_in_src() 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",
"sonar.maven.scanAll", "true");
Expand All @@ -116,7 +116,7 @@ public void project_with_java_files_not_in_src() throws Exception {
}

@Test
public void project_with_java_files_not_in_src_and_user_defined_binaries() throws Exception {
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",
"sonar.maven.scanAll", "true",
Expand All @@ -128,7 +128,7 @@ public void project_with_java_files_not_in_src_and_user_defined_binaries() throw
}

@Test
public void project_with_java_files_not_in_src_and_incomplete_user_defined_libraries() throws Exception {
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",
"sonar.maven.scanAll", "true",
Expand All @@ -139,7 +139,7 @@ public void project_with_java_files_not_in_src_and_incomplete_user_defined_libra
}

@Test
public void project_with_java_files_not_in_src_and_incomplete_user_defined_binaries() throws Exception {
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",
"sonar.maven.scanAll", "true",
Expand Down Expand Up @@ -247,7 +247,7 @@ private static Set<String> extractSonarSources(String propertiesPath, Path proje
String sources = readProps(propertiesPath)
.entrySet()
.stream()
.filter(e -> e.getKey().toString().equals("sonar.sources"))
.filter(e -> e.getKey().endsWith("sonar.sources"))
.map(Map.Entry::getValue)
.findFirst()
.orElse(null);
Expand Down
33 changes: 28 additions & 5 deletions src/test/java/org/sonarsource/scanner/maven/bootstrap/ScannerBootstrapperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ void scanAll_property_is_applied_by_default() throws MojoExecutionException {
});

verify(log, times(1)).info("Parameter sonar.maven.scanAll is enabled. The scanner will attempt to collect additional sources.");
verify(scannerBootstrapper, times(1)).collectAllSources(any(), any());
verify(scannerBootstrapper, times(1)).collectAllSources(any(), eq(false));
}

@Test
Expand All @@ -179,7 +179,7 @@ void scanAll_property_is_not_applied_when_set_explicitly() throws MojoExecutionE
});

verify(log, never()).info("Parameter sonar.maven.scanAll is enabled. The scanner will attempt to collect additional sources.");
verify(scannerBootstrapper, never()).collectAllSources(any(), any());
verify(scannerBootstrapper, never()).collectAllSources(any(), eq(false));
}

@Test
Expand All @@ -194,7 +194,22 @@ void scanAll_property_is_applied_when_set_explicitly() throws MojoExecutionExcep
});

verify(log, times(1)).info("Parameter sonar.maven.scanAll is enabled. The scanner will attempt to collect additional sources.");
verify(scannerBootstrapper, times(1)).collectAllSources(any(), any());
verify(scannerBootstrapper, times(1)).collectAllSources(any(), eq(false));
}

@Test
void scanAll_property_is_applied_when_set_explicitly_with_binaries_and_libraries() throws MojoExecutionException {
setSonarScannerScanAllAndBinariesAndLibraries();

verifyCollectedSources(sourceDirs -> {
assertThat(sourceDirs).hasSize(3);
assertThat(sourceDirs[0]).endsWith(Paths.get("src", "main", "java").toString());
assertThat(sourceDirs[1]).endsWith(Paths.get("pom.xml").toString());
assertThat(sourceDirs[2]).endsWith(Paths.get("src", "main", "resources", "index.js").toString());
});

verify(log, times(1)).info("Parameter sonar.maven.scanAll is enabled. The scanner will attempt to collect additional sources.");
verify(scannerBootstrapper, times(1)).collectAllSources(any(), eq(true));
}

@Test
Expand All @@ -212,7 +227,7 @@ void should_not_collect_all_sources_when_sonar_sources_is_overridden() throws Mo

verify(log, times(1)).info("Parameter sonar.maven.scanAll is enabled. The scanner will attempt to collect additional sources.");
verify(log, times(1)).warn("Parameter sonar.maven.scanAll is enabled but the scanner will not collect additional sources because sonar.sources has been overridden.");
verify(scannerBootstrapper, never()).collectAllSources(any(), any());
verify(scannerBootstrapper, never()).collectAllSources(any(), eq(false));
}

@Test
Expand All @@ -231,7 +246,7 @@ void should_not_collect_all_sources_when_sonar_tests_is_overridden() throws Mojo

verify(log, times(1)).info("Parameter sonar.maven.scanAll is enabled. The scanner will attempt to collect additional sources.");
verify(log, times(1)).warn("Parameter sonar.maven.scanAll is enabled but the scanner will not collect additional sources because sonar.tests has been overridden.");
verify(scannerBootstrapper, never()).collectAllSources(any(), any());
verify(scannerBootstrapper, never()).collectAllSources(any(), eq(false));
}

@Test
Expand Down Expand Up @@ -334,6 +349,14 @@ private void setSonarScannerScanAllTo(String value) {
when(session.getUserProperties()).thenReturn(withScanAllSet);
}

private void setSonarScannerScanAllAndBinariesAndLibraries() {
Properties withScanAllSet = new Properties();
withScanAllSet.put(MavenScannerProperties.PROJECT_SCAN_ALL_SOURCES, "true");
withScanAllSet.put(MavenProjectConverter.JAVA_PROJECT_MAIN_BINARY_DIRS, "target/classes");
withScanAllSet.put(MavenProjectConverter.JAVA_PROJECT_MAIN_LIBRARIES, "target/lib/log.jar");
when(session.getUserProperties()).thenReturn(withScanAllSet);
}

private void verifyCollectedSources(Consumer<String[]> sourceDirsAssertions) throws MojoExecutionException {
Map<String, String> collectedProperties = scannerBootstrapper.collectProperties();
assertThat(collectedProperties).containsKey(ScanProperties.PROJECT_SOURCE_DIRS);
Expand Down
25 changes: 23 additions & 2 deletions src/test/java/org/sonarsource/scanner/maven/bootstrap/SourceCollectorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,34 @@ void visitorCollectsConsistently() throws IOException {
void visitorIgnoresFilesInDirectoriesToIgnore() throws IOException {
Path simpleProjectPom = simpleProjectBasedDir.resolve("pom.xml");
simpleProjectPom.toFile().createNewFile();
Path rootJavaFile = simpleProjectBasedDir.resolve("ProjectRoot.java");
rootJavaFile.toFile().createNewFile();
Path subModule = simpleProjectBasedDir.resolve("submodule");
subModule.toFile().mkdirs();
Path fileInSubModule = subModule.resolve("ignore-me.php");
fileInSubModule.toFile().createNewFile();

SourceCollector visitor = new SourceCollector(Collections.emptySet(), Collections.singleton(subModule), Collections.emptySet(), false);
SourceCollector visitor = new SourceCollector(Collections.emptySet(), Collections.singleton(subModule), Collections.emptySet(), true);
Files.walkFileTree(simpleProjectBasedDir, visitor);
assertThat(visitor.getCollectedSources()).doesNotContain(fileInSubModule);
assertThat(visitor.getCollectedSources())
.contains(rootJavaFile)
.doesNotContain(fileInSubModule);
}

@Test
void visitorIgnoresJavaAndKotlinFiles() throws IOException {
Path simpleProjectPom = simpleProjectBasedDir.resolve("pom.xml");
simpleProjectPom.toFile().createNewFile();
Path rootJavaFile = simpleProjectBasedDir.resolve("ProjectRoot.java");
rootJavaFile.toFile().createNewFile();
Path rootKotlinFile = simpleProjectBasedDir.resolve("ProjectRoot.kt");
rootKotlinFile.toFile().createNewFile();

SourceCollector visitor = new SourceCollector(Collections.emptySet(), Collections.emptySet(), Collections.emptySet(), false);
Files.walkFileTree(simpleProjectBasedDir, visitor);
assertThat(visitor.getCollectedSources())
.contains(simpleProjectPom)
.doesNotContain(rootJavaFile)
.doesNotContain(rootKotlinFile);
}
}

0 comments on commit df150e4

Please sign in to comment.