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.

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

Already on GitHub? Sign in to your account

Thrown exceptions on MethodInfo #633

Closed
jkschneider opened this issue Jan 13, 2022 · 4 comments
Closed

Thrown exceptions on MethodInfo #633

jkschneider opened this issue Jan 13, 2022 · 4 comments

Comments

@jkschneider
Copy link
Contributor

jkschneider commented Jan 13, 2022

Hi 馃憢

I'd love to add thrown exceptions support to MethodInfo. Thrown exceptions show up on getTypeSignatureOrTypeDescriptor().getThrowsSignature() apparently only when at least one of the exceptions is a generic type variable.

There appears to be a particular attributeNameCpIdx named Exceptions that has the data for all other method signatures with exceptions. If you're willing, I am happy to provide a PR.

We're using classgraph quite heavily in https://github.com/openrewrite/rewrite. Thanks!

@lukehutch
Copy link
Member

I'm glad you're getting some good use out of ClassGraph! I hadn't heard of Rewrite, it looks like an awesome project.

That would be awesome, if you'd be willing to contribute this! Thanks for the offer.

Here are the pointers to what you will need to do (roughly... YMMV):

Pick up the Exceptions attribute roughly here:

https://github.com/classgraph/classgraph/blob/latest/src/main/java/io/github/classgraph/Classfile.java#L1655

Plumb it through to MethodInfo:

https://github.com/classgraph/classgraph/blob/latest/src/main/java/io/github/classgraph/Classfile.java#L1677

Note that ClassGraph goes out of its way to only lazy-load information, so don't try parsing type descriptors or try to look up classes by name, leave them in string form, and parse or look them up as needed, caching the result, inside of MethodInfo (see what has been done for MethodInfo.getTypeDescriptor etc.)

You need to index the returned exception class names in MethodInfo.findReferencedClassInfo:

https://github.com/classgraph/classgraph/blob/latest/src/main/java/io/github/classgraph/MethodInfo.java#L885

If you do parse any type signatures or look up / create any ClassInfo objects (lazily) in MethodInfo, then ensure that the ScanResult is set in those reachable objects once they are created -- see an example here:

https://github.com/classgraph/classgraph/blob/latest/src/main/java/io/github/classgraph/MethodInfo.java#L254

and then to be safe, also add code here to set the ScanResult in those cached objects if they are non-null, at class graph creation time:

https://github.com/classgraph/classgraph/blob/latest/src/main/java/io/github/classgraph/MethodInfo.java#L847

Also add the throws notation to the toString method, avoiding doubling-up exceptions, if there's any chance that exceptions will be listed twice, once in getThrowsSignatures() and once in the Exceptions attribute:

https://github.com/classgraph/classgraph/blob/latest/src/main/java/io/github/classgraph/MethodInfo.java#L1118

Sorry for the complexity of all that :-)

@lukehutch
Copy link
Member

lukehutch commented Jan 13, 2022

PS you should probably add two methods: ClassInfoList getThrownExceptions() and List<String> getThrownExceptionNames(), and these should draw from either the generic throws method signature or the Exceptions attribute if that is not available. The first of these methods should look up and cache the exception classes, by calling ScanResult#getClassInfo(exceptionClassName).

@jkschneider
Copy link
Contributor Author

Thanks for the tips -- will get working on it!

jkschneider added a commit to moderneinc/classgraph that referenced this issue Jan 14, 2022
jkschneider added a commit to moderneinc/classgraph that referenced this issue Jan 14, 2022
jkschneider added a commit to moderneinc/classgraph that referenced this issue Jan 14, 2022
jkschneider added a commit to moderneinc/classgraph that referenced this issue Jan 14, 2022
jkschneider added a commit to moderneinc/classgraph that referenced this issue Jan 14, 2022
jkschneider added a commit to moderneinc/classgraph that referenced this issue Jan 14, 2022
jkschneider added a commit to moderneinc/classgraph that referenced this issue Jan 14, 2022
jkschneider added a commit to moderneinc/classgraph that referenced this issue Jan 14, 2022
lukehutch added a commit that referenced this issue Jan 16, 2022
Add thrown exceptions to MethodInfo (#633)
@lukehutch
Copy link
Member

lukehutch commented Feb 13, 2022

Released in 4.8.139. I'm sorry for the long delay in getting a release out, I have been traveling for the last 3 weeks with little downtime. Thanks again for your contribution!

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

No branches or pull requests

2 participants