Merge pull request #5818 from pkriens/issue/5808-baseline-extendable

Fixes #5808 Handle effective final clases
bndtools · Oct 17, 2023 · 9186bcc · 9186bcc
2 parents 755ff1b + 85a44e0
commit 9186bcc
Show file tree

Hide file tree

Showing 3 changed files with 103 additions and 3 deletions.
diff --git a/biz.aQute.bndlib.tests/test/test/baseline/BaselineTest.java b/biz.aQute.bndlib.tests/test/test/baseline/BaselineTest.java
@@ -1,6 +1,7 @@
 package test.baseline;
 
 import static aQute.bnd.osgi.Constants.BUNDLE_SYMBOLICNAME;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -50,7 +51,7 @@
 @SuppressWarnings("resource")
 public class BaselineTest {
 
-	Workspace	workspace;
+	Workspace workspace;
 
 	private Workspace getWorkspace(File tmp) throws Exception {
 		if (workspace != null)
@@ -66,6 +67,94 @@ protected void tearDown() throws Exception {
 		workspace = null;
 	}
 
+	public static class PrivateConstructorsAndFinal {
+		public class Normal {}
+
+		public class Private {
+			private Private() {}
+		}
+
+		public class PrivateMultiple {
+			private PrivateMultiple() {}
+
+			private PrivateMultiple(int a) {}
+
+			private PrivateMultiple(int a, int b) {}
+
+			private PrivateMultiple(int a, int b, int c) {}
+		}
+
+		public class ProtectedPrivate {
+			protected ProtectedPrivate(int x) {}
+		}
+
+		public final class PrivateFinal {
+			private PrivateFinal() {}
+		}
+
+		public final class Final {}
+
+	}
+
+	@Test
+	public void testTreatingPrivateConstructorsAsFinalClass() throws Exception {
+		DiffPluginImpl diff = new DiffPluginImpl();
+		diff.setIgnore("METHOD");
+		try (Builder b = new Builder()) {
+			b.addClasspath(IO.getFile("bin_test"));
+			b.setProperty("Export-Package", "test.baseline");
+			Jar build = b.build();
+			assertThat(b.check()).isTrue();
+			Tree tree = diff.tree(b);
+			Tree pack = tree.get("<api>")
+				.get("test.baseline");
+			Tree Normal = pack.get("test.baseline.BaselineTest$PrivateConstructorsAndFinal$Normal");
+			Tree Final = pack.get("test.baseline.BaselineTest$PrivateConstructorsAndFinal$Final");
+			Tree PrivateFinal = pack.get("test.baseline.BaselineTest$PrivateConstructorsAndFinal$PrivateFinal");
+			Tree Private = pack.get("test.baseline.BaselineTest$PrivateConstructorsAndFinal$Private");
+			Tree ProtectedPrivate = pack.get("test.baseline.BaselineTest$PrivateConstructorsAndFinal$ProtectedPrivate");
+			Tree PrivateMultiple = pack.get("test.baseline.BaselineTest$PrivateConstructorsAndFinal$PrivateMultiple");
+
+			assertThat(Normal).isNotNull();
+			assertThat(Final).isNotNull();
+			assertThat(PrivateFinal).isNotNull();
+			assertThat(Private).isNotNull();
+			assertThat(ProtectedPrivate).isNotNull();
+			assertThat(PrivateMultiple).isNotNull();
+
+			assertThat(Normal.get("final")).isNull();
+			assertThat(PrivateFinal.get("final")).isNotNull();
+			assertThat(Private.get("final")).isNotNull();
+			assertThat(Final.get("final")).isNotNull();
+			assertThat(ProtectedPrivate.get("final")).isNull();
+			assertThat(PrivateMultiple.get("final")).isNotNull();
+
+			Tree[] ORDER = new Tree[] {
+				Normal, Final, PrivateFinal,Private
+			};
+			Delta[][] deltas = new Delta[][] {
+				{
+					Delta.UNCHANGED, Delta.MAJOR, Delta.MAJOR, Delta.MAJOR
+				}, {
+					Delta.MINOR, Delta.UNCHANGED, Delta.MAJOR, Delta.MAJOR
+				}, {
+					Delta.MINOR, Delta.MINOR, Delta.UNCHANGED, Delta.UNCHANGED
+				}, {
+					Delta.MINOR, Delta.MINOR, Delta.UNCHANGED, Delta.UNCHANGED
+				},
+			};
+			for (int y = 0; y < ORDER.length; y++) {
+				for (int x = 0; x < ORDER.length; x++) {
+					Tree older = ORDER[y];
+					Tree newer = ORDER[x];
+					System.out.println(newer.diff(older));
+					assertThat(newer.diff(older)
+						.getDelta()).isEqualTo(deltas[y][x]);
+				}
+			}
+		}
+	}
+
 	/**
 	 * Test 2 jars compiled with different compilers
 	 */

diff --git a/biz.aQute.bndlib/src/aQute/bnd/differ/JavaElement.java b/biz.aQute.bndlib/src/aQute/bnd/differ/JavaElement.java
@@ -271,6 +271,7 @@ Element classElement(final Clazz clazz) throws Exception {
 		Instructions matchers = providerMatcher.get(name.getPackageRef());
 		boolean p = matchers != null && matchers.matches(shortName);
 		final AtomicBoolean provider = new AtomicBoolean(p);
+		final AtomicBoolean isExtendable = new AtomicBoolean(false);
 
 		clazz.parseClassFileWithCollector(new ClassDataCollector() {
 			boolean			memberEnd;
@@ -286,6 +287,9 @@ public void method(MethodDef defined) {
 				if ((defined.isProtected() || defined.isPublic())) {
 					last = defined;
 					methods.add(defined);
+					if (defined.isConstructor()) {
+						isExtendable.set(true);
+					}
 				} else {
 					last = null;
 				}
@@ -563,6 +567,10 @@ else if (clazz.isEnum())
 			remove = MAJOR;
 		}
 
+		if (clazz.isFinal()) {
+			isExtendable.set(false);
+		}
+
 		for (MethodDef m : methods) {
 			if (m.isSynthetic()) { // Ignore synthetic methods
 				continue;
@@ -594,7 +602,7 @@ else if (clazz.isEnum())
 			// override a method from a super class that was not
 			// final. So we actually remove the final for methods
 			// in a final class.
-			if (clazz.isFinal())
+			if (!isExtendable.get())
 				children.remove(FINAL);
 
 			children.add(getReturn(m.getType()));
@@ -651,6 +659,9 @@ else if (clazz.isEnum())
 
 		Integer inner_access_flags = innerAccess.get(clazz.getClassName());
 		int access_flags = (inner_access_flags != null) ? inner_access_flags.intValue() : clazz.getAccess();
+		if (!isExtendable.get()) {
+			access_flags |= Modifier.FINAL;
+		}
 		access(members, access_flags, clazz.isDeprecated(), provider.get());
 
 		// And make the result

diff --git a/bndtools.core/bnd.bnd b/bndtools.core/bnd.bnd
@@ -19,7 +19,7 @@ Export-Package: org.osgi.service.metatype.annotations
 
 -includepackage: \
 	bndtools.*,\
-	org.bndtools.build.api.*,\
+	org.bndtools.build.api.*, \
 	org.bndtools.core.*,\
 	org.bndtools.facade