https://kotlinlang.org logo
Title
b

Brian Hartvigsen

05/10/2023, 10:38 PM
Is this the best place to ask about a plugin I've developed? It works in the tests, but when I run it against actual code, no results, even if I have the exact same code from the test in the actual code.
@RequiresTypeResolution
class PreferLetOverIfNotNullElseNull(
    config: Config = Config.empty
) : Rule(config) {
    override val issue = Issue(
        "PreferLetOverIfNotNullElseNull",
        Severity.Style,
        "Using `if (X == null) { } else null` is just a longer form of `X?.let { }`",
        Debt.FIVE_MINS
    )

    override val active: Boolean
        get() = true

    override fun visitIfExpression(expression: KtIfExpression) {
        super.visitIfExpression(expression)
        val elseBranch = expression.`else` ?: return

        val condition = expression.condition as? KtBinaryExpression

        if (condition?.isNonNullCheck() == true && isNullConstant(elseBranch)) {
            report(CodeSmell(issue, Entity.from(expression), issue.description))
        }
    }
}
I'm moved the report to the head of the
visitIfExpression
function and it doesn't appear to be getting called at all
(The
override val active
was just my latest attempt, I don't know how to tell if my plugin is even getting called)
I do have it set to active in my configuration and in the config/config.yml
OMG.. it's because I had it as
Severity.Style
and didn't have style checks enabled... 🤦‍♂️
nope, something weirder is going on with my ruleset...
Apparently it has to deal the length of the rule name
NoIfNotNullElseNull
works, but if I change the name to
PreferLetOverIfNotNullElseNull
the rule fails to process
b

Brais Gabin

05/11/2023, 9:36 AM
Is this the best place to ask about a plugin I’ve developed?
I’m not sure if it is the best, but it is a good one for sure 😉 This seems as an error on the configuration. when you change the name of the rule are you editting your configuration to ensure the rule and the rule set are enabled? And, for testing, I would recommend you to set a unconditional report somewhere so you know that every-single-file should be reported so the only thing that could be not working is the configuration. Once the configuration is working you can return to the real behaviour.
https://github.com/detekt/detekt-custom-rule-template this template should help a lot with the configuration.
a

Atul Gupta

05/11/2023, 11:01 AM
Offtopic: This sounds like a good idea for built in rule. @Brais Gabin / @gammax thoughts?
b

Brais Gabin

05/11/2023, 12:16 PM
What would we check exactly? This doesn't seems like a kotlin related issue, right?
a

Atul Gupta

05/11/2023, 12:50 PM
if (X == null) { } else null is just a longer form of X?.let { }
This rule prevent this
b

Brais Gabin

05/11/2023, 1:37 PM
Oh I didn’t get the idea. In general I’m not a big fan of
let
because people usually overuse it and a rule that enforce you to use it… I’m not 100% sure if is good. But for sure I would always use the
let
form on this case. We could open an issue and look for what other people think
b

Brian Hartvigsen

05/11/2023, 9:11 PM
I've got an improved version I've written (and better named as
CouldBeLet
)
If you'd like I'll open a PR, or I can do it as a marketplace ruleset
I think I followed all the guidance in CONTRIBUTING (as best I could)