Skip to content

Commit 72b5ad6

Browse files
authored
Use first line number from LineNumberTable for JavaCodeUnit's sourceCodeLocation TNG#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. TNG#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 TNG#2 // Field java/lang/System.out:Ljava/io/PrintStream; 3: bipush 10 5: invokevirtual TNG#3 // Method java/io/PrintStream.println:(I)V 8: getstatic TNG#2 // Field java/lang/System.out:Ljava/io/PrintStream; 11: bipush 11 13: invokevirtual TNG#3 // Method java/io/PrintStream.println:(I)V 16: getstatic TNG#2 // Field java/lang/System.out:Ljava/io/PrintStream; 19: bipush 12 21: invokevirtual TNG#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. TNG#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.)
2 parents a080697 + 61e140f commit 72b5ad6

File tree

10 files changed

+108
-24
lines changed

10 files changed

+108
-24
lines changed

archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -911,8 +911,8 @@ Stream<DynamicTest> MethodsTest() {
911911

912912
.ofRule("no code units that are declared in classes that reside in a package '..persistence..' "
913913
+ "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))
916916

917917
.toDynamicTests();
918918
}

archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedConstructor.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,23 @@ public static ExpectedConstructor.Creator of(Class<?> owner, Class<?>... params)
1515
public static class Creator {
1616
private final Class<?> clazz;
1717
private final Class<?>[] params;
18+
private int lineNumber;
1819

1920
private Creator(Class<?> clazz, Class<?>[] params) {
2021
this.clazz = clazz;
2122
this.params = params;
2223
}
2324

25+
public Creator inLine(int lineNumber) {
26+
this.lineNumber = lineNumber;
27+
return this;
28+
}
29+
2430
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)",
2632
formatMethod(clazz.getName(), JavaConstructor.CONSTRUCTOR_NAME, JavaClass.namesOf(params)),
2733
annotationType.getSimpleName(),
28-
clazz.getSimpleName()));
34+
clazz.getSimpleName(), lineNumber));
2935
}
3036
}
3137
}

archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedMethod.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,19 @@ public static class Creator {
1616
private final Class<?> clazz;
1717
private final String methodName;
1818
private final Class<?>[] params;
19+
private int lineNumber;
1920

2021
private Creator(Class<?> clazz, String methodName, Class<?>[] params) {
2122
this.clazz = clazz;
2223
this.methodName = methodName;
2324
this.params = params;
2425
}
2526

27+
public Creator inLine(int lineNumber) {
28+
this.lineNumber = lineNumber;
29+
return this;
30+
}
31+
2632
public ExpectedMessage toNotHaveRawReturnType(Class<?> type) {
2733
return method("does not have raw return type " + type.getName());
2834
}
@@ -37,7 +43,7 @@ public ExpectedMessage beingAnnotatedWith(Class<? extends Annotation> annotation
3743

3844
private ExpectedMessage method(String message) {
3945
String methodDescription = format("Method <%s>", formatMethod(clazz.getName(), methodName, JavaClass.namesOf(params)));
40-
String sourceCodeLocation = format("(%s.java:0)", clazz.getSimpleName());
46+
String sourceCodeLocation = format("(%s.java:%d)", clazz.getSimpleName(), lineNumber);
4147
return new ExpectedMessage(format("%s %s in %s", methodDescription, message, sourceCodeLocation));
4248
}
4349
}

archunit/src/main/java/com/tngtech/archunit/core/domain/JavaMember.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import com.tngtech.archunit.Internal;
2626
import com.tngtech.archunit.PublicAPI;
2727
import com.tngtech.archunit.base.DescribedPredicate;
28-
import com.tngtech.archunit.base.HasDescription;
2928
import com.tngtech.archunit.base.Optional;
3029
import com.tngtech.archunit.core.domain.properties.CanBeAnnotated;
3130
import com.tngtech.archunit.core.domain.properties.HasAnnotations;
@@ -59,7 +58,7 @@ public abstract class JavaMember implements
5958
this.descriptor = checkNotNull(builder.getDescriptor());
6059
this.annotations = builder.getAnnotations(this);
6160
this.owner = checkNotNull(builder.getOwner());
62-
this.sourceCodeLocation = SourceCodeLocation.of(owner);
61+
this.sourceCodeLocation = SourceCodeLocation.of(owner, builder.getFirstLineNumber());
6362
this.modifiers = checkNotNull(builder.getModifiers());
6463
}
6564

archunit/src/main/java/com/tngtech/archunit/core/importer/DomainBuilders.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ public abstract static class JavaMemberBuilder<OUTPUT, SELF extends JavaMemberBu
118118
private Set<JavaModifier> modifiers;
119119
private JavaClass owner;
120120
private ClassesByTypeName importedClasses;
121+
private int firstLineNumber;
121122

122123
private JavaMemberBuilder() {
123124
}
@@ -142,6 +143,10 @@ SELF withModifiers(Set<JavaModifier> modifiers) {
142143
return self();
143144
}
144145

