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

Arch Unit configuration with lombok [Error] #337

Closed
JaimLus opened this issue Apr 7, 2020 · 13 comments
Closed

Arch Unit configuration with lombok [Error] #337

JaimLus opened this issue Apr 7, 2020 · 13 comments

Comments

@JaimLus
Copy link

JaimLus commented Apr 7, 2020

Hi everyone,

I need help setting up the arch unit with lombok.

I am testing that my domain models are not annotated with @DaTa from lombok library, but I run the test and I get this error:

image

image

image

What recommendation do you suggest?

This is test:
image

@hankem
Copy link
Member

hankem commented Apr 7, 2020

As ArchUnit is (currently) built solely on byte code, there is unfortunately no way to check whether a class is annotated with an annoation with @Retention(SOURCE), because – just as ArchUnit's consistency check states – that information is gone after compile.

So from ArchUnit's (current) point of view, it doesn't matter how your bytecode is generated.

I understand that you may still want to avoid the dependency on lombok; so maybe there is another signature in the generated bytecode that you can look for?
(I don't have a lombok example project at hand, but if you post the result of

javap -c -p -v AMinimalClassUsingALombokDataAnnotation.class

we can have a look at the bytecode together.)

@JaimLus
Copy link
Author

JaimLus commented Apr 7, 2020

Classfile /D:/Documentos/Personales/Spring_boot/Git/arch-unit/build/classes/java/main/com/jaimlus/archunit/demo/application/dto/OneDto.class
  Last modified 7/04/2020; size 877 bytes
  MD5 checksum 41d05fe7f9ce43002e62cd155633f09a
  Compiled from "OneDto.java"
public class com.jaimlus.archunit.demo.application.dto.OneDto
  minor version: 0
  major version: 52
  flags: ACC_PUBLIC, ACC_SUPER
