`InlineVariable` to handle more complex expressions too

[Pankraz76]

What version of OpenRewrite are you using?

I am using
id 'org.openrewrite.rewrite' version '7.22.0' apply false
org.openrewrite.recipe:rewrite-static-analysis:2.23.0

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project.

/*
 * Licensed to the Apache Software Foundation (ASF) under one or more
 * contributor license agreements. See the NOTICE file distributed with
 * this work for additional information regarding copyright ownership.
 * The ASF licenses this file to You under the Apache License, Version 2.0
 * (the "License"); you may not use this file except in compliance with
 * the License. You may obtain a copy of the License at
 *
 * https://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

apply plugin: 'org.openrewrite.rewrite'

dependencies {
    rewrite('org.openrewrite.recipe:rewrite-static-analysis:2.23.0')
}

rewrite {
    activeRecipe('org.apache.kafka.openrewrite.SanityCheck')
    configFile = project.getRootProject().file("${rootDir}/config/sanity.yml")
    setExportDatatables(true)
    setFailOnDryRunResults(true)
}

What is the smallest, simplest way to reproduce the problem?

    private int nextValue(String topic) {
        AtomicInteger counter = topicCounterMap.computeIfAbsent(topic, k -> new AtomicInteger(0));
        return counter.getAndIncrement();
    }

What did you expect to see?

    private int nextValue(String topic) {
        return topicCounterMap.computeIfAbsent(topic, k -> new AtomicInteger(0)).getAndIncrement();
    }

What did you see instead?

    private int nextValue(String topic) {
        AtomicInteger counter = topicCounterMap.computeIfAbsent(topic, k -> new AtomicInteger(0));
        return counter.getAndIncrement();
    }

PS: please dont be the new checkstyle caring more and actually only about their bug-template instead of the bug.

[greg-at-moderne]

What is wrong with:

        return topicCounterMap.computeIfAbsent(topic, k -> new AtomicInteger(0)).getAndIncrement();

?

Not sure why you would expect not to see it.

[Pankraz76]

updated. thats the expect, not result.

[greg-at-moderne]

OK, when reading the code of the recipe, I think what is only supported right now is that if a return (or throw) statement contains a plain reference to the variable. More complex expressions are not supported. E.g. return counter; is a candidate for inlining, return counter.getAndIncrement() is not.

To provide some context, I am not sure we should unconditionally extend the recipe to handle more complex expressions for two reasons:

• it's sometimes a matter of style to introduce a single-use variable with a specific name; some people find it better than putting comments. Inlining such an expression might actually degrade the code (in their eyes)
• at least for some expressions, we risk producing some complex one-liner which would be less readable than the original code.

Therefore:

• I don't consider it a bug, and let me remove the bug label
• I still think something can be done here, but it needs to be designed, possibly with some heuristics