From 3c6f9b0806e06af58b7779fc4a8d4e6b6281f7ab Mon Sep 17 00:00:00 2001 From: Sheng Yu Date: Tue, 22 Oct 2024 10:26:06 +0200 Subject: [PATCH 01/21] add: recipe for RSPEC-S1170, adding "static" to "public final" constants and fields --- ...difierToPublicFinalConstantsAndFields.java | 80 +++++++++++++++++++ .../rewrite/common-static-analysis.yml | 1 + ...erToPublicFinalConstantsAndFieldsTest.java | 69 ++++++++++++++++ 3 files changed, 150 insertions(+) create mode 100644 src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFieldsTest.java diff --git a/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java b/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java new file mode 100644 index 0000000000..657c07f508 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java @@ -0,0 +1,80 @@ +/* + * Copyright 2020 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * 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.ExecutionContext; +import org.openrewrite.Recipe; +import org.openrewrite.Tree; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.J.Modifier; +import org.openrewrite.java.tree.Space; +import org.openrewrite.marker.Markers; + +import java.time.Duration; +import java.util.Collections; +import java.util.Set; + +import static java.util.Collections.emptyList; + +public class AddStaticModifierToPublicFinalConstantsAndFields extends Recipe { + + @Override + public String getDisplayName() { + return "Add `static` to `public final` variables"; + } + + @Override + public String getDescription() { + return "Finds fields declared as `public final` and adds `static` modifier, meanwhile sort all the modifiers."; + } + + @Override + public Set getTags() { + return Collections.singleton("RSPEC-S1170"); + } + + @Override + public Duration getEstimatedEffortPerOccurrence() { + return Duration.ofMinutes(3); + } + + @Override + public JavaIsoVisitor getVisitor() { + return new JavaIsoVisitor() { + + @Override + public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, + ExecutionContext executionContext) { + + if (multiVariable.hasModifier(Modifier.Type.Public) && + multiVariable.hasModifier(Modifier.Type.Final) && + !multiVariable.hasModifier(Modifier.Type.Static)) { + J.VariableDeclarations v = super.visitVariableDeclarations(multiVariable, executionContext); + + multiVariable.getModifiers().add(new J.Modifier(Tree.randomId(), + Space.format(" "), Markers.EMPTY, " ", + Modifier.Type.Static, emptyList())); + + return v.withModifiers(ModifierOrder.sortModifiers(multiVariable.getModifiers())); + + } + return super.visitVariableDeclarations(multiVariable, executionContext); + } + }; + } +} \ No newline at end of file diff --git a/src/main/resources/META-INF/rewrite/common-static-analysis.yml b/src/main/resources/META-INF/rewrite/common-static-analysis.yml index 598e11499c..5b9bd794e9 100644 --- a/src/main/resources/META-INF/rewrite/common-static-analysis.yml +++ b/src/main/resources/META-INF/rewrite/common-static-analysis.yml @@ -96,3 +96,4 @@ recipeList: - org.openrewrite.kotlin.cleanup.EqualsMethodUsage - org.openrewrite.kotlin.cleanup.ImplicitParameterInLambda - org.openrewrite.kotlin.cleanup.ReplaceCharToIntWithCode + - org.openrewrite.staticanalysis.AddStaticModifierToFinalVariables diff --git a/src/test/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFieldsTest.java b/src/test/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFieldsTest.java new file mode 100644 index 0000000000..1c481c0d32 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFieldsTest.java @@ -0,0 +1,69 @@ +/* + * Copyright 2022 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * 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.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class AddStaticModifierToPublicFinalConstantsAndFieldsTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new AddStaticModifierToPublicFinalConstantsAndFields()); + } + + @Test + void doesNotModify() { + rewriteRun( + //language=java + java( + """ + class A { + public final static String s1; + public String s2; + static String s3; + } + """ + ) + ); + } + + @DocumentExample + @Test + void addStaticModifier() { + rewriteRun( + //language=java + java( + """ + class A { + final public String s1; + public final String s2; + } + """, + """ + class A { + public static final String s1; + public static final String s2; + } + """ + ) + ); + } +} From 8771f690d76bcec77cd7294f02445e624bad512e Mon Sep 17 00:00:00 2001 From: Sheng Yu Date: Wed, 23 Oct 2024 00:08:06 +0200 Subject: [PATCH 02/21] add: recipe and unit test for RSPEC-S6202 --- .../ReplaceClassIsInstanceWithInstanceof.java | 83 +++++++++++++++++++ ...laceClassIsInstanceWithInstanceofTest.java | 63 ++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java new file mode 100644 index 0000000000..63275e7430 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java @@ -0,0 +1,83 @@ +package org.openrewrite.staticanalysis; + +import java.time.Duration; +import java.util.Collections; +import java.util.Set; + +import org.openrewrite.ExecutionContext; +import org.openrewrite.NlsRewrite.Description; +import org.openrewrite.Recipe; +import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.J.FieldAccess; +import org.openrewrite.java.tree.J.Identifier; +import org.openrewrite.java.tree.JavaType; + +public class ReplaceClassIsInstanceWithInstanceof extends Recipe { + + @Override + public String getDisplayName() { + return "Replace `A.class.isInstance(a)` with `a instanceof A`"; + } + + @Override + public @Description + String getDescription() { + return "There should be no `A.class.isInstance(a)`, it should be replaced by `a instanceof A`."; + } + + @Override + public Set getTags() { + return Collections.singleton("RSPEC-S6202"); + } + + @Override + public Duration getEstimatedEffortPerOccurrence() { + return Duration.ofMinutes(3); + } + + @Override + public JavaVisitor getVisitor() { + //use JavaVisitor instead of JavaIsoVisitor because we changed the type of LST + return new JavaVisitor() { + + private final MethodMatcher matcher = new MethodMatcher("java.lang.Class isInstance(java.lang.Object)"); + private final JavaTemplate template = JavaTemplate.builder("#{any(org.openrewrite.java.tree.Expression)} instanceof #{}").build(); + + @Override + public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + + //make sure we find the right method and the left part is something like "SomeClass.class" + if (matcher.matches(method) && isObjectClass(method.getSelect())) { + //for code like "A.class.isInstance(a)", select is "String.class", name is "isInstance", argument is "a" + Identifier objectExpression = (Identifier) method.getArguments().get(0); + + FieldAccess fieldAccessPart = (FieldAccess) method.getSelect(); + String className = ((JavaType.Class) ((Identifier) fieldAccessPart.getTarget()).getType()).getClassName(); + + return maybeAutoFormat( + (J) method, + (J) template.apply(updateCursor(method), method.getCoordinates().replace(), objectExpression, className), + ctx + ); + + } + return (J.MethodInvocation) super.visitMethodInvocation(method, ctx); + } + + private boolean isObjectClass(Expression expression) { + if (expression instanceof J.FieldAccess) { + J.FieldAccess fieldAccess = (J.FieldAccess) expression; + if (fieldAccess.getTarget() instanceof Identifier) { + Identifier identifier = (Identifier) fieldAccess.getTarget(); + return identifier.getType() instanceof JavaType.Class; + } + } + return false; + } + }; + } +} \ No newline at end of file diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java new file mode 100644 index 0000000000..221252d4ff --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java @@ -0,0 +1,63 @@ +package org.openrewrite.staticanalysis; + +import org.junit.jupiter.api.Test; +import static org.openrewrite.java.Assertions.java; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +class ReplaceClassIsInstanceWithInstanceofTest implements RewriteTest { + + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new ReplaceClassIsInstanceWithInstanceof()); + //spec.recipe(new AvoidBoxedBooleanExpressions()); + } + + @Test + void doesNotMatchMethod() { + rewriteRun( + //language=java + java( + """ + class A { + boolean foo() { + String s = ""; + return s instanceof String; + } + } + """ + ) + ); + } + + @Test + void changeInstanceOf() { + rewriteRun( + //language=java + java( + """ + class A { + boolean foo() { + String s = ""; + String s2 = null; + boolean result = String.class.isInstance(s); + return result; + } + } + """, + """ + class A { + boolean foo() { + String s = ""; + String s2 = null; + boolean result = s instanceof String; + return result; + } + } + """ + ) + ); + } + +} From dc85402c67babf9d1efb3c51f3ad77a5e5041d7a Mon Sep 17 00:00:00 2001 From: Sheng Yu Date: Wed, 23 Oct 2024 00:08:33 +0200 Subject: [PATCH 03/21] update: add recipe to list --- src/main/resources/META-INF/rewrite/common-static-analysis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/resources/META-INF/rewrite/common-static-analysis.yml b/src/main/resources/META-INF/rewrite/common-static-analysis.yml index 5b9bd794e9..5526cfac13 100644 --- a/src/main/resources/META-INF/rewrite/common-static-analysis.yml +++ b/src/main/resources/META-INF/rewrite/common-static-analysis.yml @@ -97,3 +97,4 @@ recipeList: - org.openrewrite.kotlin.cleanup.ImplicitParameterInLambda - org.openrewrite.kotlin.cleanup.ReplaceCharToIntWithCode - org.openrewrite.staticanalysis.AddStaticModifierToFinalVariables + - org.openrewrite.staticanalysis.ReplaceClassIsInstanceWithInstanceof From 554ce96632c8d2993a6e306fa3b8d046e39e6bab Mon Sep 17 00:00:00 2001 From: Sheng Yu Date: Wed, 23 Oct 2024 00:20:54 +0200 Subject: [PATCH 04/21] fix: correct the name of the recipe --- src/main/resources/META-INF/rewrite/common-static-analysis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/META-INF/rewrite/common-static-analysis.yml b/src/main/resources/META-INF/rewrite/common-static-analysis.yml index 5526cfac13..903cf861eb 100644 --- a/src/main/resources/META-INF/rewrite/common-static-analysis.yml +++ b/src/main/resources/META-INF/rewrite/common-static-analysis.yml @@ -96,5 +96,5 @@ recipeList: - org.openrewrite.kotlin.cleanup.EqualsMethodUsage - org.openrewrite.kotlin.cleanup.ImplicitParameterInLambda - org.openrewrite.kotlin.cleanup.ReplaceCharToIntWithCode - - org.openrewrite.staticanalysis.AddStaticModifierToFinalVariables + - org.openrewrite.staticanalysis.AddStaticModifierToPublicFinalConstantsAndFields - org.openrewrite.staticanalysis.ReplaceClassIsInstanceWithInstanceof From 6cc2947b844c0a0be5d1cbb6bd7659fcded5b2b7 Mon Sep 17 00:00:00 2001 From: Sheng Yu Date: Wed, 23 Oct 2024 00:33:25 +0200 Subject: [PATCH 05/21] update: for more cases in unit test --- .../ReplaceClassIsInstanceWithInstanceofTest.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java index 221252d4ff..80b69334e2 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java @@ -38,21 +38,19 @@ void changeInstanceOf() { java( """ class A { - boolean foo() { + void foo() { String s = ""; - String s2 = null; boolean result = String.class.isInstance(s); - return result; + result = Integer.class.isInstance(s); } } """, """ class A { - boolean foo() { + void foo() { String s = ""; - String s2 = null; boolean result = s instanceof String; - return result; + result = s instanceof Integer; } } """ From 41400865c73bb00845ed0c9c1ed5beed77aeffa7 Mon Sep 17 00:00:00 2001 From: Sheng Yu Date: Thu, 24 Oct 2024 22:22:09 +0200 Subject: [PATCH 06/21] update: correct copyright messages --- ...icModifierToPublicFinalConstantsAndFields.java | 3 +-- .../ReplaceClassIsInstanceWithInstanceof.java | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java b/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java index 657c07f508..2895a215cf 100644 --- a/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java +++ b/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 the original author or authors. + * Copyright 2024 the original author or authors. *

* Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.openrewrite.staticanalysis; import org.openrewrite.ExecutionContext; diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java index 63275e7430..d4d79ba32a 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java @@ -1,3 +1,18 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * 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 java.time.Duration; From 7d439034926f7ddf018bbad0913e594dccce5222 Mon Sep 17 00:00:00 2001 From: Sheng Yu Date: Thu, 24 Oct 2024 22:29:03 +0200 Subject: [PATCH 07/21] refactor: format code --- .../ReplaceClassIsInstanceWithInstanceof.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java index d4d79ba32a..3467d509f5 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java @@ -29,6 +29,7 @@ import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.J.FieldAccess; import org.openrewrite.java.tree.J.Identifier; +import org.openrewrite.java.tree.J.MethodInvocation; import org.openrewrite.java.tree.JavaType; public class ReplaceClassIsInstanceWithInstanceof extends Recipe { @@ -60,10 +61,11 @@ public JavaVisitor getVisitor() { return new JavaVisitor() { private final MethodMatcher matcher = new MethodMatcher("java.lang.Class isInstance(java.lang.Object)"); - private final JavaTemplate template = JavaTemplate.builder("#{any(org.openrewrite.java.tree.Expression)} instanceof #{}").build(); + private final JavaTemplate template = JavaTemplate.builder( + "#{any(org.openrewrite.java.tree.Expression)} instanceof #{}").build(); @Override - public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + public J visitMethodInvocation(MethodInvocation method, ExecutionContext ctx) { //make sure we find the right method and the left part is something like "SomeClass.class" if (matcher.matches(method) && isObjectClass(method.getSelect())) { @@ -73,14 +75,13 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) FieldAccess fieldAccessPart = (FieldAccess) method.getSelect(); String className = ((JavaType.Class) ((Identifier) fieldAccessPart.getTarget()).getType()).getClassName(); - return maybeAutoFormat( - (J) method, - (J) template.apply(updateCursor(method), method.getCoordinates().replace(), objectExpression, className), - ctx - ); + //upcast to type J, so J.MethodInfocation can be replaced by J.InstanceOf + return maybeAutoFormat((J)method, + (J)template.apply(updateCursor(method), method.getCoordinates().replace(), objectExpression, className), + ctx); } - return (J.MethodInvocation) super.visitMethodInvocation(method, ctx); + return (MethodInvocation) super.visitMethodInvocation(method, ctx); } private boolean isObjectClass(Expression expression) { From 40c508eb5bc36ab4e06efbf7eb36e6cf77e39c6e Mon Sep 17 00:00:00 2001 From: Yurii Date: Fri, 25 Oct 2024 13:49:59 +0200 Subject: [PATCH 08/21] Update src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- ...laceClassIsInstanceWithInstanceofTest.java | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java index 80b69334e2..9ae58ff9b7 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java @@ -1,4 +1,21 @@ -package org.openrewrite.staticanalysis; +/* + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * 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.openrewrite.DocumentExample; +import static org.openrewrite.java.Assertions.java; + import org.junit.jupiter.api.Test; import static org.openrewrite.java.Assertions.java; From 137fc0cdbf5fe03e7fd3af91fa1d904c05fbf9d9 Mon Sep 17 00:00:00 2001 From: Yurii Date: Fri, 25 Oct 2024 13:50:06 +0200 Subject: [PATCH 09/21] Update src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../ReplaceClassIsInstanceWithInstanceofTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java index 9ae58ff9b7..6ba57a94cd 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java @@ -63,12 +63,12 @@ void foo() { } """, """ - class A { - void foo() { - String s = ""; - boolean result = s instanceof String; - result = s instanceof Integer; - } + class A { + void foo() { + String s = ""; + boolean result = s instanceof String; + result = s instanceof Integer; + } } """ ) From 8f8008d04568c24d1aec1cd187532c1fa39f5535 Mon Sep 17 00:00:00 2001 From: Yurii Date: Fri, 25 Oct 2024 13:50:15 +0200 Subject: [PATCH 10/21] Update src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../ReplaceClassIsInstanceWithInstanceofTest.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java index 6ba57a94cd..75d12d88ad 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java @@ -37,11 +37,12 @@ void doesNotMatchMethod() { //language=java java( """ - class A { - boolean foo() { - String s = ""; - return s instanceof String; - } + class A { + boolean foo() { + String s = ""; + return s instanceof String; + } + @DocumentExample } """ ) From 3826d8141e9bbc0de7669e7969c5a4fa4e68336b Mon Sep 17 00:00:00 2001 From: Yurii Date: Fri, 25 Oct 2024 13:50:37 +0200 Subject: [PATCH 11/21] Update src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../staticanalysis/ReplaceClassIsInstanceWithInstanceof.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java index 3467d509f5..62e4e13bc1 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java @@ -40,7 +40,7 @@ public String getDisplayName() { } @Override - public @Description + public String getDescription() { return "There should be no `A.class.isInstance(a)`, it should be replaced by `a instanceof A`."; } From 5c6fc8b8e957df1d47329c4096f9430d3b21cff6 Mon Sep 17 00:00:00 2001 From: Yurii Date: Fri, 25 Oct 2024 13:50:48 +0200 Subject: [PATCH 12/21] Update src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../staticanalysis/ReplaceClassIsInstanceWithInstanceof.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java index 62e4e13bc1..42a85b5ed2 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java @@ -32,6 +32,10 @@ import org.openrewrite.java.tree.J.MethodInvocation; import org.openrewrite.java.tree.JavaType; +import java.time.Duration; +import java.util.Collections; +import java.util.Set; + public class ReplaceClassIsInstanceWithInstanceof extends Recipe { @Override From b4a1505f07adf4247a5c48be457ed10fb1c9783c Mon Sep 17 00:00:00 2001 From: Yurii Date: Fri, 25 Oct 2024 13:50:55 +0200 Subject: [PATCH 13/21] Update src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../staticanalysis/ReplaceClassIsInstanceWithInstanceof.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java index 42a85b5ed2..0ef834efcd 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java @@ -15,12 +15,7 @@ */ package org.openrewrite.staticanalysis; -import java.time.Duration; -import java.util.Collections; -import java.util.Set; - import org.openrewrite.ExecutionContext; -import org.openrewrite.NlsRewrite.Description; import org.openrewrite.Recipe; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; From 9160bebbd60ed36253c73d46836256507be6e727 Mon Sep 17 00:00:00 2001 From: Yurii Date: Fri, 25 Oct 2024 13:51:38 +0200 Subject: [PATCH 14/21] Update src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../AddStaticModifierToPublicFinalConstantsAndFields.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java b/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java index 2895a215cf..79cbe5b58f 100644 --- a/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java +++ b/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java @@ -72,8 +72,8 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m return v.withModifiers(ModifierOrder.sortModifiers(multiVariable.getModifiers())); } - return super.visitVariableDeclarations(multiVariable, executionContext); - } + return super.visitVariableDeclarations(multiVariable, ctx); +} }; } } \ No newline at end of file From e8e062c5cf0145d8f1d7e1886cb458a2875c9833 Mon Sep 17 00:00:00 2001 From: Yurii Date: Fri, 25 Oct 2024 13:51:48 +0200 Subject: [PATCH 15/21] Update src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../AddStaticModifierToPublicFinalConstantsAndFields.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java b/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java index 79cbe5b58f..f36194b998 100644 --- a/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java +++ b/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java @@ -58,8 +58,8 @@ public JavaIsoVisitor getVisitor() { @Override public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, - ExecutionContext executionContext) { - + ExecutionContext ctx) { + J.VariableDeclarations v = super.visitVariableDeclarations(multiVariable, ctx); if (multiVariable.hasModifier(Modifier.Type.Public) && multiVariable.hasModifier(Modifier.Type.Final) && !multiVariable.hasModifier(Modifier.Type.Static)) { From f1990fa72f356712adbe887954262acacf10a174 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 25 Oct 2024 17:51:24 +0200 Subject: [PATCH 16/21] Test additional cases pointed out in review --- ...difierToPublicFinalConstantsAndFields.java | 27 ++++++-- ...erToPublicFinalConstantsAndFieldsTest.java | 61 +++++++++++++++++++ 2 files changed, 82 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java b/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java index f36194b998..8bff245b4a 100644 --- a/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java +++ b/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java @@ -56,14 +56,29 @@ public Duration getEstimatedEffortPerOccurrence() { public JavaIsoVisitor getVisitor() { return new JavaIsoVisitor() { + @Override + public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { + J.ClassDeclaration enclosing = getCursor().getParent().firstEnclosing(J.ClassDeclaration.class); + if (enclosing == null || classDecl.hasModifier(Modifier.Type.Static)) { + return super.visitClassDeclaration(classDecl, ctx); + } + // Ignore non-static inner classes as they can't have static fields + return classDecl; + } + + @Override + public J.NewClass visitNewClass(J.NewClass newClass, ExecutionContext ctx) { + // Ignore anonymous inner classes as they can't have static fields + return newClass; + } + @Override public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { - J.VariableDeclarations v = super.visitVariableDeclarations(multiVariable, ctx); + J.VariableDeclarations v = super.visitVariableDeclarations(multiVariable, ctx); if (multiVariable.hasModifier(Modifier.Type.Public) && - multiVariable.hasModifier(Modifier.Type.Final) && - !multiVariable.hasModifier(Modifier.Type.Static)) { - J.VariableDeclarations v = super.visitVariableDeclarations(multiVariable, executionContext); + multiVariable.hasModifier(Modifier.Type.Final) && + !multiVariable.hasModifier(Modifier.Type.Static)) { multiVariable.getModifiers().add(new J.Modifier(Tree.randomId(), Space.format(" "), Markers.EMPTY, " ", @@ -73,7 +88,7 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m } return super.visitVariableDeclarations(multiVariable, ctx); -} + } }; } -} \ No newline at end of file +} diff --git a/src/test/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFieldsTest.java b/src/test/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFieldsTest.java index 1c481c0d32..9c96359009 100644 --- a/src/test/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFieldsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFieldsTest.java @@ -66,4 +66,65 @@ class A { ) ); } + + @Test + void staticInnerClass() { + rewriteRun( + //language=java + java( + """ + class Outer { + static class Inner { + final public String s1; + public final String s2; + } + } + """, + """ + class Outer { + static class Inner { + public static final String s1; + public static final String s2; + } + } + """ + ) + ); + } + + @Test + void nonStaticInnerClass() { + rewriteRun( + //language=java + java( + """ + class Outer { + class Inner { + final public String s1; + public final String s2; + } + } + """ + ) + ); + } + + @Test + void anonymousInnerClass() { + rewriteRun( + //language=java + java( + """ + class Outer { + void foo() { + new Runnable() { + final public String s1; + public final String s2; + }; + } + } + """ + ) + ); + } } From 5b0b9b213eae19172e90938efe7e1f43ba729661 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 25 Oct 2024 17:51:32 +0200 Subject: [PATCH 17/21] Format tests --- ...laceClassIsInstanceWithInstanceofTest.java | 46 +++++++++---------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java index 75d12d88ad..bb86fcdca3 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java @@ -13,38 +13,36 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import org.openrewrite.DocumentExample; -import static org.openrewrite.java.Assertions.java; - +package org.openrewrite.staticanalysis; import org.junit.jupiter.api.Test; -import static org.openrewrite.java.Assertions.java; +import org.openrewrite.DocumentExample; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import static org.openrewrite.java.Assertions.java; + class ReplaceClassIsInstanceWithInstanceofTest implements RewriteTest { - @Override public void defaults(RecipeSpec spec) { spec.recipe(new ReplaceClassIsInstanceWithInstanceof()); - //spec.recipe(new AvoidBoxedBooleanExpressions()); } @Test + @DocumentExample void doesNotMatchMethod() { rewriteRun( //language=java java( """ - class A { - boolean foo() { - String s = ""; - return s instanceof String; - } - @DocumentExample + class A { + boolean foo() { + String s = ""; + return s instanceof String; + } } - """ + """ ) ); } @@ -57,21 +55,21 @@ void changeInstanceOf() { """ class A { void foo() { - String s = ""; - boolean result = String.class.isInstance(s); - result = Integer.class.isInstance(s); - } + String s = ""; + boolean result = String.class.isInstance(s); + result = Integer.class.isInstance(s); + } } """, """ - class A { - void foo() { - String s = ""; - boolean result = s instanceof String; - result = s instanceof Integer; - } + class A { + void foo() { + String s = ""; + boolean result = s instanceof String; + result = s instanceof Integer; + } } - """ + """ ) ); } From 1bc5dceb5235aefbb69f00d763207c86260e261c Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 25 Oct 2024 17:59:44 +0200 Subject: [PATCH 18/21] Slight polish to ReplaceClassIsInstanceWithInstanceof --- .../ReplaceClassIsInstanceWithInstanceof.java | 30 ++++------ ...laceClassIsInstanceWithInstanceofTest.java | 60 +++++++++++++++---- 2 files changed, 61 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java index 0ef834efcd..0e605844ad 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java @@ -15,6 +15,7 @@ */ package org.openrewrite.staticanalysis; +import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; import org.openrewrite.java.JavaTemplate; @@ -39,8 +40,7 @@ public String getDisplayName() { } @Override - public - String getDescription() { + public String getDescription() { return "There should be no `A.class.isInstance(a)`, it should be replaced by `a instanceof A`."; } @@ -56,34 +56,28 @@ public Duration getEstimatedEffortPerOccurrence() { @Override public JavaVisitor getVisitor() { - //use JavaVisitor instead of JavaIsoVisitor because we changed the type of LST + // use JavaVisitor instead of JavaIsoVisitor because we changed the type of LST return new JavaVisitor() { private final MethodMatcher matcher = new MethodMatcher("java.lang.Class isInstance(java.lang.Object)"); - private final JavaTemplate template = JavaTemplate.builder( - "#{any(org.openrewrite.java.tree.Expression)} instanceof #{}").build(); @Override public J visitMethodInvocation(MethodInvocation method, ExecutionContext ctx) { - - //make sure we find the right method and the left part is something like "SomeClass.class" + // make sure we find the right method and the left part is something like "SomeClass.class" if (matcher.matches(method) && isObjectClass(method.getSelect())) { - //for code like "A.class.isInstance(a)", select is "String.class", name is "isInstance", argument is "a" + // for code like "A.class.isInstance(a)", select is "String.class", name is "isInstance", argument is "a" Identifier objectExpression = (Identifier) method.getArguments().get(0); - FieldAccess fieldAccessPart = (FieldAccess) method.getSelect(); - String className = ((JavaType.Class) ((Identifier) fieldAccessPart.getTarget()).getType()).getClassName(); - - //upcast to type J, so J.MethodInfocation can be replaced by J.InstanceOf - return maybeAutoFormat((J)method, - (J)template.apply(updateCursor(method), method.getCoordinates().replace(), objectExpression, className), - ctx); - + String className = ((JavaType.Class) fieldAccessPart.getTarget().getType()).getClassName(); + // upcast to type J, so J.MethodInvocation can be replaced by J.InstanceOf + J.InstanceOf instanceOf = JavaTemplate.apply("#{any(org.openrewrite.java.tree.Expression)} instanceof #{}", + getCursor(), method.getCoordinates().replace(), objectExpression, className); + return maybeAutoFormat(method, instanceOf, ctx); } - return (MethodInvocation) super.visitMethodInvocation(method, ctx); + return super.visitMethodInvocation(method, ctx); } - private boolean isObjectClass(Expression expression) { + private boolean isObjectClass(@Nullable Expression expression) { if (expression instanceof J.FieldAccess) { J.FieldAccess fieldAccess = (J.FieldAccess) expression; if (fieldAccess.getTarget() instanceof Identifier) { diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java index bb86fcdca3..2905c68d0a 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java @@ -21,6 +21,7 @@ import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.java.Assertions.javaVersion; class ReplaceClassIsInstanceWithInstanceofTest implements RewriteTest { @@ -31,7 +32,34 @@ public void defaults(RecipeSpec spec) { @Test @DocumentExample - void doesNotMatchMethod() { + void changeInstanceOf() { + rewriteRun( + //language=java + java( + """ + class A { + void foo() { + String s = ""; + boolean result = String.class.isInstance(s); + result = Integer.class.isInstance(s); + } + } + """, + """ + class A { + void foo() { + String s = ""; + boolean result = s instanceof String; + result = s instanceof Integer; + } + } + """ + ) + ); + } + + @Test + void doNotChangeWhenAlreadyInstanceOf() { rewriteRun( //language=java java( @@ -48,28 +76,38 @@ boolean foo() { } @Test - void changeInstanceOf() { + void doNotChangeWhenVariable() { rewriteRun( //language=java java( """ class A { - void foo() { + void foo(Class clazz) { String s = ""; - boolean result = String.class.isInstance(s); - result = Integer.class.isInstance(s); + boolean result = clazz.isInstance(s); } } - """, + """ + ) + ); + } + + @Test + void doNotChangeInstanceOfWithVariable() { + rewriteRun( + //language=java + java( """ class A { - void foo() { - String s = ""; - boolean result = s instanceof String; - result = s instanceof Integer; + String foo(Object obj) { + if (obj instanceof String s) { + return s; + } + return null; } } - """ + """, + spec -> spec.markers(javaVersion(17)) ) ); } From 9fb07321bfa4a0140752154939717374792cc80a Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 25 Oct 2024 18:00:05 +0200 Subject: [PATCH 19/21] Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../staticanalysis/ReplaceClassIsInstanceWithInstanceof.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java index 0e605844ad..ef9d4a6f5f 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java @@ -89,4 +89,4 @@ private boolean isObjectClass(@Nullable Expression expression) { } }; } -} \ No newline at end of file +} From 8bdf819ac8a68a2d2b8245f053bd736c093ffa9c Mon Sep 17 00:00:00 2001 From: Sheng Yu Date: Mon, 28 Oct 2024 08:35:12 +0100 Subject: [PATCH 20/21] update: Recipe for RSPEC-S1170 is removed because it needs further consideration, so we will do it in another PR --- ...difierToPublicFinalConstantsAndFields.java | 94 ------------- .../rewrite/common-static-analysis.yml | 1 - ...erToPublicFinalConstantsAndFieldsTest.java | 130 ------------------ 3 files changed, 225 deletions(-) delete mode 100644 src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java delete mode 100644 src/test/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFieldsTest.java diff --git a/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java b/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java deleted file mode 100644 index 8bff245b4a..0000000000 --- a/src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java +++ /dev/null @@ -1,94 +0,0 @@ -/* - * Copyright 2024 the original author or authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - *

- * https://www.apache.org/licenses/LICENSE-2.0 - *

- * 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.ExecutionContext; -import org.openrewrite.Recipe; -import org.openrewrite.Tree; -import org.openrewrite.java.JavaIsoVisitor; -import org.openrewrite.java.tree.J; -import org.openrewrite.java.tree.J.Modifier; -import org.openrewrite.java.tree.Space; -import org.openrewrite.marker.Markers; - -import java.time.Duration; -import java.util.Collections; -import java.util.Set; - -import static java.util.Collections.emptyList; - -public class AddStaticModifierToPublicFinalConstantsAndFields extends Recipe { - - @Override - public String getDisplayName() { - return "Add `static` to `public final` variables"; - } - - @Override - public String getDescription() { - return "Finds fields declared as `public final` and adds `static` modifier, meanwhile sort all the modifiers."; - } - - @Override - public Set getTags() { - return Collections.singleton("RSPEC-S1170"); - } - - @Override - public Duration getEstimatedEffortPerOccurrence() { - return Duration.ofMinutes(3); - } - - @Override - public JavaIsoVisitor getVisitor() { - return new JavaIsoVisitor() { - - @Override - public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { - J.ClassDeclaration enclosing = getCursor().getParent().firstEnclosing(J.ClassDeclaration.class); - if (enclosing == null || classDecl.hasModifier(Modifier.Type.Static)) { - return super.visitClassDeclaration(classDecl, ctx); - } - // Ignore non-static inner classes as they can't have static fields - return classDecl; - } - - @Override - public J.NewClass visitNewClass(J.NewClass newClass, ExecutionContext ctx) { - // Ignore anonymous inner classes as they can't have static fields - return newClass; - } - - @Override - public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, - ExecutionContext ctx) { - J.VariableDeclarations v = super.visitVariableDeclarations(multiVariable, ctx); - if (multiVariable.hasModifier(Modifier.Type.Public) && - multiVariable.hasModifier(Modifier.Type.Final) && - !multiVariable.hasModifier(Modifier.Type.Static)) { - - multiVariable.getModifiers().add(new J.Modifier(Tree.randomId(), - Space.format(" "), Markers.EMPTY, " ", - Modifier.Type.Static, emptyList())); - - return v.withModifiers(ModifierOrder.sortModifiers(multiVariable.getModifiers())); - - } - return super.visitVariableDeclarations(multiVariable, ctx); - } - }; - } -} diff --git a/src/main/resources/META-INF/rewrite/common-static-analysis.yml b/src/main/resources/META-INF/rewrite/common-static-analysis.yml index 903cf861eb..7316f3b30a 100644 --- a/src/main/resources/META-INF/rewrite/common-static-analysis.yml +++ b/src/main/resources/META-INF/rewrite/common-static-analysis.yml @@ -96,5 +96,4 @@ recipeList: - org.openrewrite.kotlin.cleanup.EqualsMethodUsage - org.openrewrite.kotlin.cleanup.ImplicitParameterInLambda - org.openrewrite.kotlin.cleanup.ReplaceCharToIntWithCode - - org.openrewrite.staticanalysis.AddStaticModifierToPublicFinalConstantsAndFields - org.openrewrite.staticanalysis.ReplaceClassIsInstanceWithInstanceof diff --git a/src/test/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFieldsTest.java b/src/test/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFieldsTest.java deleted file mode 100644 index 9c96359009..0000000000 --- a/src/test/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFieldsTest.java +++ /dev/null @@ -1,130 +0,0 @@ -/* - * Copyright 2022 the original author or authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - *

- * https://www.apache.org/licenses/LICENSE-2.0 - *

- * 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.junit.jupiter.api.Test; -import org.openrewrite.DocumentExample; -import org.openrewrite.test.RecipeSpec; -import org.openrewrite.test.RewriteTest; - -import static org.openrewrite.java.Assertions.java; - -class AddStaticModifierToPublicFinalConstantsAndFieldsTest implements RewriteTest { - - @Override - public void defaults(RecipeSpec spec) { - spec.recipe(new AddStaticModifierToPublicFinalConstantsAndFields()); - } - - @Test - void doesNotModify() { - rewriteRun( - //language=java - java( - """ - class A { - public final static String s1; - public String s2; - static String s3; - } - """ - ) - ); - } - - @DocumentExample - @Test - void addStaticModifier() { - rewriteRun( - //language=java - java( - """ - class A { - final public String s1; - public final String s2; - } - """, - """ - class A { - public static final String s1; - public static final String s2; - } - """ - ) - ); - } - - @Test - void staticInnerClass() { - rewriteRun( - //language=java - java( - """ - class Outer { - static class Inner { - final public String s1; - public final String s2; - } - } - """, - """ - class Outer { - static class Inner { - public static final String s1; - public static final String s2; - } - } - """ - ) - ); - } - - @Test - void nonStaticInnerClass() { - rewriteRun( - //language=java - java( - """ - class Outer { - class Inner { - final public String s1; - public final String s2; - } - } - """ - ) - ); - } - - @Test - void anonymousInnerClass() { - rewriteRun( - //language=java - java( - """ - class Outer { - void foo() { - new Runnable() { - final public String s1; - public final String s2; - }; - } - } - """ - ) - ); - } -} From 8d59c7772efa4e1a47dcd43f7dcbc685aa1248e8 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 28 Oct 2024 13:08:11 +0100 Subject: [PATCH 21/21] Minor polish --- .../ReplaceClassIsInstanceWithInstanceof.java | 6 ------ .../resources/META-INF/rewrite/common-static-analysis.yml | 2 +- .../ReplaceClassIsInstanceWithInstanceofTest.java | 1 + 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java index ef9d4a6f5f..4ed0d4bca8 100644 --- a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java @@ -28,7 +28,6 @@ import org.openrewrite.java.tree.J.MethodInvocation; import org.openrewrite.java.tree.JavaType; -import java.time.Duration; import java.util.Collections; import java.util.Set; @@ -49,11 +48,6 @@ public Set getTags() { return Collections.singleton("RSPEC-S6202"); } - @Override - public Duration getEstimatedEffortPerOccurrence() { - return Duration.ofMinutes(3); - } - @Override public JavaVisitor getVisitor() { // use JavaVisitor instead of JavaIsoVisitor because we changed the type of LST diff --git a/src/main/resources/META-INF/rewrite/common-static-analysis.yml b/src/main/resources/META-INF/rewrite/common-static-analysis.yml index 7316f3b30a..bb6547e9ef 100644 --- a/src/main/resources/META-INF/rewrite/common-static-analysis.yml +++ b/src/main/resources/META-INF/rewrite/common-static-analysis.yml @@ -72,6 +72,7 @@ recipeList: - org.openrewrite.staticanalysis.RenameLocalVariablesToCamelCase - org.openrewrite.staticanalysis.RenameMethodsNamedHashcodeEqualOrToString - org.openrewrite.staticanalysis.RenamePrivateFieldsToCamelCase + - org.openrewrite.staticanalysis.ReplaceClassIsInstanceWithInstanceof - org.openrewrite.staticanalysis.ReplaceLambdaWithMethodReference - org.openrewrite.staticanalysis.ReplaceStringBuilderWithString - org.openrewrite.staticanalysis.SimplifyBooleanExpression @@ -96,4 +97,3 @@ recipeList: - org.openrewrite.kotlin.cleanup.EqualsMethodUsage - org.openrewrite.kotlin.cleanup.ImplicitParameterInLambda - org.openrewrite.kotlin.cleanup.ReplaceCharToIntWithCode - - org.openrewrite.staticanalysis.ReplaceClassIsInstanceWithInstanceof diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java index 2905c68d0a..d96312d9c4 100644 --- a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java @@ -23,6 +23,7 @@ import static org.openrewrite.java.Assertions.java; import static org.openrewrite.java.Assertions.javaVersion; +@SuppressWarnings({"RedundantClassCall", "ConstantValue", "UnusedAssignment"}) class ReplaceClassIsInstanceWithInstanceofTest implements RewriteTest { @Override