Skip to content

Commit

Permalink
SONARJAVA-3114 Make sure we log only once
Browse files Browse the repository at this point in the history
  • Loading branch information
Wohops authored and quentin-jaquier-sonarsource committed Feb 1, 2021
1 parent 4ee0b44 commit be1719d
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 19 deletions.
13 changes: 8 additions & 5 deletions java-frontend/src/main/java/org/sonar/java/SonarComponents.java
Original file line number Diff line number Diff line change
Expand Up @@ -346,20 +346,23 @@ public void logUndefinedTypes() {
javaClasspath.logSuspiciousEmptyLibraries();
javaTestClasspath.logSuspiciousEmptyLibraries();
logUndefinedTypes(LOGGED_MAX_NUMBER_UNDEFINED_TYPES);

// clear the set so only new undefined types will be logged
undefinedTypes.clear();
}
}

private void logUndefinedTypes(int maxLines) {
boolean moreThanMax = undefinedTypes.size() > maxLines;

String debugMessage = "Unresolved imports/types have been detected during analysis";
debugMessage += moreThanMax ? String.format(". Logging the %d first:", maxLines) : ", with following errors:";
String warningMessage = "Unresolved imports/types have been detected during analysis. Enable DEBUG mode to see them.";
String debugMessage = moreThanMax ? String.format("First %d unresolved imports/types:", maxLines) : "Unresolved imports/types:";

String prefix = "- ";
String delimiter = System.lineSeparator() + prefix;
String delimiter = System.lineSeparator() + "- ";
String prefix = debugMessage + delimiter;
String suffix = moreThanMax ? (delimiter + "...") : "";

LOG.debug(debugMessage);
LOG.warn(warningMessage);
LOG.debug(undefinedTypes
.stream()
.sorted()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public class ClasspathForMain extends AbstractClasspath {

private final AnalysisWarningsWrapper analysisWarnings;
private boolean hasSuspiciousEmptyLibraries = false;
private boolean alreadyReported = false;

public ClasspathForMain(Configuration settings, FileSystem fs, AnalysisWarningsWrapper analysisWarnings) {
super(settings, fs, InputFile.Type.MAIN);
Expand Down Expand Up @@ -96,10 +97,11 @@ private static boolean isNotNullOrEmpty(@Nullable String string) {

@Override
public void logSuspiciousEmptyLibraries() {
if (hasSuspiciousEmptyLibraries) {
String warning = String.format(ClasspathProperties.EMPTY_LIBRARIES_WARNING_TEMPLATE, "source", ClasspathProperties.SONAR_JAVA_LIBRARIES);
if (hasSuspiciousEmptyLibraries && !alreadyReported) {
String warning = String.format(ClasspathProperties.EMPTY_LIBRARIES_WARNING_TEMPLATE, "SOURCE", ClasspathProperties.SONAR_JAVA_LIBRARIES);
LOG.warn(warning);
analysisWarnings.addUnique(warning);
alreadyReported = true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
public class ClasspathForTest extends AbstractClasspath {

private static final Logger LOG = Loggers.get(ClasspathForTest.class);

private boolean hasSuspiciousEmptyLibraries = false;
private boolean alreadyReported = false;

public ClasspathForTest(Configuration settings, FileSystem fs) {
super(settings, fs, InputFile.Type.TEST);
Expand Down Expand Up @@ -60,9 +62,10 @@ protected void init() {

@Override
public void logSuspiciousEmptyLibraries() {
if (hasSuspiciousEmptyLibraries) {
String warning = String.format(ClasspathProperties.EMPTY_LIBRARIES_WARNING_TEMPLATE, "test", ClasspathProperties.SONAR_JAVA_TEST_LIBRARIES);
if (hasSuspiciousEmptyLibraries && !alreadyReported) {
String warning = String.format(ClasspathProperties.EMPTY_LIBRARIES_WARNING_TEMPLATE, "TEST", ClasspathProperties.SONAR_JAVA_TEST_LIBRARIES);
LOG.warn(warning);
alreadyReported = true;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -450,13 +450,14 @@ void log_only_50_undefined_types() {
// triggers log
sonarComponents.logUndefinedTypes();

assertThat(logTester.logs(LoggerLevel.WARN)).containsExactly("Unresolved imports/types have been detected during analysis. Enable DEBUG mode to see them.");

List<String> debugLogs = logTester.logs(LoggerLevel.DEBUG);
assertThat(debugLogs)
.hasSize(2)
.startsWith("Unresolved imports/types have been detected during analysis. Logging the 50 first:");
String list = debugLogs.get(1);
assertThat(debugLogs).hasSize(1);

String list = debugLogs.get(0);
assertThat(list)
.startsWith("First 50 unresolved imports/types:")
.endsWith("- ...")
.doesNotContain("- Y cannot be resolved to a type")
.doesNotContain("- Z cannot be resolved to a type");
Expand All @@ -479,12 +480,13 @@ void log_all_undefined_types_if_less_than_thresold() {
// triggers log
sonarComponents.logUndefinedTypes();

assertThat(logTester.logs(LoggerLevel.WARN)).containsExactly("Unresolved imports/types have been detected during analysis. Enable DEBUG mode to see them.");

List<String> debugLogs = logTester.logs(LoggerLevel.DEBUG);
assertThat(debugLogs)
.hasSize(2)
.startsWith("Unresolved imports/types have been detected during analysis, with following errors:");
assertThat(debugLogs).hasSize(1);

assertThat(debugLogs.get(1))
assertThat(debugLogs.get(0))
.startsWith("Unresolved imports/types:")
.doesNotContain("- ...")
.contains("- A cannot be resolved to a type")
.contains("- The import org.package01 cannot be resolved");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyZeroInteractions;

Expand Down Expand Up @@ -99,12 +100,30 @@ void display_warning_for_missing_bytecode_when_libraries_empty_and_have_java_sou

javaClasspath.logSuspiciousEmptyLibraries();

String warning = "Dependencies/libraries were not provided for analysis of source files. The 'sonar.java.libraries' property is empty. "
String warning = "Dependencies/libraries were not provided for analysis of SOURCE files. The 'sonar.java.libraries' property is empty. "
+ "Verify your configuration, as you might end up with less precise results.";
verify(analysisWarnings).addUnique(warning);
assertThat(logTester.logs(LoggerLevel.WARN)).containsExactly(warning);
}

@Test
void only_display_once_warning_for_missing_bytecode_when_libraries_empty_and_have_java_sources() {
javaClasspath = createJavaClasspath();
javaClasspath.init();
assertThat(javaClasspath.getFilesFromProperty(ClasspathProperties.SONAR_JAVA_LIBRARIES)).isEmpty();
assertThat(javaClasspath.hasJavaSources()).isTrue();

javaClasspath.logSuspiciousEmptyLibraries();

// re-trigger logs
javaClasspath.logSuspiciousEmptyLibraries();

String warning = "Dependencies/libraries were not provided for analysis of SOURCE files. The 'sonar.java.libraries' property is empty. "
+ "Verify your configuration, as you might end up with less precise results.";
verify(analysisWarnings, times(1)).addUnique(warning);
assertThat(logTester.logs(LoggerLevel.WARN)).containsExactly(warning);
}

@Test
void no_warning_for_missing_bytecode_when_libraries_empty_and_have_no_java_sources() {
javaClasspath = new ClasspathForMain(settings.asConfig(), new DefaultFileSystem(new File("src/test/files/classpath/")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,24 @@ void display_warning_for_missing_bytecode_when_libraries_empty_and_have_java_sou

javaTestClasspath.logSuspiciousEmptyLibraries();

String warning = "Dependencies/libraries were not provided for analysis of test files. The 'sonar.java.test.libraries' property is empty. "
String warning = "Dependencies/libraries were not provided for analysis of TEST files. The 'sonar.java.test.libraries' property is empty. "
+ "Verify your configuration, as you might end up with less precise results.";
assertThat(logTester.logs(LoggerLevel.WARN)).containsExactly(warning);
}

@Test
void only_display_once_warning_for_missing_bytecode_when_libraries_empty_and_have_java_sources() {
javaTestClasspath = createJavaClasspath();
javaTestClasspath.init();
assertThat(javaTestClasspath.getFilesFromProperty(ClasspathProperties.SONAR_JAVA_TEST_LIBRARIES)).isEmpty();
assertThat(javaTestClasspath.hasJavaSources()).isTrue();

javaTestClasspath.logSuspiciousEmptyLibraries();

//re-trigger logs
javaTestClasspath.logSuspiciousEmptyLibraries();

String warning = "Dependencies/libraries were not provided for analysis of TEST files. The 'sonar.java.test.libraries' property is empty. "
+ "Verify your configuration, as you might end up with less precise results.";
assertThat(logTester.logs(LoggerLevel.WARN)).containsExactly(warning);
}
Expand Down

0 comments on commit be1719d

Please sign in to comment.