-
Notifications
You must be signed in to change notification settings - Fork 17
feature: Implemented ConsecutiveLiteralAppends #915
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: master
Are you sure you want to change the base?
Changes from all commits
310314f
776c0ab
8a1170c
7f21c55
96ec836
b94d49e
96b9db7
ea3a352
f475dd6
03756ce
af7005b
36c8026
545642d
cc92aa5
72974db
7bc9d74
26be102
732daa6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| /* | ||
| * Copyright 2023-2025 Benoit Lacelle - SOLVEN | ||
| * | ||
| * 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 | ||
| * | ||
| * http://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 eu.solven.cleanthat.engine.java.refactorer.mutators; | ||
|
|
||
| import java.util.Optional; | ||
| import java.util.Set; | ||
|
|
||
| import com.github.javaparser.ast.NodeList; | ||
| import com.github.javaparser.ast.expr.Expression; | ||
| import com.github.javaparser.ast.expr.MethodCallExpr; | ||
| import com.github.javaparser.ast.expr.StringLiteralExpr; | ||
| import com.google.common.collect.ImmutableSet; | ||
|
|
||
| import eu.solven.cleanthat.engine.java.IJdkVersionConstants; | ||
| import eu.solven.cleanthat.engine.java.refactorer.AJavaparserExprMutator; | ||
| import eu.solven.cleanthat.engine.java.refactorer.NodeAndSymbolSolver; | ||
| import eu.solven.cleanthat.engine.java.refactorer.helpers.MethodCallExprHelpers; | ||
| import eu.solven.cleanthat.engine.java.refactorer.meta.IMutatorDescriber; | ||
|
|
||
| /** | ||
| * Switch builder.append("string").append("builder") to builder.append("stringbuilder") | ||
| * | ||
| * @author Balazs Glatz | ||
| */ | ||
| public class ConsecutiveLiteralAppends extends AJavaparserExprMutator implements IMutatorDescriber { | ||
|
|
||
| private static final String METHOD_APPEND = "append"; | ||
|
|
||
| @Override | ||
| public String minimalJavaVersion() { | ||
| return IJdkVersionConstants.JDK_5; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isPerformanceImprovment() { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public Set<String> getTags() { | ||
| return ImmutableSet.of("String"); | ||
| } | ||
|
|
||
| @Override | ||
| public Optional<String> getPmdId() { | ||
| return Optional.of("ConsecutiveLiteralAppends"); | ||
| } | ||
|
|
||
| @Override | ||
| public String pmdUrl() { | ||
| return "https://pmd.github.io/pmd/pmd_rules_java_performance.html#consecutiveliteralappends"; | ||
| } | ||
|
|
||
| @Override | ||
| protected boolean processExpression(NodeAndSymbolSolver<Expression> expression) { | ||
| Expression node = expression.getNode(); | ||
| if (!isMethodCall(node)) { | ||
| return false; | ||
| } | ||
|
|
||
| var methodCall = node.asMethodCallExpr(); | ||
| if (!isAppendMethodWithSingleParam(methodCall)) { | ||
| return false; | ||
| } | ||
|
|
||
| String argument = getStringValue(methodCall.getArgument(0)); | ||
| if (argument == null) { | ||
| return false; | ||
| } | ||
|
|
||
| Optional<Expression> scope = methodCall.getScope(); | ||
| if (!isMethodCallOnAppendable(expression, scope)) { | ||
| return false; | ||
| } | ||
|
|
||
| var previousMethodCall = scope.get().asMethodCallExpr(); | ||
| if (!isAppendMethodWithSingleParam(previousMethodCall)) { | ||
| return false; | ||
| } | ||
|
|
||
| String previousArgument = getStringValue(previousMethodCall.getArgument(0)); | ||
| if (previousArgument == null) { | ||
| return false; | ||
| } | ||
|
|
||
| if (!isAppendableScope(expression, previousMethodCall.getScope())) { | ||
| return false; | ||
| } | ||
|
|
||
| var newArgument = new StringLiteralExpr(previousArgument + argument); | ||
| var replacement = | ||
| new MethodCallExpr(previousMethodCall.getScope().get(), METHOD_APPEND, NodeList.nodeList(newArgument)); | ||
|
|
||
| return tryReplace(expression, replacement); | ||
| } | ||
|
|
||
| private static boolean isMethodCall(Expression node) { | ||
| return node instanceof MethodCallExpr; | ||
| } | ||
|
|
||
| private static boolean isMethodCallOnAppendable(NodeAndSymbolSolver<Expression> expression, Optional<Expression> scope) { | ||
| return scope.isPresent() && scope.get() instanceof MethodCallExpr && isAppendableScope(expression, scope); | ||
| } | ||
|
|
||
| private static boolean isAppendMethodWithSingleParam(MethodCallExpr methodCall) { | ||
| return METHOD_APPEND.equals(methodCall.getNameAsString()) && methodCall.getArguments().size() == 1; | ||
| } | ||
|
|
||
| private static boolean isAppendableScope(NodeAndSymbolSolver<Expression> expression, Optional<Expression> scope) { | ||
| return MethodCallExprHelpers.scopeHasRequiredType(expression.editNode(scope), Appendable.class); | ||
| } | ||
|
|
||
| private static String getStringValue(Expression argument) { | ||
| if (argument.isStringLiteralExpr()) { | ||
| return argument.asStringLiteralExpr().getValue(); | ||
| } | ||
| if (argument.isCharLiteralExpr()) { | ||
| return argument.asCharLiteralExpr().getValue(); | ||
| } | ||
| if (argument.isIntegerLiteralExpr()) { | ||
| return argument.asIntegerLiteralExpr().asNumber().toString(); | ||
| } | ||
| if (argument.isLongLiteralExpr()) { | ||
| return argument.asLongLiteralExpr().asNumber().toString(); | ||
| } | ||
| if (argument.isDoubleLiteralExpr()) { | ||
| return String.valueOf(argument.asDoubleLiteralExpr().asDouble()); | ||
| } | ||
| if (argument.isUnaryExpr()) { | ||
| return argument.asUnaryExpr().toString(); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,201 @@ | ||
| package eu.solven.cleanthat.engine.java.refactorer.cases.do_not_format_me; | ||
|
|
||
| import eu.solven.cleanthat.engine.java.refactorer.annotations.CompareMethods; | ||
| import eu.solven.cleanthat.engine.java.refactorer.annotations.UnmodifiedMethod; | ||
| import eu.solven.cleanthat.engine.java.refactorer.meta.IJavaparserAstMutator; | ||
| import eu.solven.cleanthat.engine.java.refactorer.mutators.ConsecutiveLiteralAppends; | ||
| import eu.solven.cleanthat.engine.java.refactorer.test.AJavaparserRefactorerCases; | ||
| import org.junit.Ignore; | ||
|
|
||
| public class TestConsecutiveLiteralAppendsCases extends AJavaparserRefactorerCases { | ||
|
|
||
| @Override | ||
| public IJavaparserAstMutator getTransformer() { | ||
| return new ConsecutiveLiteralAppends(); | ||
| } | ||
|
|
||
| @UnmodifiedMethod | ||
| public static class FakeAppend { | ||
| private void append(char c) { } | ||
|
|
||
| public void pre() { | ||
| append('/'); | ||
| } | ||
| } | ||
|
|
||
| @UnmodifiedMethod | ||
| public static class FakeChainedAppend { | ||
| private StringBuilder append(String string) { | ||
| return new StringBuilder(); | ||
| } | ||
|
|
||
| public void pre() { | ||
| append("fakeAppend").append("realAppend"); | ||
| } | ||
| } | ||
|
|
||
| @UnmodifiedMethod | ||
| public static class UnchainedLiteral { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append("first"); | ||
| } | ||
| } | ||
|
|
||
| @UnmodifiedMethod | ||
| public static class UnchainedVariable { | ||
| public Object pre(StringBuilder builder, String first) { | ||
| return builder.append(first); | ||
| } | ||
| } | ||
|
|
||
| @CompareMethods | ||
| public static class TwoLiterals { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append("app").append("end"); | ||
| } | ||
|
|
||
| public Object post(StringBuilder builder) { | ||
| return builder.append("append"); | ||
| } | ||
| } | ||
|
|
||
| @Ignore("Not yet implemented") | ||
| @CompareMethods | ||
| public static class ThreeLiterals { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append("app").append("end").append("ed"); | ||
| } | ||
|
|
||
| public Object post(StringBuilder builder) { | ||
| return builder.append("appended"); | ||
| } | ||
| } | ||
|
|
||
| @CompareMethods | ||
| public static class TwoChars { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append('a').append('b'); | ||
| } | ||
|
|
||
| public Object post(StringBuilder builder) { | ||
| return builder.append("ab"); | ||
| } | ||
| } | ||
|
|
||
| @UnmodifiedMethod | ||
| public static class TwoVariables { | ||
| public Object pre(StringBuilder builder, String first, String second) { | ||
| return builder.append(first).append(second); | ||
| } | ||
| } | ||
|
|
||
| @CompareMethods | ||
| public static class CharAndString { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append('a').append("ppend"); | ||
| } | ||
|
|
||
| public Object post(StringBuilder builder) { | ||
| return builder.append("append"); | ||
| } | ||
| } | ||
|
|
||
| @CompareMethods | ||
| public static class StringAndChar { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append("map").append('s'); | ||
| } | ||
|
|
||
| public Object post(StringBuilder builder) { | ||
| return builder.append("maps"); | ||
| } | ||
| } | ||
|
|
||
| @UnmodifiedMethod | ||
| public static class VariableAndLiteral { | ||
| public Object pre(StringBuilder builder, String first) { | ||
| return builder.append(first).append("second"); | ||
| } | ||
| } | ||
|
|
||
| @UnmodifiedMethod | ||
| public static class LiteralAndVariable { | ||
| public Object pre(StringBuilder builder, String second) { | ||
| return builder.append("first").append(second); | ||
| } | ||
| } | ||
|
|
||
| @CompareMethods | ||
| public static class TwoIntegers { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append(1).append(2); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about :
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, added them! |
||
| } | ||
|
|
||
| public Object post(StringBuilder builder) { | ||
| return builder.append("12"); | ||
|
blacelle marked this conversation as resolved.
|
||
| } | ||
| } | ||
|
|
||
| @CompareMethods | ||
| public static class NonSingleDigitIntegers { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append(123).append(456); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This case is
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, I think I just didn't think about it, but thanks for pointing out, I'll try to make it work!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have adjusted it to support numbers, |
||
| } | ||
|
|
||
| public Object post(StringBuilder builder) { | ||
| return builder.append("123456"); | ||
| } | ||
| } | ||
|
|
||
| @CompareMethods | ||
| public static class IntegerOverflow { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append(2147483647).append(1); | ||
| } | ||
|
|
||
| public Object post(StringBuilder builder) { | ||
| return builder.append("21474836471"); | ||
| } | ||
| } | ||
|
|
||
| @CompareMethods | ||
| public static class Doubles { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append(1.2).append(3.4); | ||
| } | ||
|
|
||
| public Object post(StringBuilder builder) { | ||
| return builder.append("1.23.4"); | ||
| } | ||
| } | ||
|
|
||
| @CompareMethods | ||
| public static class NegativeInteger { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append(1).append(-2); | ||
| } | ||
|
|
||
| public Object post(StringBuilder builder) { | ||
| return builder.append("1-2"); | ||
| } | ||
| } | ||
|
|
||
| @CompareMethods | ||
| public static class HexadecimalIntegers { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append(0x1).append(0x2); | ||
| } | ||
|
|
||
| public Object post(StringBuilder builder) { | ||
| return builder.append("12"); | ||
| } | ||
| } | ||
|
|
||
| @UnmodifiedMethod | ||
| public static class CastIntegers { | ||
| public Object pre(StringBuilder builder) { | ||
| return builder.append((char) 1).append((char) 2); | ||
| } | ||
| } | ||
|
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.