WELD-2507: More fixes

- match the whole method signature
- better handling of class hierarchies
- improved test

impl/src/main/java/org/jboss/weld/bean/proxy/InterceptedSubclassFactory.java

@@ -193,8 +193,11 @@ void doReturn(CodeAttribute b, ClassMethod method) {
                         if (Modifier.isFinal(method.getModifiers())) {
                             finalMethods.add(methodSignature);
                         }
-                        if (method.isBridge() && !superClassAbstractAndPackagePrivate(cls, method)) {
-                            declaredBridgeMethods.add(new BridgeMethod(methodSignature, method.getGenericReturnType()));
+                        if (method.isBridge()) {
+                            BridgeMethod bridgeMethod = new BridgeMethod(methodSignature, method.getGenericReturnType());
+                            if (!hasAbstractPackagePrivateSuperClassWithImplementation(cls, bridgeMethod)) {
+                                declaredBridgeMethods.add(bridgeMethod);
+                            }
                         }
                     }
                 }
@@ -240,30 +243,26 @@ void doReturn(CodeAttribute b, ClassMethod method) {
     }
 
     /**
-     * Returns true if super class of the parameter exists and is abstract and package private. In such case we want to omit
-     * such method.
+     * Returns true if super class of the parameter exists and is abstract and package private. In such case we want to omit such method.
+     *
      * See WELD-2507 and Oracle issue - https://bugs.java.com/view_bug.do?bug_id=6342411
      *
      * @return true if the super class exists and is abstract and package private
      */
