Skip to content

Commit de3d78f

Browse files
committed
Make ClasspathResolutionTest2 test projects use realistic type
hierarchies Add cross-bundle type hierarchy chains (extends/implements) and actual API usage to test projects B-H and X, modelling real-world scenarios where type hierarchy required transitive dependencies on the compilation classpath.
1 parent 368feba commit de3d78f

File tree

17 files changed

+303
-109
lines changed

17 files changed

+303
-109
lines changed

ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/classpath/ClasspathResolutionTest2.java

Lines changed: 53 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,34 @@
5959
* Bundle B: Export-Package: b.api
6060
* Import-Package: d.api, f.api(optional)
6161
* Require-Bundle: C, E(optional), G(reexport)
62+
* b.api.MyObject extends c.api.MyObject implements d.api.Processor
63+
* b.internal.MyObject uses e.api + f.api (optional, internal only)
6264
* Bundle C: Export-Package: c.api (leaf)
65+
* c.api.Configurable interface, c.api.MyObject implements it
6366
* Bundle D: Export-Package: d.api (leaf)
64-
* Bundle E: Export-Package: e.api (leaf)
65-
* Bundle F: Export-Package: f.api (leaf)
67+
* d.api.Processor interface, d.api.MyObject with getData()
68+
* Bundle E: Export-Package: e.api (leaf, e.api.MyObject with activate())
69+
* Bundle F: Export-Package: f.api (leaf, f.api.MyObject with isAvailable())
6670
* Bundle G: Export-Package: g.api; Import-Package: h.api(optional)
67-
* Bundle H: Export-Package: h.api (leaf)
71+
* g.api.MyObject.describe() uses h.api internally (optional)
72+
* Bundle H: Export-Package: h.api (leaf, h.api.MyObject with getIdentifier())
6873
* </pre>
6974
*
75+
* <b>Type hierarchy chain exercised by A:</b>
76+
* <pre>
77+
* c.api.Configurable (interface)
78+
* ↑ implements
79+
* c.api.MyObject (class with getValue(), getConfig())
80+
* ↑ extends
81+
* b.api.MyObject (class) ← also implements d.api.Processor
82+
* </pre>
83+
* When A uses b.api.MyObject and calls inherited methods, the compiler
84+
* needs c.api.MyObject, c.api.Configurable, d.api.Processor, and
85+
* d.api.MyObject on the classpath — modelling the real-world scenario from
86+
* <a href="https://github.com/eclipse-pde/eclipse.pde/issues/2195">issue
87+
* #2195</a> where IWidgetValueProperty's type hierarchy required the
88+
* databinding bundle on the classpath.
89+
*
7090
* Each bundle also has an internal package (e.g. {@code b.internal}) that is
7191
* <b>not</b> listed in Export-Package.
7292
* <p>
@@ -452,9 +472,12 @@ public void testExpectedCompilationMarkers_forbiddenIsError() throws Exception {
452472
* because transitive dependencies are on the classpath (PR #2218). Before
453473
* PR #2218, transitive types caused "The type X cannot be resolved. It is
454474
* indirectly referenced from required type Y" errors.</li>
455-
* <li><b>No markers on accessible types:</b> b.api.MyObject (line 29) and
456-
* g.api.MyObject (line 34) produce zero markers because they match
457-
* {@code K_ACCESSIBLE} access rules for exported packages (§3.6.5).</li>
475+
* <li><b>No markers on accessible types:</b> b.api.MyObject (line 28) and
476+
* g.api.MyObject (line 64) produce zero markers because they match
477+
* {@code K_ACCESSIBLE} access rules for exported packages (§3.6.5).
478+
* Additionally, inherited method calls (e.g. {@code service.configure()},
479+
* {@code service.process()}) accessed through b.api.MyObject produce no
480+
* markers even though they involve transitive types (c.api, d.api).</li>
458481
* <li><b>Forbidden reference markers on all non-accessible types:</b> Every
459482
* type from non-exported packages of direct deps (b.internal, g.internal)
460483
* and from transitive-only bundles (C, D, E, F, H) produces exactly 2
@@ -481,10 +504,12 @@ private void assertExpectedCompilationMarkers(IProject project, int expectedSeve
481504
// ("The type 'MyObject' is not API") and one for the constructor
482505
// call ("The constructor 'MyObject()' is not API").
483506
//
484-
// Lines 29 (b.api.MyObject) and 34 (g.api.MyObject) are
485-
// K_ACCESSIBLE and must have NO markers. The total count assertion
486-
// below implicitly validates this — any extra markers would
487-
// increase the count.
507+
// Lines 28 (b.api.MyObject) and 64 (g.api.MyObject) are
508+
// K_ACCESSIBLE and must have NO markers. Lines 32-56 contain
509+
// method calls through b.api.MyObject (inherited methods from
510+
// c.api/d.api) and g.api.MyObject — also no markers. The total
511+
// count assertion below implicitly validates this — any extra
512+
// markers would increase the count.
488513
record ForbiddenRef(int line, String qualifiedType, String project) {
489514
}
490515
List<ForbiddenRef> expected = List.of(
@@ -493,7 +518,7 @@ record ForbiddenRef(int line, String qualifiedType, String project) {
493518
// B/G entries. Non-exported packages are not returned by
494519
// StateHelper.getVisiblePackages() — they have no explicit
495520
// rule, so the catch-all EXCLUDE_ALL fires (§3.6.5).
496-
new ForbiddenRef(31, "b.internal.MyObject", "B"), new ForbiddenRef(36, "g.internal.MyObject", "G"),
521+
new ForbiddenRef(62, "b.internal.MyObject", "B"), new ForbiddenRef(70, "g.internal.MyObject", "G"),
497522
// Transitive forbidden: ALL packages forbidden via the
498523
// single **/* K_NON_ACCESSIBLE rule added by
499524
// addTransitiveDependenciesWithForbiddenAccess().
@@ -502,26 +527,27 @@ record ForbiddenRef(int line, String qualifiedType, String project) {
502527
//
503528
// C: Required by B (Require-Bundle: C,
504529
// visibility:=private §3.13.1)
505-
new ForbiddenRef(45, "c.api.MyObject", "C"), new ForbiddenRef(46, "c.internal.MyObject", "C"),
530+
new ForbiddenRef(77, "c.api.MyObject", "C"), new ForbiddenRef(78, "c.internal.MyObject", "C"),
506531
// D: Package imported by B (Import-Package: d.api §3.6.4
507532
// — never re-exports)
508-
new ForbiddenRef(49, "d.api.MyObject", "D"), new ForbiddenRef(50, "d.internal.MyObject", "D"),
533+
new ForbiddenRef(81, "d.api.MyObject", "D"), new ForbiddenRef(82, "d.internal.MyObject", "D"),
509534
// E: Optionally required by B (Require-Bundle: E;optional
510535
// §3.7.5 + §3.13.1)
511-
new ForbiddenRef(53, "e.api.MyObject", "E"), new ForbiddenRef(54, "e.internal.MyObject", "E"),
536+
new ForbiddenRef(85, "e.api.MyObject", "E"), new ForbiddenRef(86, "e.internal.MyObject", "E"),
512537
// F: Optionally imported by B (Import-Package:
513538
// f.api;optional §3.7.5 + §3.6.4)
514-
new ForbiddenRef(57, "f.api.MyObject", "F"), new ForbiddenRef(58, "f.internal.MyObject", "F"),
539+
new ForbiddenRef(89, "f.api.MyObject", "F"), new ForbiddenRef(90, "f.internal.MyObject", "F"),
515540
// H: Optionally imported by G (Import-Package:
516541
// h.api;optional §3.7.5)
517-
new ForbiddenRef(61, "h.api.MyObject", "H"), new ForbiddenRef(62, "h.internal.MyObject", "H"));
542+
new ForbiddenRef(93, "h.api.MyObject", "H"), new ForbiddenRef(94, "h.internal.MyObject", "H"));
518543

519544
IMarker[] allMarkers = project.findMarkers(IJavaModelMarker.JAVA_MODEL_PROBLEM_MARKER, true,
520545
IResource.DEPTH_INFINITE);
521546

522547
// Each forbidden reference produces exactly 2 markers (type +
523548
// constructor). The total count also implicitly asserts that NO
524-
// markers exist for K_ACCESSIBLE references on lines 29 and 34.
549+
// markers exist for K_ACCESSIBLE references (line 28 b.api,
550+
// line 64 g.api) or inherited method calls (lines 32-56).
525551
assertThat(allMarkers).as(
526552
"Project %s must have exactly %d JDT markers: " + "%d forbidden references × 2 (type + "
527553
+ "constructor). Zero 'cannot be resolved' " + "errors, zero markers on accessible types.",
@@ -627,8 +653,9 @@ public void testDiscouragedAccessRulesForXInternal() throws Exception {
627653
* <p>
628654
* Expected markers on Ad/src/a/api/AClass.java:
629655
* <ul>
630-
* <li>Lines 30, 33, 36: no markers (K_ACCESSIBLE API)</li>
631-
* <li>Line 40: exactly 2 markers — discouraged type + constructor for
656+
* <li>Lines 26-36: no markers (K_ACCESSIBLE API and inherited method
657+
* calls through b.api.MyObject, g.api.MyObject, x.api.MyObject)</li>
658+
* <li>Line 44: exactly 2 markers — discouraged type + constructor for
632659
* x.internal.MyObject</li>
633660
* </ul>
634661
*/
@@ -638,14 +665,14 @@ public void testDiscouragedMarkersOnProjectAd() throws Exception {
638665
IResource.DEPTH_INFINITE);
639666

640667
// Ad only has 1 discouraged reference (x.internal.MyObject on line
641-
// 38) producing 2 markers (type + constructor). Zero markers for
642-
// exported API (b.api, g.api, x.api).
668+
// 44) producing 2 markers (type + constructor). Zero markers for
669+
// exported API (b.api, g.api, x.api) and inherited method calls.
643670
assertThat(allMarkers).as("Project Ad must have exactly 2 markers: " + "discouraged type + constructor for "
644671
+ "x.internal.MyObject. Zero errors, zero " + "markers on accessible types.").hasSize(2);
645672

646673
for (IMarker m : allMarkers) {
647674
assertThat(m.getAttribute(IMarker.LINE_NUMBER, -1))
648-
.as("Discouraged marker must be on line 40 " + "(x.internal.MyObject)").isEqualTo(40);
675+
.as("Discouraged marker must be on line 44 " + "(x.internal.MyObject)").isEqualTo(44);
649676

650677
assertThat(m.getAttribute(IMarker.SEVERITY, -1))
651678
.as("Discouraged access must be WARNING " + "(discouragedReference=warning in "
@@ -680,9 +707,10 @@ public void testDiscouragedMarkersOnProjectAd() throws Exception {
680707

681708
/**
682709
* All dependency bundles (B-H) must build without any compilation problems
683-
* — no errors AND no warnings. Each bundle's own dependencies are properly
684-
* declared in its manifest, and their source code does not reference any
685-
* forbidden types.
710+
* — no errors AND no warnings. Each bundle actively uses its declared
711+
* dependencies (e.g. B extends c.api.MyObject, implements d.api.Processor,
712+
* uses e.api/f.api internally; G uses h.api internally) but only through
713+
* properly declared imports, so no access rule violations occur.
686714
*/
687715
@Test
688716
public void testDependencyBundlesBuildClean() throws Exception {
Lines changed: 63 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,95 @@
11
package a.api;
22

33
/**
4-
* Test class that references types from multiple bundles to exercise OSGi
5-
* visibility and JDT access rules.
4+
* Test consumer that exercises OSGi visibility and JDT access rules.
65
* <p>
7-
* Bundle A declares: {@code Import-Package: b.api, g.api}
6+
* Bundle A imports: {@code b.api}, {@code g.api}. The {@code b.api.MyObject}
7+
* type has a rich type hierarchy ({@code extends c.api.MyObject implements
8+
* d.api.Processor}) that requires transitive dependencies on the compilation
9+
* classpath for type hierarchy resolution.
810
* <p>
9-
* With the correct PDE behavior (PR #2218), all bundles in the transitive
10-
* dependency closure are present on the compilation classpath:
11-
* <ul>
12-
* <li><b>Directly imported bundles</b> (B, G): exported packages are
13-
* {@code K_ACCESSIBLE}, non-exported packages caught by EXCLUDE_ALL_RULE
14-
* ({@code K_NON_ACCESSIBLE})</li>
15-
* <li><b>Transitive dependencies</b> (C, D, E, F, H): all-forbidden access
16-
* rules (single {@code **&#47;* K_NON_ACCESSIBLE} rule) — compiler can
17-
* resolve types but direct usage produces forbidden reference warnings
18-
* (configured via {@code forbiddenReference=warning})</li>
19-
* </ul>
11+
* <b>Accessible API usage:</b> Methods inherited from the transitive type
12+
* hierarchy are called through {@code b.api.MyObject} ({@code K_ACCESSIBLE}),
13+
* exercising the compiler's type resolution without producing forbidden
14+
* markers. This models the real-world scenario from issue #2195 where
15+
* {@code IWidgetValueProperty extends IValueProperty} required the databinding
16+
* bundle on the classpath.
2017
* <p>
21-
* At <b>OSGi runtime</b>, only directly imported packages (b.api, g.api)
22-
* would be accessible to A's classloader (OSGi Core R8 §3.9.4). The JDT
23-
* access rules enforce this visibility at compile time.
18+
* <b>Forbidden references:</b> Direct references to types from non-imported
19+
* packages produce forbidden reference markers, validating that PDE's access
20+
* rules correctly enforce OSGi module layer visibility at compile time.
2421
*/
2522
public class AClass {
2623
// ---- Directly imported bundles (K_ACCESSIBLE for exported packages) ----
2724

28-
// B exports b.api, A imports b.api → K_ACCESSIBLE → no marker
29-
public Object objectFromB_allowed = new b.api.MyObject();
30-
// B's b.internal is NOT exported → caught by EXCLUDE_ALL → forbidden warning
25+
// b.api.MyObject extends c.api.MyObject implements d.api.Processor.
26+
// The compiler needs transitive types (c.api.*, d.api.*) for type
27+
// hierarchy resolution — these are present as transitive forbidden deps.
28+
public b.api.MyObject service = new b.api.MyObject();
29+
30+
// Calls inherited methods from c.api.Configurable via c.api.MyObject.
31+
// No markers: accessed through b.api.MyObject (K_ACCESSIBLE).
32+
public boolean configureAndVerify() {
33+
service.configure("production");
34+
return service.isConfigured();
35+
}
36+
37+
// Inherited from c.api.MyObject — compiler resolves c.api.MyObject
38+
// from the transitive forbidden classpath entry.
39+
public Object retrieveConfig() {
40+
return service.getConfig();
41+
}
42+
43+
// From d.api.Processor interface (implemented by b.api.MyObject).
44+
// Compiler resolves d.api.Processor from transitive forbidden entry.
45+
public Object processItem(String input) {
46+
return service.process(input);
47+
}
48+
49+
// b.api method using d.api.MyObject in its signature — compiler
50+
// resolves d.api.MyObject from the transitive forbidden classpath entry.
51+
public Object processData() {
52+
return service.processData(null);
53+
}
54+
55+
// b.api method returning g.api.MyObject (G is reexported by B).
56+
// g.api is directly imported by A, so no marker.
57+
public g.api.MyObject createViaService() {
58+
return service.createService();
59+
}
60+
61+
// B's b.internal is NOT exported → caught by EXCLUDE_ALL → forbidden
3162
public Object objectFromB_forbidden = new b.internal.MyObject();
3263

3364
// G exports g.api, A imports g.api → K_ACCESSIBLE → no marker
34-
public Object objectFromG_allowed = new g.api.MyObject();
35-
// G's g.internal is NOT exported → caught by EXCLUDE_ALL → forbidden warning
65+
public g.api.MyObject gService = new g.api.MyObject();
66+
// g.api.MyObject.describe() internally uses h.api (optional dep of G)
67+
public String description = gService.describe();
68+
69+
// G's g.internal is NOT exported → caught by EXCLUDE_ALL → forbidden
3670
public Object objectFromG_forbidden = new g.internal.MyObject();
3771

3872
// ---- Transitive dependencies (all-forbidden access rules) ----
39-
// These types CAN be resolved by the compiler (on classpath for type
40-
// hierarchy validation), but produce forbidden reference warnings because
41-
// their entries have only **/* K_NON_ACCESSIBLE access rules.
42-
// At OSGi runtime, A's classloader cannot load any of these types (§3.9.4).
73+
// Direct references to forbidden types produce forbidden reference markers.
74+
// At OSGi runtime, A's classloader cannot load these types (§3.9.4).
4375

44-
// C: Required by B (Require-Bundle: C, default visibility:=private §3.13.1)
76+
// C: Required by B (Require-Bundle: C, visibility:=private §3.13.1)
4577
public Object objectFromC_forbidden1 = new c.api.MyObject();
4678
public Object objectFromC_forbidden2 = new c.internal.MyObject();
4779

48-
// D: Package imported by B (Import-Package: d.api) — never re-exports §3.6.4
80+
// D: Imported by B (Import-Package: d.api) — never re-exports §3.6.4
4981
public Object objectFromD_forbidden1 = new d.api.MyObject();
5082
public Object objectFromD_forbidden2 = new d.internal.MyObject();
5183

52-
// E: Optionally required by B (Require-Bundle: E;resolution:=optional §3.7.5)
84+
// E: Optionally required by B (Require-Bundle: E;optional §3.7.5)
5385
public Object objectFromE_forbidden1 = new e.api.MyObject();
5486
public Object objectFromE_forbidden2 = new e.internal.MyObject();
5587

56-
// F: Optionally imported by B (Import-Package: f.api;resolution:=optional)
88+
// F: Optionally imported by B (Import-Package: f.api;optional)
5789
public Object objectFromF_forbidden1 = new f.api.MyObject();
5890
public Object objectFromF_forbidden2 = new f.internal.MyObject();
5991

60-
// H: Optionally imported by G (Import-Package: h.api;resolution:=optional)
92+
// H: Optionally imported by G (Import-Package: h.api;optional)
6193
public Object objectFromH_forbidden1 = new h.api.MyObject();
6294
public Object objectFromH_forbidden2 = new h.internal.MyObject();
6395
}
Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,45 @@
11
package a.api;
22

33
/**
4-
* Test class that references only exported API from direct dependencies,
5-
* plus both public and x-internal packages from bundle X.
4+
* Test consumer that references only directly imported API from B, G, and X,
5+
* exercising the rich type hierarchy and x-internal discouraged access.
66
* <p>
77
* Bundle Ad declares:
88
* {@code Import-Package: b.api, g.api, x.api, x.internal}
99
* <p>
1010
* Expected markers:
1111
* <ul>
12-
* <li>Lines 26, 29: b.api.MyObject, g.api.MyObject → {@code K_ACCESSIBLE}
13-
* → <b>no markers</b></li>
14-
* <li>Line 32: x.api.MyObject → {@code K_ACCESSIBLE} (normal export)
15-
* → <b>no marker</b></li>
16-
* <li>Line 35: x.internal.MyObject → {@code K_DISCOURAGED}
17-
* ({@code x-internal:=true} on Export-Package) → <b>discouraged access
18-
* warning</b></li>
12+
* <li>b.api, g.api, x.api: {@code K_ACCESSIBLE} → <b>no markers</b></li>
13+
* <li>x.internal: {@code K_DISCOURAGED} ({@code x-internal:=true})
14+
* → <b>discouraged access warning</b></li>
1915
* </ul>
2016
* <p>
21-
* No transitive dependency types are referenced — Ad only uses directly
22-
* imported packages. This validates that exported API from direct
17+
* No transitive dependency types are directly referenced — Ad only uses
18+
* directly imported packages. This validates that exported API from direct
2319
* dependencies produces zero markers, and that x-internal packages produce
2420
* discouraged (not forbidden) markers.
2521
*/
2622
public class AClass {
27-
// ---- Directly imported bundles: exported API only ----
23+
// ---- Directly imported bundles: exported API usage ----
2824

29-
// B exports b.api, Ad imports b.api → K_ACCESSIBLE → no marker
30-
public Object objectFromB_allowed = new b.api.MyObject();
25+
// B exports b.api → K_ACCESSIBLE → no marker
26+
// b.api.MyObject extends c.api.MyObject implements d.api.Processor
27+
public b.api.MyObject service = new b.api.MyObject();
3128

32-
// G exports g.api, Ad imports g.api → K_ACCESSIBLE → no marker
33-
public Object objectFromG_allowed = new g.api.MyObject();
29+
// Exercise inherited methods from transitive type hierarchy — no markers
30+
public boolean setup() {
31+
service.configure("settings");
32+
return service.isConfigured();
33+
}
3434

35-
// X exports x.api (normal), Ad imports x.api → K_ACCESSIBLE → no marker
36-
public Object objectFromX_allowed = new x.api.MyObject();
35+
// G exports g.api → K_ACCESSIBLE → no marker
36+
public g.api.MyObject gService = new g.api.MyObject();
37+
public String desc = gService.describe();
3738

38-
// X exports x.internal with x-internal:=true, Ad imports x.internal
39-
// → K_DISCOURAGED → discouraged access warning
39+
// X exports x.api → K_ACCESSIBLE → no marker
40+
public x.api.MyObject xService = new x.api.MyObject();
41+
public String xName = xService.getName();
42+
43+
// X exports x.internal with x-internal:=true → K_DISCOURAGED → warning
4044
public Object objectFromX_discouraged = new x.internal.MyObject();
4145
}

0 commit comments

Comments
 (0)