diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelTarget.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelTarget.java index 90ead5bf..4a4799cd 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelTarget.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/BazelTarget.java @@ -131,7 +131,7 @@ public BazelRuleAttributes getRuleAttributes() throws CoreException { * in case of errors loading the rules info */ public String getRuleClass() throws CoreException { - return getInfo().getTarget().getRule().getRuleClass(); + return getInfo().getRule().getRuleClass(); } /** @@ -164,4 +164,18 @@ public boolean hasBazelProject() throws CoreException { public int hashCode() { return Objects.hash(bazelPackage, targetName); } + + /** + * Returns true if the target is a rule and has a attribute named tags containing the + * specified tag. + * + * @param tag + * the tag to check + * @return true if the rule has the specified tag, false otherwise + * @throws CoreException + */ + public boolean hasTag(String tag) throws CoreException { + var tags = getInfo().getRuleAttributes().getStringList("tags"); + return (tags != null) && tags.contains(tag); + } } diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java index 75f092ec..3afae43a 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java @@ -110,7 +110,8 @@ private static String queryForTargets(Collection targets) { return targetList; } - return String.format("attr('tags', '^((?!manual).)*$', %s)", targetList); + // do not exclude the manual tags here but make sure to filter out "no-ide" targets + return String.format("attr('tags', '^((?!no-ide).)*$', %s)", targetList); } private final BazelWorkspace workspace; diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BaseProvisioningStrategy.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BaseProvisioningStrategy.java index da81436b..8dcff23a 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BaseProvisioningStrategy.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BaseProvisioningStrategy.java @@ -183,7 +183,7 @@ private void addResourceFolders(BazelProject project, List rawC } private void addSourceFolders(BazelProject project, List rawClasspath, - JavaSourceInfo javaSourceInfo, boolean useTestsClasspath) { + JavaSourceInfo javaSourceInfo, boolean useTestsClasspath) throws CoreException { var virtualSourceFolder = useTestsClasspath ? getFileSystemMapper().getVirtualSourceFolderForTests(project) : getFileSystemMapper().getVirtualSourceFolder(project); var outputLocation = useTestsClasspath ? getFileSystemMapper().getOutputFolderForTests(project).getFullPath() @@ -207,13 +207,34 @@ private void addSourceFolders(BazelProject project, List rawCla var sourceFolder = dir.isEmpty() ? virtualSourceFolder : project.getProject().getFolder(dir); var inclusionPatterns = javaSourceInfo.getInclusionPatternsForSourceDirectory(dir); var exclusionPatterns = javaSourceInfo.getExclutionPatternsForSourceDirectory(dir); - rawClasspath.add( - JavaCore.newSourceEntry( - sourceFolder.getFullPath(), - inclusionPatterns, - exclusionPatterns, - outputLocation, - classpathAttributes)); + var existingEntry = + rawClasspath.stream().anyMatch(entry -> entry.getPath().equals(sourceFolder.getFullPath())); + if (existingEntry) { + if (useTestsClasspath) { + createBuildPathProblem( + project, + Status.warning( + format( + "Folder '%s' found twice on the classpath. This is likely because it's used as test as well as non-test resource. Please consider modifying the project setup!", + sourceFolder))); + } else { + createBuildPathProblem( + project, + Status.error( + format( + "Folder '%s' found twice on the classpath. This is an unexpected situation. Please consider modifying the project setup! Don't hesitate and reach out for help.", + sourceFolder))); + } + } else { + rawClasspath.add( + JavaCore.newSourceEntry( + sourceFolder.getFullPath(), + inclusionPatterns, + exclusionPatterns, + outputLocation, + classpathAttributes)); + } + } } } diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BazelQueryTargetDiscovery.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BazelQueryTargetDiscovery.java index 8f6cee1b..90118e09 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BazelQueryTargetDiscovery.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BazelQueryTargetDiscovery.java @@ -29,6 +29,8 @@ */ public class BazelQueryTargetDiscovery implements TargetDiscoveryStrategy { + private static final String TAG_NO_IDE = "no-ide"; + public static final String STRATEGY_NAME = "bazel-query"; private static Logger LOG = LoggerFactory.getLogger(BazelQueryTargetDiscovery.class); @@ -102,7 +104,7 @@ public Collection discoverTargets(BazelWorkspace bazelWorkspace, bazelPackage.getLabel(), bazelPackage.getBazelTargets()); } - targets.addAll(bazelPackage.getBazelTargets()); + bazelPackage.getBazelTargets().stream().filter(this::isVisibleToIde).forEach(targets::add); monitor.worked(1); } @@ -147,4 +149,13 @@ protected IPath findWorkspaceLocation(BazelWorkspace bazelWorkspace, IPath packa } return findWorkspaceLocation(bazelWorkspace, packagePath.removeLastSegments(1)); } + + private boolean isVisibleToIde(BazelTarget bazelTarget) { + try { + return !bazelTarget.hasTag(TAG_NO_IDE); + } catch (CoreException e) { + LOG.error("Error reading tags for target '{}': {}", bazelTarget, e.getMessage(), e); + return true; + } + } } diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/ProjectPerPackageProvisioningStrategy.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/ProjectPerPackageProvisioningStrategy.java index 9c4d7ff2..ea06fcf5 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/ProjectPerPackageProvisioningStrategy.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/ProjectPerPackageProvisioningStrategy.java @@ -31,6 +31,7 @@ import com.salesforce.bazel.eclipse.core.model.BazelWorkspace; import com.salesforce.bazel.eclipse.core.model.discovery.classpath.ClasspathEntry; import com.salesforce.bazel.eclipse.core.model.discovery.projects.JavaSourceEntry; +import com.salesforce.bazel.eclipse.core.model.discovery.projects.JavaSourceInfo; import com.salesforce.bazel.sdk.aspects.intellij.IntellijAspects.OutputGroup; import com.salesforce.bazel.sdk.command.BazelBuildWithIntelliJAspectsCommand; import com.salesforce.bazel.sdk.model.BazelLabel; @@ -178,6 +179,32 @@ public Map> computeClasspaths(Collectio } } + private void createWarningsForUnsupportedLayouts(BazelPackage bazelPackage, List packageTargets, + BazelProject project, JavaSourceInfo sourceInfo) throws CoreException { + if (sourceInfo.hasSourceFilesWithoutCommonRoot()) { + for (JavaSourceEntry file : sourceInfo.getSourceFilesWithoutCommonRoot()) { + createBuildPathProblem( + project, + Status.warning( + format( + "File '%s' could not be mapped into a common source directory. The project may not build successful in Eclipse.", + file.getPath()))); + } + } + if (!sourceInfo.hasSourceDirectories()) { + createBuildPathProblem( + project, + Status.info( + format( + "No source directories detected when analyzing package '%s' using targets '%s'", + bazelPackage.getLabel().getPackagePath(), + packageTargets.stream() + .map(BazelTarget::getLabel) + .map(BazelLabel::getLabelPath) + .collect(joining(", "))))); + } + } + @Override protected List doProvisionProjects(Collection targets, SubMonitor monitor) throws CoreException { @@ -208,29 +235,8 @@ protected List doProvisionProjects(Collection targets var javaInfo = collectJavaInfo(project, packageTargets, monitor.split(1)); // sanity check - var sourceInfo = javaInfo.getSourceInfo(); - if (sourceInfo.hasSourceFilesWithoutCommonRoot()) { - for (JavaSourceEntry file : sourceInfo.getSourceFilesWithoutCommonRoot()) { - createBuildPathProblem( - project, - Status.warning( - format( - "File '%s' could not be mapped into a common source directory. The project may not build successful in Eclipse.", - file.getPath()))); - } - } - if (!sourceInfo.hasSourceDirectories()) { - createBuildPathProblem( - project, - Status.info( - format( - "No source directories detected when analyzihng package '%s' using targets '%s'", - bazelPackage.getLabel().getPackagePath(), - packageTargets.stream() - .map(BazelTarget::getLabel) - .map(BazelLabel::getLabelPath) - .collect(joining(", "))))); - } + createWarningsForUnsupportedLayouts(bazelPackage, packageTargets, project, javaInfo.getSourceInfo()); + createWarningsForUnsupportedLayouts(bazelPackage, packageTargets, project, javaInfo.getTestSourceInfo()); // configure classpath configureRawClasspath(project, javaInfo, monitor.split(1)); diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/projects/JavaProjectInfo.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/projects/JavaProjectInfo.java index c4b51184..4347ca99 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/projects/JavaProjectInfo.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/projects/JavaProjectInfo.java @@ -179,7 +179,7 @@ public IStatus analyzeProjectRecommendations(IProgressMonitor monitor) throws Co resourceInfo = new JavaResourceInfo(resources, bazelPackage); resourceInfo.analyzeResourceDirectories(result); - testSourceInfo = new JavaSourceInfo(this.testSrcs, bazelPackage.getLocation()); + testSourceInfo = new JavaSourceInfo(this.testSrcs, bazelPackage.getLocation(), sourceInfo); testSourceInfo.analyzeSourceDirectories(result); testResourceInfo = new JavaResourceInfo(testResources, bazelPackage); diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/projects/JavaSourceInfo.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/projects/JavaSourceInfo.java index 37d3bdda..f64bd07c 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/projects/JavaSourceInfo.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/projects/JavaSourceInfo.java @@ -37,7 +37,8 @@ */ public class JavaSourceInfo { - private static final IPath INVALID = new Path("_not_following_ide_standards_"); + private static final IPath NOT_FOLLOWING_JAVA_PACKAGE_STRUCTURE = + new Path("_not_following_java_package_structure_"); private static boolean isJavaFile(java.nio.file.Path file) { return isRegularFile(file) && file.getFileName().toString().endsWith(".java"); @@ -46,6 +47,7 @@ private static boolean isJavaFile(java.nio.file.Path file) { private final Map detectedPackagePathsByFileEntryPathParent = new HashMap<>(); private final Collection srcs; private final IPath bazelPackageLocation; + private final JavaSourceInfo sharedSourceInfo; /** * A list of all source files impossible to identify a common root directory @@ -61,6 +63,26 @@ private static boolean isJavaFile(java.nio.file.Path file) { public JavaSourceInfo(Collection srcs, IPath bazelPackageLocation) { this.srcs = srcs; this.bazelPackageLocation = bazelPackageLocation; + this.sharedSourceInfo = null; + } + + /** + * Use this constructor for test sources, i.e. sources which may have targets sharing sources. + *

