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

[#6664,#505]: Enable External Workspace completions and tests - 5/n #6730

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
1 change: 1 addition & 0 deletions base/src/META-INF/blaze-base.xml
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@
<codeInsight.lineMarkerProvider
language="BUILD"
implementationClass="com.google.idea.blaze.base.query.MacroLineMarkerProvider"/>
<lookup.charFilter implementation="com.google.idea.blaze.base.lang.buildfile.completion.BuildLabelCharFilter"/>
<runLineMarkerContributor
language="BUILD"
implementationClass="com.google.idea.blaze.base.run.producers.BuildFileRunLineMarkerContributor"/>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* 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 (!lookup.isCompletion()) {
return null;
}

PsiFile file = lookup.getPsiFile();
if (file == null ||
file.getLanguage() != BuildFileLanguage.INSTANCE ||
PsiTreeUtil.getParentOfType(lookup.getPsiElement(), StringLiteral.class) == null) {
return null;
}

switch (c) {
case '@':
return Result.ADD_TO_PREFIX;

case '/':
if (lookup.getCurrentItem() instanceof ExternalWorkspaceLookupElement) {
return Result.SELECT_ITEM_AND_FINISH_LOOKUP;
}
}

return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
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.codeInsight.lookup.AutoCompletionPolicy;
import com.intellij.openapi.editor.CaretModel;
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();

// if we completed by pressing '/' (since this lookup element should never complete using '/') .
if (context.getCompletionChar() == '/') {
context.setAddCompletionChar(false);
}

CaretModel caret = context.getEditor().getCaretModel();
// 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<caret>old_workspace_path//. The following bit will remove `old_workspace_path//`
int replaceStart = context.getTailOffset();
int replaceEnd = context.getTailOffset();

int indexOfPackageStart = literal.getText().lastIndexOf("//");
if (indexOfPackageStart != -1) {
// shift to be a document offset
replaceEnd = indexOfPackageStart + 2 + literal.getTextOffset();
}

document.replaceString(replaceStart, replaceEnd, "//");
context.commitDocument();
caret.moveToOffset(replaceStart + 2);

// handle auto insertion of end quote. This will have to be if the completion is triggered inside a `"@<caret` bit
if (insertClosingQuotes()) {
QuoteType quoteType = literal.getQuoteType();
if (quoteType != QuoteType.NoQuotes && !literal.getText().endsWith(quoteType.quoteString)) {
document.insertString(literal.getTextOffset() + literal.getTextLength(), quoteType.quoteString);
context.commitDocument();
}
}

super.handleInsert(context);
}

@Override
public boolean requiresCommittedDocuments() {
return false;
}

@Override
public AutoCompletionPolicy getAutoCompletionPolicy() {
return AutoCompletionPolicy.ALWAYS_AUTOCOMPLETE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,19 @@
*/
package com.google.idea.blaze.base.lang.buildfile.references;

import com.google.idea.blaze.base.lang.buildfile.completion.BuildLookupElement;
import com.google.idea.blaze.base.lang.buildfile.completion.ExternalWorkspaceLookupElement;
import com.google.idea.blaze.base.lang.buildfile.psi.BuildElement;
import com.google.idea.blaze.base.lang.buildfile.psi.BuildFile;
import com.google.idea.blaze.base.lang.buildfile.psi.FuncallExpression;
import com.google.idea.blaze.base.lang.buildfile.psi.StringLiteral;
import com.google.idea.blaze.base.lang.buildfile.psi.util.PsiUtils;
import com.google.idea.blaze.base.model.BlazeProjectData;
import com.google.idea.blaze.base.model.primitives.WorkspacePath;
import com.google.idea.blaze.base.model.primitives.WorkspaceRoot;
import com.google.idea.blaze.base.settings.Blaze;
import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
import com.google.idea.blaze.base.sync.workspace.WorkspaceHelper;
import com.intellij.lang.ASTNode;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.TextRange;
Expand All @@ -30,6 +36,8 @@
import com.intellij.psi.PsiReferenceBase;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.ObjectUtils;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/** The external workspace component of a label (between '@' and '//') */
Expand All @@ -42,26 +50,45 @@ public ExternalWorkspaceReferenceFragment(LabelReference labelReference) {
@Override
public TextRange getRangeInElement() {
String rawText = myElement.getText();
boolean valid = LabelUtils.getExternalWorkspaceComponent(myElement.getStringContents()) != null;
if (!valid) {
String unquotedText = LabelUtils.trimToDummyIdentifier(myElement.getStringContents());
QuoteType quoteType = myElement.getQuoteType();

String externalWorkspace = LabelUtils.getExternalWorkspaceComponent(unquotedText);
if (!unquotedText.trim().isEmpty() && externalWorkspace == null) {
return TextRange.EMPTY_RANGE;
}

int endIndex = rawText.indexOf("//");
if (endIndex == -1) {
endIndex = rawText.length() - 1;
endIndex = rawText.length() - quoteType.quoteString.length();
} else {
endIndex += 2;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still don't cover this scenario, however I'm not sure if we even should

Screen.Recording.2024-09-05.at.11.23.41.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea. I don't think we should do that here.

}
return new TextRange(1, endIndex);
}

@Nullable
@Override
public FuncallExpression resolve() {
public BuildElement resolve() {
String name = LabelUtils.getExternalWorkspaceComponent(myElement.getStringContents());
if (name == null) {
return null;
}

BuildFile workspaceFile = resolveProjectWorkspaceFile(myElement.getProject());
return workspaceFile != null ? workspaceFile.findRule(name) : null;
if (workspaceFile != null) {
FuncallExpression expression = workspaceFile.findRule(name);
if (expression != null) {
return expression;
}
};

WorkspaceRoot workspaceRoot = WorkspaceHelper.getExternalWorkspace(myElement.getProject(), name);
if (workspaceRoot != null) {
return BuildReferenceManager.getInstance(myElement.getProject()).findBuildFile(workspaceRoot.directory());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior is a bit different as it points to a BUILD file, but it's out of scope of this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea. This is slightly wrong, too, since it should point to either a WORKSPACE or a MODULE.xx file anyway. It's a net improvement though and is still helpful for the next step: to root completion of packagePaths from the containing folder

}

return null;
}

@Nullable
Expand All @@ -83,8 +110,16 @@ private static BuildFile resolveProjectWorkspaceFile(Project project) {
}

@Override
public Object[] getVariants() {
return EMPTY_ARRAY;
@Nonnull
public BuildLookupElement[] getVariants() {
BlazeProjectData blazeProjectData = BlazeProjectDataManager.getInstance(myElement.getProject()).getBlazeProjectData();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit worried it could kill the UI in case of a large number of external repositories

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should come from in-memory data though.

if (blazeProjectData == null) {
return BuildLookupElement.EMPTY_ARRAY;
}

return blazeProjectData.getExternalWorkspaceData().workspaces.values().stream()
.map(ExternalWorkspaceLookupElement::new)
.toArray(BuildLookupElement[]::new);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,9 @@
import com.intellij.openapi.vfs.VirtualFileFilter;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiFile;
import com.intellij.psi.PsiReference;
import com.intellij.psi.PsiReferenceBase;
import com.intellij.util.ArrayUtil;
import com.intellij.util.IncorrectOperationException;
import org.jetbrains.annotations.NotNull;

import javax.annotation.Nullable;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import com.google.common.annotations.VisibleForTesting;
import com.google.idea.blaze.base.bazel.BuildSystemProvider;
import com.google.idea.blaze.base.lang.buildfile.psi.BuildFile;
import com.google.idea.blaze.base.model.BlazeProjectData;
import com.google.idea.blaze.base.model.primitives.ExternalWorkspace;
import com.google.idea.blaze.base.model.primitives.Label;
Expand Down Expand Up @@ -68,8 +69,8 @@ private static synchronized BlazeProjectData getBlazeProjectData(Project project
}

@Nullable
public static WorkspaceRoot resolveExternalWorkspace(Project project, String workspaceName) {
return getExternalWorkspaceRootsFile(workspaceName, project);
public static WorkspaceRoot getExternalWorkspace(Project project, String workspaceName) {
return resolveExternalWorkspaceRoot(project, workspaceName, null);
}

/**
Expand All @@ -84,7 +85,7 @@ public static File resolveBlazePackage(Project project, Label label) {
return pathResolver != null ? pathResolver.resolveToFile(label.blazePackage()) : null;
}

WorkspaceRoot root = getExternalWorkspaceRootsFile(label.externalWorkspaceName(), project);
WorkspaceRoot root = resolveExternalWorkspaceRoot(project, label.externalWorkspaceName(), null);
return root != null ? root.fileForPath(label.blazePackage()) : null;
}

Expand Down Expand Up @@ -127,9 +128,7 @@ private static Workspace resolveWorkspace(Project project, File absoluteFile) {
}

BlazeProjectData blazeProjectData = getBlazeProjectData(project);
Path bazelRootPath = Paths.get(
blazeProjectData.getBlazeInfo().getOutputBase().getAbsolutePath(),
"external").normalize();
Path bazelRootPath = getExternalSourceRoot(blazeProjectData);

logger.debug("the bazelRootPath is " + bazelRootPath);
Path path = Paths.get(absoluteFile.getAbsolutePath()).normalize();
Expand Down Expand Up @@ -187,16 +186,21 @@ private static WorkspacePath getPackagePath(
}

@VisibleForTesting
public static File getExternalSourceRoot(BlazeProjectData projectData) {
return new File(projectData.getBlazeInfo().getOutputBase(), "external");
public static Path getExternalSourceRoot(BlazeProjectData projectData) {
return Paths.get(projectData.getBlazeInfo().getOutputBase().getAbsolutePath(), "external").normalize();
}

@Nullable
private static synchronized WorkspaceRoot getExternalWorkspaceRootsFile(String workspaceName,
Project project) {
private static synchronized WorkspaceRoot resolveExternalWorkspaceRoot(
Project project, String workspaceName, @Nullable BuildFile buildFile) {
if (Blaze.getBuildSystemName(project) == BuildSystemName.Blaze) {
return null;
}

if (workspaceName == null || workspaceName.isEmpty()) {
return WorkspaceRoot.fromProjectSafe(project);
}

logger.debug("getExternalWorkspaceRootsFile for " + workspaceName);
Map<String, WorkspaceRoot> workspaceRootCache =
SyncCache.getInstance(project)
Expand Down
Loading
Loading