Improve add dependencies quick fix to handle more cases

bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/edits/AddDependenciesJob.java

@@ -2,6 +2,7 @@
 
 import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 
 import java.util.ArrayList;
@@ -28,6 +29,7 @@
 import com.salesforce.bazel.sdk.model.BazelLabel;
 
 public final class AddDependenciesJob extends WorkspaceJob {
+
     private final BazelProject bazelProject;
     private final Collection<Label> labelsToAdd;
     private final Collection<ClasspathEntry> newClasspathEntries;
@@ -69,10 +71,19 @@ public IStatus runInWorkspace(IProgressMonitor progress) throws CoreException {
             }
 
             // java_library & co
-            updateTargetsUsingAttribute(targetsToUpdate, "deps", monitor.split(1));
+            var updated = updateTargetsUsingAttribute(targetsToUpdate, "deps", monitor.split(1));
 
             // some other macros may use this
-            updateTargetsUsingAttribute(targetsToUpdate, "dependencies", monitor.split(1));
+            updated += updateTargetsUsingAttribute(targetsToUpdate, "dependencies", monitor.split(1));
+
+            // log a message if nothing was updated
+            if (updated == 0) {
+                return Status.info(
+                    format(
+                        "No suitable targets found in '%s' to add dependency %s to.",
+                        bazelProject.getBazelBuildFile(),
+                        labelsToAdd.stream().map(Label::toString).collect(joining(", "))));
+            }
 
         } catch (CoreException e) {
             return Status.error(format("Error updating '%s'. %s", bazelProject, e.getMessage()), e);
@@ -106,15 +117,23 @@ public IStatus runInWorkspace(final IProgressMonitor monitor) throws CoreExcepti
                 var modified = false;
                 Stream.of(container.getClasspathEntries()).map(ClasspathEntry::fromExisting).forEach(classpath::add);
                 for (ClasspathEntry newClasspathEntry : newClasspathEntries) {
-                    if (classpath.stream()
-                            .anyMatch(
+                    var existing = classpath.stream()
+                            .filter(
                                 c -> (c.getEntryKind() == newClasspathEntry.getEntryKind())
-                                        && c.getPath().equals(newClasspathEntry.getPath()))) {
-                        // skip if there is already an existing entry
-                        continue;
+                                        && c.getPath().equals(newClasspathEntry.getPath()))
+                            .findFirst();
+                    if (existing.isPresent()) {
+                        // remove the test attribute in case a runtime dependency is changed to become a compile dependency
+                        if (!existing.get().isTest()) {
+                            // skip if there is already an existing entry
+                            continue;
+                        }
+                        existing.get().setTest(true);
+                        modified = true;
+                    } else {
+                        classpath.add(newClasspathEntry);
+                        modified = true;
                     }
-                    classpath.add(newClasspathEntry);
-                    modified = true;
                 }
 
                 if (modified) {
@@ -130,31 +149,45 @@ public IStatus runInWorkspace(final IProgressMonitor monitor) throws CoreExcepti
         workspaceJob.schedule();
     }
 
-    private void updateTargetsUsingAttribute(List<String> targetsToUpdate, String depsAttributeName, SubMonitor monitor)
+    private int updateTargetsUsingAttribute(List<String> targetsToUpdate, String depsAttributeName, SubMonitor monitor)
             throws CoreException {
 
         // filter that targets to update based on top level macro calls actually using the attribute name
         var buildFile = bazelProject.getBazelBuildFile();
-        List<String> targetsUsingDependencies = buildFile.getTopLevelMacroCalls()
+        var affactedMacroCalls = buildFile.getTopLevelMacroCalls()
                 .stream()
                 .filter(m -> (m.getStringListArgument(depsAttributeName) != null))
                 .map(m -> buildFile.getParent().getLabel().toString() + ":" + m.getName())
-                .filter(targetsToUpdate::contains)
-                .collect(toList());
+                .toList();
+
+        // if there is only one top level macro call we use it directly
+        // otherwise we try to match based on targetsToUpdate
+        List<String> targetsForBuildozerToUpdate;
+        if (affactedMacroCalls.size() > 1) {
+            targetsForBuildozerToUpdate =
+                    affactedMacroCalls.stream().filter(targetsToUpdate::contains).collect(toList());
+        } else {
+            targetsForBuildozerToUpdate = affactedMacroCalls;
+        }
 
-        if (!targetsUsingDependencies.isEmpty()) {
+        if (!targetsForBuildozerToUpdate.isEmpty()) {
             var workspaceRoot = bazelProject.getBazelWorkspace().getLocation().toPath();
             var buildozerCommand = new BuildozerCommand(
                     workspaceRoot,
                     labelsToAdd.stream().map(l -> format("add %s %s", depsAttributeName, l)).collect(toList()),
-                    targetsUsingDependencies,
-                    "Add dependency '%s'");
+                    targetsForBuildozerToUpdate,
+                    format(
+                        "Add label(s) '%s' to '%s'",
+                        labelsToAdd.stream().map(Label::toString).collect(joining(", ")),
+                        buildFile.getLocation()));
             bazelProject.getBazelWorkspace()
                     .getCommandExecutor()
                     .runDirectlyWithinExistingWorkspaceLock(
                         buildozerCommand,
                         List.of(bazelProject.getBuildFile()),
                         monitor);
         }
+
+        return targetsForBuildozerToUpdate.size();
     }
 }

bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/classpath/ClasspathEntry.java

@@ -178,6 +178,10 @@ public boolean isExported() {
         return exported;
     }
 
+    public boolean isTest() {
+        return "true".equals(getExtraAttributes().get(IClasspathAttribute.TEST));
+    }
+
     public void setBazelTargetOrigin(Label origin) {
         getExtraAttributes().put(ATTRIBUTE_BAZEL_TARGET_NAME, origin.toString());
     }
@@ -193,4 +197,12 @@ public void setSourceAttachmentPath(IPath sourceAttachmentPath) {
     public void setSourceAttachmentRootPath(IPath sourceAttachmentRootPath) {
         this.sourceAttachmentRootPath = sourceAttachmentRootPath;
     }
+
+    public void setTest(boolean test) {
+        if (test) {
+            getExtraAttributes().put(IClasspathAttribute.TEST, "true");
+        } else {
+            getExtraAttributes().remove(IClasspathAttribute.TEST);
+        }
+    }
 }

bundles/com.salesforce.bazel.eclipse.ui/src/com/salesforce/bazel/eclipse/ui/commands/BaseJobBasedHandler.java

@@ -12,6 +12,11 @@ public abstract class BaseJobBasedHandler extends AbstractHandler {
     @Override
     public Object execute(final ExecutionEvent event) throws ExecutionException {
         final var job = createJob(event);
+        if (job == null) {
+            // assume abort
+            return null;
+        }
+
         job.setUser(true);
         job.schedule();
         return null;

bundles/com.salesforce.bazel.eclipse.ui/src/com/salesforce/bazel/eclipse/ui/jdt/BazelQuickFixProcessor.java

@@ -69,6 +69,7 @@ public IJavaCompletionProposal[] getCorrections(IInvocationContext context, IPro
                     handleImportNotFound(context, location, collector);
                     break;
                 case IProblem.IsClassPathCorrect:
+                case IProblem.IsClassPathCorrectWithReferencingType:
                     handleMissingTransitive(context, location, collector);
                     break;
             }
@@ -137,8 +138,7 @@ private void handleMissingTransitive(IInvocationContext context, IProblemLocatio
             ClassResolutionCollector collector) throws CoreException {
         var cu = context.getASTRoot();
         var problemArguments = problemLocation.getProblemArguments();
-        if (problemArguments.length == 1) {
-            var className = problemArguments[0];
+        for (String className : problemArguments) {
             if (className != null) {
                 var project = cu.getJavaElement().getJavaProject().getProject();
 
@@ -166,6 +166,7 @@ public boolean hasCorrections(ICompilationUnit unit, int problemId) {
             case IProblem.MissingTypeInConstructor:
             case IProblem.MissingTypeInLambda:
             case IProblem.IsClassPathCorrect:
+            case IProblem.IsClassPathCorrectWithReferencingType:
                 var parent = unit.getParent();
                 if (parent != null) {
                     var project = parent.getJavaProject();

bundles/com.salesforce.bazel.eclipse.ui/src/com/salesforce/bazel/eclipse/ui/jdt/JavaResolutionFactory.java

@@ -61,6 +61,7 @@ public String getName() {
         public Change perform(IProgressMonitor pm) throws CoreException {
             var job = new AddDependenciesJob(bazelProject, List.of(labelToAdd), List.of(newClasspathEntry));
             job.runInWorkspace(pm); // run directly because this should be executed in workspace lock already
+            // FIXME: if this blocks the UI we should send it to the background (needs testing)
             return null;
         }
     }