New rule: Reports the usages of <expr1>?.let { <expr2> } ?: run { <single_line_expr> }

[atulgpt]

Expected Behavior of the rule

Rule should report <expr1>?.let { <expr2> } ?: run { <single_line_expr> } as expression can be simplified as <expr1>?.let { <expr2> } ?: <single_line_expr>. Here there is an unnecessary use of run { } block

uncompliant code

val nullable: Int? = 4
val default = 1
val a = nullable?.let { 0 } ?: run { default }

compliant code

val nullable: Int? = 4
val default = 1
val a = nullable?.let { 0 } ?: default

Context

Got this review in #5981 (comment) having unnecessary run { } really makes the code more indented and increases the complexity

[3flex]

Could the rule be more generic?

It looks like what we want flagged is any use of the non-extension version of run that contains a single line expression.

[atulgpt]

Yes, I think It can be generic. But then we also have to consider single expression use of with, apply, let and if those make sense as well or not(and then we can maybe incorporate more scoping operators). Let me update the proposal

[atulgpt]

Hi, @3flex got your query yes it should be generic.

We can create similar rules for with as well as apply as well
Some more violating code

val length = with("") {
       length
}

val length = "".apply {
       length
}

val a = run { "".length }

There is one similar rule UnnecessaryLet which does the same with let.

Update 29/05/23: We also have UnnecessaryApply rule

[BraisGabin]

So the proposal would be to create UnnecessaryRun and UnnecessaryWith too? I would vote in favor of that. And these will not be related at all with the previous <expr1>?.let { <expr2> } ?:, right?

If that's the idea, we need proper definitions of what these rules should catch. Some more examples of compilant/non-compilant.

[atulgpt]

Yes @BraisGabin this suggestion is independent of the previous suggestion as that one deals through the case when due to the last nullable expression code execution pass through the next run or apply block as well.
This one mostly focuses on unnecessary block operators when there is only one expression

[detekt-ci]

This issue is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days.

[detekt-ci]

This issue was closed because it has been stalled for 7 days with no activity.

[atulgpt]

Not stale

[BraisGabin]

Sorry for the really late response. How can we make this issue actionable? Would a rule called UnnecessaryRun that detects single expresions inside run close this issue?

[atulgpt]

Yes, @BraisGabin that will close the issue. After implementing that and analyzing the learnings from that rule we can implement more rules mentioned in #6112 (comment)

[BraisGabin]

Ok, so to summarize to anyone reading and willing to contribute. We want a rule called UnnecessaryRun that detects single expressions. And we are only talking about run without any receiver. T.run is another topic.

This rule can be expanded in the future but for now just that.