-    private boolean superClassAbstractAndPackagePrivate(Class<?> clazz, Method methodFromClazz) {
+    private boolean hasAbstractPackagePrivateSuperClassWithImplementation(Class<?> clazz, BridgeMethod bridgeMethod) {
         Class<?> superClass = clazz.getSuperclass();
-        if (superClass == null) {
-            return false;
-        }
-        int modifiers = superClass.getModifiers();
-        if (Modifier.isAbstract(modifiers) && Reflections.isPackagePrivate(modifiers)) {
-            // if superclass is abstract, we need to dig deeper
-            for (Method m : superClass.getDeclaredMethods()) {
-                if (m.getReturnType().equals(methodFromClazz.getReturnType())
-                    && m.getName().equals(methodFromClazz.getName())
-                    && !Reflections.isAbstract(m)) {
-                    // this is the case we are after -> methods have same signature and the one in super class has actual implementation
-                    return true;
+        while (superClass != null) {
+            if (Modifier.isAbstract(superClass.getModifiers()) && Reflections.isPackagePrivate(superClass.getModifiers())) {
+                // if superclass is abstract, we need to dig deeper
+                for (Method method : superClass.getDeclaredMethods()) {
+                    if (bridgeMethod.signature.matches(method) && method.getGenericReturnType().equals(bridgeMethod.returnType)
+                            && !Reflections.isAbstract(method)) {
+                        // this is the case we are after -> methods have same signature and the one in super class has actual implementation
+                        return true;
+                    }
                 }
             }
-        } else {
-            return false;
+            superClass = superClass.getSuperclass();
         }
         return false;
     }

tests-arquillian/src/test/java/org/jboss/weld/tests/interceptors/inheritance/packagePrivate/AbstractPackagePrivateClass.java

@@ -22,15 +22,19 @@
  * @author <a href="mailto:manovotn@redhat.com">Matej Novotny</a>
  */
 @TestBinding
-abstract class AbstractPackagePrivateClass<S extends Number> {
+abstract class AbstractPackagePrivateClass<S extends Number> extends SuperAbstractPackagePrivateClass<Number> {
 
     @TestBinding
     public String implementedMethod() {
         return AbstractPackagePrivateClass.class.getSimpleName();
     }
-
-    @TestBinding
+
     public abstract String abstractMethod();
-
+
+    // For this method a bridge method that needs to be processed is generated
     public abstract void foo(S param);
+
+    // This method has intentionally the same name as foo(S) but different signature
+    public void foo() {
+    }
 }

tests-arquillian/src/test/java/org/jboss/weld/tests/interceptors/inheritance/packagePrivate/ActualImpl.java

@@ -25,6 +25,7 @@
 @ApplicationScoped
 public class ActualImpl extends AbstractPackagePrivateClass<Integer>{
 
+    @TestBinding
     @Override
     public String abstractMethod() {
         return ActualImpl.class.getSimpleName();
@@ -34,5 +35,5 @@ public String abstractMethod() {
     @TestBinding
     public void foo(Integer param) {
     }
-    
+
 }

tests-arquillian/src/test/java/org/jboss/weld/tests/interceptors/inheritance/packagePrivate/PackagePrivateInterceptedTest.java

@@ -16,6 +16,8 @@
  */
 package org.jboss.weld.tests.interceptors.inheritance.packagePrivate;
 
+import static org.junit.Assert.assertEquals;
+
 import javax.inject.Inject;
 
 import org.jboss.arquillian.container.test.api.Deployment;
@@ -24,7 +26,6 @@
 import org.jboss.shrinkwrap.api.BeanArchive;
 import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.weld.test.util.Utils;
-import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -37,18 +38,33 @@ public class PackagePrivateInterceptedTest {
 
     @Deployment
     public static Archive<?> deploy() {
-        return ShrinkWrap.create(BeanArchive.class, Utils.getDeploymentNameAsHash(PackagePrivateInterceptedTest.class))
-            .intercept(SomeInterceptor.class)
-            .addPackage(PackagePrivateInterceptedTest.class.getPackage());
+        return ShrinkWrap
+                .create(BeanArchive.class,
+                        Utils.getDeploymentNameAsHash(PackagePrivateInterceptedTest.class))
+                .intercept(SomeInterceptor.class)
+                .addPackage(PackagePrivateInterceptedTest.class.getPackage());
     }
-    
+
     @Inject
     AbstractPackagePrivateClass<Integer> bean;
-
+
+    @Inject
+    ActualImpl actualImpl;
+
     @Test
     public void testInvocationIntercepted() {
+        SomeInterceptor.INVOCATION_COUNT.set(0);
         bean.implementedMethod();
-        bean.foo(null);
-        Assert.assertEquals(2, SomeInterceptor.invocationCount);
+        bean.implementedMethod("foo");
+        bean.abstractMethod();
+        bean.foo(2);
+        assertEquals(4, SomeInterceptor.INVOCATION_COUNT.get());
+
+        SomeInterceptor.INVOCATION_COUNT.set(0);
+        actualImpl.implementedMethod();
+        actualImpl.implementedMethod("foo");
+        actualImpl.abstractMethod();
+        actualImpl.foo(1);
+        assertEquals(4, SomeInterceptor.INVOCATION_COUNT.get());
     }
 }

tests-arquillian/src/test/java/org/jboss/weld/tests/interceptors/inheritance/packagePrivate/SomeInterceptor.java

@@ -16,6 +16,8 @@
  */
 package org.jboss.weld.tests.interceptors.inheritance.packagePrivate;
 
+import java.util.concurrent.atomic.AtomicInteger;
+
 import javax.annotation.Priority;
 import javax.interceptor.AroundInvoke;
 import javax.interceptor.Interceptor;
@@ -30,11 +32,11 @@
 @TestBinding
 public class SomeInterceptor {
 
-    public static int invocationCount = 0;
+    static final AtomicInteger INVOCATION_COUNT = new AtomicInteger(0);
 
     @AroundInvoke
     public Object interceptService(InvocationContext invocationContext) throws Exception {
-        invocationCount++;
+        INVOCATION_COUNT.incrementAndGet();
         return invocationContext.proceed();
     }
 }

tests-arquillian/src/test/java/org/jboss/weld/tests/interceptors/inheritance/packagePrivate/SuperAbstractPackagePrivateClass.java

@@ -0,0 +1,30 @@
+/*
+ * JBoss, Home of Professional Open Source
+ * Copyright 2018, Red Hat, Inc., and individual contributors
+ * by the @authors tag. See the copyright.txt in the distribution for a
+ * full listing of individual contributors.
+ *
+ * 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.
+ */
+package org.jboss.weld.tests.interceptors.inheritance.packagePrivate;
+
+/**
+ * Class is intentionally kept as package private.
+ *
+ * @author Martin Kouba
+ */
+abstract class SuperAbstractPackagePrivateClass<S extends Number> {
+
+    @TestBinding
+    public String implementedMethod(String param) {
+        return "ping";
+    }
+}