Skip to content

Commit

Permalink
SONARJAVA-4464 Restrict the use of the sonar.java.enablePreview flag …
Browse files Browse the repository at this point in the history
…to the greatest supported Java version (#4367)
  • Loading branch information
dorian-burihabwa-sonarsource authored May 1, 2023
1 parent 0f50490 commit b1a0335
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ static CheckVerifier newVerifier() {
* @param enablePreviewFeatures defines if preview features from the specified java version should be enabled or not
*
* @return the verifier configured to consider the provided test file(s) as following the syntax of the given java version
*
* @throws IllegalArgumentException If the enablePreviewFeatures parameter is set to true but javaVersionAsInt is not the latest supported version
*/
CheckVerifier withJavaVersion(int javaVersionAsInt, boolean enablePreviewFeatures);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,20 @@ public InternalCheckVerifier withQuickFixes() {
@Override
public InternalCheckVerifier withJavaVersion(int javaVersionAsInt) {
requiresNull(javaVersion, "java version");
javaVersion = new JavaVersionImpl(javaVersionAsInt);
return this;
return withJavaVersion(javaVersionAsInt, false);
}

@Override
public InternalCheckVerifier withJavaVersion(int javaVersionAsInt, boolean enablePreviewFeatures) {
requiresNull(javaVersion, "java version");
if (enablePreviewFeatures && javaVersionAsInt != JavaVersionImpl.MAX_SUPPORTED) {
var message = String.format(
"Preview features can only be enabled when the version == latest supported Java version (%d != %d)",
javaVersionAsInt,
JavaVersionImpl.MAX_SUPPORTED
);
throw new IllegalArgumentException(message);
}
javaVersion = new JavaVersionImpl(javaVersionAsInt, enablePreviewFeatures);
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import org.sonar.java.caching.FileHashingUtils;
import org.sonar.java.caching.JavaReadCacheImpl;
import org.sonar.java.caching.JavaWriteCacheImpl;
import org.sonar.java.checks.verifier.CheckVerifier;
import org.sonar.java.model.JavaVersionImpl;
import org.sonar.java.reporting.AnalyzerMessage;
import org.sonar.java.reporting.InternalJavaIssueBuilder;
import org.sonar.java.reporting.JavaQuickFix;
Expand Down Expand Up @@ -184,6 +186,17 @@ void setting_multiple_times_java_version_fails() {
.isInstanceOf(AssertionError.class)
.hasMessage("Do not set java version multiple times!");
}

@Test
void preview_features_can_only_be_enabled_for_the_latest_java_version() {
int desiredJavaVersion = JavaVersionImpl.MAX_SUPPORTED - 1;
final CheckVerifier verifier = InternalCheckVerifier.newInstance();
assertThatThrownBy(() -> verifier.withJavaVersion(desiredJavaVersion, true))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage(
String.format("Preview features can only be enabled when the version == latest supported Java version (%d != %d)", desiredJavaVersion, JavaVersionImpl.MAX_SUPPORTED)
);
}

@Test
void preview_features_disabled_by_default() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package org.sonar.java.model;

import java.util.Locale;
import java.util.Map;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.plugins.java.api.JavaVersion;
Expand All @@ -47,13 +46,11 @@ public class JavaVersionImpl implements JavaVersion {
private final boolean previewFeaturesEnabled;

public JavaVersionImpl() {
this.javaVersion = -1;
this.previewFeaturesEnabled = false;
this(-1, false);
}

public JavaVersionImpl(int javaVersion) {
this.javaVersion = javaVersion;
this.previewFeaturesEnabled = false;
this(javaVersion, false);
}

public JavaVersionImpl(int javaVersion, boolean previewFeaturesEnabled) {
Expand All @@ -73,13 +70,13 @@ public static JavaVersion fromString(String javaVersion) {
}
}

public static JavaVersion fromMap(Map<String, String> properties) {
boolean previewFeaturesFlag = convertBooleanString(properties.getOrDefault(JavaVersion.ENABLE_PREVIEW, ""));
public static JavaVersion fromStrings(String javaVersion, String previewFeaturesFlag) {
boolean previewFeaturesEnabled = convertBooleanString(previewFeaturesFlag);
try {
int versionAsInt = convertJavaVersionString(properties.getOrDefault(JavaVersion.SOURCE_VERSION, ""));
return new JavaVersionImpl(versionAsInt, previewFeaturesFlag);
int versionAsInt = convertJavaVersionString(javaVersion);
return new JavaVersionImpl(versionAsInt, previewFeaturesEnabled);
} catch (NumberFormatException e) {
LOG.warn("Invalid java version (got \"" + properties.get(JavaVersion.SOURCE_VERSION) + "\"). "
LOG.warn("Invalid java version (got \"" + javaVersion + "\"). "
+ "The version will be ignored. Accepted formats are \"1.X\", or simply \"X\" "
+ "(for instance: \"1.5\" or \"5\", \"1.6\" or \"6\", \"1.7\" or \"7\", etc.)");
return new JavaVersionImpl();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
*/
package org.sonar.java.model;

import java.util.Map;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -41,7 +40,7 @@ void should_enable_preview() {
assertThat(shouldEnablePreviewFlag(new JavaVersionImpl(42, true))).isTrue();

assertThat(shouldEnablePreviewFlag(JavaVersionImpl.fromString("1.8"))).isFalse();
assertThat(shouldEnablePreviewFlag(JavaVersionImpl.fromMap(Map.of(JavaVersionImpl.SOURCE_VERSION, "1.8", JavaVersionImpl.ENABLE_PREVIEW, "True") ))).isTrue();
assertThat(shouldEnablePreviewFlag(JavaVersionImpl.fromStrings("1.8", "True"))).isTrue();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
*/
package org.sonar.java.model;

import java.util.Map;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
Expand Down Expand Up @@ -145,31 +144,31 @@ void test_fromString() {
@Test
void test_fromMap() {
JavaVersion version;
version = JavaVersionImpl.fromMap(Map.of(JavaVersion.ENABLE_PREVIEW, "False", JavaVersion.SOURCE_VERSION, "17"));
version = JavaVersionImpl.fromStrings("17", "False");
assertThat(version.isSet()).isTrue();
assertThat(version.isNotSet()).isFalse();
assertThat(version.asInt()).isEqualTo(17);
assertThat(version.arePreviewFeaturesEnabled()).isFalse();
version = JavaVersionImpl.fromMap(Map.of(JavaVersion.ENABLE_PREVIEW, "True", JavaVersion.SOURCE_VERSION, "17"));

version = JavaVersionImpl.fromStrings("17", "True");
assertThat(version.isSet()).isTrue();
assertThat(version.isNotSet()).isFalse();
assertThat(version.asInt()).isEqualTo(17);
assertThat(version.arePreviewFeaturesEnabled()).isTrue();
version = JavaVersionImpl.fromMap(Map.of(JavaVersion.ENABLE_PREVIEW, "True", JavaVersion.SOURCE_VERSION, ""));

version = JavaVersionImpl.fromStrings("", "True");
assertThat(version.isSet()).isFalse();
assertThat(version.isNotSet()).isTrue();
assertThat(version.asInt()).isEqualTo(-1);
assertThat(version.arePreviewFeaturesEnabled()).isFalse();

version = JavaVersionImpl.fromMap(Map.of(JavaVersion.ENABLE_PREVIEW, "True"));
version = JavaVersionImpl.fromStrings("", "True");
assertThat(version.isSet()).isFalse();
assertThat(version.isNotSet()).isTrue();
assertThat(version.asInt()).isEqualTo(-1);
assertThat(version.arePreviewFeaturesEnabled()).isFalse();

version = JavaVersionImpl.fromMap(Map.of());
version = JavaVersionImpl.fromStrings("", "");
assertThat(version.isSet()).isFalse();
assertThat(version.isNotSet()).isTrue();
assertThat(version.asInt()).isEqualTo(-1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.UnaryOperator;
Expand Down Expand Up @@ -185,11 +184,18 @@ private Iterable<InputFile> javaFiles(InputFile.Type type) {
}

private JavaVersion getJavaVersion() {
Map<String, String> javaVersionProperties = Map.of(
JavaVersion.ENABLE_PREVIEW, settings.get(JavaVersion.ENABLE_PREVIEW).orElseGet(() -> ""),
JavaVersion.SOURCE_VERSION, settings.get(JavaVersion.SOURCE_VERSION).orElseGet(() -> ""));

JavaVersion javaVersion = JavaVersionImpl.fromMap(javaVersionProperties);
Optional<String> javaVersionAsString = settings.get(JavaVersion.SOURCE_VERSION);
if (!javaVersionAsString.isPresent()) {
return new JavaVersionImpl();
}
String enablePreviewAsString = settings.get(JavaVersion.ENABLE_PREVIEW).orElse("false");

JavaVersion javaVersion = JavaVersionImpl.fromStrings(javaVersionAsString.get(), enablePreviewAsString);
if (javaVersion.arePreviewFeaturesEnabled() && javaVersion.asInt() < JavaVersionImpl.MAX_SUPPORTED) {
LOG.warn("sonar.java.enablePreview is set but will be discarded as the Java version is less than the max supported version (" +
javaVersion.asInt() + " < " + JavaVersionImpl.MAX_SUPPORTED + ")");
javaVersion = new JavaVersionImpl(javaVersion.asInt(), false);
}
LOG.info("Configured Java source version (" + JavaVersion.SOURCE_VERSION + "): " + javaVersion.asInt()
+ ", preview features enabled (" + JavaVersion.ENABLE_PREVIEW + "): " + javaVersion.arePreviewFeaturesEnabled());
return javaVersion;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import org.sonar.java.classpath.ClasspathForTest;
import org.sonar.java.jsp.Jasper;
import org.sonar.java.model.GeneratedFile;
import org.sonar.java.model.JavaVersionImpl;
import org.sonar.java.reporting.AnalyzerMessage;
import org.sonar.plugins.java.api.CheckRegistrar;
import org.sonar.plugins.java.api.JavaCheck;
Expand Down Expand Up @@ -113,7 +114,7 @@ void test_toString() throws IOException {

@Test
void test_issues_creation_on_main_file() throws IOException {
testIssueCreation(InputFile.Type.MAIN, 16);
testIssueCreation(InputFile.Type.MAIN, 18);
}

@Test
Expand All @@ -133,7 +134,7 @@ private void testIssueCreation(InputFile.Type onType, int expectedIssues) throws

jss.execute(context);
// argument 120 refers to the comment on line #120 in this file
verify(noSonarFilter, times(1)).noSonarInFile(fs.inputFiles().iterator().next(), Collections.singleton(120));
verify(noSonarFilter, times(1)).noSonarInFile(fs.inputFiles().iterator().next(), Collections.singleton(121));
verify(sonarComponents, times(expectedIssues)).reportIssue(any(AnalyzerMessage.class));

settings.setProperty(JavaVersion.SOURCE_VERSION, "wrongFormat");
Expand Down Expand Up @@ -334,16 +335,49 @@ void custom_performance_measure_file_path_can_be_empty() throws IOException {
assertThat(defaultPerformanceFile).exists();
assertThat(new String(Files.readAllBytes(defaultPerformanceFile), UTF_8)).contains("\"JavaSensor\"");
}


@Test
void test_java_version_automatically_accepts_enablePreview_flag_when_maximum_version() throws IOException {
MapSettings settings = new MapSettings();
settings.setProperty("sonar.java.source", JavaVersionImpl.MAX_SUPPORTED);
settings.setProperty("sonar.java.enablePreview", "True");
Path workDir = tmp.newFolder().toPath();
executeJavaSensorForPerformanceMeasure(settings, workDir);
assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty();
List<String> infoLogs = logTester.logs(LoggerLevel.INFO);
assertThat(infoLogs).contains("Configured Java source version (sonar.java.source): " + JavaVersionImpl.MAX_SUPPORTED +
", preview features enabled (sonar.java.enablePreview): true");
}

@Test
void test_java_version_construction_logs() throws IOException {
void test_java_version_automatically_disables_enablePreview_flag_when_version_is_less_than_maximum_version() throws IOException {
MapSettings settings = new MapSettings();
settings.setProperty("sonar.java.source", "17");
settings.setProperty("sonar.java.enablePreview", "False");
int version = JavaVersionImpl.MAX_SUPPORTED - 1;
settings.setProperty("sonar.java.source", version);
settings.setProperty("sonar.java.enablePreview", "True");
Path workDir = tmp.newFolder().toPath();
executeJavaSensorForPerformanceMeasure(settings, workDir);
String infoLogs = String.join("\n", logTester.logs(LoggerLevel.INFO));
assertThat(infoLogs).contains("Configured Java source version (sonar.java.source): 17" );
assertThat(logTester.logs(LoggerLevel.WARN)).contains(
"sonar.java.enablePreview is set but will be discarded as the Java version is less than the max supported version (" +
version + " < " + JavaVersionImpl.MAX_SUPPORTED + ")"
);
List<String> infoLogs = logTester.logs(LoggerLevel.INFO);
assertThat(infoLogs).contains("Configured Java source version (sonar.java.source): " + version +
", preview features enabled (sonar.java.enablePreview): false");
}

@Test
void getJavaVersion_does_not_try_to_check_consistency_when_sonar_java_source_is_not_set() throws IOException {
// We set the sonar.java.enablePreview flag to true but it will be ignored because there is no sonar.java.source
MapSettings settings = new MapSettings();
settings.setProperty("sonar.java.enablePreview", "true");
Path workDir = tmp.newFolder().toPath();
executeJavaSensorForPerformanceMeasure(settings, workDir);
assertThat(logTester.logs(LoggerLevel.WARN)).noneMatch(
log -> log.startsWith("sonar.java.enablePreview is set but will be discarded as the Java version is less than the max supported version")
);
List<String> infoLogs = logTester.logs(LoggerLevel.INFO);
assertThat(infoLogs).noneMatch(log -> log.startsWith("Configured Java source version (sonar.java.source):"));
}

@Test
Expand Down

0 comments on commit b1a0335

Please sign in to comment.