Skip to content

Commit

Permalink
Fix "Could not determine jdk version for project" error (#980)
Browse files Browse the repository at this point in the history
Fix "Could not determine jdk version for project" error
  • Loading branch information
ash211 authored Jan 22, 2024
1 parent bb4f09d commit 96267d3
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 20 deletions.
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) {
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();
}
}

0 comments on commit 96267d3

Please sign in to comment.