-
Notifications
You must be signed in to change notification settings - Fork 90
Feature Request: Recipe to Extract Magic Numbers to Type-and-Value-Named Constants per Class #769
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.
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
base: main
Are you sure you want to change the base?
Conversation
Introduce `ReplaceMagicNumbersWithConstants` recipe implementing Sonar rule java:S109. This recipe extracts numeric literals from method bodies into private static final constants to improve code readability and maintainability. Also add `ReplaceMagicNumbersWithConstantsTest` with test cases verifying: - constants are generated for numeric literals - literals already assigned to variables or fields are ignored - replacements preserve correctness and type consistency
-1, 0, and 1 are not considered magic numbers.
-1, 0, and 1 are not considered magic numbers.
src/main/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstants.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstants.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstants.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstants.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstants.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstants.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstants.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstantsTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstantsTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstantsTest.java
Show resolved
Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
src/main/java/org/openrewrite/staticanalysis/ReplaceMagicNumbersWithConstants.java
Outdated
Show resolved
Hide resolved
| import org.openrewrite.*; | ||
| import org.openrewrite.java.JavaTemplate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 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; |
| 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. " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 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."; |
| if (!(cursor.getParent().getParent().getValue() instanceof J.VariableDeclarations.NamedVariable) | ||
| && !isIgnoredMagicNumber(literal)) { | ||
| literals.add(literal); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 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)) { |
| grandparent.getValue() instanceof J.VariableDeclarations.NamedVariable) | ||
| || isIgnoredMagicNumber(literal)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| grandparent.getValue() instanceof J.VariableDeclarations.NamedVariable) | |
| || isIgnoredMagicNumber(literal)) { | |
| grandparent.getValue() instanceof J.VariableDeclarations.NamedVariable) || | |
| isIgnoredMagicNumber(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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 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"); |
| private String getStrValFromLiteral(J.Literal literal) { | ||
| String type = getTypeName(literal).toUpperCase(); | ||
| String valueSource = literal.getValueSource(); | ||
| if (valueSource == null) return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (valueSource == null) return null; | |
| if (valueSource == null) { | |
| return null; | |
| } |
| } | ||
|
|
||
| private String getTypeName(J.Literal literal) { | ||
| if (literal.getType() == null) return "Object"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (literal.getType() == null) return "Object"; | |
| if (literal.getType() == null) { | |
| return "Object"; | |
| } |
| import org.openrewrite.java.JavaParser; | ||
| import org.openrewrite.test.RecipeSpec; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| import org.openrewrite.java.JavaParser; | |
| import org.openrewrite.test.RecipeSpec; | |
| class ReplaceMagicNumbersWithConstantsTest implements RewriteTest { |
| @DocumentExample | ||
| @Test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @DocumentExample | |
| @Test | |
| @Test |
| } | ||
| @DocumentExample |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | |
| @DocumentExample | |
| } |
Introduce
ReplaceMagicNumbersWithConstantsrecipe implementing Sonar rule java:S109. This recipe extracts numeric literals from method bodies into private static final constants to improve code readability and maintainability.Also add
ReplaceMagicNumbersWithConstantsTestwith test cases verifying:What's changed?
Added new recipe: ReplaceMagicNumbersWithConstants
Added new test class: ReplaceMagicNumbersWithConstantsTest
What's your motivation?
To enhance code quality and readability by enforcing Sonar rule java:S109, reducing the use of magic numbers in Java code.
Anything in particular you'd like reviewers to focus on?
Verify correct handling of numeric literals and naming of generated constants
Confirm no false positives for literals already assigned to variables or fields
Ensure the recipe integrates cleanly with existing static analysis infrastructure
Have you considered any alternatives or workarounds?
Manual refactoring or partial Sonar-based analysis, but these approaches lack the automation and reproducibility provided by this recipe.