correctly search for overridden/overriding package-local members

GitOrigin-RevId: e6aabe4198d345ccce6bc9f64d28a95ca2755ef8

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl.log

@@ -0,0 +1,12 @@
+Cleaning output files:
+out/production/DeleteOverridingPackageLocalMethodImpl/a/DerivedA.class
+End of files
+Compiling files:
+src/a/DerivedA.java
+End of files
+Cleaning output files:
+out/production/DeleteOverridingPackageLocalMethodImpl/a/Client.class
+End of files
+Compiling files:
+src/a/Client.java
+End of files

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl/src/a/Base.java

@@ -0,0 +1,5 @@
+package a;
+
+public class Base {
+    void foo(String arg) {}
+}

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl/src/a/Client.java

@@ -0,0 +1,7 @@
+package a;
+
+public class Client {
+    public static void foo(DerivedA obj) {
+        obj.foo("test");
+    }
+}

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl/src/a/DerivedA.java

@@ -0,0 +1,5 @@
+package a;
+
+public class DerivedA extends b.DerivedB {
+    void foo(String arg) {}
+}

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl/src/a/DerivedA.java.new

@@ -0,0 +1,4 @@
+package a;
+
+public class DerivedA extends b.DerivedB {
+}

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl/src/b/DerivedB.java

@@ -0,0 +1,4 @@
+package b;
+
+public class DerivedB extends a.Base {
+}

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl2.log

@@ -0,0 +1,12 @@
+Cleaning output files:
+out/production/DeleteOverridingPackageLocalMethodImpl2/a/DerivedA.class
+End of files
+Compiling files:
+src/a/DerivedA.java
+End of files
+Cleaning output files:
+out/production/DeleteOverridingPackageLocalMethodImpl2/a/DerivedA2.class
+End of files
+Compiling files:
+src/a/DerivedA2.java
+End of files

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl2/src/a/Base.java

@@ -0,0 +1,5 @@
+package a;
+
+public abstract class Base {
+    abstract void foo(String arg);
+}

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl2/src/a/Client.java

@@ -0,0 +1,7 @@
+package a;
+
+public class Client {
+    public static void foo(Base obj) {
+        obj.foo("test");
+    }
+}

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl2/src/a/DerivedA.java

@@ -0,0 +1,5 @@
+package a;
+
+public abstract class DerivedA extends b.DerivedB {
+    void foo(String arg) {}
+}

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl2/src/a/DerivedA.java.new

@@ -0,0 +1,4 @@
+package a;
+
+public abstract class DerivedA extends b.DerivedB {
+}

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl2/src/a/DerivedA2.java

@@ -0,0 +1,4 @@
+package a;
+
+public class DerivedA2 extends DerivedA {
+}

java/java-tests/testData/compileServer/incremental/membersChange/deleteOverridingPackageLocalMethodImpl2/src/b/DerivedB.java

@@ -0,0 +1,4 @@
+package b;
+
+public abstract class DerivedB extends a.Base {
+}

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

@@ -378,7 +378,7 @@ public final class Mappings {
       };
     }
 
-    private void addOverridingMethods(final MethodRepr m, final ClassRepr fromClass, final Predicate<? super MethodRepr> predicate, final Collection<? super Pair<MethodRepr, ClassRepr>> container, IntSet visitedClasses) {
+    private void addOverridingMethods(MethodRepr m, ClassRepr methodClass, final ClassRepr fromClass, final Predicate<? super MethodRepr> predicate, final Collection<? super Pair<MethodRepr, ClassRepr>> container, IntSet visitedClasses) {
       if (m.name == myInitName) {
         return; // overriding is not defined for constructors
       }
@@ -398,7 +398,7 @@ public final class Mappings {
         if (!Iterators.isEmpty(reprs)) {
           Iterable<Pair<MethodRepr, ClassRepr>> overriding = Iterators.flat(Iterators.map(
             reprs,
-            r -> isVisibleIn(fromClass, m, r)? Iterators.map(r.findMethods(predicate), mm -> Pair.create(mm, r)) : Collections.emptyList())
+            r -> isVisibleIn(methodClass, m, r)? Iterators.map(r.findMethods(predicate), mm -> Pair.create(mm, r)) : Collections.emptyList())
           );
 
           Set<ClassRepr> found = new SmartHashSet<>();
@@ -409,7 +409,7 @@ public final class Mappings {
 
           for (ClassRepr r : Iterators.filter(reprs, r -> !found.contains(r))) {
             // continue with reprs, for those no overriding members were found
-            addOverridingMethods(m, r, predicate, container, _visitedClasses);
+            addOverridingMethods(m, methodClass, r, predicate, container, _visitedClasses);
           }
         }
       });
@@ -420,11 +420,11 @@ public final class Mappings {
         return Collections.emptySet(); // overriding is not defined for constructors
       }
       final Collection<Pair<MethodRepr, ClassRepr>> result = new HashSet<>();
-      addOverriddenMethods(c, MethodRepr.equalByJavaRules(m), result, null);
+      addOverriddenMethods(c, MethodRepr.equalByJavaRules(m), result, null, c);
       return result;
     }
 
