Skip to content

[LANG-1778] MethodUtils.getMatchingMethod() doesn't respect the hierarchy of methods #1414

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.

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

Merged
merged 3 commits into from
Jul 17, 2025

Conversation

wuwu2000
Copy link
Contributor

@garydgregory
Copy link
Member

@wuwu2000

Please fix build failures. You can run mvn (by itself) to run all build checks.

@wuwu2000
Copy link
Contributor Author

@garydgregory thanks, done!

@garydgregory garydgregory merged commit 1e45583 into apache:master Jul 17, 2025
20 checks passed
@garydgregory garydgregory changed the title LANG-1778 fix by reversing order [LANG-1778] MethodUtils.getMatchingMethod() doesn't respect the hierarchy of methods Jul 17, 2025
asf-gitbox-commits pushed a commit that referenced this pull request Jul 17, 2025
hierarchy of methods #1414

- Javadoc
- Sort members
- Simplify test
@garydgregory
Copy link
Member

Hello @wuwu2000

YW 😄

Do you think the same type of issue applies to other callers of org.apache.commons.lang3.reflect.MethodUtils.getAllSuperclassesAndInterfaces(Class<?>) like org.apache.commons.lang3.reflect.MethodUtils.getAnnotation(Method, Class<A>, boolean, boolean)?

What about elsewhere in the class?

@wuwu2000
Copy link
Contributor Author

So far only

  • org.apache.commons.lang3.reflect.MethodUtils.getMethodsListWithAnnotation(Class<?>, Class<? extends Annotation>, boolean, boolean)
  • org.apache.commons.lang3.reflect.MethodUtils.getAnnotation(Method, Class<A>, boolean, boolean) and
  • org.apache.commons.lang3.reflect.MethodUtils.getMatchingMethod(Class<?>, Class<? extends Annotation>, boolean, boolean)

are using this private method.

getMethodsListWithAnnotation in my opinion is uncritical. You just get every method annotated and it works fine

getAnnotation I just checked it has the same issue as getMatchingMethod had before this fix.

To be honest, the thing that getAnnotation and getMatchingMethod find the method most "near" to the implementing class is more like a opinion. I guess the consumer may expect either

  1. find the (annotation of) the method most "up"/"near to implementing class" - sorry I don't know how to express this
  2. find the (annotation of) the method of the interface
  3. get an exception if it's unclear for MethodUtil

I think option 1 is the most intuitive (that's how spring mvc annotation system works, at least similar). Maybe MethodUtils.getAnnotation and MethodUtils.getMatchingMethod need additional parameters to be able to fulfil the expectations of the consumer?

I think in nearer future the order of getAllSuperclassesAndInterfaces should be in the order concrete class => super => super.super etc.. => interfaces - sorry for my quite dirty fix only for getMatchingMethod, I wasn't sure how much I am allowed to change 🗡️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants