From 1c2330e598feab71cf38cd72ac3626b7d56ca02b Mon Sep 17 00:00:00 2001 From: Mihai Toader Date: Fri, 30 Aug 2024 15:24:10 -0700 Subject: [PATCH] [#6664]: Enable External Workspace completions and tests Also add a CharFilter impl that allows `@` to filter the lookup elements and not hide the lookup Other issues: #505 --- base/src/META-INF/blaze-base.xml | 1 + .../completion/BuildLabelCharFilter.java | 46 ++++++++++ .../completion/BuildLookupElement.java | 2 +- .../ExternalWorkspaceLookupElement.java | 83 +++++++++++++++++ .../ExternalWorkspaceReferenceFragment.java | 49 ++++++++-- .../buildfile/references/LabelReference.java | 2 - .../base/sync/workspace/WorkspaceHelper.java | 24 ++--- .../ExternalWorkspaceCompletionTest.java | 89 +++++++++++++++++++ .../BuildFileIntegrationTestCase.java | 2 +- .../blaze/cpp/BlazeConfigurationResolver.java | 2 +- .../cpp/BlazeConfigurationResolverTest.java | 4 +- 11 files changed, 280 insertions(+), 24 deletions(-) create mode 100644 base/src/com/google/idea/blaze/base/lang/buildfile/completion/BuildLabelCharFilter.java create mode 100644 base/src/com/google/idea/blaze/base/lang/buildfile/completion/ExternalWorkspaceLookupElement.java create mode 100644 base/tests/integrationtests/com/google/idea/blaze/base/lang/buildfile/completion/ExternalWorkspaceCompletionTest.java diff --git a/base/src/META-INF/blaze-base.xml b/base/src/META-INF/blaze-base.xml index 4c93d7ed3ac..6e8c748b56f 100644 --- a/base/src/META-INF/blaze-base.xml +++ b/base/src/META-INF/blaze-base.xml @@ -376,6 +376,7 @@ + diff --git a/base/src/com/google/idea/blaze/base/lang/buildfile/completion/BuildLabelCharFilter.java b/base/src/com/google/idea/blaze/base/lang/buildfile/completion/BuildLabelCharFilter.java new file mode 100644 index 00000000000..169061a83c1 --- /dev/null +++ b/base/src/com/google/idea/blaze/base/lang/buildfile/completion/BuildLabelCharFilter.java @@ -0,0 +1,46 @@ +/* + * Copyright 2024 The Bazel Authors. 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.google.idea.blaze.base.lang.buildfile.completion; + +import com.google.idea.blaze.base.lang.buildfile.language.BuildFileLanguage; +import com.google.idea.blaze.base.lang.buildfile.psi.StringLiteral; +import com.intellij.codeInsight.lookup.CharFilter; +import com.intellij.codeInsight.lookup.Lookup; +import com.intellij.psi.PsiFile; +import com.intellij.psi.util.PsiTreeUtil; +import org.jetbrains.annotations.NotNull; + +import javax.annotation.Nullable; + +/** Allows '@' to be typed (and appended to completion) inside a label from a build file. */ +public final class BuildLabelCharFilter extends CharFilter { + @Nullable + public CharFilter.Result acceptChar(char c, int prefixLength, @NotNull Lookup lookup) { + if ('@' == c && lookup.isCompletion()) { + + // check that we are in a proper place to complete. Unfortunately the CharFilter extension points are not + // langauge and / or location aware so we need the below hack to + PsiFile file = lookup.getPsiFile(); + if (file != null && + file.getLanguage() == BuildFileLanguage.INSTANCE && + PsiTreeUtil.getParentOfType(lookup.getPsiElement(), StringLiteral.class) != null) { + return Result.ADD_TO_PREFIX; + } + } + + return null; + } +} diff --git a/base/src/com/google/idea/blaze/base/lang/buildfile/completion/BuildLookupElement.java b/base/src/com/google/idea/blaze/base/lang/buildfile/completion/BuildLookupElement.java index 9e99d40ff8c..12d884ae605 100644 --- a/base/src/com/google/idea/blaze/base/lang/buildfile/completion/BuildLookupElement.java +++ b/base/src/com/google/idea/blaze/base/lang/buildfile/completion/BuildLookupElement.java @@ -43,7 +43,7 @@ public BuildLookupElement(String baseName, QuoteType quoteWrapping) { this.wrapWithQuotes = quoteWrapping != QuoteType.NoQuotes; } - private static boolean insertClosingQuotes() { + protected static boolean insertClosingQuotes() { return CodeInsightSettings.getInstance().AUTOINSERT_PAIR_QUOTE; } diff --git a/base/src/com/google/idea/blaze/base/lang/buildfile/completion/ExternalWorkspaceLookupElement.java b/base/src/com/google/idea/blaze/base/lang/buildfile/completion/ExternalWorkspaceLookupElement.java new file mode 100644 index 00000000000..f1deaebaca0 --- /dev/null +++ b/base/src/com/google/idea/blaze/base/lang/buildfile/completion/ExternalWorkspaceLookupElement.java @@ -0,0 +1,83 @@ +package com.google.idea.blaze.base.lang.buildfile.completion; + +import com.google.idea.blaze.base.lang.buildfile.psi.StringLiteral; +import com.google.idea.blaze.base.lang.buildfile.references.QuoteType; +import com.google.idea.blaze.base.model.primitives.ExternalWorkspace; +import com.intellij.codeInsight.completion.InsertionContext; +import com.intellij.openapi.editor.Document; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.PlatformIcons; +import org.jetbrains.annotations.Nullable; + +import javax.swing.*; + +public class ExternalWorkspaceLookupElement extends BuildLookupElement { + private final ExternalWorkspace workspace; + + public ExternalWorkspaceLookupElement(ExternalWorkspace workspace) { + super('@' + workspace.repoName(), QuoteType.NoQuotes); + this.workspace = workspace; + } + + @Override + public String getLookupString() { + return super.getItemText() + "//"; + } + + @Override + @Nullable + protected String getTypeText() { + return !workspace.repoName().equals(workspace.name()) ? '@' + workspace.name() : null; + } + + @Override + @Nullable + protected String getTailText() { + return "//"; + } + + @Override + public @Nullable Icon getIcon() { + return PlatformIcons.LIBRARY_ICON; + } + + @Override + public void handleInsert(InsertionContext context) { + StringLiteral literal = PsiTreeUtil.findElementOfClassAtOffset(context.getFile(), context.getStartOffset(), StringLiteral.class, false); + if (literal == null) { + super.handleInsert(context); + return; + } + + Document document = context.getDocument(); + context.commitDocument(); + // find an remove trailing package path after insert / replace. + // current element text looks like `@workspace//`. If this is complete inside an existing workspace name the + // result would look like: @workspace//old_workspace_path//. The following bit will remove `old_workspace_path//` + int indexOfPackageStart = literal.getText().lastIndexOf("//"); + if (indexOfPackageStart != -1) { + // shift to be a document offset + indexOfPackageStart += literal.getTextOffset() + 2; + if (context.getSelectionEndOffset() < indexOfPackageStart) { + document.deleteString(context.getSelectionEndOffset(), indexOfPackageStart); + context.commitDocument(); + } + } + + // handle auto insertion of end quote. This will have to be if the completion is triggered inside a `"@ workspaceRootCache = SyncCache.getInstance(project) diff --git a/base/tests/integrationtests/com/google/idea/blaze/base/lang/buildfile/completion/ExternalWorkspaceCompletionTest.java b/base/tests/integrationtests/com/google/idea/blaze/base/lang/buildfile/completion/ExternalWorkspaceCompletionTest.java new file mode 100644 index 00000000000..2a563692234 --- /dev/null +++ b/base/tests/integrationtests/com/google/idea/blaze/base/lang/buildfile/completion/ExternalWorkspaceCompletionTest.java @@ -0,0 +1,89 @@ +package com.google.idea.blaze.base.lang.buildfile.completion; + +import com.google.common.collect.ImmutableList; +import com.google.idea.blaze.base.ExternalWorkspaceFixture; +import com.google.idea.blaze.base.lang.buildfile.BuildFileIntegrationTestCase; +import com.google.idea.blaze.base.lang.buildfile.psi.BuildFile; +import com.google.idea.blaze.base.model.ExternalWorkspaceData; +import com.google.idea.blaze.base.model.primitives.ExternalWorkspace; +import com.google.idea.blaze.base.model.primitives.WorkspacePath; +import com.intellij.codeInsight.CodeInsightSettings; +import com.intellij.openapi.editor.Editor; +import com.intellij.psi.PsiFile; +import org.junit.Ignore; +import org.junit.Test; + +import static com.google.common.truth.Truth.assertThat; + +public class ExternalWorkspaceCompletionTest extends BuildFileIntegrationTestCase { + protected ExternalWorkspaceFixture workspaceOne; + protected ExternalWorkspaceFixture workspaceTwoMapped; + + @Override + protected ExternalWorkspaceData mockExternalWorkspaceData() { + workspaceOne = new ExternalWorkspaceFixture( + ExternalWorkspace.create("workspace_one", "workspace_one"), fileSystem); + + workspaceTwoMapped = new ExternalWorkspaceFixture( + ExternalWorkspace.create("workspace_two", "com_workspace_two"), fileSystem); + + return ExternalWorkspaceData.create( + ImmutableList.of(workspaceOne.workspace, workspaceTwoMapped.workspace)); + } + + @Test + public void testEmptyLabelCompletion() throws Throwable { + PsiFile file = testFixture.configureByText("BUILD", "''"); + + String[] strings = editorTest.getCompletionItemsAsStrings(); + assertThat(strings).hasLength(2); + assertThat(strings) + .asList() + .containsExactly("@com_workspace_two//", "@workspace_one//"); + } + + @Test + public void testCompleteWillIncludeSlashes () { + PsiFile file = testFixture.configureByText("BUILD", "'@com'"); + assertThat(editorTest.completeIfUnique()).isTrue(); + + assertFileContents(file, "'@com_workspace_two//'"); + } + + @Test + public void testCompleteWillFixUpRemainingSlashed () { + PsiFile file = testFixture.configureByText("BUILD", "'@com//'"); + assertThat(editorTest.completeIfUnique()).isTrue(); + + assertFileContents(file, "'@com_workspace_two//'"); + } + + @Test + public void testCompleteWillAlwaysReplaceWorkspace() { + PsiFile file = testFixture.configureByText("BUILD", "'@comxxxx//'"); + assertThat(editorTest.completeIfUnique()).isTrue(); + + assertFileContents(file, "'@com_workspace_two//'"); + } + + @Test + public void testCompleteWillRespectAutoQuoting() { + boolean old = CodeInsightSettings.getInstance().AUTOINSERT_PAIR_QUOTE; + try { + CodeInsightSettings.getInstance().AUTOINSERT_PAIR_QUOTE = false; + PsiFile file = testFixture.configureByText("BUILD", "'@com"); + + assertThat(editorTest.completeIfUnique()).isTrue(); + assertFileContents(file, "'@com_workspace_two//"); + + CodeInsightSettings.getInstance().AUTOINSERT_PAIR_QUOTE = true; + file = testFixture.configureByText("BUILD", "'@com"); + + assertThat(editorTest.completeIfUnique()).isTrue(); + assertFileContents(file, "'@com_workspace_two//'"); + + } finally { + CodeInsightSettings.getInstance().AUTOINSERT_PAIR_QUOTE = old; + } + } +} diff --git a/base/tests/utils/integration/com/google/idea/blaze/base/lang/buildfile/BuildFileIntegrationTestCase.java b/base/tests/utils/integration/com/google/idea/blaze/base/lang/buildfile/BuildFileIntegrationTestCase.java index a2f8ce53d7f..3728c040bdb 100644 --- a/base/tests/utils/integration/com/google/idea/blaze/base/lang/buildfile/BuildFileIntegrationTestCase.java +++ b/base/tests/utils/integration/com/google/idea/blaze/base/lang/buildfile/BuildFileIntegrationTestCase.java @@ -91,6 +91,6 @@ protected File getExternalSourceRoot() { BlazeProjectData blazeProjectData = BlazeProjectDataManager.getInstance(getProject()).getBlazeProjectData(); assertThat(blazeProjectData).isNotNull(); - return WorkspaceHelper.getExternalSourceRoot(blazeProjectData); + return WorkspaceHelper.getExternalSourceRoot(blazeProjectData).toFile(); } } diff --git a/cpp/src/com/google/idea/blaze/cpp/BlazeConfigurationResolver.java b/cpp/src/com/google/idea/blaze/cpp/BlazeConfigurationResolver.java index 404c0bd5855..c913d85c9ab 100644 --- a/cpp/src/com/google/idea/blaze/cpp/BlazeConfigurationResolver.java +++ b/cpp/src/com/google/idea/blaze/cpp/BlazeConfigurationResolver.java @@ -149,7 +149,7 @@ private static WorkspacePath getWorkspacePathForExternalTarget( Project project, WorkspacePathResolver workspacePathResolver) { if (target.toTargetInfo().getLabel().isExternal()) { - WorkspaceRoot externalWorkspace = WorkspaceHelper.resolveExternalWorkspace(project, + WorkspaceRoot externalWorkspace = WorkspaceHelper.getExternalWorkspace(project, target.getKey().getLabel().externalWorkspaceName()); if (externalWorkspace != null) { diff --git a/cpp/tests/unittests/com/google/idea/blaze/cpp/BlazeConfigurationResolverTest.java b/cpp/tests/unittests/com/google/idea/blaze/cpp/BlazeConfigurationResolverTest.java index 1b69d7450f2..fc479a858c6 100644 --- a/cpp/tests/unittests/com/google/idea/blaze/cpp/BlazeConfigurationResolverTest.java +++ b/cpp/tests/unittests/com/google/idea/blaze/cpp/BlazeConfigurationResolverTest.java @@ -826,7 +826,7 @@ public void testExternalDependencyResolvedWhenIsPartOfProject() throws IOExcepti .build(); File externalRoot = WorkspaceHelper.getExternalSourceRoot( - BlazeProjectDataManager.getInstance(project).getBlazeProjectData()); + BlazeProjectDataManager.getInstance(project).getBlazeProjectData()).toFile(); File spyExternalDependencyRoot = spy(new File(externalRoot, "external_dependency")); doReturn(true).when(spyExternalDependencyRoot).isDirectory(); @@ -844,7 +844,7 @@ public void testExternalDependencyResolvedWhenIsPartOfProject() throws IOExcepti try (MockedStatic mockedStatic = Mockito.mockStatic(WorkspaceHelper.class)) { mockedStatic.when( - () -> WorkspaceHelper.resolveExternalWorkspace(Mockito.any(MockProject.class), + () -> WorkspaceHelper.getExternalWorkspace(Mockito.any(MockProject.class), Mockito.any(String.class))).thenReturn(new WorkspaceRoot(spyExternalDependencyRoot)); assertThatResolving(projectView, targetMap).producesConfigurationsFor(