Skip to content

fix: FinalizeMethodArguments considers unary and += expressions #180

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.

Sign up for GitHub

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

Merged
timtebeek merged 6 commits into from
Oct 2, 2023
Merged

fix: FinalizeMethodArguments considers unary and += expressions #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c522be5
fix: FinalizeMethodArguments considers unary and += expressions
AlejandroBertolo Oct 2, 2023
bf5c622
chore: add language tag
AlejandroBertolo Oct 2, 2023
a4debfa
chore: mark contributed code
AlejandroBertolo Oct 2, 2023
a4575ae
Update src/test/java/org/openrewrite/staticanalysis/FinalizeMethodArg…
AlejandroBertolo Oct 2, 2023
ca9c0a6
Polish
timtebeek Oct 2, 2023
74bfae2
Merge branch 'main' into bugfix/GH-176-finalizeMethodArguments-unary
timtebeek Oct 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions src/main/java/org/openrewrite/staticanalysis/FinalizeMethodArguments.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,38 @@ public J.Assignment visitAssignment(J.Assignment a, AtomicBoolean hasAssignment)

return assignment;
}

@Override
public J.Unary visitUnary(final J.Unary unary, final AtomicBoolean hasAssignment) {
if (hasAssignment.get()) {
return unary;
}

final J.Unary u = super.visitUnary(unary, hasAssignment);
if (u.getOperator().isModifying() && u.getExpression() instanceof J.Identifier) {
final J.Identifier i = (J.Identifier) u.getExpression();
if (i.getSimpleName().equals(this.variable.getSimpleName())) {
hasAssignment.set(true);
}
}
return u;
}

@Override
public J.AssignmentOperation visitAssignmentOperation(final J.AssignmentOperation assignOp, final AtomicBoolean hasAssignment) {
if (hasAssignment.get()) {
return assignOp;
}

final J.AssignmentOperation a = super.visitAssignmentOperation(assignOp, hasAssignment);
if (a.getVariable() instanceof J.Identifier) {
final J.Identifier i = (J.Identifier) a.getVariable();
if (i.getSimpleName().equals(this.variable.getSimpleName())) {
hasAssignment.set(true);
}
}
return a;
}
}

private static Statement updateParam(final Statement p) {
Expand Down
258 changes: 149 additions & 109 deletions src/test/java/org/openrewrite/staticanalysis/FinalizeMethodArgumentsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,12 @@ void doNotAddFinalIfAssigned() {
//language=java
java(
"""
package a;
class A {
void SubeventUtils(String a) {
a = "abc";
}
}
"""
class A {
void SubeventUtils(String a) {
a = "abc";
}
}
"""
)
);
}
Expand All @@ -93,12 +92,10 @@ void doNotAddFinalIfInterface() {
//language=java
java(
"""
package a;
public interface MarketDeleteService {

void deleteMarket(Long marketId, String deletionTimestamp);
}
"""
public interface MarketDeleteService {
void deleteMarket(Long marketId, String deletionTimestamp);
}
"""
)
);
}
Expand All @@ -110,25 +107,17 @@ void replaceWithFinalModifier() {
//language=java
java(
"""
package com.test;

class TestClass {

private void getAccaCouponData(String responsiveRequestConfig, String card) {

}
}
""",
class TestClass {
private void getAccaCouponData(String responsiveRequestConfig, String card) {
}
}
""",
"""
package com.test;

class TestClass {

private void getAccaCouponData(final String responsiveRequestConfig, final String card) {

}
}
"""
class TestClass {
private void getAccaCouponData(final String responsiveRequestConfig, final String card) {
}
}
"""
)
);
}
Expand All @@ -139,103 +128,154 @@ void replaceWithFinalModifierWhenAnnotated() {
//language=java
java(
"""
public class Test {
public void test(@Override Integer test) {}
}
""",
class Test {
public void test(@Override Integer test) {}
}
""",
"""
public class Test {
public void test(@Override final Integer test) {}
}
"""
class Test {
public void test(@Override final Integer test) {}
}
"""
)
);
}

