diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstants.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstants.java new file mode 100644 index 0000000000..5e332c52aa --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstants.java @@ -0,0 +1,194 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * 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. + */ +package org.openrewrite.staticanalysis; + +import org.openrewrite.*; +import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; + +import java.time.Duration; +import java.util.*; + +/** + * This recipe replaces magic number literals in method bodies with named constants following the Sonar java:S109 rule. + * All detected magic numbers (excluding those explicitly assigned to variables or fields, and -1, 0, 1) will be extracted as + * private static final constants at the top of the class. + * The original numeric usages are replaced with the new constant name to improve code readability and maintainability. + */ +public class ReplaceMagicNumbersWithConstants extends Recipe { + private static final String CUSTOM_MODIFIERS = "private static final"; + + @Override + public @NlsRewrite.DisplayName String getDisplayName() { + return "Replace magic numbers with constants"; + } + + @Override + public @NlsRewrite.Description String getDescription() { + return "Replaces magic number literals in method bodies with named constants to improve code readability and maintainability. " + + "Magic numbers are replaced by private static final constants declared at the top of the class, following Sonar's java:S109 rule. " + + "The recipe does not create constants for literals that are already assigned to fields or variables, nor for typical non-magic numbers (such as 0, 1, or -1). " + + "Currently, only numeric primitive literals are handled; string and character literals are unaffected. " + + "If a constant for a value already exists, or the constant name would conflict with an existing symbol, the recipe will skip that value."; + } + + @Override + public TreeVisitor getVisitor() { + return new JavaVisitor() { + @Override + public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { + J.ClassDeclaration cd = (J.ClassDeclaration) super.visitClassDeclaration(classDecl, ctx); + + List literals = new ArrayList<>(); + // COLLECT magic numbers not in variable initializers and not -1/0/1, ONLY numbers + new JavaVisitor() { + @Override + public J visitLiteral(J.Literal literal, ExecutionContext ctx2) { + try { + Object value = literal.getValue(); + if (!(value instanceof Number)) { + return literal; + } + Cursor cursor = getCursor(); + if (!(cursor.getParent().getParent().getValue() instanceof J.VariableDeclarations.NamedVariable) + && !isIgnoredMagicNumber(literal)) { + literals.add(literal); + } + return literal; + } catch (Exception e) { + System.out.println("Exception in visitLiteral 1: " + e.getMessage() + e.getStackTrace() + e); + return literal; + } + } + }.visit(classDecl, ctx); + + // Deduplicate by constant name! + List newFieldSources = new ArrayList<>(); + Set alreadyCreated = new HashSet<>(); + for (J.Literal literal : literals) { + String constantName = getStrValFromLiteral(literal); + + if (alreadyCreated.contains(constantName)) { + continue; + } + boolean existsInCode = cd.getBody().getStatements().stream() + .filter(J.VariableDeclarations.class::isInstance) + .map(J.VariableDeclarations.class::cast) + .flatMap(vars -> vars.getVariables().stream()) + .anyMatch(var -> var.getSimpleName().equals(constantName)); + if (!existsInCode) { + String modifiers = CUSTOM_MODIFIERS; + String typeName = getTypeName(literal); + String fieldSource = modifiers + " " + typeName + " " + constantName + " = " + literal.getValueSource() + ";"; + newFieldSources.add(fieldSource); + alreadyCreated.add(constantName); + } + } + if (newFieldSources.isEmpty()) { + return cd; + } + + String templateStr = String.join("\n", newFieldSources); + JavaTemplate template = JavaTemplate.builder(templateStr) + .contextSensitive() + .build(); + Cursor bodyCursor = new Cursor(getCursor(), cd.getBody()); + J.Block updatedBody = template.apply(bodyCursor, cd.getBody().getCoordinates().firstStatement()); + + return cd.withBody(updatedBody); + } + + @Override + public J visitLiteral(J.Literal literal, ExecutionContext ctx) { + try { + Object value = literal.getValue(); + if (!(value instanceof Number)) { + return literal; + } + Cursor cursor = getCursor(); + Cursor parent = cursor != null ? cursor.getParent() : null; + Cursor grandparent = parent != null ? parent.getParent() : null; + + // Do NOT replace in variable/field initializers or for ignored numbers + if ((grandparent != null && + grandparent.getValue() instanceof J.VariableDeclarations.NamedVariable) + || isIgnoredMagicNumber(literal)) { + return super.visitLiteral(literal, ctx); + } + String constantName = getStrValFromLiteral(literal); + if (constantName != null) { + JavaTemplate template = JavaTemplate.builder(constantName).build(); + return template.apply(getCursor(), literal.getCoordinates().replace()); + } + return super.visitLiteral(literal, ctx); + } catch (Exception e) { + return literal; + } + } + }; + } + private String printCursorPath(Cursor cursor) { + StringBuilder sb = new StringBuilder(); + while (cursor != null) { + Object val = cursor.getValue(); + sb.append(val == null ? "null" : val.getClass().getSimpleName()); + sb.append(" > "); + cursor = cursor.getParent(); + } + sb.append("ROOT"); + return sb.toString(); + } + @Override + public Set getTags() { + return Collections.singleton("RSPEC-109"); + } + + @Override + public Duration getEstimatedEffortPerOccurrence() { + return Duration.ofSeconds(10); + } + + private String getStrValFromLiteral(J.Literal literal) { + String type = getTypeName(literal).toUpperCase(); + String valueSource = literal.getValueSource(); + if (valueSource == null) return null; + if (valueSource.startsWith("-")) { + valueSource = "NEGATIVE_" + valueSource.substring(1); + } + return type + "_" + valueSource.replace(".", "_"); + } + + private String getTypeName(J.Literal literal) { + if (literal.getType() == null) return "Object"; + JavaType type = literal.getType(); + if (type instanceof JavaType.Primitive) { + return ((JavaType.Primitive) type).getKeyword(); + } + return type.toString(); // fallback + } + + private boolean isIgnoredMagicNumber(J.Literal literal) { + Object value = literal.getValue(); + if (value instanceof Number) { + double d = ((Number) value).doubleValue(); + // Only ignore -1, 0, 1 for all numeric types. + return d == -1.0 || d == 0.0 || d == 1.0; + } + return false; + } +} diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstantsTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstantsTest.java new file mode 100644 index 0000000000..0dc3ff9fac --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstantsTest.java @@ -0,0 +1,181 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * 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. + */ +package org.openrewrite.staticanalysis; +/* + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * 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. + */ +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.java.JavaParser; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; +import org.openrewrite.test.TypeValidation; + +import static org.openrewrite.java.Assertions.java; + +public class ReplaceMagicNumbersWithConstantsTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new ReplaceMagicNumbersWithConstants()); + } + + @DocumentExample + @Test + void assignMagicNumbersToConstantsTest() { + rewriteRun( + spec -> spec + .typeValidationOptions(TypeValidation.none()), + java( +""" +public class OrderProcessor { + private static final double myVariable = 99.99; + public double calculateShippingCost(double orderTotal) { + int localVar = 5; + orderTotal = localVar + 10; + if (orderTotal < 51.0) { + System.out.println("Order total is less than 51.0"); + } + if (orderTotal < 51.0) { + System.out.println("Order total is less than 51.0"); + } + if (orderTotal < 51.0) { + System.out.println("Order total is less than 51.0"); + } + if (orderTotal < 51.0) { + return 7.99; + } else { + return 0.0; + } + } +} +""", + """ +public class OrderProcessor { + private static final int INT_10 = 10; + private static final double DOUBLE_51_0 = 51.0; + private static final double DOUBLE_7_99 = 7.99; + private static final double myVariable = 99.99; + public double calculateShippingCost(double orderTotal) { + int localVar = 5; + orderTotal = localVar + INT_10; + if (orderTotal < DOUBLE_51_0) { + System.out.println("Order total is less than 51.0"); + } + if (orderTotal < DOUBLE_51_0) { + System.out.println("Order total is less than 51.0"); + } + if (orderTotal < DOUBLE_51_0) { + System.out.println("Order total is less than 51.0"); + } + if (orderTotal < DOUBLE_51_0) { + return DOUBLE_7_99; + } else { + return 0.0; + } + } +} + """ + ) + ); + } + + @DocumentExample + @Test + void assignMagicNumbersToConstantsBasicTest() { + rewriteRun( + spec -> spec + .typeValidationOptions(TypeValidation.none()), + java( + """ + public class OrderProcessor { + public double calculateShippingCost(double orderTotal) { + if (orderTotal < 51.0) { + return 7.99; + } + } + } + """, + """ +public class OrderProcessor { + private static final double DOUBLE_51_0 = 51.0; + private static final double DOUBLE_7_99 = 7.99; + public double calculateShippingCost(double orderTotal) { + if (orderTotal < DOUBLE_51_0) { + return DOUBLE_7_99; + } + } +} + """ + ) + ); + } + @DocumentExample + @Test + void assignMagicNumbersToConstantsM1_0_1_AreIgnoredTest() { + rewriteRun( + spec -> spec + .typeValidationOptions(TypeValidation.none()), + java( + """ + public class OrderProcessor { + private static final double myVariable = 99.99; + public double calculateShippingCost(double orderTotal) { + int localVar0 = 0; + orderTotal = localVar0 - 1; + orderTotal = localVar0 + 0; + orderTotal = localVar0 + 1; + if (orderTotal < 51.0) { + return 7.99; + } else { + return 0.0; + } + } + } + """, + """ +public class OrderProcessor { + private static final double DOUBLE_51_0 = 51.0; + private static final double DOUBLE_7_99 = 7.99; + private static final double myVariable = 99.99; + public double calculateShippingCost(double orderTotal) { + int localVar0 = 0; + orderTotal = localVar0 - 1; + orderTotal = localVar0 + 0; + orderTotal = localVar0 + 1; + if (orderTotal < DOUBLE_51_0) { + return DOUBLE_7_99; + } else { + return 0.0; + } + } +} + """ + ) + ); + } +}