-
Notifications
You must be signed in to change notification settings - Fork 305
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
---|---|---|
@@ -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 |
---|---|---|
|
@@ -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; | ||
|
@@ -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 '//') */ | ||
|
@@ -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; | ||
} | ||
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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
} | ||
|
||
return null; | ||
} | ||
|
||
@Nullable | ||
|
@@ -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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.