Correctly handle removal of 'protected' access flag on fields (IDEA-184854);

Fix InheritanceConstraint: when member access is changed to protected or package-private, more precise calculation of affected files

java/java-tests/testData/compileServer/incremental/fieldModifiers/setPackagePrivate.log

@@ -0,0 +1,17 @@
+Cleaning output files:
+out/production/SetPackagePrivate/qqq/Util$1.class
+out/production/SetPackagePrivate/qqq/Util$Base.class
+out/production/SetPackagePrivate/qqq/Util$BaseImpl.class
+out/production/SetPackagePrivate/qqq/Util.class
+End of files
+Compiling files:
+src/qqq/Util.java
+End of files
+Cleaning output files:
+out/production/SetPackagePrivate/ppp/Client$MyUtil$1.class
+out/production/SetPackagePrivate/ppp/Client$MyUtil.class
+out/production/SetPackagePrivate/ppp/Client.class
+End of files
+Compiling files:
+src/ppp/Client.java
+End of files

java/java-tests/testData/compileServer/incremental/fieldModifiers/setPackagePrivate/src/ppp/Client.java

@@ -0,0 +1,16 @@
+package ppp;
+
+import qqq.Util;
+
+public class Client {
+  private static class MyUtil extends Util {
+    public static void perform() {
+      BaseImpl impl = new BaseImpl() {
+        @Override
+        protected void init() {
+          System.out.println(myMessage);
+        }
+      };
+    }
+  }
+}

java/java-tests/testData/compileServer/incremental/fieldModifiers/setPackagePrivate/src/qqq/Util.java

@@ -0,0 +1,12 @@
+package qqq;
+
+public class Util {
+  private static class Base {
+    protected String myMessage = "msg";
+  }
+
+  protected static class BaseImpl extends Base {
+    protected void init() {
+    }
+  }
+}

java/java-tests/testData/compileServer/incremental/fieldModifiers/setPackagePrivate/src/qqq/Util.java.new

@@ -0,0 +1,12 @@
+package qqq;
+
+public class Util {
+  private static class Base {
+    String myMessage = "msg";
+  }
+
+  protected static class BaseImpl extends Base {
+    protected void init() {
+    }
+  }
+}

java/java-tests/testData/compileServer/incremental/fieldModifiers/setProtected.log

@@ -1,12 +1,12 @@
 Cleaning output files:
-out/production/SetProtected/A.class
+out/production/SetProtected/qqq/A.class
 End of files
 Compiling files:
-src/A.java
+src/qqq/A.java
 End of files
 Cleaning output files:
-out/production/SetProtected/B.class
+out/production/SetProtected/ppp/D.class
 End of files
 Compiling files:
-src/B.java
-End of files
+src/ppp/D.java
+End of files

java/java-tests/testData/compileServer/incremental/fieldModifiers/setProtected/src/ppp/D.java

@@ -0,0 +1,8 @@
+package ppp;
+import qqq.*;
+
+public class D {
+    void f (A a) {
+        int y = a.x;
+    }
+}

java/java-tests/testData/compileServer/incremental/fieldModifiers/setProtected/src/qqq/A.java

@@ -1,3 +1,5 @@
+package qqq;
+
 public class A {
     public int x;
 }

java/java-tests/testData/compileServer/incremental/fieldModifiers/setProtected/src/qqq/A.java.new

@@ -1,3 +1,5 @@
+package qqq;
+
 public class A {
     protected int x;
 }

java/java-tests/testData/compileServer/incremental/fieldModifiers/setProtected/src/qqq/B.java