-    private boolean hasOverriddenMethods(final ClassRepr fromClass, final Predicate<? super MethodRepr> predicate, IntSet visitedClasses) {
+    private boolean hasOverriddenMethods(final ClassRepr fromClass, final Predicate<? super MethodRepr> predicate, IntSet visitedClasses, ClassRepr visibilityScope) {
       if (visitedClasses == null) {
         visitedClasses = new IntOpenHashSet();
         visitedClasses.add(fromClass.name);
@@ -435,11 +435,11 @@ public final class Mappings {
         }
         for (ClassRepr superClass : reprsByName(superName.className, ClassRepr.class)) {
           for (MethodRepr mm : superClass.findMethods(predicate)) {
-            if (isVisibleIn(superClass, mm, fromClass)) {
+            if (isVisibleIn(superClass, mm, visibilityScope)) {
               return true;
             }
           }
-          if (hasOverriddenMethods(superClass, predicate, visitedClasses)) {
+          if (hasOverriddenMethods(superClass, predicate, visitedClasses, visibilityScope)) {
             return true;
           }
         }
@@ -469,7 +469,7 @@ public final class Mappings {
       return false;
     }
 
-    private void addOverriddenMethods(final ClassRepr fromClass, final Predicate<? super MethodRepr> predicate, final Collection<? super Pair<MethodRepr, ClassRepr>> container, IntSet visitedClasses) {
+    private void addOverriddenMethods(final ClassRepr fromClass, final Predicate<? super MethodRepr> predicate, final Collection<? super Pair<MethodRepr, ClassRepr>> container, IntSet visitedClasses, final ClassRepr visibilityScope) {
       if (visitedClasses == null) {
         visitedClasses = new IntOpenHashSet();
         visitedClasses.add(fromClass.name);
@@ -480,9 +480,8 @@ public final class Mappings {
         }
         Iterable<ClassRepr> superClasses = Iterators.collect(reprsByName(superName.className, ClassRepr.class), new SmartList<>());
         if (!Iterators.isEmpty(superClasses)) {
-          Iterable<Pair<MethodRepr, ClassRepr>> pairs = Iterators.flat(Iterators.map(
-            superClasses,
-            superClass -> Iterators.filter(Iterators.map(superClass.findMethods(predicate), mm -> isVisibleIn(superClass, mm, fromClass)? Pair.create(mm, superClass) : null), Iterators.notNullFilter()))
+          Iterable<Pair<MethodRepr, ClassRepr>> pairs = Iterators.flat(
+            Iterators.map(superClasses, superClass -> Iterators.map(superClass.findMethods(mm -> predicate.test(mm) && isVisibleIn(superClass, mm, visibilityScope)), mm -> Pair.create(mm, superClass)))
           );
 
           Set<ClassRepr> found = new SmartHashSet<>();
@@ -493,7 +492,7 @@ public final class Mappings {
 
           for (ClassRepr superClass : Iterators.filter(superClasses, superClass -> !found.contains(superClass))) {
             // continue with those, for whom the matching method was not found
-            addOverriddenMethods(superClass, predicate, container, visitedClasses);
+            addOverriddenMethods(superClass, predicate, container, visitedClasses, visibilityScope);
           }
         }
         else {
@@ -502,7 +501,7 @@ public final class Mappings {
       }
     }
 
-    void addOverriddenFields(final FieldRepr f, final ClassRepr fromClass, final Collection<? super Pair<FieldRepr, ClassRepr>> container, IntSet visitedClasses) {
+    void addOverriddenFields(final FieldRepr f, final ClassRepr fromClass, final Collection<? super Pair<FieldRepr, ClassRepr>> container, IntSet visitedClasses, ClassRepr visibilityScope) {
       if (visitedClasses == null) {
         visitedClasses = new IntOpenHashSet();
         visitedClasses.add(fromClass.name);
@@ -513,17 +512,17 @@ public final class Mappings {
         }
         for (ClassRepr superClass : reprsByName(supername.className, ClassRepr.class)) {
           final FieldRepr ff = superClass.findField(f.name);
-          if (ff != null && isVisibleIn(superClass, ff, fromClass)) {
+          if (ff != null && isVisibleIn(superClass, ff, visibilityScope)) {
             container.add(Pair.create(ff, superClass));
           }
           else{
-            addOverriddenFields(f, superClass, container, visitedClasses);
+            addOverriddenFields(f, superClass, container, visitedClasses, visibilityScope);
           }
         }
       }
     }
 
-    boolean hasOverriddenFields(final FieldRepr f, final ClassRepr fromClass, IntSet visitedClasses) {
+    boolean hasOverriddenFields(final FieldRepr f, final ClassRepr fromClass, IntSet visitedClasses, ClassRepr visibilityScope) {
       if (visitedClasses == null) {
         visitedClasses = new IntOpenHashSet();
         visitedClasses.add(fromClass.name);
@@ -534,10 +533,10 @@ public final class Mappings {
         }
         for (ClassRepr superClass : reprsByName(supername.className, ClassRepr.class)) {
           final FieldRepr ff = superClass.findField(f.name);
-          if (ff != null && isVisibleIn(superClass, ff, fromClass)) {
+          if (ff != null && isVisibleIn(superClass, ff, visibilityScope)) {
             return true;
           }
-          if (hasOverriddenFields(f, superClass, visitedClasses)) {
+          if (hasOverriddenFields(f, superClass, visitedClasses, visibilityScope)) {
             return true;
           }
         }
@@ -654,7 +653,7 @@ public final class Mappings {
     }
 
     boolean isMethodVisible(final ClassRepr classRepr, final MethodRepr m) {
-      return !classRepr.findMethods(MethodRepr.equalByJavaRules(m)).isEmpty() || hasOverriddenMethods(classRepr, MethodRepr.equalByJavaRules(m), null);
+      return !classRepr.findMethods(MethodRepr.equalByJavaRules(m)).isEmpty() || hasOverriddenMethods(classRepr, MethodRepr.equalByJavaRules(m), null, classRepr);
     }
 
     boolean isFieldVisible(final int className, final FieldRepr field) {
@@ -667,7 +666,7 @@ public final class Mappings {
         if (r.getFields().contains(field)) {
           return true;
         }
-        if (hasOverriddenFields(field, r, null)) {
+        if (hasOverriddenFields(field, r, null, r)) {
           return true;
         }
       }
@@ -1211,7 +1210,7 @@ public final class Mappings {
 
         final Supplier<IntSet> propagated = lazy(()-> myFuture.propagateMethodAccess(addedMethod, it.name));
 
-        if (!addedMethod.isPrivate() && addedMethod.myArgumentTypes.length > 0 && !myPresent.hasOverriddenMethods(it, MethodRepr.equalByJavaRules(addedMethod), null)) {
+        if (!addedMethod.isPrivate() && addedMethod.myArgumentTypes.length > 0 && !myPresent.hasOverriddenMethods(it, MethodRepr.equalByJavaRules(addedMethod), null, it)) {
           debug("Conservative case on overriding methods, affecting method usages");
           // do not propagate constructors access, since constructors are always concrete and not accessible via references to subclasses
           myFuture.affectMethodUsages(addedMethod, addedMethod.name == myInitName? null : propagated.get(), addedMethod.createMetaUsage(myContext, it.name), state.myAffectedUsages, state.myDependants);
@@ -1235,7 +1234,7 @@ public final class Mappings {
           debug("Processing overridden by specificity methods");
 
           final Collection<Pair<MethodRepr, ClassRepr>> overridden = new HashSet<>();
-          myFuture.addOverriddenMethods(it, lessSpecificCond, overridden, null);
+          myFuture.addOverriddenMethods(it, lessSpecificCond, overridden, null, it);
           for (final Pair<MethodRepr, ClassRepr> pair : overridden) {
             final MethodRepr method = pair.first;
             final ClassRepr methodClass = pair.second;
@@ -1254,7 +1253,7 @@ public final class Mappings {
 
           final Predicate<MethodRepr> overrides = MethodRepr.equalByJavaRules(addedMethod);
           final Collection<Pair<MethodRepr, ClassRepr>> overriding = new HashSet<>();
-          myFuture.addOverridingMethods(addedMethod, it, lessSpecificCond, overriding, null);
+          myFuture.addOverridingMethods(addedMethod, it, it, lessSpecificCond, overriding, null);
           for (final Pair<MethodRepr, ClassRepr> pair : overriding) {
             final MethodRepr method = pair.first;
             final ClassRepr methodClass = pair.second;
@@ -1331,7 +1330,15 @@ public final class Mappings {
           myFuture.affectStaticMemberImportUsages(removedMethod.name, it.name, propagated.get(), state.myAffectedUsages, state.myDependants);
         }
 
-        if (overriddenMethods.isEmpty()) {
+        if (removedMethod.isPackageLocal()) {
+          // Sometimes javac cannot find an overridden package local method in superclasses, when superclasses are defined in different packages.
+          // This results in compilation error when the code is compiled from the very beginning.
+          // So even if we correctly find a corresponding overridden method and the bytecode compatibility remains,
+          // we still need to affect package local method usages to behave similar to javac.
+          debug("Removed method is package-local, affecting method usages");
+          myFuture.affectMethodUsages(removedMethod, propagated.get(), removedMethod.createUsage(myContext, it.name), state.myAffectedUsages, state.myDependants);
+        }
+        else if (overriddenMethods.isEmpty()) {
           debug("No overridden methods found, affecting method usages");
           myFuture.affectMethodUsages(removedMethod, propagated.get(), removedMethod.createUsage(myContext, it.name), state.myAffectedUsages, state.myDependants);
         }
@@ -1347,7 +1354,7 @@ public final class Mappings {
         }
 
         final Collection<Pair<MethodRepr, ClassRepr>> overridingMethods = new HashSet<>();
-        myFuture.addOverridingMethods(removedMethod, it, MethodRepr.equalByJavaRules(removedMethod), overridingMethods, null);
+        myFuture.addOverridingMethods(removedMethod, it, it, MethodRepr.equalByJavaRules(removedMethod), overridingMethods, null);
 
         for (final Pair<MethodRepr, ClassRepr> p : overridingMethods) {
           final Iterable<File> fNames = classToSourceFileGet(p.second.name);
@@ -1483,7 +1490,7 @@ public final class Mappings {
 
               final List<Pair<MethodRepr, ClassRepr>> overridingMethods = new LinkedList<>();
 
-              myFuture.addOverridingMethods(m, it, MethodRepr.equalByJavaRules(m), overridingMethods, null);
+              myFuture.addOverridingMethods(m, it, it, MethodRepr.equalByJavaRules(m), overridingMethods, null);
 
               for(final Pair<MethodRepr, ClassRepr> p : overridingMethods) {
                 final ClassRepr aClass = p.getSecond();
@@ -1698,7 +1705,7 @@ public final class Mappings {
         }
 
         final Collection<Pair<FieldRepr, ClassRepr>> overriddenFields = new HashSet<>();
-        myFuture.addOverriddenFields(f, classRepr, overriddenFields, null);
+        myFuture.addOverriddenFields(f, classRepr, overriddenFields, null, classRepr);
 
         for (final Pair<FieldRepr, ClassRepr> p : overriddenFields) {
           final FieldRepr ff = p.first;

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

@@ -1,18 +1,4 @@
-/*
- * Copyright 2000-2012 JetBrains s.r.o.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package org.jetbrains.ether;
 
 public class MemberChangeTest extends IncrementalTestCase {
@@ -184,6 +170,14 @@ public class MemberChangeTest extends IncrementalTestCase {
     doTest();
   }
 
+  public void testDeleteOverridingPackageLocalMethodImpl() {
+    doTest().assertFailed(); // should fail because of javac bug
+  }
+
+  public void testDeleteOverridingPackageLocalMethodImpl2() {
+    doTest().assertFailed();
+  }
+
   public void testHierarchy() {
     doTest();
   }