Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
12 changes: 12 additions & 0 deletions java-frontend/src/main/java/org/sonar/java/SonarComponents.java
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,14 @@ public class SonarComponents extends CheckRegistrar.RegistrarContext {
* relying on (transitive) dependencies that do not respect modularization as defined by the JLS.
*/
public static final String SONAR_IGNORE_UNNAMED_MODULE_FOR_SPLIT_PACKAGE = "sonar.java.ignoreUnnamedModuleForSplitPackage";

/**
* Describes whether the analysis should fail and halt when a StackOverflowError is encountered.
* True by default, we still want to give the possibility to disable this behavior in case of projects
* with large amounts of unreviewed AI-generated code causing such errors.
*/
public static final String SONAR_FAIL_ON_STACKOVERFLOW = "sonar.java.failOnStackOverflow";

private static final Version SONARLINT_6_3 = Version.parse("6.3");
private static final Version SONARQUBE_9_2 = Version.parse("9.2");
@VisibleForTesting
Expand Down Expand Up @@ -492,6 +500,10 @@ public boolean shouldIgnoreUnnamedModuleForSplitPackage() {
return context.config().getBoolean(SONAR_IGNORE_UNNAMED_MODULE_FOR_SPLIT_PACKAGE).orElse(false);
}

public boolean shouldFailOnStackOverflow() {
return context.config().getBoolean(SONAR_FAIL_ON_STACKOVERFLOW).orElse(true);
}

private static long computeIdealBatchSize() {
// We take a fraction of the total memory available though -Xmx.
// If we assume that the average size of a file is 5KB and the average CI should have 1GB of memory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,9 @@ public void simpleScan(InputFile inputFile, JParserConfig.Result result, Consume
interruptIfFailFast(e, inputFile);
} catch (StackOverflowError error) {
LOG.error(String.format(LOG_ERROR_STACKOVERFLOW, inputFile), error);
throw error;
if (sonarComponents == null || sonarComponents.shouldFailOnStackOverflow()) {
throw error;
}
} finally {
telemetry.aggregateAsCounter(telemetryAnalysisKeys.sizeCharsKey(), InputFileUtils.charCount(inputFile, 0));
telemetry.aggregateAsCounter(telemetryAnalysisKeys.timeMsKey(), currentTimeMillis() - startTime);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
Expand Down Expand Up @@ -1064,6 +1065,23 @@ void shouldIgnoreUnnamedModuleForSplitPackage_returns_true_when_analysis_paramet
assertThat(sonarComponents.shouldIgnoreUnnamedModuleForSplitPackage()).isTrue();
}

@Test
void shouldFailOnStackOverflow_returns_true_by_default() {
MapSettings settings = new MapSettings();
SonarComponents sonarComponents = new SonarComponents(null, null, null, null, null, null);
sonarComponents.setSensorContext(SensorContextTester.create(new File("")).setSettings(settings));
assertThat(sonarComponents.shouldFailOnStackOverflow()).isTrue();
}

@ParameterizedTest
@ValueSource(strings = {"true", "false"})
void shouldFailOnStackOverflow_returns_correct_value_when_set(String value) {
MapSettings settings = new MapSettings().setProperty("sonar.java.failOnStackOverflow", value);
SonarComponents sonarComponents = new SonarComponents(null, null, null, null, null, null);
sonarComponents.setSensorContext(SensorContextTester.create(new File("")).setSettings(settings));
assertThat(sonarComponents.shouldFailOnStackOverflow()).isEqualTo(Boolean.valueOf(value));
}

@Nested
class Logging {
private final DecimalFormat formatter = new DecimalFormat("00");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.fail;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn;
Expand Down Expand Up @@ -478,6 +479,60 @@ public void scanFile(JavaFileScannerContext context) {

}

@Test
void test_should_fail_on_stackoverflow_by_default() {
InputFile trivialCompilationUnit = TestUtils.inputFile("src/test/resources/AstScannerNoParseError.txt");
var files = List.of(trivialCompilationUnit);
var problematicVisitor = new JavaFileScanner(){
@Override
public void scanFile(JavaFileScannerContext context) {
throw new StackOverflowError();
}
};
List<JavaFileScanner> visitors = List.of(problematicVisitor);

// Assert that by default, StackOverflowError is propagated
var emptySettings = new MapSettings();
assertThrows(StackOverflowError.class, () ->
scanFilesWithVisitorsAndContext(files, visitors, emptySettings, JavaVersionImpl.MAX_SUPPORTED));
}

@Test
void test_should_not_fail_on_stackoverflow_when_set() {
InputFile trivialCompilationUnit = TestUtils.inputFile("src/test/resources/AstScannerNoParseError.txt");
var files = List.of(trivialCompilationUnit);
var problematicVisitor = new JavaFileScanner(){
@Override
public void scanFile(JavaFileScannerContext context) {
throw new StackOverflowError();
}
};
List<JavaFileScanner> visitors = List.of(problematicVisitor);

// Assert that when configured to not fail on exception, StackOverflowError is swallowed, but logged
MapSettings settings = new MapSettings().setProperty(SonarComponents.SONAR_FAIL_ON_STACKOVERFLOW, false);
assertDoesNotThrow(() -> scanFilesWithVisitorsAndContext(files, visitors, settings, JavaVersionImpl.MAX_SUPPORTED));
assertThat(logTester.logs(Level.ERROR))
.containsExactly("A stack overflow error occurred while analyzing file: 'src/test/resources/AstScannerNoParseError.txt'");
}

@Test
void test_should_fail_on_stackoverflow_when_sonar_components_null() {
InputFile trivialCompilationUnit = TestUtils.inputFile("src/test/resources/AstScannerNoParseError.txt");
var files = List.of(trivialCompilationUnit);
var problematicVisitor = new JavaFileScanner(){
@Override
public void scanFile(JavaFileScannerContext context) {
throw new StackOverflowError();
}
};
List<JavaFileScanner> visitors = List.of(problematicVisitor);
JavaAstScanner scanner = new JavaAstScanner(null, new NoOpTelemetry(), TelemetryKey.JAVA_ANALYSIS_MAIN);
VisitorsBridge visitorBridge = new VisitorsBridge(visitors, new ArrayList<>(), null, new JavaVersionImpl(JavaVersionImpl.MAX_SUPPORTED));
scanner.setVisitorBridge(visitorBridge);
assertThrows(StackOverflowError.class, () -> scanner.scan(files));
}

private void scanSingleFile(InputFile file, boolean failOnException) {
scanFilesWithVisitors(Collections.singletonList(file), Collections.emptyList(), -1, failOnException, false);
}
Expand All @@ -501,12 +556,16 @@ private void scanWithJavaVersion(int version, List<InputFile> inputFiles, List<J
}

private void scanFilesWithVisitors(List<InputFile> inputFiles, List<JavaFileScanner> visitors,
int javaVersion, boolean failOnException, boolean autoscanMode) {
context.setSettings(new MapSettings()
int javaVersion, boolean failOnException, boolean autoscanMode) {
MapSettings settings = new MapSettings()
.setProperty(SonarComponents.FAIL_ON_EXCEPTION_KEY, failOnException)
.setProperty(SonarComponents.SONAR_AUTOSCAN, autoscanMode)
);
.setProperty(SonarComponents.SONAR_AUTOSCAN, autoscanMode);

scanFilesWithVisitorsAndContext(inputFiles, visitors, settings, javaVersion);
}

private void scanFilesWithVisitorsAndContext(List<InputFile> inputFiles, List<JavaFileScanner> visitors, MapSettings contextMap, int javaVersion) {
context.setSettings(contextMap);
DefaultFileSystem fileSystem = context.fileSystem();
ClasspathForMain classpathForMain = new ClasspathForMain(context.config(), fileSystem);
ClasspathForTest classpathForTest = new ClasspathForTest(context.config(), fileSystem);
Expand Down