@@ -1,3 +1,5 @@
+package qqq;
+
 public class B {
     void f (A a) {
         int y = a.x;

java/java-tests/testData/compileServer/incremental/fieldModifiers/setProtected/src/qqq/C.java

@@ -1,3 +1,5 @@
+package qqq;
+
 public class C extends A {
     int f (){
         return x;

java/java-tests/testData/compileServer/incremental/methodModifiers/setProtected.log

@@ -3,10 +3,4 @@ out/production/SetProtected/A.class
 End of files
 Compiling files:
 src/A.java
-End of files
-Cleaning output files:
-out/production/SetProtected/B.class
-End of files
-Compiling files:
-src/B.java
-End of files
+End of files

java/java-tests/testData/compileServer/incremental/methodModifiers/setProtectedFromPublic.log

@@ -0,0 +1,12 @@
+Cleaning output files:
+out/production/SetProtectedFromPublic/qqq/A.class
+End of files
+Compiling files:
+src/qqq/A.java
+End of files
+Cleaning output files:
+out/production/SetProtectedFromPublic/ppp/D.class
+End of files
+Compiling files:
+src/ppp/D.java
+End of files

java/java-tests/testData/compileServer/incremental/methodModifiers/setProtectedFromPublic/src/ppp/D.java

@@ -0,0 +1,8 @@
+package ppp;
+import qqq.*;
+
+class D {
+   void f (A a) {
+       a.f();
+   }
+}

java/java-tests/testData/compileServer/incremental/methodModifiers/setProtectedFromPublic/src/qqq/A.java

@@ -0,0 +1,7 @@
+package qqq;
+
+public class A {
+    public void f (){
+
+    }
+}

java/java-tests/testData/compileServer/incremental/methodModifiers/setProtectedFromPublic/src/qqq/A.java.new

@@ -0,0 +1,7 @@
+package qqq;
+
+public class A {
+   protected void f() {
+
+   }
+}

java/java-tests/testData/compileServer/incremental/methodModifiers/setProtectedFromPublic/src/qqq/B.java

@@ -0,0 +1,7 @@
+package qqq;
+
+class B {
+   void f (A a) {
+       a.f();
+   }
+}

java/java-tests/testData/compileServer/incremental/methodModifiers/setProtectedFromPublic/src/qqq/C.java

@@ -0,0 +1,7 @@
+package qqq;
+
+class C extends A {
+    void g(){
+        f();
+    }
+}

jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Difference.java

@@ -25,10 +25,10 @@ import java.util.*;
  */
 public abstract class Difference {
 
-  public static boolean weakerAccess(final int me, final int then) {
-    return ((me & Opcodes.ACC_PRIVATE) > 0 && (then & Opcodes.ACC_PRIVATE) == 0) ||
-           ((me & Opcodes.ACC_PROTECTED) > 0 && (then & Opcodes.ACC_PUBLIC) > 0) ||
-           (isPackageLocal(me) && (then & Opcodes.ACC_PROTECTED) > 0);
+  public static boolean weakerAccess(final int me, final int than) {
+    return ((me & Opcodes.ACC_PRIVATE) > 0 && (than & Opcodes.ACC_PRIVATE) == 0) ||
+           ((me & Opcodes.ACC_PROTECTED) > 0 && (than & Opcodes.ACC_PUBLIC) > 0) ||
+           (isPackageLocal(me) && (than & (Opcodes.ACC_PROTECTED | Opcodes.ACC_PUBLIC)) > 0);
   }
 
   private static boolean isPackageLocal(final int access) {
@@ -161,7 +161,7 @@ public abstract class Difference {
 
   public abstract boolean no();
 
-  public abstract boolean weakedAccess();
+  public abstract boolean accessRestricted();
 
   public abstract int addedModifiers();
 

jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/DifferenceImpl.java

@@ -22,8 +22,8 @@ class DifferenceImpl extends Difference{
     return myDelegate.no();
   }
 
-  public boolean weakedAccess() {
-    return myDelegate.weakedAccess();
+  public boolean accessRestricted() {
+    return myDelegate.accessRestricted();
   }
 
   public int addedModifiers() {

jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java

@@ -792,6 +792,11 @@ public class Mappings {
     public class InheritanceConstraint extends PackageConstraint {
       public final int rootClass;
 
+      public InheritanceConstraint(ClassRepr rootClass) {
+        super(rootClass.getPackageName());
+        this.rootClass = rootClass.name;
+      }
+
       public InheritanceConstraint(final int rootClass) {
         super(ClassRepr.getPackageName(myContext.getValue(rootClass)));
         this.rootClass = rootClass;
@@ -800,7 +805,7 @@ public class Mappings {
       @Override
       public boolean checkResidence(final int residence) {
         final Boolean inheritorOf = isInheritorOf(residence, rootClass, null);
-        return inheritorOf == null || !inheritorOf || super.checkResidence(residence);
+        return (inheritorOf == null || !inheritorOf) && super.checkResidence(residence);
       }
     }
   }
@@ -1444,7 +1449,7 @@ public class Mappings {
             myFuture.affectMethodUsages(m, propagated, m.createUsage(myContext, it.name), usages, state.myDependants);
 
             for (final UsageRepr.Usage usage : usages) {
-              state.myUsageConstraints.put(usage, myFuture.new InheritanceConstraint(it.name));
+              state.myUsageConstraints.put(usage, myFuture.new PackageConstraint(it.getPackageName()));
             }
 
             state.myAffectedUsages.addAll(usages);
@@ -1510,7 +1515,7 @@ public class Mappings {
                   }
 
                   for (final UsageRepr.Usage usage : usages) {
-                    state.myUsageConstraints.put(usage, myFuture.new InheritanceConstraint(it.name));
+                    state.myUsageConstraints.put(usage, myFuture.new InheritanceConstraint(it));
                   }
                   constrained = true;
                 }
@@ -1628,13 +1633,13 @@ public class Mappings {
               UsageConstraint constraint;
 
               if ((ff.isProtected() && f.isPublic()) || (f.isProtected() && ff.isPublic()) || (ff.isPackageLocal() && f.isProtected())) {
-                constraint = myFuture.new InheritanceConstraint(cc.name).negate();
+                constraint = myFuture.new InheritanceConstraint(cc).negate();
               }
               else if (ff.isPublic() && ff.isPackageLocal()) {
                 constraint = myFuture.new PackageConstraint(cc.getPackageName()).negate();
               }
               else {
-                final Util.InheritanceConstraint inherit = myFuture.new InheritanceConstraint(cc.name);
+                final Util.InheritanceConstraint inherit = myFuture.new InheritanceConstraint(cc);
                 final Util.PackageConstraint matchPackage = myFuture.new PackageConstraint(cc.getPackageName());
                 constraint = inherit.negate().and(matchPackage.negate());
               }
@@ -1706,7 +1711,7 @@ public class Mappings {
           final int changedModifiers = d.addedModifiers() | d.removedModifiers();
           final boolean harmful = (changedModifiers & (Opcodes.ACC_STATIC | Opcodes.ACC_FINAL)) > 0;
           final boolean accessChanged = (changedModifiers & (Opcodes.ACC_PUBLIC | Opcodes.ACC_PRIVATE | Opcodes.ACC_PROTECTED)) > 0;
-          final boolean becameLessAccessible = accessChanged && !d.weakedAccess();
+          final boolean becameLessAccessible = accessChanged && d.accessRestricted();
           final boolean valueChanged = (d.base() & Difference.VALUE) > 0;
 
           if (harmful || valueChanged || becameLessAccessible) {
@@ -1726,13 +1731,12 @@ public class Mappings {
 
         if (d.base() != Difference.NONE) {
           final TIntHashSet propagated = myFuture.propagateFieldAccess(field.name, it.name);
-          boolean affected = false;
 
           if ((d.base() & Difference.TYPE) > 0 || (d.base() & Difference.SIGNATURE) > 0) {
             debug("Type or signature changed --- affecting field usages");
-            myFuture
-              .affectFieldUsages(field, propagated, field.createUsage(myContext, it.name), state.myAffectedUsages, state.myDependants);
-            affected = true;
+            myFuture.affectFieldUsages(
+              field, propagated, field.createUsage(myContext, it.name), state.myAffectedUsages, state.myDependants
+            );
           }
           else if ((d.base() & Difference.ACCESS) > 0) {
             if ((d.addedModifiers() & Opcodes.ACC_STATIC) > 0 ||
@@ -1740,9 +1744,9 @@ public class Mappings {
                 (d.addedModifiers() & Opcodes.ACC_PRIVATE) > 0 ||
                 (d.addedModifiers() & Opcodes.ACC_VOLATILE) > 0) {
               debug("Added/removed static modifier or added private/volatile modifier --- affecting field usages");
-              myFuture
-                .affectFieldUsages(field, propagated, field.createUsage(myContext, it.name), state.myAffectedUsages, state.myDependants);
-              affected = true;
+              myFuture.affectFieldUsages(
+                field, propagated, field.createUsage(myContext, it.name), state.myAffectedUsages, state.myDependants
+              );
             }
             else {
               final Set<UsageRepr.Usage> usages = new THashSet<>();
@@ -1751,26 +1755,31 @@ public class Mappings {
                 debug("Added final modifier --- affecting field assign usages");
                 myFuture.affectFieldUsages(field, propagated, field.createAssignUsage(myContext, it.name), usages, state.myDependants);
                 state.myAffectedUsages.addAll(usages);
-                affected = true;
               }
 
               if ((d.removedModifiers() & Opcodes.ACC_PUBLIC) > 0) {
                 debug("Removed public modifier, affecting field usages with appropriate constraint");
-                if (!affected) {
-                  myFuture.affectFieldUsages(field, propagated, field.createUsage(myContext, it.name), usages, state.myDependants);
-                  state.myAffectedUsages.addAll(usages);
-                  affected = true;
-                }
+                myFuture.affectFieldUsages(field, propagated, field.createUsage(myContext, it.name), usages, state.myDependants);
+                state.myAffectedUsages.addAll(usages);
 
                 for (final UsageRepr.Usage usage : usages) {
                   if ((d.addedModifiers() & Opcodes.ACC_PROTECTED) > 0) {
-                    state.myUsageConstraints.put(usage, myFuture.new InheritanceConstraint(it.name));
+                    state.myUsageConstraints.put(usage, myFuture.new InheritanceConstraint(it));
                   }
                   else {
                     state.myUsageConstraints.put(usage, myFuture.new PackageConstraint(it.getPackageName()));
                   }
                 }
               }
+              else if ((d.removedModifiers() & Opcodes.ACC_PROTECTED) > 0 && d.accessRestricted()) {
+                debug("Removed protected modifier and the field became less accessible, affecting field usages with package constraint");
+                myFuture.affectFieldUsages(field, propagated, field.createUsage(myContext, it.name), usages, state.myDependants);
+                state.myAffectedUsages.addAll(usages);
+
+                for (final UsageRepr.Usage usage : usages) {
+                  state.myUsageConstraints.put(usage, myFuture.new PackageConstraint(it.getPackageName()));
+                }
+              }
             }
           }
 
@@ -1916,7 +1925,7 @@ public class Mappings {
             final UsageRepr.Usage usage = changedClass.createUsage();
 
             state.myAffectedUsages.add(usage);
-            state.myUsageConstraints.put(usage, myFuture.new InheritanceConstraint(changedClass.name));
+            state.myUsageConstraints.put(usage, myFuture.new InheritanceConstraint(changedClass));
           }
 
           if (diff.packageLocalOn()) {

jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Proto.java

@@ -178,8 +178,8 @@ class Proto implements RW.Savable, Streamable {
       }
 
       @Override
-      public boolean weakedAccess() {
-        return Difference.weakerAccess(past.access, access);
+      public boolean accessRestricted() {
+        return Difference.weakerAccess(access, past.access);
       }
 
       @Override

jps/jps-builders/testSrc/org/jetbrains/ether/FieldModifierTest.java

@@ -39,6 +39,10 @@ public class FieldModifierTest extends IncrementalTestCase {
     doTest();
   }
 
+  public void testSetPackagePrivate() {
+    doTest();
+  }
+
   public void testSetStatic() {
     doTest();
   }

jps/jps-builders/testSrc/org/jetbrains/ether/MethodModifierTest.java

@@ -47,6 +47,10 @@ public class MethodModifierTest extends IncrementalTestCase {
     doTest();
   }
 
+  public void testSetProtectedFromPublic() {
+    doTest();
+  }
+
 
   public void testUnsetFinal() {
     doTest();