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

isEnum method for JavaClass #5

Closed
storozhukBM opened this issue May 18, 2017 · 4 comments
Closed

isEnum method for JavaClass #5

storozhukBM opened this issue May 18, 2017 · 4 comments
Labels
enhancement
Milestone
Release 0.5.0

Comments

@storozhukBM
Copy link
Contributor

Hi @codecholeric,

Thank you for this great library. It is awesome and saves me a lot of time.
Recently I've faced a case where i wanted to check that JavaClass is enum.
Currently I can solve this by: javaClass.reflect().isEnum(), but it would be more convenient to have such check in JavaClass.

If you don't mind I'll create corresponding PR.
@codecholeric
Copy link
Collaborator

Hey,

glad it helps you :-) I actually thought about this myself in the past, the reason it's not in, is simply, that I wanted to wait, if this is really relevant in daily life. Which you just proved it is ;-)
I think this should be part of JavaClass, so performance is better, and it's decoupled from the classpath.

A PR would be highly appreciated of course :-) In case it helps to get started, I think this would involve

  • Adding a test to ClassFileImporterTest
  • In JavaClassProcessor check the access parameter for org.objectweb.asm.Opcodes.ACC_ENUM (similar to Opcodes.ACC_INTERFACE, which is already checked)

I wonder, if it would make sense, to support List<JavaEnumConstant> getEnumConstants() as well (or List<String> getEnumConstants()?)...
What do you think?

@storozhukBM
Copy link
Contributor Author

storozhukBM commented May 19, 2017 •
edited
Loading

I've made some simple changes in PR #6
As for getEnumConstants() I'm also not sure about it, currently I don't have such use case.

@codecholeric
Copy link
Collaborator

Thanks a lot, I've merged your PR :-) If you want to try the current 0.5.0-SNAPSHOT version with your changes, you can include the Sonatype snapshot repo

<repository>
    <id>sonatype-snapshots</id>
    <url>https://oss.sonatype.org/content/repositories/snapshots</url>
</repository>

(note that there is a breaking change where @AnalyzeClasses(importOption = ...) became @AnalyzeClasses(importOptions = ...))

As for getEnumConstants(), guess I'll handle that like isEnum() and wait, if anybody ever really needs this feature.

Anyway, thanks for contributing, I'll close this issue.

@storozhukBM
Copy link
Contributor Author

storozhukBM commented May 19, 2017 •
edited
Loading

Thank you too, @codecholeric.

@codecholeric codecholeric added this to the Release 0.5.0 milestone May 20, 2017
@JaimLus JaimLus mentioned this issue Apr 7, 2020
Closed
codecholeric added a commit to crizzis/ArchUnit that referenced this issue Mar 27, 2022
@codecholeric
for some reason the bytecode looks different with JDK 7 and two of th…
94dce24 
…e three try catch blocks start from a handler label.

At least with my JDK 7 it looks like this:

```
private void method(int, boolean);
    descriptor: (IZ)V
    flags: (0x0002) ACC_PRIVATE
    Code:
      stack=4, locals=6, args_size=3
         0: new           TNG#3                  // class java/net/Socket
         3: dup
         4: ldc           TNG#4                  // String
         6: iconst_0
         7: invokespecial TNG#5                  // Method java/net/Socket."<init>":(Ljava/lang/String;I)V
        10: astore_3
        11: new           TNG#6                  // class java/io/BufferedReader
        14: dup
        15: aconst_null
        16: invokespecial TNG#7                  // Method java/io/BufferedReader."<init>":(Ljava/io/Reader;)V
        19: astore        4
        21: aload         4
        23: invokevirtual TNG#8                  // Method java/io/BufferedReader.close:()V
        26: return
        27: astore        5
        29: aload         4
        31: invokevirtual TNG#8                  // Method java/io/BufferedReader.close:()V
        34: aload         5
        36: athrow
        37: astore_3
        38: return
      Exception table:
         from    to  target type
            27    29    27   any
             0    26    37   Class java/lang/Exception
            27    37    37   Class java/lang/Exception
```

We can possibly ignore this, since we will drop JDK 7 support soon anyway, but let's maybe ponder about it, why it looks so different. That of the two declared try-catch-blocks only one try catch block is visible is definitely strange (and worse than having 3 try-catch-blocks where one is synthetic). But this example is anyway quite strange, because the bytecode immediately changes e.g. if we remove one of the unused method parameters.

Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
Release 0.5.0
Development

No branches or pull requests
2 participants
@storozhukBM @codecholeric