146+
void recordLineNumber(int lineNumber) {
147+
this.firstLineNumber = this.firstLineNumber == 0 ? lineNumber : Math.min(this.firstLineNumber, lineNumber);
148+
}
149+
145150
@SuppressWarnings("unchecked")
146151
SELF self() {
147152
return (SELF) this;
@@ -178,6 +183,10 @@ public JavaClass getOwner() {
178183
return owner;
179184
}
180185

186+
public int getFirstLineNumber() {
187+
return firstLineNumber;
188+
}
189+
181190
@Override
182191
public final OUTPUT build(JavaClass owner, ClassesByTypeName importedClasses) {
183192
this.owner = owner;

archunit/src/main/java/com/tngtech/archunit/core/importer/JavaClassProcessor.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ public void visitCode() {
320320
@Override
321321
public void visitLineNumber(int line, Label start) {
322322
LOG.trace("Examining line number {}", line);
323+
codeUnitBuilder.recordLineNumber(line);
323324
actualLineNumber = line;
324325
accessHandler.setLineNumber(actualLineNumber);
325326
}

archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterTest.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@
156156
import com.tngtech.archunit.core.importer.testexamples.integration.ClassD;
157157
import com.tngtech.archunit.core.importer.testexamples.integration.ClassXDependingOnClassesABCD;
158158
import com.tngtech.archunit.core.importer.testexamples.integration.InterfaceOfClassX;
159+
import com.tngtech.archunit.core.importer.testexamples.methodimport.ClassWithMultipleMethods;
159160
import com.tngtech.archunit.core.importer.testexamples.methodimport.ClassWithObjectVoidAndIntIntSerializableMethod;
160161
import com.tngtech.archunit.core.importer.testexamples.methodimport.ClassWithStringStringMethod;
161162
import com.tngtech.archunit.core.importer.testexamples.methodimport.ClassWithThrowingMethod;
@@ -739,6 +740,36 @@ public void imports_methods_with_correct_throws_declarations() throws Exception
739740
assertThat(method.getExceptionTypes()).matches(FirstCheckedException.class, SecondCheckedException.class);
740741
}
741742

743+
@Test
744+
public void imports_members_with_sourceCodeLocation() throws Exception {
745+
ImportedClasses importedClasses = classesIn("testexamples/methodimport");
746+
String sourceFileName = "ClassWithMultipleMethods.java";
747+
748+
JavaClass javaClass = importedClasses.get(ClassWithMultipleMethods.class);
749+
assertThat(javaClass.getField("usage").getSourceCodeLocation())
750+
.hasToString("(" + sourceFileName + ":0)"); // the byte code has no line number associated with a field
751+
assertThat(javaClass.getConstructor().getSourceCodeLocation())
752+
.hasToString("(" + sourceFileName + ":3)"); // auto-generated constructor seems to get line of class definition
753+
assertThat(javaClass.getStaticInitializer().get().getSourceCodeLocation())
754+
.hasToString("(" + sourceFileName + ":5)"); // auto-generated static initializer seems to get line of first static variable definition
755+
assertThat(javaClass.getMethod("methodDefinedInLine7").getSourceCodeLocation())
756+
.hasToString("(" + sourceFileName + ":7)");
757+
assertThat(javaClass.getMethod("methodWithBodyStartingInLine10").getSourceCodeLocation())
758+
.hasToString("(" + sourceFileName + ":10)");
759+
assertThat(javaClass.getMethod("emptyMethodDefinedInLine15").getSourceCodeLocation())
760+
.hasToString("(" + sourceFileName + ":15)");
761+
assertThat(javaClass.getMethod("emptyMethodEndingInLine19").getSourceCodeLocation())
762+
.hasToString("(" + sourceFileName + ":19)");
763+
764+
javaClass = importedClasses.get(ClassWithMultipleMethods.InnerClass.class);
765+
assertThat(javaClass.getMethod("methodWithBodyStartingInLine24").getSourceCodeLocation())
766+
.hasToString("(" + sourceFileName + ":24)");
767+
768+
javaClass = importedClasses.get(ClassWithMultipleMethods.InnerClass.class.getName() + "$1");
769+
assertThat(javaClass.getMethod("run").getSourceCodeLocation())
770+
.hasToString("(" + sourceFileName + ":27)");
771+
}
772+
742773
@Test
743774
public void imports_methods_with_one_annotation_correctly() throws Exception {
744775
JavaClass clazz = classesIn("testexamples/annotationmethodimport").get(ClassWithAnnotatedMethods.class);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package com.tngtech.archunit.core.importer.testexamples.methodimport;
2+
3+
public class ClassWithMultipleMethods {
4+
5+
static String usage = "ClassFileImporterTest's @Test imports_methods_with_correct_sourceCodeLocation";
6+
7+
int methodDefinedInLine7() { return 7; }
8+
9+
void methodWithBodyStartingInLine10() {
10+
System.out.println(10);
11+
System.out.println(11);
12+
System.out.println(12);
13+
}
14+
15+
void emptyMethodDefinedInLine15() { }
16+
17+
void emptyMethodEndingInLine19() {
18+
19+
}
20+
21+
public static class InnerClass {
22+
23+
void methodWithBodyStartingInLine24() {
24+
new Runnable() {
25+
@Override
26+
public void run() {
27+
System.out.println(27);
28+
}
29+
};
30+
}
31+
}
32+
}

archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenClassShouldTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -758,8 +758,8 @@ public void classes_should_have_only_private_constructor(ArchRule rule) {
758758
assertThat(rule).checking(importClasses(ClassWithPrivateConstructors.class))
759759
.hasNoViolation();
760760
assertThat(rule).checking(importClasses(ClassWithPublicAndPrivateConstructor.class))
761-
.hasOnlyViolations(String.format("Constructor <%s.<init>(%s)> is not private in (%s.java:0)",
762-
ClassWithPublicAndPrivateConstructor.class.getName(), String.class.getName(), getClass().getSimpleName()));
761+
.hasOnlyViolations(String.format("Constructor <%s.<init>(%s)> is not private in (%s.java:%d)",
762+
ClassWithPublicAndPrivateConstructor.class.getName(), String.class.getName(), getClass().getSimpleName(), 1538));
763763
}
764764

765765
@DataProvider

archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MembersShouldConjunctionTest.java

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,11 @@ public void orShould_ORs_conditions(ArchRule rule) {
4444
RightOne.class.getName(), RightTwo.class.getName()));
4545
assertThat(report.getDetails()).containsOnly(
4646
String.format("%s and %s",
47-
isNotDeclaredInMessage(WrongOne.class, CONSTRUCTOR_NAME, RightOne.class),
48-
isNotDeclaredInMessage(WrongOne.class, CONSTRUCTOR_NAME, RightTwo.class)),
47+
isNotDeclaredInMessage(WrongOne.class, CONSTRUCTOR_NAME, RightOne.class, 111),
48+
isNotDeclaredInMessage(WrongOne.class, CONSTRUCTOR_NAME, RightTwo.class, 111)),
4949
String.format("%s and %s",
50-
isNotDeclaredInMessage(WrongOne.class, "wrongMethod1", RightOne.class),
51-
isNotDeclaredInMessage(WrongOne.class, "wrongMethod1", RightTwo.class)));
50+
isNotDeclaredInMessage(WrongOne.class, "wrongMethod1", RightOne.class, 113),
51+
isNotDeclaredInMessage(WrongOne.class, "wrongMethod1", RightTwo.class, 113)));
5252
}
5353