@Test
void replaceWithFinalModifierNoArguments() {
void doNotReplaceWithFinalModifier() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.none()),
//language=java
java(
"""
package com.test;

class TestClass {

private void getAccaCouponData() {

}
}
"""
package responsive.utils.subevent;

import responsive.enums.subevent.SubeventTypes;
import responsive.model.dto.card.SubEvent;
import java.util.List;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;

import static responsive.enums.matchdata.MatchDataTitleSeparator.AT;
import static responsive.enums.matchdata.MatchDataTitleSeparator.VS;
import static java.lang.String.format;
import static org.apache.commons.lang3.StringUtils.splitByWholeSeparator;

/**
* Created by mza05 on 13/10/2017.
*/
@Component
class SubeventUtils {

private static final String SUBEVENT_FORMAT = "%s%s%s";
private final List<Integer> categoryGroupIdForChangeSubeventName;

public SubeventUtils(
@Value("#{'${responsive.category.group.id.change.subevent.name}'.split(',')}") final List<Integer> categoryGroupIdForChangeSubeventName) {
this.categoryGroupIdForChangeSubeventName = categoryGroupIdForChangeSubeventName;
}

public static boolean isSubeventOfSpecifiedType(final SubEvent subEvent, final List<SubeventTypes> requiredTypes) {

if (subEvent.getType() == null) {
return false;
}
return requiredTypes.stream()
.anyMatch(requiredType -> requiredType.getType().equalsIgnoreCase(subEvent.getType()));

}

/**
* Change SubeventName by CategoryGroupId and rebub
* @param subeventName
* @param categoryGroupId
* @return
*/
public String getSubeventNameForCategoryGroupId(final String subeventName, final Integer categoryGroupId) {

if (subeventName != null && categoryGroupId != null
&& subeventName.contains(AT.getSeparator())
&& categoryGroupIdForChangeSubeventName.contains(categoryGroupId)) {
final var subeventTeamSplit = splitByWholeSeparator(subeventName, AT.getSeparator());
if (subeventTeamSplit.length > 0) {
return format(SUBEVENT_FORMAT, subeventTeamSplit[0], VS.getSeparator(), subeventTeamSplit[1]);
}
}

return subeventName;
}
}
"""
)
);
}

@Test
void doNotReplaceWithFinalModifier() {
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/176")
void doNotReplaceIfAssignedThroughUnaryOrAccumulator() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.none()),
//language=java
java(
"""
package responsive.utils.subevent;

import responsive.enums.subevent.SubeventTypes;
import responsive.model.dto.card.SubEvent;
import java.util.List;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;

import static responsive.enums.matchdata.MatchDataTitleSeparator.AT;
import static responsive.enums.matchdata.MatchDataTitleSeparator.VS;
import static java.lang.String.format;
import static org.apache.commons.lang3.StringUtils.splitByWholeSeparator;

/**
* Created by mza05 on 13/10/2017.
*/
@Component
public class SubeventUtils {

private static final String SUBEVENT_FORMAT = "%s%s%s";
private final List<Integer> categoryGroupIdForChangeSubeventName;

public SubeventUtils(
@Value("#{'${responsive.category.group.id.change.subevent.name}'.split(',')}") final List<Integer> categoryGroupIdForChangeSubeventName) {
this.categoryGroupIdForChangeSubeventName = categoryGroupIdForChangeSubeventName;
}

public static boolean isSubeventOfSpecifiedType(final SubEvent subEvent, final List<SubeventTypes> requiredTypes) {

if (subEvent.getType() == null) {
return false;
}
return requiredTypes.stream()
.anyMatch(requiredType -> requiredType.getType().equalsIgnoreCase(subEvent.getType()));

}

/**
* Change SubeventName by CategoryGroupId and rebub
* @param subeventName
* @param categoryGroupId
* @return
*/
public String getSubeventNameForCategoryGroupId(final String subeventName, final Integer categoryGroupId) {

if (subeventName != null && categoryGroupId != null
&& subeventName.contains(AT.getSeparator())
&& categoryGroupIdForChangeSubeventName.contains(categoryGroupId)) {
final var subeventTeamSplit = splitByWholeSeparator(subeventName, AT.getSeparator());
if (subeventTeamSplit.length > 0) {
return format(SUBEVENT_FORMAT, subeventTeamSplit[0], VS.getSeparator(), subeventTeamSplit[1]);
}
}

return subeventName;
}
}
"""
class Test {
protected int addFinalToThisVar(int unalteredVariable) {
return unalteredVariable;
}

protected int increment(int variableToIncrement) {
variableToIncrement++;
return variableToIncrement;
}

protected int preIncrement(int variableToPreIncrement) {
return ++variableToPreIncrement;
}

protected int decrement(int variableToDecrement) {
variableToDecrement--;
return variableToDecrement;
}

protected int preDecrement(int variableToPreDecrement) {
return --variableToPreDecrement;
}

protected int accumulate(int add, int accumulator) {
accumulator += add;
return accumulator;
}
}
""",
"""
class Test {
protected int addFinalToThisVar(final int unalteredVariable) {
return unalteredVariable;
}

protected int increment(int variableToIncrement) {
variableToIncrement++;
return variableToIncrement;
}

protected int preIncrement(int variableToPreIncrement) {
return ++variableToPreIncrement;
}

protected int decrement(int variableToDecrement) {
variableToDecrement--;
return variableToDecrement;
}

protected int preDecrement(int variableToPreDecrement) {
return --variableToPreDecrement;
}

protected int accumulate(int add, int accumulator) {
accumulator += add;
return accumulator;
}
}
"""
)
);
}
Expand Down