Add referenced class objects to JavaClass
#518
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
codecholeric
force-pushed
the
add-loaded-class-constants-to-JavaClass
branch
from
8e4841a to
6fb8b40
Compare
While `toString()` is not really intended as API we should always make sure there is some reasonable string output for debugging purposes. Signed-off-by: Peter Gafert <[email protected]>
We should unify this and let all domain objects that have a designated source code location implement `HasSourceCodeLocation`. Signed-off-by: Peter Gafert <[email protected]>
A method taking a `String` as parameter should clearly state what format the string has. A `String` is not an "ASM type". Signed-off-by: Peter Gafert <[email protected]>
codecholeric
force-pushed
the
add-loaded-class-constants-to-JavaClass
branch
from
6fb8b40 to
5c794ae
Compare
codecholeric
changed the title
JavaClassJavaClass
codecholeric
force-pushed
the
add-loaded-class-constants-to-JavaClass
branch
from
5c794ae to
37f103a
Compare
|
Thanks a lot for the review @kaebiscs ! I agree that |
To detect usages of class objects we need to look which classes are referenced in the constant pool. So far the following usage of `SomeClass` would not have been detected by ArchUnit:
```
class Example {
private Map<Class<?>, Value> map = Map.of(SomeClass.class, someValue);
}
```
In other words, pure usages of `Foo.class` could not be detected, because there was no "access" to `Foo` in the bytecode. However, for each such occurrence the class would actually be present in the constant pool of the referring class. While ASM does not allow direct access to the constant pool, it does allow us to hook into `ldc` instructions, i.e. load constant instructions in the bytecode, that will in turn reference the respective class. The way to detect this is principally an `instanceof` check for `org.objectweb.asm.Type` in `void visitLdcInsn(Object value)`. As far as I could see, any reference of another class as a constant would cause this method to be called with the respective ASM `Type` object.
It would actually be possible to import a lot more constants here. I have looked a little into adding all supported constant types, so it would e.g. be possible to have assertions on `String` values, but then decided to let it go for now. Strings would still be simple, but as soon as `Integer` comes into play (which could also be imported), there are a lot of internal optimizations by the JVM (e.g. `iconst_1`, ...), which makes it hard to do this in a consistent way. I think the most valuable feature by far is to detect constant loads of classes (since those cause dependencies), so I decided to keep it simple for now.
Signed-off-by: Peter Gafert <[email protected]>
codecholeric
force-pushed
the
add-loaded-class-constants-to-JavaClass
branch
from
37f103a to
806b04d
Compare
Signed-off-by: Peter Gafert <[email protected]>
codecholeric
force-pushed
the
add-loaded-class-constants-to-JavaClass
branch
from
806b04d to
1263ae5
Compare
Closed
codecholeric
added a commit
that referenced
this pull request
Feb 21, 2021
So far ArchUnit has not been able to detect the pure usage of class objects as dependencies. E.g. the following example would not have detected a dependency on `Evil`, since besides the reference to the class object no further dependency on `Evil` (like a field access or method call) has occurred.
```
class Example {
final Map<Class<?>, Object> association = Map.of(Evil.class, anything);
}
```
With this PR `JavaClass` now knows its `referencedClassObjects`, including the respective line number. Furthermore class objects are now parts of the `dependencies{From/To}Self` of a `JavaClass`.
Resolves: #309
Issue: #446
Resolves: #474
Resolves: #515
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So far ArchUnit has not been able to detect the pure usage of class objects as dependencies. E.g. the following example would not have detected a dependency on
Evil, since besides the reference to the class object no further dependency onEvil(like a field access or method call) has occurred.With this PR
JavaClassnow knows itsreferencedClassObjects, including the respective line number. Furthermore class objects are now parts of thedependencies{From/To}Selfof aJavaClass.Resolves: #309
Issue: #446
Resolves: #474
Resolves: #515