Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

More easily detect overridden methods & calls to super #1040

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

More easily detect overridden methods & calls to super #1040

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
380958a
Added Method#isOverridden
java-coding-prodigy Jan 6, 2023
e569b0e
Fixed the method and added a test
java-coding-prodigy Jan 7, 2023
9c68442
Merge branch 'main' into main
java-coding-prodigy Jan 7, 2023
9f58651
Added support for methods in grandparent class and used ArchUnit API …
java-coding-prodigy Jan 8, 2023
1903590
Merge branch 'main' of github.com:TNG/ArchUnit
java-coding-prodigy Jan 8, 2023
3e97e3f
Fixed issue with `isOverridden`
java-coding-prodigy Jan 8, 2023
3ea2f36
Added testing for overriden method in grandparent class
java-coding-prodigy Jan 8, 2023
c901839
Merge branch 'main' of github.com:java-coding-prodigy/ArchUnit
java-coding-prodigy Jan 8, 2023
9e74551
Undo accidental copyright change
java-coding-prodigy Jan 8, 2023
c9e63f6
Check for super methods in interfaces as well
java-coding-prodigy Jan 8, 2023
02eae70
Import fix
java-coding-prodigy Jan 8, 2023
e67cb29
Merge branch 'main' into main
java-coding-prodigy Feb 8, 2023
d7c3481
Got this so far
java-coding-prodigy May 17, 2023
2acc5d6
Got this so far
java-coding-prodigy May 17, 2023
9c8a0a2
Merge branch 'main' into main
java-coding-prodigy May 18, 2023
82c8bfa
Merge branch 'main' into main
java-coding-prodigy Jul 25, 2023
1767d88
Merge branch 'main' into main
java-coding-prodigy Oct 6, 2023
b8b74e6
Merge branch 'TNG:main' into main
java-coding-prodigy Oct 15, 2023
c13a74a
Finally passed the test.
java-coding-prodigy Nov 11, 2023
fa2bebc
Merge branch 'main' into main
java-coding-prodigy Nov 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions archunit/src/main/java/com/tngtech/archunit/core/domain/JavaMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,21 @@
*/
package com.tngtech.archunit.core.domain;

import java.lang.reflect.Method;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Supplier;

import com.tngtech.archunit.PublicAPI;
import com.tngtech.archunit.base.ArchUnitException.InconsistentClassPathException;
import com.tngtech.archunit.base.MayResolveTypesViaReflection;
import com.tngtech.archunit.base.ResolvesTypesViaReflection;
import com.tngtech.archunit.base.Suppliers;
import com.tngtech.archunit.core.importer.DomainBuilders;

import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Supplier;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please adhere to the import order specified in CONTRIBUTING.md, i.e. keep the java.* imports at the top?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I'll do so in JavaMethodTest as well

import static com.google.common.collect.Sets.union;
import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
import static com.tngtech.archunit.core.domain.Formatters.formatMethod;
Expand Down Expand Up @@ -125,6 +126,12 @@ public String getDescription() {
return "Method <" + getFullName() + ">";
}

@PublicAPI(usage = ACCESS)
public boolean isOverridden() {
Method method = methodSupplier.get();
return Arrays.asList(method.getDeclaringClass().getSuperclass().getDeclaredMethods()).contains(method);
Copy link
Member

@hankem hankem Jan 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to avoid the (expensive) reflection API, but more importantly: Does this even work for you? Please provide a unit test that proves the correctnes of your code.

I'd start with something like this:

public class JavaMethodTest {
    @Test
    public void isOverridden() {
        class Base {
            void method1() {}
            void method1(int x) {}
        }
        class Child extends Base {
            void method1() {}
            void method2() {}
        }

        JavaClass childClass = new ClassFileImporter().importClass(Child.class);
        assertThat(childClass.getMethod("method1").isOverridden()).isTrue();
        assertThat(childClass.getMethod("method1", int.class).isOverridden()).isFalse();
        assertThat(childClass.getMethod("method2").isOverridden()).isFalse();
}

Your implementation however gives me childClass.getMethod("method1").isOverridden() == false.

}

@ResolvesTypesViaReflection
@MayResolveTypesViaReflection(reason = "Just part of a bigger resolution process")
private class ReflectMethodSupplier implements Supplier<Method> {
Expand Down