feat: fix ToArrayCallWithZeroLengthArrayArgument (#241)

code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/QodanaRules.java

@@ -12,6 +12,7 @@
 import xyz.keksdose.spoon.code_solver.analyzer.qodana.rules.PointlessBooleanExpression;
 import xyz.keksdose.spoon.code_solver.analyzer.qodana.rules.ProtectedMemberInFinalClass;
 import xyz.keksdose.spoon.code_solver.analyzer.qodana.rules.SizeReplaceableByIsEmpty;
+import xyz.keksdose.spoon.code_solver.analyzer.qodana.rules.ToArrayCallWithZeroLengthArrayArgument;
 import xyz.keksdose.spoon.code_solver.analyzer.qodana.rules.UnnecessaryInterfaceModifier;
 import xyz.keksdose.spoon.code_solver.analyzer.qodana.rules.UnnecessaryLocalVariable;
 import xyz.keksdose.spoon.code_solver.analyzer.qodana.rules.UnnecessaryModifier;
@@ -37,7 +38,9 @@ public enum QodanaRules implements AnalyzerRule {
     PROTECTED_MEMBER_IN_FINAL_CLASS("ProtectedMemberInFinalClass", ProtectedMemberInFinalClass::new),
     UNNECESSARY_MODIFIER("UnnecessaryModifier", UnnecessaryModifier::new),
     POINTLESS_BOOLEAN_EXPRESSION("PointlessBooleanExpression", PointlessBooleanExpression::new),
-    INNER_CLASS_MAY_BE_STATIC("InnerClassMayBeStatic", InnerClassMayBeStatic::new);
+    INNER_CLASS_MAY_BE_STATIC("InnerClassMayBeStatic", InnerClassMayBeStatic::new),
+    TO_ARRAY_CALL_WITH_ZERO_LENGTH_ARRAY_ARGUMENT(
+            "ToArrayCallWithZeroLengthArrayArgument", ToArrayCallWithZeroLengthArrayArgument::new);
 
     private final String ruleId;
     private final Function<AnalyzerResult, AbstractRefactoring> refactoring;

code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/rules/ToArrayCallWithZeroLengthArrayArgument.java

@@ -0,0 +1,77 @@
+package xyz.keksdose.spoon.code_solver.analyzer.qodana.rules;
+
+import java.nio.file.Path;
+import java.util.List;
+import spoon.reflect.code.CtNewArray;
+import spoon.reflect.declaration.CtElement;
+import spoon.reflect.declaration.CtType;
+import xyz.keksdose.spoon.code_solver.analyzer.PositionScanner;
+import xyz.keksdose.spoon.code_solver.api.analyzer.AnalyzerResult;
+import xyz.keksdose.spoon.code_solver.history.Change;
+import xyz.keksdose.spoon.code_solver.history.ChangeListener;
+import xyz.keksdose.spoon.code_solver.history.MarkdownString;
+import xyz.keksdose.spoon.code_solver.transformations.BadSmell;
+
+public class ToArrayCallWithZeroLengthArrayArgument extends AbstractRefactoring {
+
+    private static BadSmell badSmell = new BadSmell() {
+
+        @Override
+        public MarkdownString getDescription() {
+            return MarkdownString.fromMarkdown(
+                    """
+        The performance of the empty array version is the same, and sometimes even better, compared
+        to the pre-sized version. Also, passing a pre-sized array is dangerous for a concurrent or
+        synchronized collection as a data race is possible between the <code>size</code> and <code>toArray</code>
+        calls. This may result in extra <code>null</code>s at the end of the array if the collection was concurrently
+        shrunk during the operation.</p>
+        See https://shipilev.net/blog/2016/arrays-wisdom-ancients/ for more details.
+        """);
+        }
+
+        @Override
+        public MarkdownString getName() {
+            return MarkdownString.fromMarkdown("ToArrayCallWithZeroLengthArrayArgument");
+        }
+    };
+
+    public ToArrayCallWithZeroLengthArrayArgument(AnalyzerResult result) {
+        super(result);
+    }
+
+    @Override
+    public void refactor(ChangeListener listener, CtType<?> type) {
+        if (!isSameType(type, Path.of(result.filePath()))) {
+            return;
+        }
+        for (CtNewArray<?> toArrayCall : filterMatches(PositionScanner.findLineOnly(type, result.position()))) {
+            var dimensionExpression = toArrayCall.getDimensionExpressions().get(0);
+            var zeroExpression = dimensionExpression.getFactory().createLiteral(0);
+            dimensionExpression.replace(zeroExpression);
+            String message = "Replaced " + toMarkdown(dimensionExpression) + " with " + zeroExpression;
+            String markdown = "Replaced " + toMarkdown(dimensionExpression) + " with " + zeroExpression;
+            Change change = new Change(badSmell, MarkdownString.fromMarkdown(message, markdown), type, result);
+            listener.setChanged(type, change);
+        }
+    }
+
+    @SuppressWarnings("rawtypes")
+    private List<CtNewArray> filterMatches(List<CtElement> findLineOnly) {
+        return findLineOnly.stream()
+                .filter(CtNewArray.class::isInstance)
+                .map(CtNewArray.class::cast)
+                .filter(ctNewArray -> ctNewArray.getDimensionExpressions().size() == 1)
+                .filter(ctNewArray ->
+                        !ctNewArray.getDimensionExpressions().get(0).toString().equals("0"))
+                .toList();
+    }
+
+    @Override
+    public List<BadSmell> getHandledBadSmells() {
+        return List.of(badSmell);
+    }
+
+    private String toMarkdown(Object input) {
+        return "`" + input + "`";
+    }
+}

code-transformation/src/test/java/xyz/keksdose/spoon/code_solver/transformations/qodana/ToArrayCallWithZeroLengthArrayArgumentTest.java

@@ -0,0 +1,24 @@
+package xyz.keksdose.spoon.code_solver.transformations.qodana;
+
+import java.io.File;
+import java.io.IOException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import xyz.keksdose.spoon.code_solver.analyzer.qodana.rules.ToArrayCallWithZeroLengthArrayArgument;
+import xyz.keksdose.spoon.code_solver.api.analyzer.AnalyzerResult;
+import xyz.keksdose.spoon.code_solver.api.analyzer.Position;
+import xyz.keksdose.spoon.code_solver.transformations.TestAnalyzerResult;
+import xyz.keksdose.spoon.code_solver.transformations.TransformationTestUtils;
+
+public class ToArrayCallWithZeroLengthArrayArgumentTest {
+
+    @Test
+    void spoonTreeBuilderCompiler(@TempDir File dir) throws IOException {
+        Position position = new Position(136, 0, 136, 0, 4526, 0);
+        AnalyzerResult result = new TestAnalyzerResult("TreeBuilderCompiler.java", position);
+        String resourcePath = "projects/refactorings/ToArrayCallWithZeroLengthArrayArgument/TreeBuilderCompiler.java";
+        var copy = TransformationTestUtils.transform(
+                new ToArrayCallWithZeroLengthArrayArgument(result), resourcePath, dir);
+        TransformationTestUtils.compareContent(copy, resourcePath);
+    }
+}

code-transformation/src/test/resources/projects/refactorings/ToArrayCallWithZeroLengthArrayArgument/TreeBuilderCompiler.java

@@ -0,0 +1,138 @@
+/*
+ * SPDX-License-Identifier: (MIT OR CECILL-C)
+ *
+ * Copyright (C) 2006-2019 INRIA and contributors
+ *
+ * Spoon is available either under the terms of the MIT License (see LICENSE-MIT.txt) of the Cecill-C License (see LICENSE-CECILL-C.txt). You as the user are entitled to choose the terms under which to adopt Spoon.
+ */
+package spoon.support.compiler.jdt;
+
+import java.io.PrintWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Arrays;
+
+import org.eclipse.jdt.core.compiler.CharOperation;
+import org.eclipse.jdt.core.compiler.CompilationProgress;
+import org.eclipse.jdt.internal.compiler.CompilationResult;
+import org.eclipse.jdt.internal.compiler.ICompilerRequestor;
+import org.eclipse.jdt.internal.compiler.IErrorHandlingPolicy;
+import org.eclipse.jdt.internal.compiler.IProblemFactory;
+import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration;
+import org.eclipse.jdt.internal.compiler.batch.CompilationUnit;
+import org.eclipse.jdt.internal.compiler.env.ICompilationUnit;
+import org.eclipse.jdt.internal.compiler.env.INameEnvironment;
+import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
+import org.eclipse.jdt.internal.compiler.util.Messages;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import spoon.support.Level;
+
+
+class TreeBuilderCompiler extends org.eclipse.jdt.internal.compiler.Compiler {
+
+	private static final Logger LOGGER = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+	private boolean ignoreSyntaxErrors;
+
+	private Level level;
+
+	TreeBuilderCompiler(INameEnvironment environment, IErrorHandlingPolicy policy, CompilerOptions options,
+						ICompilerRequestor requestor, IProblemFactory problemFactory, PrintWriter out,
+						boolean ignoreSyntaxErrors, Level level, CompilationProgress progress) {
+		super(environment, policy, options, requestor, problemFactory, out, progress);
+		this.ignoreSyntaxErrors = ignoreSyntaxErrors;
+		this.level = level;
+	}
+
+	// This code is directly inspired from Compiler class.
+	private void sortModuleDeclarationsFirst(ICompilationUnit[] sourceUnits) {
+		Arrays.sort(sourceUnits, (u1, u2) -> {
+			char[] fn1 = u1.getFileName();
+			char[] fn2 = u2.getFileName();
+			boolean isMod1 = CharOperation.endsWith(fn1, JDTConstants.MODULE_INFO_FILE_NAME) || CharOperation.endsWith(fn1, JDTConstants.MODULE_INFO_CLASS_NAME);
+			boolean isMod2 = CharOperation.endsWith(fn2, JDTConstants.MODULE_INFO_FILE_NAME) || CharOperation.endsWith(fn2, JDTConstants.MODULE_INFO_CLASS_NAME);
+			if (isMod1 == isMod2) {
+				return 0;
+			}
+			return isMod1 ? -1 : 1;
+		});
+	}
+
+	// this method is not meant to be in the public API
+	protected CompilationUnitDeclaration[] buildUnits(CompilationUnit[] sourceUnits) {
+
+		// //////////////////////////////////////////////////////////////////////////
+		// This code is largely inspired from JDT's
+		// CompilationUnitResolver.resolve
+
+		this.reportProgress(Messages.compilation_beginningToCompile);
+
+		this.sortModuleDeclarationsFirst(sourceUnits);
+
+		CompilationUnit[] filteredSourceUnits = null;
+		if (ignoreSyntaxErrors || level.toInt() > Level.ERROR.toInt()) {
+			// syntax is optionally checked here to prevent crashes inside JDT
+			filteredSourceUnits = ignoreSyntaxErrors(sourceUnits);
+		}
+		// build and record parsed units
+		if (ignoreSyntaxErrors) {
+			beginToCompile(filteredSourceUnits);
+		} else {
+			beginToCompile(sourceUnits);
+		}
+
+		CompilationUnitDeclaration unit;
+		int i = 0;
+
+		// process all units (some more could be injected in the loop by the lookup environment)
+		for (; i < this.totalUnits; i++) {
+			unit = unitsToProcess[i];
+			this.reportProgress(Messages.bind(Messages.compilation_processing, new String(unit.getFileName())));
+			this.parser.getMethodBodies(unit);
+
+			// fault in fields & methods
+			if (unit.scope != null) {
+				unit.scope.faultInTypes();
+			}
+
+			// verify inherited methods
+			if (unit.scope != null) {
+				unit.scope.verifyMethods(lookupEnvironment.methodVerifier());
+			}
+
+			// type checking
+			unit.resolve();
+			// flow analysis
+			unit.analyseCode();
+
+			unit.ignoreFurtherInvestigation = false;
+			requestor.acceptResult(unit.compilationResult);
+			this.reportWorked(1, i);
+		}
+
+		ArrayList<CompilationUnitDeclaration> unitsToReturn = new ArrayList<>();
+		for (CompilationUnitDeclaration cud : this.unitsToProcess) {
+			if (cud != null) {
+				unitsToReturn.add(cud);
+			}
+		}
+		return unitsToReturn.toArray(new CompilationUnitDeclaration[0]);
+	}
+
+	private CompilationUnit[] ignoreSyntaxErrors(CompilationUnit[] sourceUnits) {
+		ArrayList<CompilationUnit> sourceUnitList = new ArrayList<>();
+		int maxUnits = sourceUnits.length;
+		for (int i = 0; i < maxUnits; i++) {
+			CompilationResult unitResult = new CompilationResult(sourceUnits[i], i, maxUnits, this.options.maxProblemsPerUnit);
+			CompilationUnitDeclaration parsedUnit = this.parser.parse(sourceUnits[i], unitResult);
+			if (parsedUnit.hasErrors()) {
+				LOGGER.warn("Syntax error detected in: " + String.valueOf(sourceUnits[i].getFileName()));
+			} else {
+				sourceUnitList.add(sourceUnits[i]);
+			}
+		}
+		this.initializeParser();
+		return sourceUnitList.toArray(new CompilationUnit[sourceUnitList.size()]);
+	}
+}