From 2fdb2c5c0f3dbe8879b479937723c655d25ed17c Mon Sep 17 00:00:00 2001 From: Claudia Rogoz Date: Tue, 21 May 2024 18:03:09 +0200 Subject: [PATCH] Stops optimizing imports via palantir-java-formatter (#1089) Stops optimizing imports via palantir-java-formatter --- changelog/@unreleased/pr-1089.v2.yml | 5 + .../PalantirJavaFormatFormattingService.java | 11 +- .../PalantirJavaFormatImportOptimizer.java | 92 --------- ...PalantirJavaFormatImportOptimizerTest.java | 185 ------------------ 4 files changed, 6 insertions(+), 287 deletions(-) create mode 100644 changelog/@unreleased/pr-1089.v2.yml delete mode 100644 idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatImportOptimizer.java delete mode 100644 idea-plugin/src/test/java/com/palantir/javaformat/intellij/PalantirJavaFormatImportOptimizerTest.java diff --git a/changelog/@unreleased/pr-1089.v2.yml b/changelog/@unreleased/pr-1089.v2.yml new file mode 100644 index 00000000..d98515d6 --- /dev/null +++ b/changelog/@unreleased/pr-1089.v2.yml @@ -0,0 +1,5 @@ +type: fix +fix: + description: Stops optimizing imports via palantir-java-formatter + links: + - https://github.com/palantir/palantir-java-format/pull/1089 diff --git a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatFormattingService.java b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatFormattingService.java index 5f5f81f4..e28a716b 100644 --- a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatFormattingService.java +++ b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatFormattingService.java @@ -23,7 +23,6 @@ import com.intellij.formatting.service.AsyncDocumentFormattingService; import com.intellij.formatting.service.AsyncFormattingRequest; import com.intellij.ide.highlighter.JavaFileType; -import com.intellij.lang.ImportOptimizer; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.NlsSafe; import com.intellij.openapi.util.TextRange; @@ -61,7 +60,7 @@ protected FormattingTask createFormattingTask(@NotNull AsyncFormattingRequest re @Override public @NotNull Set getFeatures() { - return Set.of(Feature.FORMAT_FRAGMENTS, Feature.OPTIMIZE_IMPORTS); + return Set.of(Feature.FORMAT_FRAGMENTS); } @Override @@ -70,14 +69,6 @@ public boolean canFormat(@NotNull PsiFile file) { && PalantirJavaFormatSettings.getInstance(file.getProject()).isEnabled(); } - @Override - public @NotNull Set getImportOptimizers(@NotNull PsiFile file) { - Project project = file.getProject(); - PalantirJavaFormatSettings settings = PalantirJavaFormatSettings.getInstance(project); - Optional formatter = formatterProvider.get(project, settings); - return Set.of(new PalantirJavaFormatImportOptimizer(formatter)); - } - private static final class PalantirJavaFormatFormattingTask implements FormattingTask { private final AsyncFormattingRequest request; private final Optional formatterService; diff --git a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatImportOptimizer.java b/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatImportOptimizer.java deleted file mode 100644 index d07eb11a..00000000 --- a/idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatImportOptimizer.java +++ /dev/null @@ -1,92 +0,0 @@ -/* - * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.palantir.javaformat.intellij; - -import com.google.common.util.concurrent.Runnables; -import com.intellij.ide.highlighter.JavaFileType; -import com.intellij.lang.ImportOptimizer; -import com.intellij.openapi.editor.Document; -import com.intellij.openapi.project.Project; -import com.intellij.psi.PsiDocumentManager; -import com.intellij.psi.PsiFile; -import com.palantir.javaformat.java.FormatterException; -import com.palantir.javaformat.java.FormatterService; -import java.util.Optional; -import org.jetbrains.annotations.NotNull; - -public class PalantirJavaFormatImportOptimizer implements ImportOptimizer { - - private final Optional formatterService; - - public PalantirJavaFormatImportOptimizer(Optional formatterService) { - this.formatterService = formatterService; - } - - @Override - public boolean supports(@NotNull PsiFile file) { - return JavaFileType.INSTANCE.equals(file.getFileType()) - && PalantirJavaFormatSettings.getInstance(file.getProject()).isEnabled(); - } - - @Override - public @NotNull Runnable processFile(@NotNull PsiFile file) { - if (formatterService.isEmpty()) { - Notifications.displayParsingErrorNotification(file.getProject(), file.getName()); - return Runnables.doNothing(); - } - Project project = file.getProject(); - - PsiDocumentManager documentManager = PsiDocumentManager.getInstance(project); - Document document = documentManager.getDocument(file); - - if (document == null) { - return Runnables.doNothing(); - } - - final String origText = document.getText(); - String text; - try { - text = formatterService.get().fixImports(origText); - } catch (FormatterException e) { - Notifications.displayParsingErrorNotification(project, file.getName()); - return Runnables.doNothing(); - } - - // pointless to change document text if it hasn't changed, plus this can interfere with - // e.g. PalantirJavaFormattingService's output, i.e. it can overwrite the results from the main - // formatter. - if (text.equals(origText)) { - return Runnables.doNothing(); - } - - return () -> { - if (documentManager.isDocumentBlockedByPsi(document)) { - documentManager.doPostponedOperationsAndUnblockDocument(document); - } - - // similarly to above, don't overwrite new document text if it has changed - we use - // getCharsSequence() as we should have `writeAction()` (which I think means effectively a - // write-lock) and it saves calling getText(), which apparently is expensive. - CharSequence newText = document.getCharsSequence(); - if (CharSequence.compare(origText, newText) != 0) { - return; - } - - document.setText(text); - }; - } -} diff --git a/idea-plugin/src/test/java/com/palantir/javaformat/intellij/PalantirJavaFormatImportOptimizerTest.java b/idea-plugin/src/test/java/com/palantir/javaformat/intellij/PalantirJavaFormatImportOptimizerTest.java deleted file mode 100644 index 5b097731..00000000 --- a/idea-plugin/src/test/java/com/palantir/javaformat/intellij/PalantirJavaFormatImportOptimizerTest.java +++ /dev/null @@ -1,185 +0,0 @@ -/* - * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.palantir.javaformat.intellij; - -import static org.assertj.core.api.Assertions.assertThat; - -import com.google.common.collect.ImmutableList; -import com.intellij.codeInsight.actions.OptimizeImportsProcessor; -import com.intellij.formatting.service.FormattingService; -import com.intellij.lang.ImportOptimizer; -import com.intellij.openapi.command.WriteCommandAction; -import com.intellij.openapi.project.Project; -import com.intellij.openapi.projectRoots.JavaSdk; -import com.intellij.openapi.projectRoots.Sdk; -import com.intellij.openapi.roots.ProjectRootManager; -import com.intellij.openapi.vfs.VirtualFile; -import com.intellij.psi.PsiDocumentManager; -import com.intellij.psi.PsiFile; -import com.intellij.testFramework.ExtensionTestUtil; -import com.intellij.testFramework.LightProjectDescriptor; -import com.intellij.testFramework.fixtures.DefaultLightProjectDescriptor; -import com.intellij.testFramework.fixtures.IdeaProjectTestFixture; -import com.intellij.testFramework.fixtures.IdeaTestFixtureFactory; -import com.intellij.testFramework.fixtures.JavaCodeInsightTestFixture; -import com.intellij.testFramework.fixtures.JavaTestFixtureFactory; -import com.intellij.testFramework.fixtures.TestFixtureBuilder; -import com.palantir.javaformat.intellij.PalantirJavaFormatSettings.State; -import java.io.File; -import java.io.IOException; -import java.util.Set; -import java.util.stream.Collectors; -import org.jetbrains.annotations.NotNull; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -public class PalantirJavaFormatImportOptimizerTest { - private JavaCodeInsightTestFixture fixture; - private DelegatingFormatter delegatingFormatter; - - @BeforeEach - public void setUp() throws Exception { - TestFixtureBuilder projectBuilder = IdeaTestFixtureFactory.getFixtureFactory() - .createLightFixtureBuilder(getProjectDescriptor(), getClass().getName()); - fixture = JavaTestFixtureFactory.getFixtureFactory().createCodeInsightFixture(projectBuilder.getFixture()); - fixture.setUp(); - - delegatingFormatter = new DelegatingFormatter(); - ExtensionTestUtil.maskExtensions( - FormattingService.EP_NAME, ImmutableList.of(delegatingFormatter), fixture.getProjectDisposable()); - - PalantirJavaFormatSettings settings = PalantirJavaFormatSettings.getInstance(fixture.getProject()); - State resetState = new State(); - resetState.setEnabled("true"); - settings.loadState(resetState); - } - - protected Project getProject() { - return fixture.getProject(); - } - - @NotNull - protected LightProjectDescriptor getProjectDescriptor() { - return new DefaultLightProjectDescriptor() { - @Override - public Sdk getSdk() { - try { - return JavaSdk.getInstance() - .createJdk( - "java 1.11", new File(System.getProperty("java.home")).getCanonicalPath(), false); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - }; - } - - @AfterEach - public void tearDown() throws Exception { - fixture.tearDown(); - } - - @Test - public void removesUnusedImports() throws Exception { - PsiFile file = createPsiFile( - "com/foo/ImportTest.java", - "package com.foo;", - "import java.util.List;", - "import java.util.ArrayList;", - "import java.util.Map;", - "public class ImportTest {", - "static final Map map;", - "}"); - OptimizeImportsProcessor processor = new OptimizeImportsProcessor(file.getProject(), file); - WriteCommandAction.runWriteCommandAction(file.getProject(), () -> { - ProjectRootManager.getInstance(getProject()) - .setProjectSdk(getProjectDescriptor().getSdk()); - processor.run(); - PsiDocumentManager.getInstance(file.getProject()).commitAllDocuments(); - }); - - assertThat(file.getText()).doesNotContain("List"); - assertThat(file.getText()).contains("import java.util.Map;"); - assertThat(delegatingFormatter.wasInvoked()).isTrue(); - } - - @Test - public void reordersImports() throws Exception { - PsiFile file = createPsiFile( - "com/foo/ImportTest.java", - "package com.foo;", - "import java.util.List;", - "import java.util.ArrayList;", - "import java.util.Map;", - "public class ImportTest {", - "static final ArrayList arrayList;", - "static final List list;", - "static final Map map;", - "}"); - OptimizeImportsProcessor processor = new OptimizeImportsProcessor(file.getProject(), file); - WriteCommandAction.runWriteCommandAction(file.getProject(), () -> { - ProjectRootManager.getInstance(getProject()) - .setProjectSdk(getProjectDescriptor().getSdk()); - processor.run(); - PsiDocumentManager.getInstance(file.getProject()).commitAllDocuments(); - }); - - assertThat(file.getText()) - .contains("import java.util.ArrayList;\n" + "import java.util.List;\n" + "import java.util.Map;\n"); - assertThat(delegatingFormatter.wasInvoked()).isTrue(); - } - - private PsiFile createPsiFile(String path, String... contents) throws IOException { - VirtualFile virtualFile = fixture.getTempDirFixture().createFile(path, String.join("\n", contents)); - fixture.configureFromExistingVirtualFile(virtualFile); - PsiFile psiFile = fixture.getFile(); - assertThat(psiFile).isNotNull(); - return psiFile; - } - - private static final class DelegatingFormatter extends PalantirJavaFormatFormattingService { - - private boolean invoked = false; - - private boolean wasInvoked() { - return invoked; - } - - @Override - public @NotNull Set getImportOptimizers(@NotNull PsiFile file) { - return super.getImportOptimizers(file).stream().map(this::wrap).collect(Collectors.toUnmodifiableSet()); - } - - private ImportOptimizer wrap(ImportOptimizer delegate) { - return new ImportOptimizer() { - @Override - public boolean supports(@NotNull PsiFile file) { - return delegate.supports(file); - } - - @Override - public @NotNull Runnable processFile(@NotNull PsiFile file) { - return () -> { - invoked = true; - delegate.processFile(file).run(); - }; - } - }; - } - } -}