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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add toDescriptor to ResolvedMethodDeclaration #3819

Merged
merged 8 commits into from
Dec 31, 2022

Conversation

vanHekthor
Copy link
Contributor

Fixes #3808.

Adds toDescriptor() to ResolvedMethodDeclarations with all the needed implementations. It was also necessary to add toDescriptor() for ResolvedType.

For convenience, a TypeUtils class was added that contains utility methods for getting the descriptors for java.lang.reflect.Method and primitive classes (compare to ASM).

Found issue:

Inner classes(/enums) do not have the correct fully qualified name. You get Class.InnerClass instead of Class$InnerClass. This also effects my test cases for the valueOf and values method of enums.
For now, it should be fine, but I think this needs to be addressed in a future issue and PR.

- add necessary implementations
- add toDescriptor to ResolvedType with necessary implementations
- extend DescriptorTest with new cases
@vanHekthor vanHekthor force-pushed the issue3808 branch 2 times, most recently from 718f582 to acd4cff Compare December 30, 2022 15:19
- remove else if cases
- get correct descriptor for primitive classes (e.g. int.class)
- and also their wrapper classes (Integer.class)
- add toDescriptor to Primitive
@jlerbsc
Copy link
Collaborator

jlerbsc commented Dec 31, 2022

Can you add unit test for all primitive type and remove the unit test dependency with the javassist library.

- TypeDescriptorTest testing descriptors for all primitive types
- rename DescriptorTest to MethodDescriptorTest
@vanHekthor
Copy link
Contributor Author

Can you add unit test for all primitive type and remove the unit test dependency with the javassist library.

Did that in the latest commits e6901c3 and 358e536.

@jlerbsc
Copy link
Collaborator

jlerbsc commented Dec 31, 2022

Thank you for your contribution which will be the last of the year. It will be available in the next version (end of January) because version 3.24.10 was released this afternoon.

@jlerbsc jlerbsc closed this Dec 31, 2022
@jlerbsc jlerbsc reopened this Dec 31, 2022
@codecov
Copy link

codecov bot commented Dec 31, 2022

Codecov Report

Merging #3819 (655af91) into master (e2590f3) will increase coverage by 0.033%.
The diff coverage is 81.632%.

❗ Current head 655af91 differs from pull request most recent head 13ceb6a. Consider uploading reports for the commit 13ceb6a to get more accurate results

Impacted file tree graph

@@               Coverage Diff               @@
##              master     #3819       +/-   ##
===============================================
+ Coverage     57.571%   57.604%   +0.033%     
- Complexity      2182      2185        +3     
===============================================
  Files            638       639        +1     
  Lines          33911     33959       +48     
  Branches        5864      5870        +6     
===============================================
+ Hits           19523     19562       +39     
- Misses         12306     12314        +8     
- Partials        2082      2083        +1     
Flag Coverage Δ
AlsoSlowTests 57.604% <81.632%> (+0.033%) ⬆️
javaparser-core 50.262% <0.000%> (-0.082%) ⬇️
javaparser-symbol-solver 36.641% <81.632%> (+0.063%) ⬆️
jdk-10 57.600% <81.632%> (+0.033%) ⬆️
jdk-11 57.600% <81.632%> (+0.033%) ⬆️
jdk-12 57.600% <81.632%> (+0.039%) ⬆️
jdk-13 57.600% <81.632%> (+0.033%) ⬆️
jdk-14 57.600% <81.632%> (+0.033%) ⬆️
jdk-15 57.600% <81.632%> (+0.033%) ⬆️
jdk-16 57.600% <81.632%> (+0.033%) ⬆️
jdk-8 57.602% <81.632%> (+0.033%) ⬆️
jdk-9 57.600% <81.632%> (+0.033%) ⬆️
macos-latest 57.595% <81.632%> (+0.033%) ⬆️
ubuntu-latest 57.590% <81.632%> (+0.033%) ⬆️
windows-latest 57.587% <81.632%> (+0.033%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...parser/resolution/types/ResolvedPrimitiveType.java 78.571% <0.000%> (-1.139%) ⬇️
...thub/javaparser/resolution/types/ResolvedType.java 65.517% <0.000%> (-2.340%) ⬇️
...aparser/resolution/types/ResolvedTypeVariable.java 83.783% <0.000%> (-2.328%) ⬇️
.../javaparser/resolution/types/ResolvedVoidType.java 80.000% <0.000%> (-20.000%) ⬇️
...in/java/com/github/javaparser/utils/TypeUtils.java 82.142% <82.142%> (ø)
.../com/github/javaparser/ast/type/PrimitiveType.java 77.611% <100.000%> (+2.202%) ⬆️
...javaparser/resolution/types/ResolvedArrayType.java 87.878% <100.000%> (+1.671%) ⬆️
...parser/resolution/types/ResolvedReferenceType.java 79.670% <100.000%> (+0.112%) ⬆️
...rmodel/declarations/JavaParserEnumDeclaration.java 60.126% <100.000%> (+0.511%) ⬆️
...odel/declarations/JavaParserMethodDeclaration.java 75.757% <100.000%> (+0.757%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2590f3...13ceb6a. Read the comment docs.

@vanHekthor
Copy link
Contributor Author

Thank you for your contribution which will be the last of the year. It will be available in the next version (end of January) because version 3.24.10 was released this afternoon.

Very nice, thanks!

Seems like maven_test (windows-latest, 8) failed the second time (even though it was successful the first time).
I have also fixed the import order now which caused the checkstyle test to fail.

@jlerbsc
Copy link
Collaborator

jlerbsc commented Dec 31, 2022

Seems like maven_test (windows-latest, 8) failed the second time (even though it was successful the first time).
I have also fixed the import order now which caused the checkstyle test to fail.

Please stop modifying the PR so that I can validate it.

@jlerbsc jlerbsc merged commit dc6418e into javaparser:master Dec 31, 2022
@jlerbsc jlerbsc added this to the next release milestone Dec 31, 2022
@jlerbsc jlerbsc added the PR: Added A PR that introduces new behaviour (e.g. functionality, tests) label Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Added A PR that introduces new behaviour (e.g. functionality, tests)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get method descriptor for ResolvedMethodDeclaration / MethodCallExpr
2 participants