Skip to content

KotlinSafeCallOperatorFilter should filter "unoptimized" chains #1818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 6, 2025

Conversation

Godin
Copy link
Member

@Godin Godin commented Dec 16, 2024

For the following Example.kt

data class A(val b: B)
data class B(val c: String)

fun example(a: A?): String? =
    a
        ?.b
        ?.c

Kotlin compiler versions 1.6.21, 1.7.21, 1.8.22, 1.9.22, 2.0.21 and 2.1.0 produce

         0: aload_0
         1: ifnull        20
         4: aload_0
         5: invokevirtual #13                 // Method A.getB:()LB;
         8: astore_1
         9: aload_1
        10: ifnull        20
        13: aload_1
        14: invokevirtual #19                 // Method B.getC:()Ljava/lang/String;
        17: goto          21
        20: aconst_null
        21: areturn

which is not handled by #1810


For the following Example.kt

data class A(val b: B)
data class B(val c: String)

fun example(a: A?): String? =
    a?.also {
        println(it)
    }?.b?.c

Kotlin compiler versions 2.0.21 and 2.1.0 generate similar

  public static final java.lang.String example(A);
    descriptor: (LA;)Ljava/lang/String;
    flags: (0x0019) ACC_PUBLIC, ACC_STATIC, ACC_FINAL
    Code:
      stack=2, locals=4, args_size=1
         0: aload_0
         1: ifnull        34
         4: aload_0
         5: astore_1
         6: aload_1
         7: astore_2
         8: iconst_0
         9: istore_3
        10: getstatic     #31                 // Field java/lang/System.out:Ljava/io/PrintStream;
        13: aload_2
        14: invokevirtual #37                 // Method java/io/PrintStream.println:(Ljava/lang/Object;)V
        17: nop
        18: aload_1
        19: invokevirtual #13                 // Method A.getB:()LB;
        22: astore_1
        23: aload_1
        24: ifnull        34
        27: aload_1
        28: invokevirtual #19                 // Method B.getC:()Ljava/lang/String;
        31: goto          35
        34: aconst_null
        35: areturn

whereas Kotlin compiler versions 1.6.21, 1.7.21, 1.8.22, 1.9.22 produce

  public static final java.lang.String example(A);
    descriptor: (LA;)Ljava/lang/String;
    flags: (0x0019) ACC_PUBLIC, ACC_STATIC, ACC_FINAL
    Code:
      stack=2, locals=4, args_size=1
         0: aload_0
         1: dup
         2: ifnull        32
         5: astore_1
         6: aload_1
         7: astore_2
         8: iconst_0
         9: istore_3
        10: getstatic     #31                 // Field java/lang/System.out:Ljava/io/PrintStream;
        13: aload_2
        14: invokevirtual #37                 // Method java/io/PrintStream.println:(Ljava/lang/Object;)V
        17: nop
        18: aload_1
        19: invokevirtual #13                 // Method A.getB:()LB;
        22: dup
        23: ifnull        32
        26: invokevirtual #19                 // Method B.getC:()Ljava/lang/String;
        29: goto          34
        32: pop
        33: aconst_null
        34: areturn

which is handled by #1810


For https://github.com/SonarSource/sonar-kotlin/tree/2.22.0.5972
after #1810 we find 65 safe call operator chains
and after this change we find 78
ie 13 more.

For https://github.com/JetBrains/kotlin/tree/v2.0.0
after #1810 we find 2161
and after this 2275
ie 114 more.

@Godin Godin added this to the 0.8.13 milestone Dec 16, 2024
@Godin Godin marked this pull request as ready for review December 17, 2024 07:22
@Godin Godin requested a review from leveretka December 17, 2024 07:22
@Godin Godin force-pushed the kotlin_safe_call_chain branch from 70b6190 to 3a70e1c Compare December 17, 2024 20:42
Copy link
Collaborator

@leveretka leveretka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

It's an interesting case, thanks for taking care of it!

@Godin Godin enabled auto-merge (squash) January 6, 2025 09:39

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@Godin Godin merged commit be4f9fd into jacoco:master Jan 6, 2025
34 checks passed
@Godin Godin deleted the kotlin_safe_call_chain branch January 6, 2025 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants