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

Dependency do not support constant #515

Closed
GeeJoe opened this issue Jan 23, 2021 · 7 comments · Fixed by #518
Closed

Dependency do not support constant #515

GeeJoe opened this issue Jan 23, 2021 · 7 comments · Fixed by #518

Comments

@GeeJoe
Copy link

GeeJoe commented Jan 23, 2021

I tried to scan a class's dependency using JavaClass.directDependenciesToSelf but I found the constant in the class which is using by another class was not in the dependency list.

What should I do?

@hankem
Copy link
Member

hankem commented Jan 23, 2021

I believe that the statement from #298 (comment) is unfortunately still true:

there is at the moment no workaround with ArchUnit to detect a dependency that is only observable through the constant pool.

Somebody needs to find some time to implement #309.

@codecholeric
Copy link
Collaborator

I just played around a little and I think it will work to inspect ldc instructions with the current ASM version 😃 I'll see if I can find some time the next days to implement this!

codecholeric added a commit that referenced this issue Jan 31, 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
codecholeric added a commit that referenced this issue 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
@GeeJoe
Copy link
Author

GeeJoe commented Mar 5, 2021

I just played around a little and I think it will work to inspect ldc instructions with the current ASM version 😃 I'll see if I can find some time the next days to implement this!

Did you resolve this issue? I find the problem still exist in com.tngtech.archunit:archunit:0.17.0

@codecholeric
Copy link
Collaborator

Are you sure? Can you show me an example? AFAIS something like this should now be detected, right?

class One {
    private Class<?> causesDependency = Two.class;
}

class Two {
}

What is your concrete case?

@GeeJoe
Copy link
Author

GeeJoe commented Mar 8, 2021

Like

class One {
    public static final String TAG = "Hello";
}

class Two {
     private String tag = One.TAG;
}

If I try to get getDirectDependenciesFromSlef() on class Two,the result won't contain class One

@hankem
Copy link
Member

hankem commented Mar 8, 2021

There is some confusion: Since ArchUnit 0.16.0, JavaClass/JavaCodeUnit know their references to other class objects (like Two.class in the example above), but primitive constants (like your String TAG) have a different signature in the bytecode (see also #298).

The bytecode for your example
class Two
  minor version: 0
  major version: 55
  flags: (0x0020) ACC_SUPER
  this_class: #5                          // Two
  super_class: #6                         // java/lang/Object
  interfaces: 0, fields: 1, methods: 1, attributes: 1
Constant pool:
   #1 = Methodref          #6.#15         // java/lang/Object."<init>":()V
   #2 = Class              #16            // One
   #3 = String             #17            // Hello
   #4 = Fieldref           #5.#18         // Two.tag:Ljava/lang/String;
   #5 = Class              #19            // Two
   #6 = Class              #20            // java/lang/Object
   #7 = Utf8               tag
   #8 = Utf8               Ljava/lang/String;
   #9 = Utf8               <init>
  #10 = Utf8               ()V
  #11 = Utf8               Code
  #12 = Utf8               LineNumberTable
  #13 = Utf8               SourceFile
  #14 = Utf8               OneAndTwo.java
  #15 = NameAndType        #9:#10         // "<init>":()V
  #16 = Utf8               One
  #17 = Utf8               Hello
  #18 = NameAndType        #7:#8          // tag:Ljava/lang/String;
  #19 = Utf8               Two
  #20 = Utf8               java/lang/Object
{
  private java.lang.String tag;
    descriptor: Ljava/lang/String;
    flags: (0x0002) ACC_PRIVATE

  Two();
    descriptor: ()V
    flags: (0x0000)
    Code:
      stack=2, locals=1, args_size=1
         0: aload_0
         1: invokespecial #1                  // Method java/lang/Object."<init>":()V
         4: aload_0
         5: ldc           #3                  // String Hello
         7: putfield      #4                  // Field tag:Ljava/lang/String;
        10: return
}

initializes Two.tag from the result of a ldc #3 instruction to load a constant that is actually copied into Two's own pool:

   #3 = String             #17            // Hello
  // ...
  #17 = Utf8               Hello

That's why the usage of the primitive constant One.TAG is currently not recognized as a dependency.

@GeeJoe
Copy link
Author

GeeJoe commented Mar 8, 2021

Thanks for your detailed explanation! Hope that you can implement this soon. :)

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 a pull request may close this issue.

3 participants