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

Deprecated replaceWith - support in diktat #1343

Open
orchestr7 opened this issue Jun 3, 2022 · 6 comments
Open

Deprecated replaceWith - support in diktat #1343

orchestr7 opened this issue Jun 3, 2022 · 6 comments
Labels
enhancement New feature or request

Comments

@orchestr7
Copy link
Member

Kotlin has outstanding logic (deprecation scenarios) - library author can help user to fix the code.
Probably in the future, when we will have all information about the project (and new framework)

@Deprecated("Old stuff", ReplaceWith("test2"))
@orchestr7 orchestr7 added the enhancement New feature or request label Jun 3, 2022
@nulls
Copy link
Member

nulls commented Jun 7, 2022

Do you mean support it in diktat with autofix? "replaceWith" doesn't mean replacement 1:1

@orchestr7
Copy link
Member Author

Do you mean support it in diktat with autofix? "replaceWith" doesn't mean replacement 1:1

No, it actually does the logic in 90% of scenarios. IDEA uses it: https://dev.to/mreichelt/the-hidden-kotlin-gem-you-didn-t-think-you-ll-love-deprecations-with-replacewith-3blo

@nulls
Copy link
Member

nulls commented Jun 7, 2022

If we will support it as autofix logic -- we will produce irrelevant code in 10% of scenarios.

@orchestr7
Copy link
Member Author

orchestr7 commented Jun 7, 2022

If we will support it as autofix logic -- we will produce irrelevant code in 10% of scenarios.

How IDEA filters out false positives?
They do absolutely the same

@nulls
Copy link
Member

nulls commented Jun 7, 2022

Yep, looks like I didn't get properly kotlin.ReplaceWith
Tested locally -- IDEA doesn't check that ReplaceWith.expression is valid (developer is who triggers a replacement in IDEA). But I guess there is the assumption: if ReplaceWith.expression is set, it must be applicable and valid.

Make the following example (inspired by com.pinterest.ktlint.core.KtLint.ExperimentalParams vs com.pinterest.ktlint.core.KtLint.Params)

  1. replaceWith is valid
data class ParamsOld(
    val arg1: String,
    val arg2: String,
)

@Deprecated(
    message = "extended params",
    replaceWith = ReplaceWith("this.doLogicNew(ParamsNew(params.arg1, params.arg2, arg3))", imports = ["ParamsNew"])
)
fun doLogicOld(params: ParamsOld, arg3: String) {
    println("${params.arg1} -> ${params.arg2} -> ${arg3}")
}

data class ParamsNew(
    val arg1: String,
    val arg2: String,
    val arg3: String,
)

fun doLogicNew(params: ParamsNew) {
    println("${params.arg1} -> ${params.arg2} -> ${params.arg3}")
}

Usage:

fun test() {
    doLogicOld(ParamsOld("1", "2"), "3")
}

IDEA transforms it to (problem with whitespaces on new line)

fun test() {
    val params = ParamsOld("1", "2")
 doLogicNew(ParamsNew(params.arg1, params.arg2, "3"))
}
  1. replaceWith is invalid
// same example as previously, but
@Deprecated(
    message = "extended params",
    replaceWith = ReplaceWith("this.doLogicInvalid(ParamsNew(params.arg1, params.arg2, arg3))", imports = ["ParamsNew"])
)
fun doLogicOld(params: ParamsOld, arg3: String) {
    println("${params.arg1} -> ${params.arg2} -> ${arg3}")
}

IDEA transforms it to (funny, but it doesn't have problem with whitespaces on new line)

fun test() {
    val params = ParamsOld("1", "2")
    this.doLogicInvalid(ParamsNew(params.arg1, params.arg2, "3"))
}

@0x6675636b796f75676974687562
Copy link
Member

In response to the previous comment:

But I guess there is the assumption: if ReplaceWith.expression is set, it must be applicable and valid.

Exactly. And if my memory fails me not, there's an equivalent quickfix action in IDEA itself.

Additionally, a user can always switch a particular DiKTat inspection off is they desire so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants