From e9b7bfd0b2f8c24c37709b3f22cc21ded0688605 Mon Sep 17 00:00:00 2001 From: Andrew Ash Date: Fri, 12 Jan 2024 16:20:46 -0800 Subject: [PATCH 1/4] Fix "Could not determine jdk version for project" error --- .../intellij/FormatterProvider.java | 32 +++++++++++-------- .../intellij/PalantirCodeStyleManager.java | 9 ++++-- .../intellij/FormatterProviderTest.java | 6 ++-- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java index 18d3e0724..a352a40c7 100644 --- a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java +++ b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java @@ -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; @@ -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; @@ -51,10 +51,10 @@ 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 implementationCache = + private final LoadingCache> implementationCache = Caffeine.newBuilder().maximumSize(1).build(FormatterProvider::createFormatter); - FormatterService get(Project project, PalantirJavaFormatSettings settings) { + Optional get(Project project, PalantirJavaFormatSettings settings) { return implementationCache.get(new FormatterCacheKey( project, getSdkVersion(project), @@ -62,8 +62,12 @@ FormatterService get(Project project, PalantirJavaFormatSettings settings) { settings.injectedVersionIsOutdated())); } - private static FormatterService createFormatter(FormatterCacheKey cacheKey) { - int jdkMajorVersion = cacheKey.jdkMajorVersion; + private static Optional createFormatter(FormatterCacheKey cacheKey) { + if (cacheKey.jdkMajorVersion.isEmpty()) { + return Optional.empty(); + } + + int jdkMajorVersion = cacheKey.jdkMajorVersion.getAsInt(); List implementationClasspath = getImplementationUrls(cacheKey.implementationClassPath, cacheKey.useBundledImplementation); @@ -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(); } /** @@ -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( @@ -146,13 +150,13 @@ 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; @@ -185,13 +189,13 @@ private static List listDirAsUrlsUnchecked(Path dir) { private static final class FormatterCacheKey { private final Project project; - private final int jdkMajorVersion; + private final OptionalInt jdkMajorVersion; private final Optional> implementationClassPath; private final boolean useBundledImplementation; FormatterCacheKey( Project project, - int jdkMajorVersion, + OptionalInt jdkMajorVersion, Optional> implementationClassPath, boolean useBundledImplementation) { this.project = project; @@ -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); diff --git a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirCodeStyleManager.java b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirCodeStyleManager.java index 0095fef54..d9cbc9c7d 100644 --- a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirCodeStyleManager.java +++ b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirCodeStyleManager.java @@ -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; @@ -194,9 +195,13 @@ private void formatInternal(PsiFile file, Collection ranges */ private void format(Document document, Collection ranges) { PalantirJavaFormatSettings settings = PalantirJavaFormatSettings.getInstance(getProject()); - FormatterService formatter = formatterProvider.get(project, settings); + Optional formatter = formatterProvider.get(project, settings); + if (formatter.isEmpty()) { + // Return early if we couldn't find a formatter. + return; + } - performReplacements(document, getReplacements(formatter, document.getText(), ranges)); + performReplacements(document, getReplacements(formatter.get(), document.getText(), ranges)); } private void performReplacements(final Document document, final Map replacements) { diff --git a/idea-plugin/src/test/java/com/palantir/javaformat/intellij/FormatterProviderTest.java b/idea-plugin/src/test/java/com/palantir/javaformat/intellij/FormatterProviderTest.java index 8cf772cb8..8df0a96ba 100644 --- a/idea-plugin/src/test/java/com/palantir/javaformat/intellij/FormatterProviderTest.java +++ b/idea-plugin/src/test/java/com/palantir/javaformat/intellij/FormatterProviderTest.java @@ -23,16 +23,16 @@ 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); } } From 5d4345ecd11840cfacb2a9b7df1faeb89a480810 Mon Sep 17 00:00:00 2001 From: Andrew Ash Date: Fri, 12 Jan 2024 16:22:20 -0800 Subject: [PATCH 2/4] log warning --- .../palantir/javaformat/intellij/PalantirCodeStyleManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirCodeStyleManager.java b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirCodeStyleManager.java index d9cbc9c7d..7d683027b 100644 --- a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirCodeStyleManager.java +++ b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirCodeStyleManager.java @@ -197,7 +197,7 @@ private void format(Document document, Collection ranges) { PalantirJavaFormatSettings settings = PalantirJavaFormatSettings.getInstance(getProject()); Optional formatter = formatterProvider.get(project, settings); if (formatter.isEmpty()) { - // Return early if we couldn't find a formatter. + log.warn("Could not find a formatter! Making no changes"); return; } From 4a67d9a9eb15089c7c5994fd7509cf386da240df Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Sat, 13 Jan 2024 00:28:06 +0000 Subject: [PATCH 3/4] Add generated changelog entries --- changelog/@unreleased/pr-980.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-980.v2.yml diff --git a/changelog/@unreleased/pr-980.v2.yml b/changelog/@unreleased/pr-980.v2.yml new file mode 100644 index 000000000..3d59c9a46 --- /dev/null +++ b/changelog/@unreleased/pr-980.v2.yml @@ -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 From b7e8ec0d9a19b02b285585317cf2dced7604e930 Mon Sep 17 00:00:00 2001 From: Andrew Ash Date: Sun, 21 Jan 2024 15:27:05 -0800 Subject: [PATCH 4/4] Return OptionalInt.empty() instead of null --- .../com/palantir/javaformat/intellij/FormatterProvider.java | 2 +- .../palantir/javaformat/intellij/FormatterProviderTest.java | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java index a352a40c7..d05f37661 100644 --- a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java +++ b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/FormatterProvider.java @@ -159,7 +159,7 @@ static OptionalInt parseSdkJavaVersion(String version) { return OptionalInt.of(Integer.parseInt(normalizedVersion)); } catch (NumberFormatException e) { log.error("Could not parse sdk version: {}", version, e); - return null; + return OptionalInt.empty(); } } diff --git a/idea-plugin/src/test/java/com/palantir/javaformat/intellij/FormatterProviderTest.java b/idea-plugin/src/test/java/com/palantir/javaformat/intellij/FormatterProviderTest.java index 8df0a96ba..c1fd27352 100644 --- a/idea-plugin/src/test/java/com/palantir/javaformat/intellij/FormatterProviderTest.java +++ b/idea-plugin/src/test/java/com/palantir/javaformat/intellij/FormatterProviderTest.java @@ -35,4 +35,9 @@ void testParseSdkJavaVersion_majorMinorPatch() { void testParseSdkJavaVersion_ea() { assertThat(FormatterProvider.parseSdkJavaVersion("15-ea")).hasValue(15); } + + @Test + void testParseSdkJavaVersion_invalidVersion_isEmpty() { + assertThat(FormatterProvider.parseSdkJavaVersion("not-a-version")).isEmpty(); + } }