Skip to content

Commit

Permalink
bndtools container: Address regression in Open Type Hierarch\y
Browse files Browse the repository at this point in the history 
The JDT type hierarchy search does not work properly when a
source folder is not-accessible following by a binary jar which is.
This caused types in the jar to be missing from the hierarchy result.

So we add back the `source=project` -buildpath clause option. The
value `project` is the default. So saying `source=none` on a -buildpath
entry, will cause Bndtools to mark the source project as discouraged
to force the compiler to use the classes in the generated jar.

Fixes #5250

Signed-off-by: BJ Hargrave <bj@hargrave.dev>
  • Loading branch information
@bjhargrave
bjhargrave committed May 19, 2022
1 parent b914286 commit 9911193
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 41 deletions.
4 changes: 2 additions & 2 deletions bndtools.builder/src/org/bndtools/builder/classpath/BndContainer.java
View file
Expand Up @@ -88,7 +88,7 @@ void refresh() throws CoreException {
resources = null;
}

private static final IClasspathAttribute TEST = JavaCore.newClasspathAttribute("test",
static final IClasspathAttribute TEST = JavaCore.newClasspathAttribute("test",
Boolean.TRUE.toString());
private static final IClasspathAttribute PROJECT = JavaCore.newClasspathAttribute(Constants.VERSION_ATTRIBUTE,
Constants.VERSION_ATTR_PROJECT);
Expand Down Expand Up @@ -146,7 +146,7 @@ IRuntimeClasspathEntry[] getRuntimeClasspathEntries() throws JavaModelException
return runtime.toArray(EMPTY_RUNTIMEENTRIES);
}

