Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "Could not determine jdk version for project" error #980

Merged
merged 5 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-980.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: fix
fix:
description: Fix "Could not determine jdk version for project" error
links:
- https://github.com/palantir/palantir-java-format/pull/980
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.github.benmanes.caffeine.cache.Caffeine;
import com.github.benmanes.caffeine.cache.LoadingCache;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Iterables;
import com.intellij.openapi.application.ApplicationInfo;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.projectRoots.JdkUtil;
Expand All @@ -40,6 +39,7 @@
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.OptionalInt;
import java.util.ServiceLoader;
import java.util.jar.Attributes.Name;
import java.util.stream.Collectors;
Expand All @@ -51,19 +51,23 @@ final class FormatterProvider {
private static final Logger log = LoggerFactory.getLogger(FormatterProvider.class);

// Cache to avoid creating a URLClassloader every time we want to format from IntelliJ
private final LoadingCache<FormatterCacheKey, FormatterService> implementationCache =
private final LoadingCache<FormatterCacheKey, Optional<FormatterService>> implementationCache =
Caffeine.newBuilder().maximumSize(1).build(FormatterProvider::createFormatter);

FormatterService get(Project project, PalantirJavaFormatSettings settings) {
Optional<FormatterService> get(Project project, PalantirJavaFormatSettings settings) {
return implementationCache.get(new FormatterCacheKey(
project,
getSdkVersion(project),
settings.getImplementationClassPath(),
settings.injectedVersionIsOutdated()));
}

private static FormatterService createFormatter(FormatterCacheKey cacheKey) {
int jdkMajorVersion = cacheKey.jdkMajorVersion;
private static Optional<FormatterService> createFormatter(FormatterCacheKey cacheKey) {
if (cacheKey.jdkMajorVersion.isEmpty()) {
return Optional.empty();
}

int jdkMajorVersion = cacheKey.jdkMajorVersion.getAsInt();
List<Path> implementationClasspath =
getImplementationUrls(cacheKey.implementationClassPath, cacheKey.useBundledImplementation);

Expand All @@ -73,14 +77,14 @@ private static FormatterService createFormatter(FormatterCacheKey cacheKey) {
jdkMajorVersion, ApplicationInfo.getInstance().getBuild())) {
Path jdkPath = getJdkPath(cacheKey.project);
log.info("Using bootstrapping formatter with jdk version {} and path: {}", jdkMajorVersion, jdkPath);
return new BootstrappingFormatterService(jdkPath, jdkMajorVersion, implementationClasspath);
return Optional.of(new BootstrappingFormatterService(jdkPath, jdkMajorVersion, implementationClasspath));
}

// Use "in-process" formatter service
log.info("Using in-process formatter for jdk version {}", jdkMajorVersion);
URL[] implementationUrls = toUrlsUnchecked(implementationClasspath);
ClassLoader classLoader = new URLClassLoader(implementationUrls, FormatterService.class.getClassLoader());
return Iterables.getOnlyElement(ServiceLoader.load(FormatterService.class, classLoader));
return ServiceLoader.load(FormatterService.class, classLoader).findFirst();
}

/**
Expand Down Expand Up @@ -131,13 +135,13 @@ private static Path getJdkPath(Project project) {
.orElseThrow(() -> new IllegalStateException("Could not determine jdk path for project " + project));
}

private static Integer getSdkVersion(Project project) {
private static OptionalInt getSdkVersion(Project project) {
return getProjectJdk(project)
.map(FormatterProvider::parseSdkJavaVersion)
.orElseThrow(() -> new IllegalStateException("Could not determine jdk version for project " + project));
}

private static Integer parseSdkJavaVersion(Sdk sdk) {
private static OptionalInt parseSdkJavaVersion(Sdk sdk) {
// Parses the actual version out of "SDK#getVersionString" which returns 'java version "15"'
// or 'openjdk version "15.0.2"'.
String version = Preconditions.checkNotNull(
Expand All @@ -146,16 +150,16 @@ private static Integer parseSdkJavaVersion(Sdk sdk) {
}

@VisibleForTesting
static Integer parseSdkJavaVersion(String version) {
static OptionalInt parseSdkJavaVersion(String version) {
ash211 marked this conversation as resolved.
Show resolved Hide resolved
int indexOfVersionDelimiter = version.indexOf('.');
String normalizedVersion =
indexOfVersionDelimiter >= 0 ? version.substring(0, indexOfVersionDelimiter) : version;
normalizedVersion = normalizedVersion.replaceAll("-ea", "");
try {
return Integer.parseInt(normalizedVersion);
return OptionalInt.of(Integer.parseInt(normalizedVersion));
} catch (NumberFormatException e) {
log.error("Could not parse sdk version: {}", version, e);
return null;
return OptionalInt.empty();
}
}

Expand Down Expand Up @@ -185,13 +189,13 @@ private static List<Path> listDirAsUrlsUnchecked(Path dir) {

private static final class FormatterCacheKey {
private final Project project;
private final int jdkMajorVersion;
private final OptionalInt jdkMajorVersion;
private final Optional<List<URI>> implementationClassPath;
private final boolean useBundledImplementation;

FormatterCacheKey(
Project project,
int jdkMajorVersion,
OptionalInt jdkMajorVersion,
Optional<List<URI>> implementationClassPath,
boolean useBundledImplementation) {
this.project = project;
Expand All @@ -209,7 +213,7 @@ public boolean equals(Object o) {
return false;
}
FormatterCacheKey that = (FormatterCacheKey) o;
return jdkMajorVersion == that.jdkMajorVersion
return Objects.equals(jdkMajorVersion, that.jdkMajorVersion)
&& useBundledImplementation == that.useBundledImplementation
&& Objects.equals(project, that.project)
&& Objects.equals(implementationClassPath, that.implementationClassPath);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import com.palantir.javaformat.java.FormatterService;
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
import java.util.TreeMap;
import java.util.stream.Collectors;
import org.jetbrains.annotations.NotNull;
Expand Down Expand Up @@ -194,9 +195,13 @@ private void formatInternal(PsiFile file, Collection<? extends TextRange> ranges
*/
private void format(Document document, Collection<? extends TextRange> ranges) {
PalantirJavaFormatSettings settings = PalantirJavaFormatSettings.getInstance(getProject());
FormatterService formatter = formatterProvider.get(project, settings);
Optional<FormatterService> formatter = formatterProvider.get(project, settings);
if (formatter.isEmpty()) {
log.warn("Could not find a formatter! Making no changes");
return;
}

performReplacements(document, getReplacements(formatter, document.getText(), ranges));
performReplacements(document, getReplacements(formatter.get(), document.getText(), ranges));
}

private void performReplacements(final Document document, final Map<TextRange, String> replacements) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,21 @@ public final class FormatterProviderTest {

@Test
void testParseSdkJavaVersion_major() {
assertThat(FormatterProvider.parseSdkJavaVersion("15")).isEqualTo(15);
assertThat(FormatterProvider.parseSdkJavaVersion("15")).hasValue(15);
}

@Test
void testParseSdkJavaVersion_majorMinorPatch() {
assertThat(FormatterProvider.parseSdkJavaVersion("15.0.2")).isEqualTo(15);
assertThat(FormatterProvider.parseSdkJavaVersion("15.0.2")).hasValue(15);
}

@Test
void testParseSdkJavaVersion_ea() {
assertThat(FormatterProvider.parseSdkJavaVersion("15-ea")).isEqualTo(15);
assertThat(FormatterProvider.parseSdkJavaVersion("15-ea")).hasValue(15);
}

@Test
void testParseSdkJavaVersion_invalidVersion_isEmpty() {
assertThat(FormatterProvider.parseSdkJavaVersion("not-a-version")).isEmpty();
}
}