Skip to content
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
/*
* Copyright 2024 the original author or authors.
* <p>
* 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
* <p>
* https://docs.moderne.io/licensing/moderne-source-available-license
* <p>
* 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;
Comment on lines +18 to +19
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import org.openrewrite.*;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.Cursor;
import org.openrewrite.ExecutionContext;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import static java.util.Collections.singleton;

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. "
Comment on lines +37 to +43
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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. "
public String getDisplayName() {
public 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.";

+ "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<?, ExecutionContext> getVisitor() {
return new JavaVisitor<ExecutionContext>() {
@Override
public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) {
J.ClassDeclaration cd = (J.ClassDeclaration) super.visitClassDeclaration(classDecl, ctx);

List<J.Literal> literals = new ArrayList<>();
// COLLECT magic numbers not in variable initializers and not -1/0/1, ONLY numbers
new JavaVisitor<ExecutionContext>() {
@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);
Comment on lines +68 to +70
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!(cursor.getParent().getParent().getValue() instanceof J.VariableDeclarations.NamedVariable)
&& !isIgnoredMagicNumber(literal)) {
literals.add(literal);
if (!(cursor.getParent().getParent().getValue() instanceof J.VariableDeclarations.NamedVariable) &&
!isIgnoredMagicNumber(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<String> newFieldSources = new ArrayList<>();
Set<String> 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)) {
Comment on lines +129 to +130
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
grandparent.getValue() instanceof J.VariableDeclarations.NamedVariable)
|| isIgnoredMagicNumber(literal)) {
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
Comment on lines +145 to +156
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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
return singleton("RSPEC-109");

public Set<String> 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (valueSource == null) return null;
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";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (literal.getType() == null) return "Object";
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;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
/*
* Copyright 2025 the original author or authors.
* <p>
* 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
* <p>
* https://docs.moderne.io/licensing/moderne-source-available-license
* <p>
* 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.
* <p>
* 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
* <p>
* https://docs.moderne.io/licensing/moderne-source-available-license
* <p>
* 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;
Comment on lines +34 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import org.openrewrite.java.JavaParser;
import org.openrewrite.test.RecipeSpec;
class ReplaceMagicNumbersWithConstantsTest implements RewriteTest {

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
Comment on lines +107 to +108
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@DocumentExample
@Test
@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
Comment on lines +136 to +137
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
@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;
}
}
}
"""
)
);
}
}