Non-Java source files were reported in a wrong way, since formatLocat…

…ion(..) assumed that the file name will always be simpleName of enclosing class + ".java". ASM allows to read the source file name from Java bytecode, provided it was compiled into the class-file. This patch adds Optional<sourceFileName> to Source and a 2-step approach within formatLocation(..) that will first try to derive the location from the real source file name, and only if that is not present will use the old heuristic to determine the source file name. Issue: #77 Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
TNG · Aug 3, 2018 · d7fef52 · d7fef52
1 parent 9e8af8c
commit d7fef52
Show file tree

Hide file tree

Showing 8 changed files with 127 additions and 21 deletions.
diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/DomainObjectCreationContext.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/DomainObjectCreationContext.java
@@ -29,6 +29,7 @@
 import com.google.common.collect.SetMultimap;
 import com.tngtech.archunit.Internal;
 import com.tngtech.archunit.base.Function;
+import com.tngtech.archunit.base.Optional;
 import com.tngtech.archunit.core.importer.DomainBuilders;
 import com.tngtech.archunit.core.importer.DomainBuilders.ConstructorCallTargetBuilder;
 import com.tngtech.archunit.core.importer.DomainBuilders.FieldAccessTargetBuilder;
@@ -126,8 +127,8 @@ public static JavaEnumConstant createJavaEnumConstant(JavaEnumConstantBuilder bu
         return new JavaEnumConstant(builder);
     }
 
