Is this the best place to ask about a plugin I've ...

# detekt

[Brian Hartvigsen]

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))
        }
    }
}

[Brian Hartvigsen]

I'm moved the report to the head of the visitIfExpression function and it doesn't appear to be getting called at all

[Brian Hartvigsen]

(The override val active was just my latest attempt, I don't know how to tell if my plugin is even getting called)

[Brian Hartvigsen]

I do have it set to active in my configuration and in the config/config.yml

[Brian Hartvigsen]

OMG.. it's because I had it as Severity.Style and didn't have style checks enabled... 🤦‍♂️

[Brian Hartvigsen]

nope, something weirder is going on with my ruleset...

[Brian Hartvigsen]

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

[Brais Gabin]

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.

[Brais Gabin]

https://github.com/detekt/detekt-custom-rule-template this template should help a lot with the configuration.

[Atul Gupta]

Offtopic: This sounds like a good idea for built in rule. @Brais Gabin / @gammax thoughts?

[Brais Gabin]

What would we check exactly? This doesn't seems like a kotlin related issue, right?

[Atul Gupta]

if (X == null) { } else null is just a longer form of X?.let { }

This rule prevent this

[Brais Gabin]

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

[Brian Hartvigsen]

I've got an improved version I've written (and better named as CouldBeLet)

[Brian Hartvigsen]

If you'd like I'll open a PR, or I can do it as a marketplace ruleset

[Brian Hartvigsen]

https://github.com/detekt/detekt/pull/6091

[Brian Hartvigsen]

I think I followed all the guidance in CONTRIBUTING (as best I could)