diff --git a/src/main/java/org/openrewrite/staticanalysis/FinalizeMethodArguments.java b/src/main/java/org/openrewrite/staticanalysis/FinalizeMethodArguments.java index b6070fb513..d7191cdb22 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FinalizeMethodArguments.java +++ b/src/main/java/org/openrewrite/staticanalysis/FinalizeMethodArguments.java @@ -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) { diff --git a/src/test/java/org/openrewrite/staticanalysis/FinalizeMethodArgumentsTest.java b/src/test/java/org/openrewrite/staticanalysis/FinalizeMethodArgumentsTest.java index b790bf752c..33982ae337 100644 --- a/src/test/java/org/openrewrite/staticanalysis/FinalizeMethodArgumentsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/FinalizeMethodArgumentsTest.java @@ -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"; + } + } + """ ) ); } @@ -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); + } + """ ) ); } @@ -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) { + } + } + """ ) ); } @@ -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 categoryGroupIdForChangeSubeventName; + + public SubeventUtils( + @Value("#{'${responsive.category.group.id.change.subevent.name}'.split(',')}") final List categoryGroupIdForChangeSubeventName) { + this.categoryGroupIdForChangeSubeventName = categoryGroupIdForChangeSubeventName; + } + + public static boolean isSubeventOfSpecifiedType(final SubEvent subEvent, final List 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 categoryGroupIdForChangeSubeventName; - - public SubeventUtils( - @Value("#{'${responsive.category.group.id.change.subevent.name}'.split(',')}") final List categoryGroupIdForChangeSubeventName) { - this.categoryGroupIdForChangeSubeventName = categoryGroupIdForChangeSubeventName; - } - - public static boolean isSubeventOfSpecifiedType(final SubEvent subEvent, final List 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; + } + } + """ ) ); }