Constant pool:
   #1 = Methodref          #5.#30         // java/lang/Object."<init>":()V
   #2 = Class              #31            // com/jaimlus/archunit/demo/application/dto/OneDto
   #3 = Methodref          #2.#32         // com/jaimlus/archunit/demo/application/dto/OneDto.canEqual:(Ljava/lang/Object;)Z
   #4 = String             #33            // OneDto()
   #5 = Class              #34            // java/lang/Object
   #6 = Utf8               <init>
   #7 = Utf8               ()V
   #8 = Utf8               Code
   #9 = Utf8               LineNumberTable
  #10 = Utf8               LocalVariableTable
  #11 = Utf8               this
  #12 = Utf8               Lcom/jaimlus/archunit/demo/application/dto/OneDto;
  #13 = Utf8               equals
  #14 = Utf8               (Ljava/lang/Object;)Z
  #15 = Utf8               o
  #16 = Utf8               Ljava/lang/Object;
  #17 = Utf8               other
  #18 = Utf8               StackMapTable
  #19 = Class              #31            // com/jaimlus/archunit/demo/application/dto/OneDto
  #20 = Utf8               MethodParameters
  #21 = Utf8               canEqual
  #22 = Utf8               hashCode
  #23 = Utf8               ()I
  #24 = Utf8               result
  #25 = Utf8               I
  #26 = Utf8               toString
  #27 = Utf8               ()Ljava/lang/String;
  #28 = Utf8               SourceFile
  #29 = Utf8               OneDto.java
  #30 = NameAndType        #6:#7          // "<init>":()V
  #31 = Utf8               com/jaimlus/archunit/demo/application/dto/OneDto
  #32 = NameAndType        #21:#14        // canEqual:(Ljava/lang/Object;)Z
  #33 = Utf8               OneDto()
  #34 = Utf8               java/lang/Object
{
  public com.jaimlus.archunit.demo.application.dto.OneDto();
    descriptor: ()V
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: invokespecial #1                  // Method java/lang/Object."<init>":()V
         4: return
      LineNumberTable:
        line 5: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       5     0  this   Lcom/jaimlus/archunit/demo/application/dto/OneDto;

  public boolean equals(java.lang.Object);
    descriptor: (Ljava/lang/Object;)Z
    flags: ACC_PUBLIC
    Code:
      stack=2, locals=3, args_size=2
         0: aload_1
         1: aload_0
         2: if_acmpne     7
         5: iconst_1
         6: ireturn
         7: aload_1
         8: instanceof    #2                  // class com/jaimlus/archunit/demo/application/dto/OneDto
        11: ifne          16
        14: iconst_0
        15: ireturn
        16: aload_1
        17: checkcast     #2                  // class com/jaimlus/archunit/demo/application/dto/OneDto
        20: astore_2
        21: aload_2
        22: aload_0
        23: invokevirtual #3                  // Method canEqual:(Ljava/lang/Object;)Z
        26: ifne          31
        29: iconst_0
        30: ireturn
        31: iconst_1
        32: ireturn
      LineNumberTable:
        line 5: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0      33     0  this   Lcom/jaimlus/archunit/demo/application/dto/OneDto;
            0      33     1     o   Ljava/lang/Object;
           21      12     2 other   Lcom/jaimlus/archunit/demo/application/dto/OneDto;
      StackMapTable: number_of_entries = 3
        frame_type = 7 /* same */
        frame_type = 8 /* same */
        frame_type = 252 /* append */
          offset_delta = 14
          locals = [ class com/jaimlus/archunit/demo/application/dto/OneDto ]
    MethodParameters:
      Name                           Flags
      o                              final

  protected boolean canEqual(java.lang.Object);
    descriptor: (Ljava/lang/Object;)Z
    flags: ACC_PROTECTED
    Code:
      stack=1, locals=2, args_size=2
         0: aload_1
         1: instanceof    #2                  // class com/jaimlus/archunit/demo/application/dto/OneDto
         4: ireturn
      LineNumberTable:
        line 5: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       5     0  this   Lcom/jaimlus/archunit/demo/application/dto/OneDto;
            0       5     1 other   Ljava/lang/Object;
    MethodParameters:
      Name                           Flags
      other                          final

  public int hashCode();
    descriptor: ()I
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=2, args_size=1
         0: iconst_1
         1: istore_1
         2: iconst_1
         3: ireturn
      LineNumberTable:
        line 5: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       4     0  this   Lcom/jaimlus/archunit/demo/application/dto/OneDto;
            2       2     1 result   I

  public java.lang.String toString();
    descriptor: ()Ljava/lang/String;
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
         0: ldc           #4                  // String OneDto()
         2: areturn
      LineNumberTable:
        line 5: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       3     0  this   Lcom/jaimlus/archunit/demo/application/dto/OneDto;
}
SourceFile: "OneDto.java"`

Thi is class:
image

This is the package where to find the class with the previous bytecode.
image

Is this the correct package? or is another.

@hankem
Copy link
Member

hankem commented Apr 8, 2020

The LineNumberTables in your byte code pretend that all methods start in line 5, so I thought we could build an ArchRule classes().should(not(seemToContainGeneratedCode)) based on something like:

imports

import com.tngtech.archunit.core.domain.JavaClass;
import com.tngtech.archunit.core.domain.JavaMethod;
import com.tngtech.archunit.core.domain.SourceCodeLocation;
import com.tngtech.archunit.lang.ArchCondition;
import com.tngtech.archunit.lang.ConditionEvents;
import com.tngtech.archunit.lang.SimpleConditionEvent;

import java.util.List;

import static com.google.common.collect.FluentIterable.concat;
import static com.google.common.collect.Sets.newHashSet;

static final ArchCondition<JavaClass> seemToContainGeneratedCode =
  new ArchCondition<JavaClass>("seem to contain generated code") {
    @Override
    public void check(JavaClass javaClass, ConditionEvents events) {
      List<SourceCodeLocation> locations = concat(
          javaClass.tryGetMethod("equals", Object.class).asSet(),
          javaClass.tryGetMethod("hashCode").asSet(),
          javaClass.tryGetMethod("toString").asSet()
      ).transform(JavaMethod::getSourceCodeLocation).toList();
      boolean allMethodsDefinedInSameLine =
              locations.size() == 3 && newHashSet(locations).size() == 1;
      events.add(new SimpleConditionEvent(javaClass, allMethodsDefinedInSameLine, String.format(
          "%s has equals(Object), hashCode() and toString() defined in the same line %s",
          javaClass.getName(), locations.get(0)
      )));
    }
  };

However, it turns out that for JavaMethods, getSourceCodeLocation() currently gives line number 0 in any case, so this cannot be used to indicate Lombok generated classes yet. ☹
(I do believe that the sourceCodeLocation could be fixed in a future version of ArchUnit, though.)

As you have already decompiled the example byte code: Could you copy and paste this decompiled code in a new Java source file, compile it and disassemble the new class file (using javap -c -p -v) as above? Then we could see whether there are differences other than line numbers.

@codecholeric
Copy link
Collaborator

Didn't we look into that and couldn't find a solution in the past? 🤔 #75

codecholeric added a commit that referenced this issue Apr 15, 2020
…odeLocation #344

So far, the `sourceCodeLocation` of `JavaMember`s does not contain a line number, as it cannot reliably be inferred from the byte code (cf. #75).

However, if the class file contains a [`LineNumberTable`](https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.12) for a method, using the smallest line number from the `LineNumberTable` for the member's `sourceCodeLocation` – even if this probably corresponds to the first executable statement and not to the method header definition – seems to be more useful to me than using the fallback line number `0` in any case.

So for example, I would infer the line number `10` from the following byte code:
```
  void methodWithBodyStartingInLine10();
    descriptor: ()V
    flags: (0x0000)
    Code:
      stack=2, locals=1, args_size=1
         0: getstatic     #2                  // Field java/lang/System.out:Ljava/io/PrintStream;
         3: bipush        10
         5: invokevirtual #3                  // Method java/io/PrintStream.println:(I)V
         8: getstatic     #2                  // Field java/lang/System.out:Ljava/io/PrintStream;
        11: bipush        11
        13: invokevirtual #3                  // Method java/io/PrintStream.println:(I)V
        16: getstatic     #2                  // Field java/lang/System.out:Ljava/io/PrintStream;
        19: bipush        12
        21: invokevirtual #3                  // Method java/io/PrintStream.println:(I)V
        24: return
      LineNumberTable:
        line 10: 0
        line 11: 8
        line 12: 16
        line 13: 24