+ * Bazel allows to re-use sources in multiple targets. It will then compile those multiple times. An example setup + * is where all code is exposed as java_library as well as many targets for java_test with + * only one test class. If this is the case, we want to not issue "split package" warnings when the test class is + * already handled at the java_library level. + *

+ * + * @param srcs + * @param bazelPackageLocation + * @param sharedSourceInfo + */ + public JavaSourceInfo(Collection srcs, IPath bazelPackageLocation, JavaSourceInfo sharedSourceInfo) { + this.srcs = srcs; + this.bazelPackageLocation = bazelPackageLocation; + this.sharedSourceInfo = sharedSourceInfo; } @SuppressWarnings("unchecked") @@ -84,7 +106,7 @@ public void analyzeSourceDirectories(MultiStatus result) throws CoreException { "Java file '%s' (with detected package '%s') does not meet IDE standards. Please move into a folder hierarchy which follows Java package structure!", fileEntry.getPath(), fileEntry.getDetectedPackagePath()))); - return INVALID; + return NOT_FOLLOWING_JAVA_PACKAGE_STRUCTURE; } // build second index of parent for all entries with a potential source root @@ -169,7 +191,7 @@ public void analyzeSourceDirectories(MultiStatus result) throws CoreException { // (eg., glob(["src/test/java/some/package/only/*.java"]) for (var potentialSourceRootAndSourceEntries : sourceEntriesBySourceRoot.entrySet()) { var potentialSourceRoot = potentialSourceRootAndSourceEntries.getKey(); - if (INVALID.equals(potentialSourceRoot)) { + if (NOT_FOLLOWING_JAVA_PACKAGE_STRUCTURE.equals(potentialSourceRoot)) { continue; } if (!(potentialSourceRootAndSourceEntries.getValue() instanceof List)) { @@ -198,11 +220,25 @@ public void analyzeSourceDirectories(MultiStatus result) throws CoreException { } } + /* + * Bazel allows to re-use sources in multiple targets. It will then compile those multiple times. An example setup + * is where all code is exposed as java_library as well as many targets for java_test with + * only one test class. If this is the case, we want to not issue "split package" warnings when the test class is + * already handled at the java_library level. + */ + if ((sharedSourceInfo != null) && sharedSourceInfo.hasSourceDirectories()) { + var sharedSourceDirectories = sharedSourceInfo.getSourceDirectories(); + potentialSplitPackageOrSubsetFolders.removeIf( + potentialSourceRoot -> sharedSourceDirectories.contains(potentialSourceRoot) + || sharedSourceDirectories.stream().anyMatch(p -> p.isPrefixOf(potentialSourceRoot))); + } + // when there are no split packages we found a good setup if (potentialSplitPackageOrSubsetFolders.isEmpty()) { // collect remaining files without a root - if (sourceEntriesBySourceRoot.containsKey(INVALID)) { - sourceFilesWithoutCommonRoot = (List) sourceEntriesBySourceRoot.remove(INVALID); + if (sourceEntriesBySourceRoot.containsKey(NOT_FOLLOWING_JAVA_PACKAGE_STRUCTURE)) { + sourceFilesWithoutCommonRoot = + (List) sourceEntriesBySourceRoot.remove(NOT_FOLLOWING_JAVA_PACKAGE_STRUCTURE); } // create source directories