Filter unreachable branches in Kotlin open functions with default arguments #887
Conversation
| } | ||
|
|
||
| @Test | ||
| public void all_branches_should_have_line_number() { |
There was a problem hiding this comment.
@marchof here is some notes about this new test:
Unfortunately in our validation tests all assertions are about code that has line numbers, so validation test was green without this - see first commit.
Similar generic assertion for instructions counter is not possible, because it does not hold for KotlinInlineTarget.
There was a problem hiding this comment.
Nice, I like this!
I actually tried with instructions and it fails with most Kotlin tests.
I wonder whether we should add this test to the base class and explicitly overwrite it for those tests where it does not hold true (with comments why this is).
There was a problem hiding this comment.
I actually tried with instructions and it fails with most Kotlin tests.
You're right - instruction counters won't be equal for most Kotlin targets, because first entry in LineNumberTable is past non-null checks of parameters:
public static final void main(java.lang.String[]);
descriptor: ([Ljava/lang/String;)V
flags: (0x0019) ACC_PUBLIC, ACC_STATIC, ACC_FINAL
Code:
stack=8, locals=2, args_size=1
0: aload_0
1: ldc #10 // String args
3: invokestatic #16 // Method kotlin/jvm/internal/Intrinsics.checkParameterIsNotNull:(Ljava/lang/Object;Ljava/lang/String;)V
6: new #18 // class org/jacoco/core/test/validation/kotlin/targets/KotlinDataClassTarget$DataClass
...
LineNumberTable:
line 40: 6
And in fact I wasn't precise enough about description of "similar generic assertion": what holds for every target except KotlinInlineTarget - is equality of number of missed instructions with line number and without, i.e. that all missed instructions are visible in source, while covered might be invisible, which is ok since they are covered 😉
explicitly overwrite it for those tests where it does not hold true (with comments why this is)
Haven't thought about override. Nice idea! 👍 Added it.
For open functions with default arguments
Kotlin compiler generates check that default arguments are not used for super calls - https://github.com/JetBrains/kotlin/blob/v1.3.31/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java#L1341-L1359 , even if currently such calls not allowed - compiler rejects following
I suppose that check is introduced for future versions of language, where such calls might be allowed.
This PR adds handling of open functions to
KotlinDefaultArgumentsFilter.Fixes #882