private static boolean hasAttribute(IClasspathEntry cpe, IClasspathAttribute attr) {
static boolean hasAttribute(IClasspathEntry cpe, IClasspathAttribute attr) {
return Arrays.stream(cpe.getExtraAttributes())
.anyMatch(attr::equals);
}
Expand Down
115 changes: 76 additions & 39 deletions bndtools.builder/src/org/bndtools/builder/classpath/BndContainerInitializer.java
View file
@@ -1,6 +1,8 @@
package org.bndtools.builder.classpath;

import static java.util.stream.Collectors.toList;
import static org.bndtools.builder.classpath.BndContainer.TEST;
import static org.bndtools.builder.classpath.BndContainer.hasAttribute;

import java.io.File;
import java.io.IOException;
Expand All @@ -10,6 +12,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.jar.Manifest;
Expand Down Expand Up @@ -200,13 +203,9 @@ private static File getContainerFile(IProject p) {
private static class Updater {
private static final IAccessRule DISCOURAGED = JavaCore.newAccessRule(new Path("**"),
IAccessRule.K_DISCOURAGED | IAccessRule.IGNORE_IF_BETTER);
private static final IAccessRule NON_ACCESSIBLE = JavaCore.newAccessRule(new Path("**"),
IAccessRule.K_NON_ACCESSIBLE | IAccessRule.IGNORE_IF_BETTER);
private static final IClasspathAttribute EMPTY_INDEX = JavaCore.newClasspathAttribute(
IClasspathAttribute.INDEX_LOCATION_ATTRIBUTE_NAME,
"platform:/plugin/" + BndtoolsBuilder.PLUGIN_ID + "/org/bndtools/builder/classpath/empty.index");
private static final IClasspathAttribute TEST = JavaCore.newClasspathAttribute("test",
Boolean.TRUE.toString());
private static final IClasspathAttribute WITHOUT_TEST_CODE = JavaCore
.newClasspathAttribute("without_test_code", Boolean.TRUE.toString());
private static final Pattern packagePattern = Pattern.compile("(?<=^|\\.)\\*(?=\\.|$)|\\.");
Expand Down Expand Up @@ -302,7 +301,7 @@ static void setClasspathContainer(IJavaProject javaProject, BndContainer contain
}, null);
}

private Void calculateProjectClasspath() {
private Void calculateProjectClasspath() throws CoreException {
if (!project.isOpen()) {
return null;
}
Expand All @@ -326,17 +325,11 @@ private Void calculateProjectClasspath() {
return null;
}

private void calculateContainersClasspath(String instruction, Collection<Container> containers) {
boolean testattr = false;
if (instruction.equals(Constants.TESTPATH)) {
try {
testattr = Arrays.stream(javaProject.getRawClasspath())
.filter(cpe -> cpe.getEntryKind() == IClasspathEntry.CPE_SOURCE)
.map(IClasspathEntry::getExtraAttributes)
.flatMap(Arrays::stream)
.anyMatch(TEST::equals);
} catch (JavaModelException e) {}
}
private void calculateContainersClasspath(String instruction, Collection<Container> containers)
throws CoreException {
boolean testattr = instruction.equals(Constants.TESTPATH) && Arrays.stream(javaProject.getRawClasspath())
.filter(cpe -> cpe.getEntryKind() == IClasspathEntry.CPE_SOURCE)
.anyMatch(cpe -> hasAttribute(cpe, TEST));
for (Container c : containers) {
File file = c.getFile();
assert file.isAbsolute();
Expand Down Expand Up @@ -383,33 +376,74 @@ private void calculateContainersClasspath(String instruction, Collection<Contain
if (testattr) {
extraAttrs.add(WITHOUT_TEST_CODE);
}
IPath projectPath = root.getFile(path)
.getProject()
.getFullPath();
if (isVersionProject(c)) {
addProjectEntry(projectPath, accessRules, extraAttrs);
} else {
IProject otherProject = root.getFile(path)
.getProject();
if (isVersionProject(c)) { // version=project
addProjectEntry(otherProject.getFullPath(), accessRules, extraAttrs);
} else { // version=latest or version=snapshot
/*
* When the source value is not equal to "project"
* (the default value), we make the source folder
* discouraged. This can be necessary when using the
* Eclipse Transformer Bnd Analyzer plugin which can
* make the class files in the jar incompatible with
* the class files in the source output folder. We
* tried to always make the source folder
* discouraged which worked fine for compiling but
* it messed up the Open Type Hierarchy support in
* Eclipse. See
* https://github.com/bndtools/bnd/issues/5250. So
* we maintain the prior behavior unless source is
* set to a value other than "project". e.g.
* source=none
*/
String source = c.getAttributes()
.getOrDefault("source", "project");
List<IAccessRule> projectAccessRules = Objects.equals(source, "project") ? accessRules
: Collections.emptyList();
/*
* If not version=project, add project entry for
* source code before library entry for generated
* jar. We use NON_ACCESSIBLE access to make eclipse
* use the classes in the library entry and not the
* project entry. We use the project entry for
* source code only.
* Add project entry for source code before library
* entry for generated jar.
*/
addProjectEntry(projectPath, Collections.singletonList(NON_ACCESSIBLE), extraAttrs);
addProjectEntry(otherProject.getFullPath(), projectAccessRules, extraAttrs);
/*
* Supply an empty index for the generated JAR of a
* workspace project dependency. This prevents the
* non-editable source files in the generated jar
* from appearing in the Open Type dialog.
*/
extraAttrs.add(EMPTY_INDEX);
addLibraryEntry(path, file, accessRules, extraAttrs);
/*
* Compute source attachment path for library entry.
*/
IPath sourceAttachmentPath = calculateSourceAttachmentPath(path, file);
IPath sourceAttachmentRootPath = null; // default
if (sourceAttachmentPath == null) {
IJavaProject otherJavaProject = JavaCore.create(otherProject);
if (otherJavaProject != null) {
for (IClasspathEntry raw : otherJavaProject.getRawClasspath()) {
if ((raw.getEntryKind() == IClasspathEntry.CPE_SOURCE)
&& !hasAttribute(raw, TEST)) {
sourceAttachmentPath = raw.getSourceAttachmentPath();
if (sourceAttachmentPath != null) {
sourceAttachmentRootPath = raw.getSourceAttachmentRootPath();
} else {
sourceAttachmentPath = raw.getPath();
}
break;
}
}
}
}
addLibraryEntry(path, file, accessRules, extraAttrs, sourceAttachmentPath,
sourceAttachmentRootPath);
}
break;
default :
addLibraryEntry(path, file, accessRules, extraAttrs);
IPath sourceAttachmentPath = calculateSourceAttachmentPath(path, file);
IPath sourceAttachmentRootPath = null; // default
addLibraryEntry(path, file, accessRules, extraAttrs, sourceAttachmentPath,
sourceAttachmentRootPath);
break;
}
}
Expand Down Expand Up @@ -441,13 +475,15 @@ private void addProjectEntry(IPath path, List<IAccessRule> accessRules, List<ICl
}
combinedAccessRules.addAll(accessRules);
}
builder.entry(i, JavaCore.newProjectEntry(path, toAccessRulesArray(combinedAccessRules), false,
entry.getExtraAttributes(), false));
IClasspathEntry projectEntry = JavaCore.newProjectEntry(path, toAccessRulesArray(combinedAccessRules),
false, entry.getExtraAttributes(), false);
builder.entry(i, projectEntry);
return;
}
// Add a new project entry for the project
builder.entry(JavaCore.newProjectEntry(path, toAccessRulesArray(accessRules), false,
toClasspathAttributesArray(extraAttrs), false));
IClasspathEntry projectEntry = JavaCore.newProjectEntry(path, toAccessRulesArray(accessRules), false,
toClasspathAttributesArray(extraAttrs), false);
builder.entry(projectEntry);
}

private IPath calculateSourceAttachmentPath(IPath path, File file) {
Expand Down Expand Up @@ -490,10 +526,11 @@ private JarInfo getJarInfo(File file) {
}

private void addLibraryEntry(IPath path, File file, List<IAccessRule> accessRules,
List<IClasspathAttribute> extraAttrs) {
IPath sourceAttachmentPath = calculateSourceAttachmentPath(path, file);
builder.entry(JavaCore.newLibraryEntry(path, sourceAttachmentPath, null, toAccessRulesArray(accessRules),
toClasspathAttributesArray(extraAttrs), false));
List<IClasspathAttribute> extraAttrs, IPath sourceAttachmentPath, IPath sourceAttachmentRootPath) {
IClasspathEntry libraryEntry = JavaCore.newLibraryEntry(path, sourceAttachmentPath,
sourceAttachmentRootPath, toAccessRulesArray(accessRules), toClasspathAttributesArray(extraAttrs),
false);
builder.entry(libraryEntry);
builder.updateLastModified(file.lastModified());
}

Expand Down

0 comments on commit 9911193

Please sign in to comment.