record line numbers when importing methods and use first line number for JavaCodeUnit's sourceCodeLocation

hankem · codecholeric · commit 61e140f73dce · 2020-04-15T18:23:51.000+02:00
Signed-off-by: Manfred Hanke &lt;Manfred.Hanke@tngtech.com&gt;
diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java
@@ -911,8 +911,8 @@ Stream<DynamicTest> MethodsTest() {
 
                 .ofRule("no code units that are declared in classes that reside in a package '..persistence..' "
                         + "should be annotated with @" + Secured.class.getSimpleName())
-                .by(ExpectedConstructor.of(SomeJpa.class).beingAnnotatedWith(Secured.class))
-                .by(ExpectedMethod.of(OtherJpa.class, "getEntityManager").beingAnnotatedWith(Secured.class))
+                .by(ExpectedConstructor.of(SomeJpa.class).inLine(15).beingAnnotatedWith(Secured.class))
+                .by(ExpectedMethod.of(OtherJpa.class, "getEntityManager").inLine(31).beingAnnotatedWith(Secured.class))
 
                 .toDynamicTests();
     }
diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedConstructor.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedConstructor.java
@@ -15,17 +15,23 @@ public static ExpectedConstructor.Creator of(Class<?> owner, Class<?>... params)
     public static class Creator {
         private final Class<?> clazz;
         private final Class<?>[] params;
+        private int lineNumber;
 
         private Creator(Class<?> clazz, Class<?>[] params) {
             this.clazz = clazz;
             this.params = params;
         }
 
+        public  Creator inLine(int lineNumber) {
+            this.lineNumber = lineNumber;
+            return this;
+        }
+
         public ExpectedMessage beingAnnotatedWith(Class<? extends Annotation> annotationType) {
-            return new ExpectedMessage(String.format("Constructor <%s> is annotated with @%s in (%s.java:0)",
+            return new ExpectedMessage(String.format("Constructor <%s> is annotated with @%s in (%s.java:%d)",
                     formatMethod(clazz.getName(), JavaConstructor.CONSTRUCTOR_NAME, JavaClass.namesOf(params)),
                     annotationType.getSimpleName(),
-                    clazz.getSimpleName()));
+                    clazz.getSimpleName(), lineNumber));
         }
     }
 }
diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedMethod.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedMethod.java
@@ -16,13 +16,19 @@ public static class Creator {
         private final Class<?> clazz;
         private final String methodName;
         private final Class<?>[] params;
+        private int lineNumber;
 
         private Creator(Class<?> clazz, String methodName, Class<?>[] params) {
             this.clazz = clazz;
             this.methodName = methodName;
             this.params = params;
         }
 
+        public Creator inLine(int lineNumber) {
+            this.lineNumber = lineNumber;
+            return this;
+        }
+
         public ExpectedMessage toNotHaveRawReturnType(Class<?> type) {
             return method("does not have raw return type " + type.getName());
         }
@@ -37,7 +43,7 @@ public ExpectedMessage beingAnnotatedWith(Class<? extends Annotation> annotation
 
         private ExpectedMessage method(String message) {
             String methodDescription = format("Method <%s>", formatMethod(clazz.getName(), methodName, JavaClass.namesOf(params)));
-            String sourceCodeLocation = format("(%s.java:0)", clazz.getSimpleName());
+            String sourceCodeLocation = format("(%s.java:%d)", clazz.getSimpleName(), lineNumber);
             return new ExpectedMessage(format("%s %s in %s", methodDescription, message, sourceCodeLocation));
         }
     }
diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaMember.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaMember.java
@@ -25,7 +25,6 @@
 import com.tngtech.archunit.Internal;
 import com.tngtech.archunit.PublicAPI;
 import com.tngtech.archunit.base.DescribedPredicate;
-import com.tngtech.archunit.base.HasDescription;
 import com.tngtech.archunit.base.Optional;
 import com.tngtech.archunit.core.domain.properties.CanBeAnnotated;
 import com.tngtech.archunit.core.domain.properties.HasAnnotations;
@@ -59,7 +58,7 @@ public abstract class JavaMember implements
         this.descriptor = checkNotNull(builder.getDescriptor());
         this.annotations = builder.getAnnotations(this);
         this.owner = checkNotNull(builder.getOwner());
-        this.sourceCodeLocation = SourceCodeLocation.of(owner);
+        this.sourceCodeLocation = SourceCodeLocation.of(owner, builder.getFirstLineNumber());
         this.modifiers = checkNotNull(builder.getModifiers());
     }
 
diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/DomainBuilders.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/DomainBuilders.java
@@ -118,6 +118,7 @@ public abstract static class JavaMemberBuilder<OUTPUT, SELF extends JavaMemberBu
         private Set<JavaModifier> modifiers;
         private JavaClass owner;
         private ClassesByTypeName importedClasses;
+        private int firstLineNumber;
 
         private JavaMemberBuilder() {
         }
@@ -142,6 +143,10 @@ SELF withModifiers(Set<JavaModifier> modifiers) {
             return self();
         }
 
+        void recordLineNumber(int lineNumber) {
+            this.firstLineNumber = this.firstLineNumber == 0 ? lineNumber : Math.min(this.firstLineNumber, lineNumber);
+        }
+
         @SuppressWarnings("unchecked")
         SELF self() {
             return (SELF) this;
@@ -178,6 +183,10 @@ public JavaClass getOwner() {
             return owner;
         }
 
+        public int getFirstLineNumber() {
+            return firstLineNumber;
+        }
+
         @Override
         public final OUTPUT build(JavaClass owner, ClassesByTypeName importedClasses) {
             this.owner = owner;
diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/JavaClassProcessor.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/JavaClassProcessor.java
@@ -320,6 +320,7 @@ public void visitCode() {
         @Override
         public void visitLineNumber(int line, Label start) {
             LOG.trace("Examining line number {}", line);
+            codeUnitBuilder.recordLineNumber(line);
             actualLineNumber = line;
             accessHandler.setLineNumber(actualLineNumber);
         }
diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterTest.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterTest.java
@@ -156,6 +156,7 @@
 import com.tngtech.archunit.core.importer.testexamples.integration.ClassD;
 import com.tngtech.archunit.core.importer.testexamples.integration.ClassXDependingOnClassesABCD;
 import com.tngtech.archunit.core.importer.testexamples.integration.InterfaceOfClassX;
+import com.tngtech.archunit.core.importer.testexamples.methodimport.ClassWithMultipleMethods;
 import com.tngtech.archunit.core.importer.testexamples.methodimport.ClassWithObjectVoidAndIntIntSerializableMethod;
 import com.tngtech.archunit.core.importer.testexamples.methodimport.ClassWithStringStringMethod;
 import com.tngtech.archunit.core.importer.testexamples.methodimport.ClassWithThrowingMethod;
@@ -739,6 +740,36 @@ public void imports_methods_with_correct_throws_declarations() throws Exception
         assertThat(method.getExceptionTypes()).matches(FirstCheckedException.class, SecondCheckedException.class);
     }
 
+    @Test
+    public void imports_members_with_sourceCodeLocation() throws Exception {
+        ImportedClasses importedClasses = classesIn("testexamples/methodimport");
+        String sourceFileName = "ClassWithMultipleMethods.java";
+
+        JavaClass javaClass = importedClasses.get(ClassWithMultipleMethods.class);
+        assertThat(javaClass.getField("usage").getSourceCodeLocation())
+                .hasToString("(" + sourceFileName + ":0)");  // the byte code has no line number associated with a field
+        assertThat(javaClass.getConstructor().getSourceCodeLocation())
+                .hasToString("(" + sourceFileName + ":3)");  // auto-generated constructor seems to get line of class definition
+        assertThat(javaClass.getStaticInitializer().get().getSourceCodeLocation())
+                .hasToString("(" + sourceFileName + ":5)");  // auto-generated static initializer seems to get line of first static variable definition
+        assertThat(javaClass.getMethod("methodDefinedInLine7").getSourceCodeLocation())
+                .hasToString("(" + sourceFileName + ":7)");
+        assertThat(javaClass.getMethod("methodWithBodyStartingInLine10").getSourceCodeLocation())
+                .hasToString("(" + sourceFileName + ":10)");
+        assertThat(javaClass.getMethod("emptyMethodDefinedInLine15").getSourceCodeLocation())
+                .hasToString("(" + sourceFileName + ":15)");
+        assertThat(javaClass.getMethod("emptyMethodEndingInLine19").getSourceCodeLocation())
+                .hasToString("(" + sourceFileName + ":19)");
+
+        javaClass = importedClasses.get(ClassWithMultipleMethods.InnerClass.class);
+        assertThat(javaClass.getMethod("methodWithBodyStartingInLine24").getSourceCodeLocation())
+                .hasToString("(" + sourceFileName + ":24)");
+
+        javaClass = importedClasses.get(ClassWithMultipleMethods.InnerClass.class.getName() + "$1");
+        assertThat(javaClass.getMethod("run").getSourceCodeLocation())
+                .hasToString("(" + sourceFileName + ":27)");
+    }
+
     @Test
     public void imports_methods_with_one_annotation_correctly() throws Exception {
         JavaClass clazz = classesIn("testexamples/annotationmethodimport").get(ClassWithAnnotatedMethods.class);
diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/methodimport/ClassWithMultipleMethods.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/methodimport/ClassWithMultipleMethods.java
@@ -0,0 +1,32 @@
+package com.tngtech.archunit.core.importer.testexamples.methodimport;
+
+public class ClassWithMultipleMethods {
+
+    static String usage = "ClassFileImporterTest's @Test imports_methods_with_correct_sourceCodeLocation";
+
+    int methodDefinedInLine7() { return 7; }
+
+    void methodWithBodyStartingInLine10() {
+        System.out.println(10);
+        System.out.println(11);
+        System.out.println(12);
+    }
+
+    void emptyMethodDefinedInLine15() { }
+
+    void emptyMethodEndingInLine19() {
+
+    }
+
+    public static class InnerClass {
+
+        void methodWithBodyStartingInLine24() {
+            new Runnable() {
+                @Override
+                public void run() {
+                    System.out.println(27);
+                }
+            };
+        }
+    }
+}
diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenClassShouldTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenClassShouldTest.java
@@ -758,8 +758,8 @@ public void classes_should_have_only_private_constructor(ArchRule rule) {
         assertThat(rule).checking(importClasses(ClassWithPrivateConstructors.class))
                 .hasNoViolation();
         assertThat(rule).checking(importClasses(ClassWithPublicAndPrivateConstructor.class))
-                .hasOnlyViolations(String.format("Constructor <%s.<init>(%s)> is not private in (%s.java:0)",
-                        ClassWithPublicAndPrivateConstructor.class.getName(), String.class.getName(), getClass().getSimpleName()));
+                .hasOnlyViolations(String.format("Constructor <%s.<init>(%s)> is not private in (%s.java:%d)",
+                        ClassWithPublicAndPrivateConstructor.class.getName(), String.class.getName(), getClass().getSimpleName(), 1538));
     }
 
     @DataProvider
diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MembersShouldConjunctionTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MembersShouldConjunctionTest.java
@@ -44,11 +44,11 @@ public void orShould_ORs_conditions(ArchRule rule) {
                         RightOne.class.getName(), RightTwo.class.getName()));
         assertThat(report.getDetails()).containsOnly(
                 String.format("%s and %s",
-                        isNotDeclaredInMessage(WrongOne.class, CONSTRUCTOR_NAME, RightOne.class),
-                        isNotDeclaredInMessage(WrongOne.class, CONSTRUCTOR_NAME, RightTwo.class)),
+                        isNotDeclaredInMessage(WrongOne.class, CONSTRUCTOR_NAME, RightOne.class, 111),
+                        isNotDeclaredInMessage(WrongOne.class, CONSTRUCTOR_NAME, RightTwo.class, 111)),
                 String.format("%s and %s",
-                        isNotDeclaredInMessage(WrongOne.class, "wrongMethod1", RightOne.class),
-                        isNotDeclaredInMessage(WrongOne.class, "wrongMethod1", RightTwo.class)));
+                        isNotDeclaredInMessage(WrongOne.class, "wrongMethod1", RightOne.class, 113),
+                        isNotDeclaredInMessage(WrongOne.class, "wrongMethod1", RightTwo.class, 113)));
     }
 
     @DataProvider