-    public static Source createSource(URI uri) {
-        return new Source(uri);
+    public static Source createSource(URI uri, Optional<String> sourceFileName) {
+        return new Source(uri, sourceFileName);
     }
 
     static class AccessContext {

diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/Formatters.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/Formatters.java
@@ -21,12 +21,13 @@
 import com.google.common.base.Joiner;
 import com.google.common.primitives.Ints;
 import com.tngtech.archunit.PublicAPI;
+import com.tngtech.archunit.base.Optional;
 import com.tngtech.archunit.core.domain.properties.HasName;
 
 import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
 
 public final class Formatters {
-    private static final String LOCATION_TEMPLATE = "(%s.java:%d)";
+    private static final String LOCATION_TEMPLATE = "(%s:%d)";
 
     private Formatters() {
     }
@@ -122,13 +123,17 @@ private static boolean isAnonymousRest(String lastPart) {
      */
     @PublicAPI(usage = ACCESS)
     public static String formatLocation(JavaClass clazz, int lineNumber) {
-        return String.format(LOCATION_TEMPLATE, getLocationClass(clazz).getSimpleName(), lineNumber);
+        Optional<String> recordedSourceFileName = clazz.getSource().isPresent()
+                ? clazz.getSource().get().getFileName()
+                : Optional.<String>absent();
+        String sourceFileName = recordedSourceFileName.isPresent() ? recordedSourceFileName.get() : guessSourceFileName(clazz);
+        return String.format(LOCATION_TEMPLATE, sourceFileName, lineNumber);
     }
 
-    private static JavaClass getLocationClass(JavaClass location) {
+    private static String guessSourceFileName(JavaClass location) {
         while (location.getEnclosingClass().isPresent()) {
             location = location.getEnclosingClass().get();
         }
-        return location;
+        return location.getSimpleName() + ".java";
     }
 }
diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/Source.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/Source.java
@@ -28,6 +28,7 @@
 import com.tngtech.archunit.PublicAPI;
 import com.tngtech.archunit.base.Optional;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
 
 /**
@@ -44,10 +45,12 @@
  */
 public class Source {
     private final URI uri;
+    private final Optional<String> fileName;
     private final Md5sum md5sum;
 
-    Source(URI uri) {
-        this.uri = uri;
+    Source(URI uri, Optional<String> fileName) {
+        this.uri = checkNotNull(uri);
+        this.fileName = checkNotNull(fileName);
         md5sum = Md5sum.of(uri);
     }
 
@@ -56,6 +59,11 @@ public URI getUri() {
         return uri;
     }
 
+    @PublicAPI(usage = ACCESS)
+    public Optional<String> getFileName() {
+        return fileName;
+    }
+
     @PublicAPI(usage = ACCESS)
     public Md5sum getMd5sum() {
         return md5sum;

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
@@ -15,6 +15,7 @@
  */
 package com.tngtech.archunit.core.importer;
 
+import java.net.URI;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -53,6 +54,7 @@
 import com.tngtech.archunit.core.domain.Source;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.createSource;
 import static com.tngtech.archunit.core.domain.JavaConstructor.CONSTRUCTOR_NAME;
 
 @Internal
@@ -275,7 +277,8 @@ JavaConstructor construct(JavaConstructorBuilder builder, ClassesByTypeName impo
 
     @Internal
     public static final class JavaClassBuilder {
-        private Optional<Source> source = Optional.absent();
+        private Optional<URI> sourceURI = Optional.absent();
+        private Optional<String> sourceFileName = Optional.absent();
         private JavaType javaType;
         private boolean isInterface;
         private boolean isEnum;
@@ -284,8 +287,13 @@ public static final class JavaClassBuilder {
         JavaClassBuilder() {
         }
 
-        JavaClassBuilder withSource(Source source) {
-            this.source = Optional.of(source);
+        JavaClassBuilder withSourceUri(URI sourceUri) {
+            this.sourceURI = Optional.of(sourceUri);
+            return this;
+        }
+
+        JavaClassBuilder withSourceFileName(String sourceFileName) {
+            this.sourceFileName = Optional.of(sourceFileName);
             return this;
         }
 
@@ -315,7 +323,7 @@ JavaClass build() {
         }
 
         public Optional<Source> getSource() {
-            return source;
+            return sourceURI.isPresent() ? Optional.of(createSource(sourceURI.get(), sourceFileName)) : Optional.<Source>absent();
         }
 
         public JavaType getJavaType() {

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
@@ -58,7 +58,6 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
-import static com.tngtech.archunit.core.domain.DomainObjectCreationContext.createSource;
 import static com.tngtech.archunit.core.domain.JavaConstructor.CONSTRUCTOR_NAME;
 import static com.tngtech.archunit.core.domain.JavaStaticInitializer.STATIC_INITIALIZER_NAME;
 import static com.tngtech.archunit.core.importer.ClassFileProcessor.ASM_API_VERSION;
@@ -90,6 +89,13 @@ Optional<JavaClass> createJavaClass() {
         return javaClassBuilder != null ? Optional.of(javaClassBuilder.build()) : Optional.<JavaClass>absent();
     }
 
+    @Override
+    public void visitSource(String source, String debug) {
+        if (source != null) {
+            javaClassBuilder.withSourceFileName(source);
+        }
+    }
+
     @Override
     public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) {
         LOG.info("Analysing class '{}'", name);
@@ -106,7 +112,7 @@ public void visit(int version, int access, String name, String signature, String
         LOG.debug("Found superclass {} on class '{}'", superClassName.orNull(), name);
 
         javaClassBuilder = new DomainBuilders.JavaClassBuilder()
-                .withSource(createSource(sourceURI))
+                .withSourceUri(sourceURI)
                 .withType(javaType)
                 .withInterface(opCodeForInterfaceIsPresent)
                 .withEnum(opCodeForEnumIsPresent)

diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/FormattersTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/FormattersTest.java
@@ -0,0 +1,51 @@
+package com.tngtech.archunit.core.domain;
+
+import java.util.ArrayList;
+
+import com.tngtech.archunit.testutil.ArchConfigurationRule;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import static com.tngtech.archunit.core.domain.Formatters.formatLocation;
+import static com.tngtech.archunit.core.domain.TestUtils.importClassWithContext;
+import static com.tngtech.archunit.testutil.Assertions.assertThat;
+
+public class FormattersTest {
+    @Rule
+    public final ArchConfigurationRule configuration = new ArchConfigurationRule();
+
+    @Before
+    public void setUp() {
+        // We need this to create a JavaClass without source, i.e. a stub because the class is missing and cannot be resolved
+        configuration.resolveAdditionalDependenciesFromClassPath(false);
+    }
+
+    @Test
+    public void format_location() {
+        JavaClass classWithSource = importClassWithContext(Object.class);
+
+        assertThat(classWithSource.getSource()).as("source").isPresent();
+        assertThat(formatLocation(classWithSource, 7)).isEqualTo("(Object.java:7)");
+
+        JavaClass classWithoutSource = getClassWithoutSource();
+
+        assertThat(classWithoutSource.getSource()).as("source").isAbsent();
+        assertThat(formatLocation(classWithoutSource, 7)).isEqualTo(String.format("(%s.java:7)", classWithoutSource.getSimpleName()));
+    }
+
+    private JavaClass getClassWithoutSource() {
+        for (JavaAccess<?> javaAccess : importClassWithContext(SomeClass.class).getAccessesFromSelf()) {
+            if (javaAccess.getTargetOwner().isEquivalentTo(ArrayList.class)) {
+                return javaAccess.getTargetOwner();
+            }
+        }
+        throw new RuntimeException("Could not create any java class without source");
+    }
+
+    private static class SomeClass {
+        public SomeClass() {
+            new ArrayList<>();
+        }
+    }
+}
diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/SourceTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/SourceTest.java
@@ -2,6 +2,7 @@
 
 import java.io.IOException;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
@@ -13,6 +14,7 @@
 import com.google.common.collect.Lists;
 import com.google.common.io.ByteStreams;
 import com.tngtech.archunit.ArchConfiguration;
+import com.tngtech.archunit.base.Optional;
 import com.tngtech.archunit.core.domain.Source.Md5sum;
 import com.tngtech.java.junit.dataprovider.DataProvider;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
@@ -23,9 +25,9 @@
 import org.junit.runner.RunWith;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.tngtech.archunit.testutil.Assertions.assertThat;
 import static com.tngtech.java.junit.dataprovider.DataProviders.$;
 import static com.tngtech.java.junit.dataprovider.DataProviders.$$;
-import static org.assertj.core.api.Assertions.assertThat;
 
 @RunWith(DataProviderRunner.class)
 public class SourceTest {
@@ -34,6 +36,15 @@ public void tearDown() {
         ArchConfiguration.get().reset();
     }
 
+    @Test
+    public void source_file_name() throws URISyntaxException {
+        Source source = new Source(urlOf(Object.class).toURI(), Optional.of("SomeClass.java"));
+        assertThat(source.getFileName()).as("source file name").contains("SomeClass.java");
+
+        source = new Source(urlOf(Object.class).toURI(), Optional.<String>absent());
+        assertThat(source.getFileName()).as("source file name").isAbsent();
+    }
+
     @DataProvider
     public static List<List<?>> expectedHexCodes() {
         List<List<?>> testCases = new ArrayList<>();
@@ -61,7 +72,7 @@ public static Object[][] classes() {
     @Test
     @UseDataProvider("classes")
     public void calculates_md5_correctly(URL url) throws Exception {
-        Source source = new Source(url.toURI());
+        Source source = newSource(url);
 
         assertThat(source.getUri()).as("source URI").isEqualTo(url.toURI());
         assertThat(source.getMd5sum().asBytes()).isEqualTo(expectedMd5BytesAt(url));
@@ -70,12 +81,12 @@ public void calculates_md5_correctly(URL url) throws Exception {
     @Test
     @UseDataProvider("classes")
     public void equals_hashcode_and_toString(URL url) throws Exception {
-        Source source = new Source(url.toURI());
-        Source equalSource = new Source(url.toURI());
+        Source source = newSource(url);
+        Source equalSource = newSource(url);
 
         assertThat(source).as("source").isEqualTo(equalSource);
         assertThat(source.hashCode()).as("hashcode").isEqualTo(equalSource.hashCode());
-        assertThat(source).as("source").isNotEqualTo(new Source(urlOf(Object.class).toURI()));
+        assertThat(source).as("source").isNotEqualTo(newSource(urlOf(Object.class)));
         String expectedToString = String.format("%s [md5='%s']", url, Md5sum.toHex(expectedMd5BytesAt(url)));
         assertThat(source.toString()).as("source.toString()").isEqualTo(expectedToString);
     }
@@ -131,7 +142,7 @@ public void negative_equals_of_md5sums(Md5sum first, Md5sum second) {
 
     @Test
     public void compensates_error_on_md5_calculation() throws Exception {
-        Source source = new Source(new URI("bummer"));
+        Source source = newSource(new URI("bummer"));
 
         assertThat(source.getMd5sum()).isEqualTo(Md5sum.UNDETERMINED);
     }
@@ -143,7 +154,15 @@ public void disables_md5_calculation_via_config() throws Exception {
         assertThat(Md5sum.of("any".getBytes())).isEqualTo(Md5sum.DISABLED);
 
         // NOTE: This tests that URIs are note resolved, which costs performance, if it would be resolved, we would get UNDETERMINED
-        assertThat(new Source(new URI("bummer")).getMd5sum()).isEqualTo(Md5sum.DISABLED);
+        assertThat(newSource(new URI("bummer")).getMd5sum()).isEqualTo(Md5sum.DISABLED);
+    }
+
+    private Source newSource(URL url) throws URISyntaxException {
+        return newSource(url.toURI());
+    }
+
+    private Source newSource(URI uri) {
+        return new Source(uri, Optional.<String>absent());
     }
 
     private static byte[] expectedMd5BytesAt(URL url) throws IOException, NoSuchAlgorithmException {

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
@@ -1727,11 +1727,19 @@ public void class_has_source_of_import() throws Exception {
         JavaClass clazzFromFile = new ClassFileImporter().importClass(ClassToImportOne.class);
         Source source = clazzFromFile.getSource().get();
         assertThat(source.getUri()).isEqualTo(urlOf(ClassToImportOne.class).toURI());
+        assertThat(source.getFileName()).contains(ClassToImportOne.class.getSimpleName() + ".java");
         assertThat(source.getMd5sum()).isEqualTo(md5sumOf(bytesAt(urlOf(ClassToImportOne.class))));
 
+        clazzFromFile = new ClassFileImporter().importClass(ClassWithInnerClass.Inner.class);
+        source = clazzFromFile.getSource().get();
+        assertThat(source.getUri()).isEqualTo(urlOf(ClassWithInnerClass.Inner.class).toURI());
+        assertThat(source.getFileName()).contains(ClassWithInnerClass.class.getSimpleName() + ".java");
+        assertThat(source.getMd5sum()).isEqualTo(md5sumOf(bytesAt(urlOf(ClassWithInnerClass.Inner.class))));
+
         JavaClass clazzFromJar = new ClassFileImporter().importClass(Rule.class);
         source = clazzFromJar.getSource().get();
         assertThat(source.getUri()).isEqualTo(urlOf(Rule.class).toURI());
+        assertThat(source.getFileName()).contains(Rule.class.getSimpleName() + ".java");
         assertThat(source.getMd5sum()).isEqualTo(md5sumOf(bytesAt(urlOf(Rule.class))));
 
         ArchConfiguration.get().setMd5InClassSourcesEnabled(false);