usage constrains fixes and optimizations for 'added' fields; more tests added

2026-09-27 10:03:11 +07:00 · 2018-01-15 10:44:31 +01:00
parent 035a4a1331
commit c022771c55
56 changed files with 436 additions and 24 deletions
@@ -0,0 +1,12 @@
+Cleaning output files:
+out/production/HidePackagePrivateWithPackagePrivate/ppp/Derived.class
+End of files
+Compiling files:
+src/ppp/Derived.java
+End of files
+Cleaning output files:
+out/production/HidePackagePrivateWithPackagePrivate/ppp/ClientInside.class
+End of files
+Compiling files:
+src/ppp/ClientInside.java
+End of files
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Base {
+  String x = "base";
+}
@@ -0,0 +1,7 @@
+package ppp;
+
+public class ClientInside {
+  void foo(Derived d) {
+    String str = d.x;
+  }
+}
@@ -0,0 +1,4 @@
+package ppp;
+
+public class Derived extends Base{
+}
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Derived extends Base{
+  String x = "derived";
+}
@@ -0,0 +1,12 @@
+Cleaning output files:
+out/production/HidePackagePrivateWithProtected/ppp/Derived.class
+End of files
+Compiling files:
+src/ppp/Derived.java
+End of files
+Cleaning output files:
+out/production/HidePackagePrivateWithProtected/ppp/ClientInside.class
+End of files
+Compiling files:
+src/ppp/ClientInside.java
+End of files
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Base {
+  String x = "base";
+}
@@ -0,0 +1,7 @@
+package ppp;
+
+public class ClientInside {
+  void foo(Derived d) {
+    String str = d.x;
+  }
+}
@@ -0,0 +1,4 @@
+package ppp;
+
+public class Derived extends Base{
+}
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Derived extends Base{
+  protected String x = "derived";
+}
@@ -0,0 +1,12 @@
+Cleaning output files:
+out/production/HidePackagePrivateWithPublic/ppp/Derived.class
+End of files
+Compiling files:
+src/ppp/Derived.java
+End of files
+Cleaning output files:
+out/production/HidePackagePrivateWithPublic/ppp/ClientInside.class
+End of files
+Compiling files:
+src/ppp/ClientInside.java
+End of files
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Base {
+  String x = "base";
+}
@@ -0,0 +1,7 @@
+package ppp;
+
+public class ClientInside {
+  void foo(Derived d) {
+    String str = d.x;
+  }
+}
@@ -0,0 +1,4 @@
+package ppp;
+
+public class Derived extends Base{
+}
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Derived extends Base{
+  public String x = "derived";
+}
@@ -0,0 +1,12 @@
+Cleaning output files:
+out/production/HideProtectedWithPackagePrivate/ppp/Derived.class
+End of files
+Compiling files:
+src/ppp/Derived.java
+End of files
+Cleaning output files:
+out/production/HideProtectedWithPackagePrivate/qqq/DerivedOutside.class
+End of files
+Compiling files:
+src/qqq/DerivedOutside.java
+End of files
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Base {
+  protected String x = "base";
+}
@@ -0,0 +1,7 @@
+package ppp;
+
+public class ClientInside {
+  void foo(Derived d) {
+    String str = d.x;
+  }
+}
@@ -0,0 +1,4 @@
+package ppp;
+
+public class Derived extends Base{
+}
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Derived extends Base{
+  String x = "derived";
+}
@@ -0,0 +1,8 @@
+package qqq;
+import ppp.*;
+
+public class DerivedOutside extends Derived{
+  void foo() {
+    String str = x;
+  }
+}
@@ -0,0 +1,14 @@
+Cleaning output files:
+out/production/HideProtectedWithProtected/ppp/Derived.class
+End of files
+Compiling files:
+src/ppp/Derived.java
+End of files
+Cleaning output files:
+out/production/HideProtectedWithProtected/ppp/ClientInside.class
+out/production/HideProtectedWithProtected/qqq/DerivedOutside.class
+End of files
+Compiling files:
+src/ppp/ClientInside.java
+src/qqq/DerivedOutside.java
+End of files
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Base {
+  protected String x = "base";
+}
@@ -0,0 +1,7 @@
+package ppp;
+
+public class ClientInside {
+  void foo(Derived d) {
+    String str = d.x;
+  }
+}
@@ -0,0 +1,4 @@
+package ppp;
+
+public class Derived extends Base{
+}
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Derived extends Base{
+  protected String x = "derived";
+}
@@ -0,0 +1,8 @@
+package qqq;
+import ppp.*;
+
+public class DerivedOutside extends Derived{
+  void foo() {
+    String str = x;
+  }
+}
@@ -0,0 +1,14 @@
+Cleaning output files:
+out/production/HideProtectedWithPublic/ppp/Derived.class
+End of files
+Compiling files:
+src/ppp/Derived.java
+End of files
+Cleaning output files:
+out/production/HideProtectedWithPublic/ppp/ClientInside.class
+out/production/HideProtectedWithPublic/qqq/DerivedOutside.class
+End of files
+Compiling files:
+src/ppp/ClientInside.java
+src/qqq/DerivedOutside.java
+End of files
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Base {
+  protected String x = "base";
+}
@@ -0,0 +1,7 @@
+package ppp;
+
+public class ClientInside {
+  void foo(Derived d) {
+    String str = d.x;
+  }
+}
@@ -0,0 +1,4 @@
+package ppp;
+
+public class Derived extends Base{
+}
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Derived extends Base{
+  public String x = "derived";
+}
@@ -0,0 +1,8 @@
+package qqq;
+import ppp.*;
+
+public class DerivedOutside extends Derived{
+  void foo() {
+    String str = x;
+  }
+}
@@ -0,0 +1,14 @@
+Cleaning output files:
+out/production/HidePublicWithPackagePrivate/ppp/Derived.class
+End of files
+Compiling files:
+src/ppp/Derived.java
+End of files
+Cleaning output files:
+out/production/HidePublicWithPackagePrivate/qqq/ClientOutside.class
+out/production/HidePublicWithPackagePrivate/qqq/DerivedOutside.class
+End of files
+Compiling files:
+src/qqq/ClientOutside.java
+src/qqq/DerivedOutside.java
+End of files
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Base {
+  public String x = "base";
+}
@@ -0,0 +1,7 @@
+package ppp;
+
+public class ClientInside {
+  void foo(Derived d) {
+    String str = d.x;
+  }
+}
@@ -0,0 +1,4 @@
+package ppp;
+
+public class Derived extends Base{
+}
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Derived extends Base{
+  String x = "derived";
+}
@@ -0,0 +1,8 @@
+package qqq;
+import ppp.*;
+
+public class ClientOutside {
+  void foo(Derived d) {
+    String str = d.x;
+  }
+}
@@ -0,0 +1,8 @@
+package qqq;
+import ppp.*;
+
+public class DerivedOutside extends Derived{
+  void foo() {
+    String str = x;
+  }
+}
@@ -0,0 +1,12 @@
+Cleaning output files:
+out/production/HidePublicWithProtected/ppp/Derived.class
+End of files
+Compiling files:
+src/ppp/Derived.java
+End of files
+Cleaning output files:
+out/production/HidePublicWithProtected/qqq/ClientOutside.class
+End of files
+Compiling files:
+src/qqq/ClientOutside.java
+End of files
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Base {
+  public String x = "base";
+}
@@ -0,0 +1,7 @@
+package ppp;
+
+public class ClientInside {
+  void foo(Derived d) {
+    String str = d.x;
+  }
+}
@@ -0,0 +1,4 @@
+package ppp;
+
+public class Derived extends Base{
+}
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Derived extends Base{
+  protected String x = "derived";
+}
@@ -0,0 +1,8 @@
+package qqq;
+import ppp.*;
+
+public class ClientOutside {
+  void foo(Derived d) {
+    String str = d.x;
+  }
+}
@@ -0,0 +1,8 @@
+package qqq;
+import ppp.*;
+
+public class DerivedOutside extends Derived{
+  void foo() {
+    String str = x;
+  }
+}
@@ -0,0 +1,16 @@
+Cleaning output files:
+out/production/HidePublicWithPublic/ppp/Derived.class
+End of files
+Compiling files:
+src/ppp/Derived.java
+End of files
+Cleaning output files:
+out/production/HidePublicWithPublic/ppp/ClientInside.class
+out/production/HidePublicWithPublic/qqq/ClientOutside.class
+out/production/HidePublicWithPublic/qqq/DerivedOutside.class
+End of files
+Compiling files:
+src/ppp/ClientInside.java
+src/qqq/ClientOutside.java
+src/qqq/DerivedOutside.java
+End of files
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Base {
+  public String x = "base";
+}
@@ -0,0 +1,7 @@
+package ppp;
+
+public class ClientInside {
+  void foo(Derived d) {
+    String str = d.x;
+  }
+}
@@ -0,0 +1,4 @@
+package ppp;
+
+public class Derived extends Base{
+}
@@ -0,0 +1,5 @@
+package ppp;
+
+public class Derived extends Base{
+  public String x = "derived";
+}
@@ -0,0 +1,8 @@
+package qqq;
+import ppp.*;
+
+public class ClientOutside {
+  void foo(Derived d) {
+    String str = d.x;
+  }
+}
@@ -0,0 +1,8 @@
+package qqq;
+import ppp.*;
+
+public class DerivedOutside extends Derived{
+  void foo() {
+    String str = x;
+  }
+}
@@ -1618,38 +1618,35 @@ public class Mappings {
        for (final Pair<FieldRepr, ClassRepr> p : overriddenFields) {
          final FieldRepr ff = p.first;
          final ClassRepr cc = p.second;
-
-          if (!ff.isPrivate()) {
+          if (ff.isPrivate()) {
+            continue;
+          }
+          final boolean sameKind = f.myType.equals(ff.myType) && f.isStatic() == ff.isStatic() && f.isSynthetic() == ff.isSynthetic() && f.isFinal() == ff.isFinal();
+          if (!sameKind || Difference.weakerAccess(f.access, ff.access)) {
            final TIntHashSet propagated = myPresent.propagateFieldAccess(ff.name, cc.name);
-            final Set<UsageRepr.Usage> localUsages = new HashSet<>();

+            final Set<UsageRepr.Usage> affectedUsages = new HashSet<>();
            debug("Affecting usages of overridden field in class ", cc.name);
-            myFuture.affectFieldUsages(ff, propagated, ff.createUsage(myContext, cc.name), localUsages, state.myDependants);
+            myFuture.affectFieldUsages(ff, propagated, ff.createUsage(myContext, cc.name), affectedUsages, state.myDependants);

-            if (f.isPrivate() || (f.isPublic() && (ff.isPublic() || ff.isPackageLocal())) || (f.isProtected() && ff.isProtected()) || (f.isPackageLocal() && ff.isPackageLocal())) {
-              // nothing
-            }
-            else {
-              UsageConstraint constraint;
-
-              if ((ff.isProtected() && f.isPublic()) || (f.isProtected() && ff.isPublic()) || (ff.isPackageLocal() && f.isProtected())) {
-                constraint = myFuture.new InheritanceConstraint(cc).negate();
+            if (sameKind) {
+              // check if we can reduce the number of usages going to be recompiled
+              UsageConstraint constraint = null;
+              if (f.isProtected()) {
+                // no need to recompile usages in field class' package and hierarchy, since newly added field is accessible in this scope
+                constraint = myFuture.new InheritanceConstraint(cc);
              }
-              else if (ff.isPublic() && ff.isPackageLocal()) {
-                constraint = myFuture.new PackageConstraint(cc.getPackageName()).negate();
+              else if (f.isPackageLocal()) {
+                // no need to recompile usages in field class' package, since newly added field is accessible in this scope
+                constraint = myFuture.new PackageConstraint(cc.getPackageName());
              }
-              else {
-                final Util.InheritanceConstraint inherit = myFuture.new InheritanceConstraint(cc);
-                final Util.PackageConstraint matchPackage = myFuture.new PackageConstraint(cc.getPackageName());
-                constraint = inherit.negate().and(matchPackage.negate());
-              }
-
-              for (final UsageRepr.Usage usage : localUsages) {
-                state.myUsageConstraints.put(usage, constraint);
+              if (constraint != null) {
+                for (final UsageRepr.Usage usage : affectedUsages) {
+                  state.myUsageConstraints.put(usage, constraint);
+                }
              }
            }
-
-            state.myAffectedUsages.addAll(localUsages);
+            state.myAffectedUsages.addAll(affectedUsages);
          }
        }
      }
@@ -54,4 +54,40 @@ public class FieldModifierTest extends IncrementalTestCase {
  public void testUnsetStaticFinal() {
    doTest();
  }
+
+  public void testHidePackagePrivateWithPackagePrivate() {
+    doTest();
+  }
+
+  public void testHidePackagePrivateWithProtected() {
+    doTest();
+  }
+
+  public void testHidePackagePrivateWithPublic() {
+    doTest();
+  }
+
+  public void testHideProtectedWithPackagePrivate() {
+    doTest();
+  }
+
+  public void testHideProtectedWithProtected() {
+    doTest();
+  }
+
+  public void testHideProtectedWithPublic() {
+    doTest();
+  }
+
+  public void testHidePublicWithPackagePrivate() {
+    doTest();
+  }
+
+  public void testHidePublicWithProtected() {
+    doTest();
+  }
+
+  public void testHidePublicWithPublic() {
+    doTest();
+  }
 }