5454
@DataProvider
@@ -75,24 +75,24 @@ public void andShould_ANDs_conditions(ArchRule rule) {
7575
"members should not be declared in %s and should not be declared in %s",
7676
WrongOne.class.getName(), WrongTwo.class.getName()));
7777
assertThat(report.getDetails()).containsOnly(
78-
isDeclaredInMessage(WrongOne.class, CONSTRUCTOR_NAME),
79-
isDeclaredInMessage(WrongOne.class, "wrongMethod1"),
80-
isDeclaredInMessage(WrongTwo.class, CONSTRUCTOR_NAME),
81-
isDeclaredInMessage(WrongTwo.class, "wrongMethod2"));
78+
isDeclaredInMessage(WrongOne.class, CONSTRUCTOR_NAME, 111),
79+
isDeclaredInMessage(WrongOne.class, "wrongMethod1", 113),
80+
isDeclaredInMessage(WrongTwo.class, CONSTRUCTOR_NAME, 117),
81+
isDeclaredInMessage(WrongTwo.class, "wrongMethod2", 119));
8282
}
8383

84-
private String isDeclaredInMessage(Class<?> clazz, String methodName) {
85-
return message(clazz, methodName, "", clazz);
84+
private String isDeclaredInMessage(Class<?> clazz, String methodName, int lineNumber) {
85+
return message(clazz, methodName, "", clazz, lineNumber);
8686
}
8787

88-
private String isNotDeclaredInMessage(Class<?> clazz, String methodName, Class<?> expectedTarget) {
89-
return message(clazz, methodName, "not ", expectedTarget);
88+
private String isNotDeclaredInMessage(Class<?> clazz, String methodName, Class<?> expectedTarget, int lineNumber) {
89+
return message(clazz, methodName, "not ", expectedTarget, lineNumber);
9090
}
9191

92-
private String message(Class<?> clazz, String methodName, String optionalNot, Class<?> expectedTarget) {
93-
return String.format("%s <%s.%s()> is %sdeclared in %s in (%s.java:0)",
92+
private String message(Class<?> clazz, String methodName, String optionalNot, Class<?> expectedTarget, int lineNumber) {
93+
return String.format("%s <%s.%s()> is %sdeclared in %s in (%s.java:%d)",
9494
CONSTRUCTOR_NAME.equals(methodName) ? "Constructor" : "Method",
95-
clazz.getName(), methodName, optionalNot, expectedTarget.getName(), getClass().getSimpleName());
95+
clazz.getName(), methodName, optionalNot, expectedTarget.getName(), getClass().getSimpleName(), lineNumber);
9696
}
9797

9898
@SuppressWarnings("unused")

0 commit comments

Comments
 (0)