PB3 always includes protected fields of superclasses (regardless of package)

[drekbour]

The following ...

class Parent {
  protected x;
}

@GeneratePojoBuilder
class Child extends Parent {
  public y;
}

... should generate withX and withY (which it does).

If Parent is in a different package to Child, x is not accessible to ChildBuilder so withX should not be generated. This is a regression from v2.

• Ditto any other means of accessing writers (setters, constructor)
• This accessibility check isn't actually driven by the packages of Parent and Child but of the (configurable) package of the ChildBuilder which could be a third location seperate to both of them

[mkarneim]

Marc,
I'll have a look at it.
Thank you for this feedback!

[drekbour]

Following is a patch for the test case that highlights the problem (i.e. testAnalyzeWithPojoExtendsClassWithPrivateAndProtectedFieldsFromAnotherPackage currently fails). I can't be bothered to fork+PR just a test so leave this one in your hands...

Include locally with

git checkout -b access-check-76
git am --signoff < 76.patch

Patch below

From c424417004025a1ca57d0b5b06da2d0ab3faa6e1 Mon Sep 17 00:00:00 2001
From: Marc Carter <[email protected]>
Date: Tue, 16 Sep 2014 09:34:20 +0100
Subject: [PATCH] Add failing testcase for #76 (accessibility check of
 protected superclass members)

---
 .../JavaModelAnalyzer_PojoWithSuperclass_Test.java |   40 ++++++++++++++++++-
 .../analysis/with/superclass/SubclassPojo3.java    |    9 ++++
 .../analysis/with/superclass/SuperclassPojo2.java  |    2 +
 .../with/superclass/p2/SuperclassPojo3.java        |    8 ++++
 4 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100644 src/testdata/java/net/karneim/pojobuilder/analysis/with/superclass/SubclassPojo3.java
 create mode 100644 src/testdata/java/net/karneim/pojobuilder/analysis/with/superclass/p2/SuperclassPojo3.java

