[#6664] Resolve a workspace label across external repositories - 4/n (#…

…6700)

* [#6664] Resolve a workspace label across external repositories - 4/n

* Fix tests execution

* Minor simplification

* Small cleanups

* Set the `bazel.read.external.workspace.data` registry key to true

* Improve fixtures and simplify tests

* Fix compilation

* Address some comments

* Replace the length based caret setting with the `configureByText`

base/src/META-INF/blaze-base.xml

@@ -411,7 +411,7 @@
     <registryKey defaultValue="false"
                  description="The Bazel Plugin uses the --target_pattern_file option to pass the targets via a file. By default, the file is deleted immediately after the build execution is finished. Set this flag to true to avoid file deletion"
                  key="bazel.sync.keep.target.files"/>
-    <registryKey defaultValue="false"
+    <registryKey defaultValue="true"
                  description="Causes the plugin to read external workspace data, enabling features such as code completion for external targets. This may incur the cost of an additional 'bazel mod' call, which could cause issues with older versions or setups."
                  key="bazel.read.external.workspace.data"/>
     <editorNotificationProvider implementation="com.google.idea.blaze.base.wizard2.BazelNotificationProvider"/>

base/src/com/google/idea/blaze/base/lang/buildfile/references/LabelReference.java

@@ -32,9 +32,12 @@
 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;
 
 /** Converts a blaze label into an absolute path, then resolves that path to a PsiElements */

base/src/com/google/idea/blaze/base/model/ExternalWorkspaceData.java

@@ -7,6 +7,9 @@
 import com.google.idea.blaze.base.ideinfo.ProtoWrapper;
 import com.google.idea.blaze.base.model.primitives.ExternalWorkspace;
 
+import javax.annotation.Nullable;
+
+
 public final class ExternalWorkspaceData implements ProtoWrapper<ProjectData.ExternalWorkspaceData> {
   public ImmutableMap<String, ExternalWorkspace> workspaces;
 
@@ -22,25 +25,32 @@ public static ExternalWorkspaceData create(ImmutableList<ExternalWorkspace> work
             .stream()
             .collect(
                 ImmutableMap.toImmutableMap(
-                    ExternalWorkspace::name,
+                    ExternalWorkspace::repositoryName,
                     Functions.identity()))
     );
   }
 
   @Override
   public ProjectData.ExternalWorkspaceData toProto() {
-    ImmutableList<ProjectData.ExternalWorkspace> protoWorkspaces = workspaces
-        .values()
-        .stream()
-        .map(ExternalWorkspace::toProto)
-        .collect(ImmutableList.toImmutableList());
-
-    return ProjectData.ExternalWorkspaceData.newBuilder()
-        .addAllWorkspaces(protoWorkspaces)
-        .build();
+    ProjectData.ExternalWorkspaceData.Builder builder = ProjectData.ExternalWorkspaceData.newBuilder();
+
+    for (ExternalWorkspace externalWorkspace : workspaces.values()) {
+      builder = builder.addWorkspaces(externalWorkspace.toProto());
+    }
+
+    return builder.build();
   }
 
   public static ExternalWorkspaceData fromProto(ProjectData.ExternalWorkspaceData proto) {
-    return new ExternalWorkspaceData(proto.getWorkspacesList().stream().map(ExternalWorkspace::fromProto).collect(ImmutableList.toImmutableList()));
+    return new ExternalWorkspaceData(
+        proto.getWorkspacesList()
+            .stream()
+            .map(ExternalWorkspace::fromProto)
+            .collect(ImmutableList.toImmutableList()));
+  }
+
+  @Nullable
+  public ExternalWorkspace getByRepoName(String name) {
+    return workspaces.get(name);
   }
 }

base/src/com/google/idea/blaze/base/model/primitives/ExternalWorkspace.java

@@ -13,24 +13,28 @@ public abstract class ExternalWorkspace implements ProtoWrapper<ProjectData.Exte
   public abstract String name();
 
   @Nullable