@@ -75,24 +75,24 @@ public void andShould_ANDs_conditions(ArchRule rule) {
                         "members should not be declared in %s and should not be declared in %s",
                         WrongOne.class.getName(), WrongTwo.class.getName()));
         assertThat(report.getDetails()).containsOnly(
-                isDeclaredInMessage(WrongOne.class, CONSTRUCTOR_NAME),
-                isDeclaredInMessage(WrongOne.class, "wrongMethod1"),
-                isDeclaredInMessage(WrongTwo.class, CONSTRUCTOR_NAME),
-                isDeclaredInMessage(WrongTwo.class, "wrongMethod2"));
+                isDeclaredInMessage(WrongOne.class, CONSTRUCTOR_NAME, 111),
+                isDeclaredInMessage(WrongOne.class, "wrongMethod1", 113),
+                isDeclaredInMessage(WrongTwo.class, CONSTRUCTOR_NAME, 117),
+                isDeclaredInMessage(WrongTwo.class, "wrongMethod2", 119));
     }
 
-    private String isDeclaredInMessage(Class<?> clazz, String methodName) {
-        return message(clazz, methodName, "", clazz);
+    private String isDeclaredInMessage(Class<?> clazz, String methodName, int lineNumber) {
+        return message(clazz, methodName, "", clazz, lineNumber);
     }
 
-    private String isNotDeclaredInMessage(Class<?> clazz, String methodName, Class<?> expectedTarget) {
-        return message(clazz, methodName, "not ", expectedTarget);
+    private String isNotDeclaredInMessage(Class<?> clazz, String methodName, Class<?> expectedTarget, int lineNumber) {
+        return message(clazz, methodName, "not ", expectedTarget, lineNumber);
     }
 
-    private String message(Class<?> clazz, String methodName, String optionalNot, Class<?> expectedTarget) {
-        return String.format("%s <%s.%s()> is %sdeclared in %s in (%s.java:0)",
+    private String message(Class<?> clazz, String methodName, String optionalNot, Class<?> expectedTarget, int lineNumber) {
+        return String.format("%s <%s.%s()> is %sdeclared in %s in (%s.java:%d)",
                 CONSTRUCTOR_NAME.equals(methodName) ? "Constructor" : "Method",
-                clazz.getName(), methodName, optionalNot, expectedTarget.getName(), getClass().getSimpleName());
+                clazz.getName(), methodName, optionalNot, expectedTarget.getName(), getClass().getSimpleName(), lineNumber);
     }
 
     @SuppressWarnings("unused")

Original file line number	Diff line number	Diff line change
`@@ -911,8 +911,8 @@ Stream<DynamicTest> MethodsTest() {`
`911`	`911`
`912`	`912`	`.ofRule("no code units that are declared in classes that reside in a package '..persistence..' "`
`913`	`913`	`+ "should be annotated with @" + Secured.class.getSimpleName())`
`914`		`- .by(ExpectedConstructor.of(SomeJpa.class).beingAnnotatedWith(Secured.class))`
`915`		`- .by(ExpectedMethod.of(OtherJpa.class, "getEntityManager").beingAnnotatedWith(Secured.class))`
	`914`	`+ .by(ExpectedConstructor.of(SomeJpa.class).inLine(15).beingAnnotatedWith(Secured.class))`
	`915`	`+ .by(ExpectedMethod.of(OtherJpa.class, "getEntityManager").inLine(31).beingAnnotatedWith(Secured.class))`
`916`	`916`
`917`	`917`	`.toDynamicTests();`
`918`	`918`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,17 +15,23 @@ public static ExpectedConstructor.Creator of(Class<?> owner, Class<?>... params)`
`15`	`15`	`public static class Creator {`
`16`	`16`	`private final Class<?> clazz;`
`17`	`17`	`private final Class<?>[] params;`
	`18`	`+ private int lineNumber;`
`18`	`19`
`19`	`20`	`private Creator(Class<?> clazz, Class<?>[] params) {`
`20`	`21`	`this.clazz = clazz;`
`21`	`22`	`this.params = params;`
`22`	`23`	`}`
`23`	`24`
	`25`	`+ public Creator inLine(int lineNumber) {`
	`26`	`+ this.lineNumber = lineNumber;`
	`27`	`+ return this;`
	`28`	`+ }`
	`29`	`+`
`24`	`30`	`public ExpectedMessage beingAnnotatedWith(Class<? extends Annotation> annotationType) {`
`25`		`- return new ExpectedMessage(String.format("Constructor <%s> is annotated with @%s in (%s.java:0)",`
	`31`	`+ return new ExpectedMessage(String.format("Constructor <%s> is annotated with @%s in (%s.java:%d)",`
`26`	`32`	`formatMethod(clazz.getName(), JavaConstructor.CONSTRUCTOR_NAME, JavaClass.namesOf(params)),`
`27`	`33`	`annotationType.getSimpleName(),`
`28`		`- clazz.getSimpleName()));`
	`34`	`+ clazz.getSimpleName(), lineNumber));`
`29`	`35`	`}`
`30`	`36`	`}`
`31`	`37`	`}`
Original file line number	Diff line number	Diff line change
`@@ -320,6 +320,7 @@ public void visitCode() {`
`320`	`320`	`@Override`
`321`	`321`	`public void visitLineNumber(int line, Label start) {`
`322`	`322`	`LOG.trace("Examining line number {}", line);`
	`323`	`+ codeUnitBuilder.recordLineNumber(line);`
`323`	`324`	`actualLineNumber = line;`
`324`	`325`	`accessHandler.setLineNumber(actualLineNumber);`
`325`	`326`	`}`