From d47caf5d1b352dd47db4c0f957449ec7fbe6e23d Mon Sep 17 00:00:00 2001 From: Martin Wittlinger Date: Mon, 24 Oct 2022 17:51:07 +0200 Subject: [PATCH 1/3] feat: add qodana result logger --- .../analyzer/qodana/QodanaAnalyzer.java | 59 +++++++++++++++++-- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/QodanaAnalyzer.java b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/QodanaAnalyzer.java index 22ca3e479..35a3dee20 100644 --- a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/QodanaAnalyzer.java +++ b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/QodanaAnalyzer.java @@ -4,11 +4,14 @@ import com.contrastsecurity.sarif.SarifSchema210; import com.fasterxml.jackson.databind.ObjectMapper; import com.github.dockerjava.api.DockerClient; +import com.github.dockerjava.api.async.ResultCallback; import com.github.dockerjava.api.async.ResultCallbackTemplate; import com.github.dockerjava.api.command.CreateContainerResponse; +import com.github.dockerjava.api.command.LogContainerCmd; import com.github.dockerjava.api.command.WaitContainerResultCallback; import com.github.dockerjava.api.model.AccessMode; import com.github.dockerjava.api.model.Bind; +import com.github.dockerjava.api.model.Frame; import com.github.dockerjava.api.model.HostConfig; import com.github.dockerjava.api.model.Image; import com.github.dockerjava.api.model.Volume; @@ -19,6 +22,7 @@ import com.github.dockerjava.httpclient5.ApacheDockerHttpClient; import com.github.dockerjava.transport.DockerHttpClient; import com.google.common.flogger.FluentLogger; +import java.io.Closeable; import java.io.File; import java.io.IOException; import java.io.StringReader; @@ -122,19 +126,37 @@ private void copyQodanaRules(Path sourceRoot) { private List startQodanaContainer(DockerClient dockerClient, CreateContainerResponse container) throws InterruptedException { - dockerClient.startContainerCmd(container.getId()).exec(); + String containerId = container.getId(); + if (containerId == null) { + logger.atSevere().log("Container id is null"); + return List.of(); + } + logger.atInfo().log("Starting qodana container %s", containerId); + dockerClient.startContainerCmd(containerId).exec(); WaitContainerResultCallback exec = - dockerClient.waitContainerCmd(container.getId()).exec(new WaitContainerResultCallback()); + dockerClient.waitContainerCmd(containerId).exec(new WaitContainerResultCallback()); List results = new ArrayList<>(); dockerClient - .waitContainerCmd(container.getId()) + .waitContainerCmd(containerId) .exec(new ResultCallbackTemplate() { @Override public void onNext(WaitResponse object) { try { exec.awaitCompletion(); - - results.addAll(parseSarif(Paths.get(resultPathString))); + logger.atInfo().log("Qodana finished"); + if (!Paths.get(resultPathString).toFile().exists()) { + StringBuilder sb = new StringBuilder(); + LogContainerCmd logContainerCmd = dockerClient.logContainerCmd(containerId); + logContainerCmd + .withStdOut(true) + .withStdErr(true) + .withTailAll() + .exec(new ResultCallbackImplementation(sb)); + logger.atSevere().log(sb.toString()); + logger.atSevere().log("Qodana did not create result file"); + } else { + results.addAll(parseSarif(Paths.get(resultPathString))); + } } catch (Exception e) { logger.atSevere().withCause(e).log("Could not parse sarif"); } @@ -217,6 +239,33 @@ private List parseSarif(Path resultPath) throws IOException { return results; } + private final class ResultCallbackImplementation implements ResultCallback { + private final StringBuilder sb; + + private ResultCallbackImplementation(StringBuilder sb) { + this.sb = sb; + } + + @Override + public void close() throws IOException {} + + @Override + public void onStart(Closeable closeable) {} + + @Override + public void onNext(Frame object) { + sb.append(new String(object.getPayload())); + } + + @Override + public void onError(Throwable throwable) { + sb.append(throwable.getMessage()); + } + + @Override + public void onComplete() {} + } + public static class Builder { private String resultFolder = "laughing-cache"; From 656a39c2ccbba76387f3433f46217fe4a29992ad Mon Sep 17 00:00:00 2001 From: Martin Wittlinger Date: Mon, 24 Oct 2022 20:24:21 +0200 Subject: [PATCH 2/3] fix: improve logging --- .../xyz/keksdose/spoon/code_solver/TransformationEngine.java | 3 +++ .../analyzer/qodana/rules/UnnecessaryToStringCall.java | 3 +++ 2 files changed, 6 insertions(+) diff --git a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/TransformationEngine.java b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/TransformationEngine.java index 771829a04..3f1969eed 100644 --- a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/TransformationEngine.java +++ b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/TransformationEngine.java @@ -85,6 +85,9 @@ public Changelog applyToGivenPath(String path) { addProcessors(pm, changeListener); pm.process(model.getAllTypes()); Collection> newTypes = model.getAllTypes(); + LOGGER.atInfo().log("Applying transformations to %s done", path); + LOGGER.atInfo().log( + "%s Changes found", changeListener.getChangelog().getChanges().size()); printing.printChangedTypes(changeListener, newTypes); return changeListener.getChangelog(); } diff --git a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/rules/UnnecessaryToStringCall.java b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/rules/UnnecessaryToStringCall.java index 2b6855949..b61fe4d0d 100644 --- a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/rules/UnnecessaryToStringCall.java +++ b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/rules/UnnecessaryToStringCall.java @@ -2,6 +2,7 @@ import static xyz.keksdose.spoon.code_solver.history.MarkdownString.fromMarkdown; +import com.google.common.flogger.FluentLogger; import java.nio.file.Path; import java.util.List; import spoon.reflect.code.CtInvocation; @@ -16,6 +17,7 @@ public class UnnecessaryToStringCall extends AbstractRefactoring { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final BadSmell UNNECESSARY_TO_STRING_CALL = new BadSmell() { @Override public MarkdownString getName() { @@ -36,6 +38,7 @@ public UnnecessaryToStringCall(AnalyzerResult result) { @Override public void refactor(ChangeListener listener, CtType type) { if (type.isAnonymous() || !isSameType(type, Path.of(result.filePath()))) { + logger.atFinest().log("skipping %s", type.getQualifiedName()); return; } for (CtInvocation toStringInvocation : From 56f6c0a9a94826b83e8f3edad8c653b3562eb8a5 Mon Sep 17 00:00:00 2001 From: Martin Wittlinger Date: Mon, 24 Oct 2022 20:55:04 +0200 Subject: [PATCH 3/3] feat:add UnnecessaryModifier rule --- .../analyzer/qodana/QodanaRules.java | 3 + .../qodana/rules/UnnecessaryModifier.java | 66 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/rules/UnnecessaryModifier.java diff --git a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/QodanaRules.java b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/QodanaRules.java index 7a2604289..e31d2d23e 100644 --- a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/QodanaRules.java +++ b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/QodanaRules.java @@ -13,6 +13,7 @@ import xyz.keksdose.spoon.code_solver.analyzer.qodana.rules.SizeReplaceableByIsEmpty; 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; import xyz.keksdose.spoon.code_solver.analyzer.qodana.rules.UnnecessaryReturn; import xyz.keksdose.spoon.code_solver.analyzer.qodana.rules.UnnecessaryToStringCall; import xyz.keksdose.spoon.code_solver.analyzer.qodana.rules.UnusedImport; @@ -33,7 +34,9 @@ public enum QodanaRules implements AnalyzerRule { UNNECESSARY_TO_STRING_CALL("UnnecessaryToStringCall", UnnecessaryToStringCall::new), UNUSED_IMPORT("UnusedImport", UnusedImport::new), PROTECTED_MEMBER_IN_FINAL_CLASS("ProtectedMemberInFinalClass", ProtectedMemberInFinalClass::new), + UNNECESSARY_MODIFIER("UnnecessaryModifier", UnnecessaryModifier::new), POINTLESS_BOOLEAN_EXPRESSION("PointlessBooleanExpression", PointlessBooleanExpression::new); + private final String ruleId; private final Function refactoring; diff --git a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/rules/UnnecessaryModifier.java b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/rules/UnnecessaryModifier.java new file mode 100644 index 000000000..761178cbd --- /dev/null +++ b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/qodana/rules/UnnecessaryModifier.java @@ -0,0 +1,66 @@ +package xyz.keksdose.spoon.code_solver.analyzer.qodana.rules; + +import java.nio.file.Path; +import java.util.HashSet; +import java.util.List; +import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtModifiable; +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 UnnecessaryModifier extends AbstractRefactoring { + + public UnnecessaryModifier(AnalyzerResult result) { + super(result); + } + + private static final BadSmell UNNECESSARY_MODIFIER = new BadSmell() { + @Override + public MarkdownString getName() { + return MarkdownString.fromRaw("Unnecessary-Modifier"); + } + + @Override + public MarkdownString getDescription() { + return MarkdownString.fromRaw( + "Some modifiers are not needed, because they are already the default and implicit. These modifiers can be removed."); + } + }; + + @Override + public void refactor(ChangeListener listener, CtType type) { + if (type.isAnonymous() || !isSameType(type, Path.of(result.filePath()))) { + return; + } + String modifier = result.messageMarkdown().split("`")[1]; + if (modifier == null || modifier.isEmpty()) { + return; + } + for (CtElement match : PositionScanner.findLineOnly(type, result.position())) { + if (match instanceof CtModifiable ctModifierHandler) { + var modifiers = new HashSet<>(ctModifierHandler.getModifiers()); + modifiers.removeIf(v -> v.toString().equals(modifier.toLowerCase())); + ctModifierHandler.setModifiers(modifiers); + listener.setChanged( + type.getTopLevelType(), + new Change( + UNNECESSARY_MODIFIER, + MarkdownString.fromMarkdown( + "Unnecessary modifier " + modifier + " removed", + "Unnecessary modifier `" + modifier + "` removed"), + type.getTopLevelType(), + result)); + } + } + } + + @Override + public List getHandledBadSmells() { + return List.of(UNNECESSARY_MODIFIER); + } +}