-  public abstract String repoName();
+  protected abstract String repoName();
+
+  public String repositoryName() {
+    return repoName() != null ? repoName() : name();
+  }
 
   public static ExternalWorkspace fromProto(ProjectData.ExternalWorkspace proto) {
     return create(proto.getName(), proto.getRepoName());
   }
 
   @Override
   public ProjectData.ExternalWorkspace toProto() {
-    ProjectData.ExternalWorkspace.Builder builder = ProjectData.ExternalWorkspace.newBuilder().setName(name());
-    if (repoName() != null && !repoName().isEmpty()) {
-      builder = builder.setRepoName(repoName());
-    }
-    return builder.build();
+    return
+        ProjectData.ExternalWorkspace.newBuilder()
+            .setName(name())
+            .setRepoName(repoName())
+            .build();
   }
 
   public static ExternalWorkspace create(String name, String repoName) {
     ExternalWorkspace.Builder builder = ExternalWorkspace.builder().setName(name);
-    if (repoName != null && !repoName.isEmpty()) {
+    if (repoName != null && !repoName.isEmpty() && repoName.compareTo(name) != 0) {
       builder = builder.setRepoName(repoName);
     }
     return builder.build();
@@ -42,10 +46,10 @@ public static ExternalWorkspace.Builder builder() {
 
   @AutoValue.Builder
   public abstract static class Builder {
-    public abstract Builder setName(String name);
+    abstract Builder setName(String name);
 
-    public abstract Builder setRepoName(String repoName);
+    abstract Builder setRepoName(String repoName);
 
-    public abstract ExternalWorkspace build();
+    abstract ExternalWorkspace build();
   }
 }

base/src/com/google/idea/blaze/base/sync/workspace/WorkspaceHelper.java

@@ -18,6 +18,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.idea.blaze.base.bazel.BuildSystemProvider;
 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;
 import com.google.idea.blaze.base.model.primitives.TargetName;
 import com.google.idea.blaze.base.model.primitives.WorkspacePath;
@@ -35,7 +36,6 @@
 import java.io.File;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import javax.annotation.Nullable;
@@ -169,8 +169,8 @@ private static Label deriveLabel(
         TargetName.createIfValid(
             FileUtil.getRelativePath(workspace.root.fileForPath(packagePath), file));
     return targetName != null
-        ? Label.create(workspace.externalWorkspaceName, packagePath, targetName)
-        : null;
+               ? Label.create(workspace.externalWorkspaceName, packagePath, targetName)
+               : null;
   }
 
   private static WorkspacePath getPackagePath(
@@ -193,33 +193,42 @@ public static File getExternalSourceRoot(BlazeProjectData projectData) {
 
   @Nullable
   private static synchronized WorkspaceRoot getExternalWorkspaceRootsFile(String workspaceName,
-      Project project) {
+                                                                          Project project) {
     if (Blaze.getBuildSystemName(project) == BuildSystemName.Blaze) {
       return null;
     }
     logger.debug("getExternalWorkspaceRootsFile for " + workspaceName);
-    Map<String, WorkspaceRoot> workspaceRootCache = SyncCache.getInstance(project)
-        .get(WorkspaceHelper.class, (p, data) -> new ConcurrentHashMap<String, WorkspaceRoot>());
+    Map<String, WorkspaceRoot> workspaceRootCache =
+        SyncCache.getInstance(project)
+            .get(WorkspaceHelper.class, (p, data) -> new ConcurrentHashMap<>());
 
     //the null cache value case could happen when the blazeProjectData is null.
-    if(workspaceRootCache == null) {
+    if (workspaceRootCache == null) {
       return null;
     }
 
-    WorkspaceRoot root = null;
     if (workspaceRootCache.containsKey(workspaceName)) {
-      root = workspaceRootCache.get(workspaceName);
-    } else if (getBlazeProjectData(project) != null) {
-      File externalDir = new File(getBlazeProjectData(project).getBlazeInfo().getOutputBase(),
-          "external/" + workspaceName);
+      return workspaceRootCache.get(workspaceName);
+    }
+
+    BlazeProjectData blazeProjectData = BlazeProjectDataManager.getInstance(project).getBlazeProjectData();
+    if (blazeProjectData != null) {
+      File externalBase = new File(blazeProjectData.getBlazeInfo().getOutputBase(), "external");
 
-      if (externalDir.exists() || isInTestMode()) {
-        root = new WorkspaceRoot(externalDir);
+      File workspaceDir = new File(externalBase, workspaceName);
+      ExternalWorkspace workspace = blazeProjectData.getExternalWorkspaceData().getByRepoName(workspaceName);
+      if (workspace != null) {
+        workspaceDir = new File(externalBase, workspace.name());
+      }
+
+      if (workspaceDir.exists() || isInTestMode()) {
+        WorkspaceRoot root = new WorkspaceRoot(workspaceDir);
         workspaceRootCache.put(workspaceName, root);
+        return root;
       }
     }
 
-    return root;
+    return null;
   }
 
   //The unit test use the TempFileSystem to create VirtualFile which does not exist on disk.

base/tests/integrationtests/com/google/idea/blaze/base/lang/buildfile/completion/RuleTargetCompletionTest.java

@@ -24,6 +24,7 @@
 import com.google.idea.blaze.base.lang.buildfile.language.semantics.RuleDefinition;
 import com.google.idea.blaze.base.lang.buildfile.psi.BuildFile;
 import com.google.idea.blaze.base.model.primitives.WorkspacePath;
+import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
 import com.intellij.codeInsight.lookup.LookupElement;
 import com.intellij.openapi.editor.Editor;
 import javax.annotation.Nullable;
@@ -188,6 +189,11 @@ public void testLocalPathIgnoredForNonLocalLabels() throws Throwable {
     assertThat(completionItems).asList().doesNotContain("'//java/com/google:other_rule'");
   }
 
+  @Test
+  public void testExternalRepoCompletion() throws Throwable {
+
+  }
+
   private static void setBuildLanguageSpecRules(
       MockBuildLanguageSpecProvider specProvider, String... ruleNames) {
     ImmutableMap.Builder<String, RuleDefinition> rules = ImmutableMap.builder();

base/tests/integrationtests/com/google/idea/blaze/base/lang/buildfile/findusages/ExternalWorkspaceFindUsagesTest.java

@@ -24,6 +24,7 @@
 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.lang.buildfile.search.FindUsages;
+import com.google.idea.blaze.base.model.BlazeProjectData;
 import com.google.idea.blaze.base.model.primitives.WorkspacePath;
 import com.google.idea.blaze.base.settings.BuildSystemName;
 import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
@@ -140,16 +141,4 @@ public void testFindUsagesFromExternalWorkspaceFileShortFormLabel() {
     assertThat(references).hasLength(1);
     assertThat(references[0].getElement()).isEqualTo(label);
   }
-
-  private PsiFile createFileInExternalWorkspace(
-      String workspaceName, WorkspacePath path, String... contents) {
-    String filePath =
-        Paths.get(getExternalSourceRoot().getPath(), workspaceName, path.relativePath()).toString();
-    return fileSystem.createPsiFile(filePath, contents);
-  }
-
-  private File getExternalSourceRoot() {
-    return WorkspaceHelper.getExternalSourceRoot(
-        BlazeProjectDataManager.getInstance(getProject()).getBlazeProjectData());
-  }
 }

base/tests/integrationtests/com/google/idea/blaze/base/lang/buildfile/references/ExternalWorkspaceReferenceBzlModeTest.java

@@ -0,0 +1,92 @@
+package com.google.idea.blaze.base.lang.buildfile.references;
+
+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.Label;
+import com.google.idea.blaze.base.model.primitives.TargetName;
+import com.google.idea.blaze.base.model.primitives.WorkspacePath;
+import com.intellij.codeInsight.navigation.actions.GotoDeclarationAction;
+import com.intellij.openapi.editor.Editor;
+import com.intellij.psi.PsiElement;
+import com.intellij.psi.PsiFile;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertNotNull;
+
+@RunWith(JUnit4.class)
+public class ExternalWorkspaceReferenceBzlModeTest 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.w, workspaceTwoMapped.w));
+  }
+
+  @Before
+  public void doSetupExternalWorkspaces() {
+    workspaceOne.createBuildFile(
+        new WorkspacePath("p1/p2/BUILD"),
+        "java_library(name = 'rule1')");
+
+    workspaceTwoMapped.createBuildFile(
+        new WorkspacePath("p1/p2/BUILD"),
+        "java_library(name = 'rule1')");
+  }
+
+  @Test
+  public void testUnmappedExternalWorkspace() throws Throwable {
+    Label targetLabel = workspaceOne.createLabel(
+        new WorkspacePath("p1/p2"), TargetName.create("rule1"));
+
+    PsiFile file = testFixture.configureByText("BUILD",
+        String.format("""
+            java_library(
+                name = 'lib',
+                deps = ['%s<caret>']
+            )""", targetLabel));
+    Editor editor = editorTest.openFileInEditor(file.getVirtualFile());
+
+    PsiElement target =
+        GotoDeclarationAction.findTargetElement(
+            getProject(), editor, editor.getCaretModel().getOffset());
+
+    assertThat(target).isNotNull();
+  }
+
+  @Test
+  public void testRemappedExternalWorkspace() throws Throwable {
+    Label targetLabel = workspaceTwoMapped.createLabel(
+        new WorkspacePath("p1/p2"), TargetName.create("rule1"));
+
+    PsiFile file = testFixture.configureByText("BUILD",
+        String.format("""
+            java_library(
+                name = 'lib',
+                deps = ['%s<caret>']
+            )""", targetLabel));
+    Editor editor = editorTest.openFileInEditor(file.getVirtualFile());
+
+    PsiElement target =
+        GotoDeclarationAction.findTargetElement(
+            getProject(), editor, editor.getCaretModel().getOffset());
+
+    assertThat(target).isNotNull();
+  }
+}

base/tests/integrationtests/com/google/idea/blaze/base/lang/buildfile/references/ExternalWorkspaceReferenceTest.java

@@ -25,13 +25,10 @@
 import com.google.idea.blaze.base.lang.buildfile.psi.LoadStatement;
 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.ExternalWorkspaceData;
 import com.google.idea.blaze.base.model.primitives.WorkspacePath;
 import com.google.idea.blaze.base.settings.BuildSystemName;
-import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
-import com.google.idea.blaze.base.sync.workspace.WorkspaceHelper;
 import com.intellij.psi.PsiFile;
-import java.io.File;
-import java.nio.file.Paths;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -223,15 +220,8 @@ public void testRelativeLoadInExternalWorkspace() {
     assertThat(load.getImportedSymbolElements()[0].getLoadedElement()).isEqualTo(function);
   }
 
-  private PsiFile createFileInExternalWorkspace(
-      String workspaceName, WorkspacePath path, String... contents) {
-    String filePath =
-        Paths.get(getExternalSourceRoot().getPath(), workspaceName, path.relativePath()).toString();
-    return fileSystem.createPsiFile(filePath, contents);
-  }
-
-  private File getExternalSourceRoot() {
-    return WorkspaceHelper.getExternalSourceRoot(
-        BlazeProjectDataManager.getInstance(getProject()).getBlazeProjectData());
+  @Override
+  protected ExternalWorkspaceData mockExternalWorkspaceData() {
+    return super.mockExternalWorkspaceData();
   }
 }