JGiven icon indicating copy to clipboard operation
JGiven copied to clipboard

fix: Kotlin name mangling

Open TimothyEarley opened this issue 1 year ago • 5 comments

Problem

Kotlin mangles some names when value classes are used. See https://kotlinlang.org/docs/inline-classes.html#mangling This mangled name is then used in the reports.

Solution

This commit removes the dash and following chars from the default method name. Thus leading to better readable reports.

This should not affect Java projects as far as I can tell since "-" is not valid in method names (see https://docs.oracle.com/javase/specs/jls/se7/html/jls-3.html#jls-3.8).

Alternatives

  • Instead of having this case covered by JGiven, we could also override the method name in all affected methods using @As (repeated test name), @As with a custom provider, or Kotlins @JvmName (repeated test name).

  • Another alternative would be to allow setting the default AsProvider (same as with the formatter currently). I presume this would mean adding the config in all our test classes, but not on each method individually.

Thanks for your time!

TimothyEarley avatar Feb 27 '24 12:02 TimothyEarley

Hi, thanks for the submission. I will take a look as soon as possible

l-1squared avatar Feb 29 '24 05:02 l-1squared

Looks good so far, I need to check, whether it might affect the spock package, since that uses groovy.

l-1squared avatar Mar 15 '24 06:03 l-1squared

@TimothyEarley I started writing https://github.com/TNG/JGiven/pull/1574, hoping that it might alleviate your issues

l-1squared avatar Mar 27 '24 06:03 l-1squared

Thanks for continuing to look into it!

Configuring a default As Provider should help alleviate the problem (and then the method described below could be in a separate Kotlin Provider).

Another approach I just tested is using Kotlin Reflection. With the kotlin-reflect library we can use the following code snippet to get the source method name (as opposed to the JVM name) in the DefaultAsProvider:

var kMethod = ReflectJvmMapping.getKotlinFunction(method);
if (kMethod != null) {
    return methodNameToReadableText( kMethod.getName() );
} else {
    return methodNameToReadableText( method.getName() );
}

Docs for kMethod.getName(): https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect/-k-callable/name.html

This would remove manipulating the method name by hand and should be safe in other languages (kMethod would be null) at the cost of adding a dependency on the kotlin-reflect library. Let me know if you would like a commit trying these out (with kotlin tests?)

TimothyEarley avatar Mar 27 '24 08:03 TimothyEarley

Hi, sorry for keeping you waiting, I was busy celebrating easter.

I think that using the kotlin reflect library might be a good tool to solve the issue for a hypothetical jgiven-kotlin package; using it in core just doesn't have the right ring in my ears. It would be an rather unexpected feature with an unexpected dependency at an unexpected place ( wo do advertise as "plain java" after all ). Therefore, if we are willing to tackle kotlin-specific problems when using jgiven. they should go in https://github.com/TNG/JGiven/pull/407

l-1squared avatar Apr 02 '24 04:04 l-1squared

Closing this PR, I think a more custom solution based on a tailored AsProvider is more appropriate

l-1squared avatar Sep 27 '24 11:09 l-1squared