diff --git a/src/test/java/net/karneim/pojobuilder/analysis/with/superclass/JavaModelAnalyzer_PojoWithSuperclass_Test.java b/src/test/java/net/karneim/pojobuilder/analysis/with/superclass/JavaModelAnalyzer_PojoWithSuperclass_Test.java
index a091d24..bd0eca6 100644
--- a/src/test/java/net/karneim/pojobuilder/analysis/with/superclass/JavaModelAnalyzer_PojoWithSuperclass_Test.java
+++ b/src/test/java/net/karneim/pojobuilder/analysis/with/superclass/JavaModelAnalyzer_PojoWithSuperclass_Test.java
@@ -63,6 +63,7 @@ public class JavaModelAnalyzer_PojoWithSuperclass_Test {
     assertThat(builderType.getName())
         .isEqualTo("net.karneim.pojobuilder.analysis.with.superclass.SubclassPojo1Builder");
     assertThat(output.getBuilderModel().getProperties()).hasSize(3);
+
     PropertyM nameProperty = output.getBuilderModel().getProperties().get(new Key("name", "java.lang.String"));
     assertThat(nameProperty).isNotNull();
     assertThat(nameProperty.getFieldAccess()).isNotNull();
@@ -80,7 +81,7 @@ public class JavaModelAnalyzer_PojoWithSuperclass_Test {
   }

   @Test
-  public void testAnalyzeWithPojoExtendsClassWithPField() throws Exception {
+  public void testAnalyzeWithPojoExtendsClassWithPrivateAndProtectedFields() throws Exception {
     // Given:
     String pojoClassname = SubclassPojo2.class.getCanonicalName();
     TypeElement pojoType = elements.getTypeElement(pojoClassname);
@@ -96,15 +97,48 @@ public class JavaModelAnalyzer_PojoWithSuperclass_Test {
     assertThat(builderType).isNotNull();
     assertThat(builderType.getName())
         .isEqualTo("net.karneim.pojobuilder.analysis.with.superclass.SubclassPojo2Builder");
-    assertThat(output.getBuilderModel().getProperties()).hasSize(1);
+    assertThat(output.getBuilderModel().getProperties()).hasSize(2);
+
     PropertyM visibleMemberProperty = output.getBuilderModel().getProperties().get(new Key("visibleMember", "int"));
     assertThat(visibleMemberProperty).isNotNull();
     assertThat(visibleMemberProperty.getFieldAccess()).isNotNull();
     assertThat(visibleMemberProperty.getFieldAccess().getModifier()).contains(Modifier.PUBLIC);
-    assertThat(output.getBuilderModel().getProperties().get(new Key("hiddenMember", "float"))).isNull();;

+    PropertyM protectedMemberProperty = output.getBuilderModel().getProperties().get(new Key("protectedMember", "float"));
+    assertThat(protectedMemberProperty).isNotNull();
+    assertThat(protectedMemberProperty.getFieldAccess()).isNotNull();
+    assertThat(protectedMemberProperty.getFieldAccess().getModifier()).contains(Modifier.PROTECTED);
+
+    assertThat(output.getBuilderModel().getProperties().get(new Key("hiddenMember", "float"))).isNull();
   }

+  @Test
+  public void testAnalyzeWithPojoExtendsClassWithPrivateAndProtectedFieldsFromAnotherPackage() throws Exception {
+    // Given:
+    String pojoClassname = SubclassPojo3.class.getCanonicalName();
+    TypeElement pojoType = elements.getTypeElement(pojoClassname);
+    Input input = inputFactory.getInput(pojoType);
+
+    // When:
+    Output output = underTest.analyze(input);
+
+    // Then:
+    assertThat(output).isNotNull();
+    assertThat(output.getBuilderModel().getPojoType().getName()).isEqualTo(pojoClassname);
+    TypeM builderType = output.getBuilderModel().getType();
+    assertThat(builderType).isNotNull();
+    assertThat(builderType.getName())
+            .isEqualTo("net.karneim.pojobuilder.analysis.with.superclass.SubclassPojo3Builder");
+    assertThat(output.getBuilderModel().getProperties()).hasSize(1);
+
+    PropertyM visibleMemberProperty = output.getBuilderModel().getProperties().get(new Key("visibleMember", "int"));
+    assertThat(visibleMemberProperty).isNotNull();
+    assertThat(visibleMemberProperty.getFieldAccess()).isNotNull();
+    assertThat(visibleMemberProperty.getFieldAccess().getModifier()).contains(Modifier.PUBLIC);
+
+    assertThat(output.getBuilderModel().getProperties().get(new Key("protectedMember", "float"))).isNull();

+    assertThat(output.getBuilderModel().getProperties().get(new Key("hiddenMember", "float"))).isNull();
+  }

 }
diff --git a/src/testdata/java/net/karneim/pojobuilder/analysis/with/superclass/SubclassPojo3.java b/src/testdata/java/net/karneim/pojobuilder/analysis/with/superclass/SubclassPojo3.java
new file mode 100644
index 0000000..f669ba4
--- /dev/null
+++ b/src/testdata/java/net/karneim/pojobuilder/analysis/with/superclass/SubclassPojo3.java
@@ -0,0 +1,9 @@
+package net.karneim.pojobuilder.analysis.with.superclass;
+
+import net.karneim.pojobuilder.GeneratePojoBuilder;
+import net.karneim.pojobuilder.analysis.with.superclass.p2.SuperclassPojo3;
+
+@GeneratePojoBuilder
+public class SubclassPojo3 extends SuperclassPojo3 {
+  public int visibleMember;
+}
diff --git a/src/testdata/java/net/karneim/pojobuilder/analysis/with/superclass/SuperclassPojo2.java b/src/testdata/java/net/karneim/pojobuilder/analysis/with/superclass/SuperclassPojo2.java
index 22532b6..b366ce3 100644
--- a/src/testdata/java/net/karneim/pojobuilder/analysis/with/superclass/SuperclassPojo2.java
+++ b/src/testdata/java/net/karneim/pojobuilder/analysis/with/superclass/SuperclassPojo2.java
@@ -3,4 +3,6 @@ package net.karneim.pojobuilder.analysis.with.superclass;
 public class SuperclassPojo2 {
   @SuppressWarnings("unused")
   private float hiddenMember = 1.5F;
+
+  protected float protectedMember = 1.5F;
 }
diff --git a/src/testdata/java/net/karneim/pojobuilder/analysis/with/superclass/p2/SuperclassPojo3.java b/src/testdata/java/net/karneim/pojobuilder/analysis/with/superclass/p2/SuperclassPojo3.java
new file mode 100644
index 0000000..d2ec316
--- /dev/null
+++ b/src/testdata/java/net/karneim/pojobuilder/analysis/with/superclass/p2/SuperclassPojo3.java
@@ -0,0 +1,8 @@
+package net.karneim.pojobuilder.analysis.with.superclass.p2;
+
+public class SuperclassPojo3 {
+    @SuppressWarnings("unused")
+    private float hiddenMember = 1.5F;
+
+    protected float protectedMember = 1.5F;
+}
-- 
1.7.9

[mkarneim]

Haha,
Just saw that you posted this just some minutes before I submitted the new release.