Skip to content
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

False positive performance SpreadOperator in case of pass-through vararg #3145

Closed
christianhujer opened this issue Oct 15, 2020 · 2 comments · Fixed by #3157
Closed

False positive performance SpreadOperator in case of pass-through vararg #3145

christianhujer opened this issue Oct 15, 2020 · 2 comments · Fixed by #3157
Assignees
Milestone

Comments

@christianhujer
Copy link

Expected Behavior

When using the spread operator to pass on a vararg array from a vararg function implementation to a vararg function call, the spread operator should not be reported.

Observed Behavior

The spread operator passing a vararg array from a vararg function implementation to a vararg function call is reported by Detekt as a performance issue because of the spread operator.

This is wrong because, in the case of passing vararg to vararg, the spread operator does not (anymore since Kotlin 1.1.60, 1.2-Beta2) create a new array and thus is not a performance issue. Refer to the following ticket on Kotlin: https://youtrack.jetbrains.com/issue/KT-20462

Steps to Reproduce

Call one vararg method from another using the spread operator.
A popular example is to call runApplication(args) from main(vararg args: String) in Spring applications.

The following code should be accepted as correct by detekt:

package com.example

import org.springframework.boot.autoconfigure.SpringBootApplication
import org.springframework.boot.runApplication
import org.springframework.context.ApplicationContext

@SpringBootApplication
class ExampleApplication

lateinit var appContext: ApplicationContext
    private set

fun main(vararg args: String) {
    appContext = runApplication<ExampleApplication>(*args) // ⇐ Point of Interest
}

Context

The impact is low.
The situation of passing through vararg is relatively rare.
It is easy to deal with the situation by using a @SuppressWarnings("SpreadOperator") annotation on the corresponding method.

Your Environment

  • Version of detekt used: 1.14.1
  • Version of Gradle used (if applicable): 6.6.1
  • Operating System and version: Kubuntu 20.04 LTS
  • Link to your project (if it's a public repository): -
@christianhujer
Copy link
Author

As an enhancement, the check could look at the actual byte code generated. If the spread operator leads to a new array instantiation, the spread operator is a performance issue and should be reported. If the spread operator passes through an existing array, the spread operator is not a performance issue and should not be reported.

@arturbosch arturbosch self-assigned this Oct 18, 2020
arturbosch added a commit that referenced this issue Oct 19, 2020
…as they do not create an array copy as of Kotlin 1.1 - Closes #3145
@arturbosch arturbosch added this to the 1.14.2 milestone Oct 19, 2020
arturbosch added a commit that referenced this issue Oct 20, 2020
…meter (#3157)

* Do not report vararg parameters which are passed as vararg arguments as they do not create an array copy as of Kotlin 1.1  - Closes #3145

* State that spread operator may lead to a performance penalty not that it must

* Do not report vararg pass through arguments for non type resolution case - Closes #3145

* Exclude guard clauses for ReturnCount
@christianhujer
Copy link
Author

Thank you, works like a charm now! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants