testClassesShouldResideInTheSamePackageAsImplementation fails on duplicated simple class name

[timtebeek]

Quite like the new library check to ensure test classes are in the same package.
One small nitpick is that it's currently unable to handle duplicated simple names:

A structure such as this one:

src/main/java/some/package/SomeClass.java
src/test/java/some/package/SomeClassTest.java
src/test/java/some/other/package/SomeClassTest.java

Will fail with:

java.lang.IllegalStateException: Duplicate key SomeClassTest (attempted merging values JavaClass{name='some.package.SomeClassTest'} and JavaClass{name='some.other.package.SomeClassTest'})
	at java.base/java.util.stream.Collectors.duplicateKeyException(Collectors.java:135)
	at java.base/java.util.stream.Collectors.lambda$uniqKeysMapAccumulator$1(Collectors.java:182)
	at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:992)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
	at com.tngtech.archunit.library.GeneralCodingRules$1.init(GeneralCodingRules.java:458)
	at com.tngtech.archunit.lang.ArchRule$Factory$SimpleArchRule.evaluate(ArchRule.java:229)
	at com.tngtech.archunit.lang.ArchRule$Assertions.check(ArchRule.java:96)
	at com.tngtech.archunit.lang.ArchRule$Factory$SimpleArchRule.check(ArchRule.java:211)
	at com.tngtech.archunit.junit.internal.ArchUnitTestDescriptor$ArchUnitRuleDescriptor.execute(ArchUnitTestDescriptor.java:166)
	at com.tngtech.archunit.junit.internal.ArchUnitTestDescriptor$ArchUnitRuleDescriptor.execute(ArchUnitTestDescriptor.java:149)

Should people ideally not be doing this in the first place? Yes of course.
But the library should probably highlight that with a violation, not an exception.

I'm guessing it could be enough to use the FQCN, with some variable renaming:

-.collect(toMap(JavaClass::getSimpleName, identity()));
+.collect(toMap(JavaClass::getName, identity()));

But figured to first report it here to see if this is something to even fix at all.

Tagging @mslowiak as he contributed the feature recently. :)

[codecholeric]

Ah, true, didn't think of that case 😬 From my point of view we should probably just do a multi-map simpleName -> classes, then when checking for violations we see if any of the candidates is in the correct package. If yes we count the rule as satisfied. Would that make sense?

[timtebeek]

That would work as well; wouldn't mind which way to go. :)

[mslowiak]

Oh well, didn't though about this case. I will raise PR for that ⚔️