```
(My example's method header is actually defined in line 9, but I prefer 10 as opposed to 0... 😉)

Note that I do even get a line number for an empty method from the following byte code:
```
  void emptyMethodDefinedInLine15();
    descriptor: ()V
    flags: (0x0000)
    Code:
      stack=0, locals=1, args_size=1
         0: return
      LineNumberTable:
        line 15: 0
```

Even if line numbers inferred in this way do not exactly point to the method header definition, they can be used to  compare different methods (i.e., the ordering should be correct). This would allow for new rules like, for example (irrespective of whether I'd personally want to have them as arch rules or not):
* Public methods are defined _before_ private methods.
* `equals`, `hashCode` and `toString` are not generated by a framework (or a developer... 🙈) in the _same_ line (cf. #337)

With this pull request, line numbers are recorded for `JavaCodeUnit`s (`JavaMethod`s, `JavaConstructor`s, `JavaStaticInitializer`s) if the class file has a `LineNumberTable`.
(The reported line number `0` for `JavaField`s is unchanged, as it cannot be inferred from byte code.)
@JaimLus
Copy link
Author

JaimLus commented Apr 16, 2020

Hello everyone. I know you have a possible solution to this problem, but I don't understand or don't know how to solve it.
I see that they mention about lineNumberTable, but this number should change if more non-lombok annotations are added to the class?

@hankem
Copy link
Member

hankem commented Apr 16, 2020

The ArchCondition<JavaClass> seemToContainGeneratedCode given above does not test for a specific line number, but that equals, hashCode, and toString are defined in the same line.

So yes, when your class is modified, the line numbers may change, but that shouldn't affect the condition.

I've maybe tried too hard to keep the code for the condition short, so it's probably not obvious... 😉
It first builds a List of the SourceCodeLocations of those three methods (which might not be present, so I'm using tryGetMethod to get an Optional<JavaMethod> and then .asSet() to convert it into a Set with either one or no element), and then transform the JavaMethods to SourceCodeLocations.
If the size() of this List is 3, it means that we've actually found all three methods – which should be the case when using Lombok's @Data annotation. If the size() of the HashSet built from this List<SourceCodeLocation> is, however, only 1, it means that all of the three methods are actually defined in the same line.

TL;DR: I believe that with the next release of ArchUnit, you can use the code above to detect classes that most likely use generated equals, hashCode, and toString methods.

@JaimLus
Copy link
Author

JaimLus commented Apr 20, 2020

Ok, i will try this solution and tell you how it went.

@codecholeric
Copy link
Collaborator

@JaimLus @hankem But the solution only works if we release the current master of ArchUnit, right? Because right now you always get line number 0? Or am I misunderstanding sth.?

@hankem
Copy link
Member

hankem commented Apr 20, 2020

Right! That's why I wrote:

with the next release of ArchUnit

@JaimLus
Copy link
Author

JaimLus commented Apr 20, 2020

Ok ok ok, agreed

@hankem
Copy link
Member

hankem commented Jul 14, 2020

@JaimLus, did you already have a chance to try the suggestion from #337 (comment) with ArchUnit 0.14.1?
If that works, we can probably close this issue, can't we?

@JaimLus
Copy link
Author

JaimLus commented Jul 18, 2020

I have not really tried what the comment says with the new version of arch unit. I think that we close the issue, and if when testing does not work, there we see if the case is reopened or a new one is created.
Thank you.

@hankem
Copy link
Member

hankem commented Jul 19, 2020

All right, so be it. 👍 Thank you!

@hankem hankem closed this as completed Jul 19, 2020
codecholeric added a commit that referenced this issue Feb 21, 2021
…odeLocation #344

So far, the `sourceCodeLocation` of `JavaMember`s does not contain a line number, as it cannot reliably be inferred from the byte code (cf. #75).

However, if the class file contains a [`LineNumberTable`](https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.12) for a method, using the smallest line number from the `LineNumberTable` for the member's `sourceCodeLocation` – even if this probably corresponds to the first executable statement and not to the method header definition – seems to be more useful to me than using the fallback line number `0` in any case.

So for example, I would infer the line number `10` from the following byte code:
```
  void methodWithBodyStartingInLine10();
    descriptor: ()V
    flags: (0x0000)
    Code:
      stack=2, locals=1, args_size=1
         0: getstatic     #2                  // Field java/lang/System.out:Ljava/io/PrintStream;
         3: bipush        10
         5: invokevirtual #3                  // Method java/io/PrintStream.println:(I)V
         8: getstatic     #2                  // Field java/lang/System.out:Ljava/io/PrintStream;
        11: bipush        11
        13: invokevirtual #3                  // Method java/io/PrintStream.println:(I)V
        16: getstatic     #2                  // Field java/lang/System.out:Ljava/io/PrintStream;
        19: bipush        12
        21: invokevirtual #3                  // Method java/io/PrintStream.println:(I)V
        24: return
      LineNumberTable:
        line 10: 0
        line 11: 8
        line 12: 16
        line 13: 24
```
(My example's method header is actually defined in line 9, but I prefer 10 as opposed to 0... 😉)

Note that I do even get a line number for an empty method from the following byte code:
```
  void emptyMethodDefinedInLine15();
    descriptor: ()V
    flags: (0x0000)
    Code:
      stack=0, locals=1, args_size=1
         0: return
      LineNumberTable:
        line 15: 0
```

Even if line numbers inferred in this way do not exactly point to the method header definition, they can be used to  compare different methods (i.e., the ordering should be correct). This would allow for new rules like, for example (irrespective of whether I'd personally want to have them as arch rules or not):
* Public methods are defined _before_ private methods.
* `equals`, `hashCode` and `toString` are not generated by a framework (or a developer... 🙈) in the _same_ line (cf. #337)

With this pull request, line numbers are recorded for `JavaCodeUnit`s (`JavaMethod`s, `JavaConstructor`s, `JavaStaticInitializer`s) if the class file has a `LineNumberTable`.
(The reported line number `0` for `JavaField`s is unchanged, as it cannot be inferred from byte code.)
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

3 participants