diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index a9883e287..f4674dac1 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -27,7 +27,7 @@ jsvg = "1.6.1" llzip = "2.6.2" logback-classic = { strictly = "1.4.11" } # newer releases break in jar releases mapping-io = "0.6.1" -mockito = "5.13.0" +mockito = "5.14.2" natural-order = "1.1" openrewrite = "8.37.1" picocli = "4.7.6" diff --git a/recaf-core/src/main/java/software/coley/recaf/services/decompile/BaseDecompilerConfig.java b/recaf-core/src/main/java/software/coley/recaf/services/decompile/BaseDecompilerConfig.java index 153fee991..09f4329e8 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/decompile/BaseDecompilerConfig.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/decompile/BaseDecompilerConfig.java @@ -17,7 +17,7 @@ public class BaseDecompilerConfig extends BasicConfigContainer implements Decomp * @param id * Container ID. */ - public BaseDecompilerConfig( @Nonnull String id) { + public BaseDecompilerConfig(@Nonnull String id) { super(SERVICE_DECOMPILE_IMPL, id); } diff --git a/recaf-core/src/main/java/software/coley/recaf/services/decompile/NoopDecompilerConfig.java b/recaf-core/src/main/java/software/coley/recaf/services/decompile/NoopDecompilerConfig.java index 520bdae75..c7152e9a6 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/decompile/NoopDecompilerConfig.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/decompile/NoopDecompilerConfig.java @@ -1,19 +1,19 @@ package software.coley.recaf.services.decompile; -import software.coley.recaf.config.BasicConfigContainer; -import software.coley.recaf.config.ConfigGroups; +import software.coley.recaf.util.ExcludeFromJacocoGeneratedReport; /** * Dummy config for {@link NoopJvmDecompiler} and {@link NoopAndroidDecompiler}. * * @author Matt Coley */ -public class NoopDecompilerConfig extends BasicConfigContainer implements DecompilerConfig { +@ExcludeFromJacocoGeneratedReport(justification = "Config POJO") +public class NoopDecompilerConfig extends BaseDecompilerConfig implements DecompilerConfig { /** * New dummy config. */ public NoopDecompilerConfig() { - super(ConfigGroups.SERVICE_DECOMPILE, "decompiler-noop" + CONFIG_SUFFIX); + super("noop"); } @Override diff --git a/recaf-core/src/main/java/software/coley/recaf/services/decompile/cfr/CfrConfig.java b/recaf-core/src/main/java/software/coley/recaf/services/decompile/cfr/CfrConfig.java index 74c15f83f..b8c98bf1f 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/decompile/cfr/CfrConfig.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/decompile/cfr/CfrConfig.java @@ -13,6 +13,7 @@ import software.coley.recaf.config.BasicConfigValue; import software.coley.recaf.config.ConfigGroups; import software.coley.recaf.services.decompile.BaseDecompilerConfig; +import software.coley.recaf.util.ExcludeFromJacocoGeneratedReport; import software.coley.recaf.util.ReflectUtil; import software.coley.recaf.util.StringUtil; @@ -28,6 +29,7 @@ */ @ApplicationScoped @SuppressWarnings("all") // ignore unused refs / typos +@ExcludeFromJacocoGeneratedReport(justification = "Config POJO") public class CfrConfig extends BaseDecompilerConfig { private final ObservableObject stringbuffer = new ObservableObject<>(BooleanOption.DEFAULT); private final ObservableObject stringbuilder = new ObservableObject<>(BooleanOption.DEFAULT); diff --git a/recaf-core/src/main/java/software/coley/recaf/services/decompile/fallback/FallbackConfig.java b/recaf-core/src/main/java/software/coley/recaf/services/decompile/fallback/FallbackConfig.java index 28f5cc209..5b6352923 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/decompile/fallback/FallbackConfig.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/decompile/fallback/FallbackConfig.java @@ -3,6 +3,7 @@ import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; import software.coley.recaf.services.decompile.BaseDecompilerConfig; +import software.coley.recaf.util.ExcludeFromJacocoGeneratedReport; /** * Config for {@link FallbackDecompiler} @@ -10,6 +11,7 @@ * @author Matt Coley */ @ApplicationScoped +@ExcludeFromJacocoGeneratedReport(justification = "Config POJO") public class FallbackConfig extends BaseDecompilerConfig { @Inject public FallbackConfig() { diff --git a/recaf-core/src/main/java/software/coley/recaf/services/decompile/fallback/print/ClassPrinter.java b/recaf-core/src/main/java/software/coley/recaf/services/decompile/fallback/print/ClassPrinter.java index 6cabe8d7f..37a088c27 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/decompile/fallback/print/ClassPrinter.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/decompile/fallback/print/ClassPrinter.java @@ -456,7 +456,7 @@ private void appendStaticInitializer(@Nonnull Printer out, @Nonnull MethodMember @Override protected void buildDeclarationFlags(@Nonnull StringBuilder sb) { // Force only printing the modifier 'static' even if other flags are present - sb.append("static "); + sb.append("static"); } @Override diff --git a/recaf-core/src/main/java/software/coley/recaf/services/decompile/procyon/ProcyonConfig.java b/recaf-core/src/main/java/software/coley/recaf/services/decompile/procyon/ProcyonConfig.java index 0f30c69c9..c4c101dec 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/decompile/procyon/ProcyonConfig.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/decompile/procyon/ProcyonConfig.java @@ -12,6 +12,7 @@ import software.coley.recaf.config.BasicConfigValue; import software.coley.recaf.config.ConfigGroups; import software.coley.recaf.services.decompile.BaseDecompilerConfig; +import software.coley.recaf.util.ExcludeFromJacocoGeneratedReport; /** * Config for {@link ProcyonDecompiler} @@ -19,6 +20,7 @@ * @author Matt Coley */ @ApplicationScoped +@ExcludeFromJacocoGeneratedReport(justification = "Config POJO") public class ProcyonConfig extends BaseDecompilerConfig { private final ObservableBoolean includeLineNumbersInBytecode = new ObservableBoolean(true); private final ObservableBoolean showSyntheticMembers = new ObservableBoolean(false); diff --git a/recaf-core/src/main/java/software/coley/recaf/services/decompile/vineflower/DummyResultSaver.java b/recaf-core/src/main/java/software/coley/recaf/services/decompile/vineflower/DummyResultSaver.java index d94a89cc6..3e864ad65 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/decompile/vineflower/DummyResultSaver.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/decompile/vineflower/DummyResultSaver.java @@ -1,6 +1,7 @@ package software.coley.recaf.services.decompile.vineflower; import org.jetbrains.java.decompiler.main.extern.IResultSaver; +import software.coley.recaf.util.ExcludeFromJacocoGeneratedReport; import java.util.jar.Manifest; @@ -9,6 +10,7 @@ * * @author therathatter */ +@ExcludeFromJacocoGeneratedReport(justification = "We don't use VF file IO, everything stays in memory") public class DummyResultSaver implements IResultSaver { @Override public void saveFolder(String s) { diff --git a/recaf-core/src/main/java/software/coley/recaf/services/decompile/vineflower/VineflowerConfig.java b/recaf-core/src/main/java/software/coley/recaf/services/decompile/vineflower/VineflowerConfig.java index 574894221..4e51b0908 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/decompile/vineflower/VineflowerConfig.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/decompile/vineflower/VineflowerConfig.java @@ -10,6 +10,7 @@ import software.coley.observables.ObservableObject; import software.coley.recaf.config.BasicConfigValue; import software.coley.recaf.services.decompile.BaseDecompilerConfig; +import software.coley.recaf.util.ExcludeFromJacocoGeneratedReport; import java.lang.reflect.Field; import java.util.HashMap; @@ -23,6 +24,7 @@ */ @ApplicationScoped @SuppressWarnings("all") // ignore unused refs / typos +@ExcludeFromJacocoGeneratedReport(justification = "Config POJO") public class VineflowerConfig extends BaseDecompilerConfig { private final ObservableObject loggingLevel = new ObservableObject<>(Level.WARN); private final ObservableBoolean removeBridge = new ObservableBoolean(true); diff --git a/recaf-core/src/main/java/software/coley/recaf/util/ExcludeFromJacocoGeneratedReport.java b/recaf-core/src/main/java/software/coley/recaf/util/ExcludeFromJacocoGeneratedReport.java new file mode 100644 index 000000000..d403d0bb1 --- /dev/null +++ b/recaf-core/src/main/java/software/coley/recaf/util/ExcludeFromJacocoGeneratedReport.java @@ -0,0 +1,28 @@ +package software.coley.recaf.util; + +import jakarta.annotation.Nonnull; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * JaCoCo excludes coverage metrics from classes and methods annotated with names including {@code "Generated"}. + *

+ *

USE THIS CLASS SPARINGLY

+ * Only annotate things with this if they are POJO's! + * Some classes like data models and config objects contribute to coverage metrics with things like getter/setters + * that do not realistically need to be covered. + * + * @author Matt Coley + */ +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.METHOD, ElementType.TYPE}) +public @interface ExcludeFromJacocoGeneratedReport { + /** + * @return Reason why we exclude the annotated element. + */ + @Nonnull + String justification(); +} \ No newline at end of file diff --git a/recaf-core/src/test/java/software/coley/recaf/services/decompile/DecompileManagerTest.java b/recaf-core/src/test/java/software/coley/recaf/services/decompile/DecompileManagerTest.java index 7a16c570b..eb5d66f7d 100644 --- a/recaf-core/src/test/java/software/coley/recaf/services/decompile/DecompileManagerTest.java +++ b/recaf-core/src/test/java/software/coley/recaf/services/decompile/DecompileManagerTest.java @@ -2,7 +2,9 @@ import jakarta.annotation.Nonnull; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import software.coley.observables.ObservableBoolean; import software.coley.recaf.info.ClassInfo; import software.coley.recaf.info.JvmClassInfo; import software.coley.recaf.services.decompile.cfr.CfrDecompiler; @@ -14,6 +16,7 @@ import software.coley.recaf.test.TestBase; import software.coley.recaf.test.TestClassUtils; import software.coley.recaf.test.dummy.HelloWorld; +import software.coley.recaf.util.ReflectUtil; import software.coley.recaf.workspace.model.Workspace; import java.io.IOException; @@ -28,20 +31,37 @@ * Tests for {@link DecompilerManager}. */ public class DecompileManagerTest extends TestBase { + private static final ObservableBoolean OB_TRUE = new ObservableBoolean(true); + private static final ObservableBoolean OB_FALSE = new ObservableBoolean(false); static final TestJvmBytecodeFilter bytecodeFilter = new TestJvmBytecodeFilter(); static final TestOutputTextFilter textFilter = new TestOutputTextFilter(); static DecompilerManager decompilerManager; + static DecompilerManagerConfig decompilerManagerConfig; static Workspace workspace; - static JvmClassInfo classToDecompile; + static JvmClassInfo classHelloWorld; @BeforeAll static void setup() throws IOException { decompilerManager = recaf.get(DecompilerManager.class); - classToDecompile = TestClassUtils.fromRuntimeClass(HelloWorld.class); - workspace = TestClassUtils.fromBundle(TestClassUtils.fromClasses(classToDecompile)); + + // Setup workspace to pull from + classHelloWorld = TestClassUtils.fromRuntimeClass(HelloWorld.class); + workspace = TestClassUtils.fromBundle(TestClassUtils.fromClasses(classHelloWorld)); workspaceManager.setCurrent(workspace); } + @BeforeEach + void setupEach() { + // We don't want to cache decompilations for this test, but we also + // do not want to edit the shared config in tests. + // Thus, we will make new config instances each test run so there's no cross-test pollution. + decompilerManagerConfig = new DecompilerManagerConfig(); + decompilerManagerConfig.getCacheDecompilations().setValue(false); + assertDoesNotThrow(() -> ReflectUtil.quietSet(unwrapProxy(decompilerManager), + DecompilerManager.class.getDeclaredField("config"), + decompilerManagerConfig)); + } + @Test void testCfr() { JvmDecompiler decompiler = decompilerManager.getJvmDecompiler(CfrDecompiler.NAME); @@ -81,7 +101,7 @@ void testFiltersUsed() { decompilerManager.addOutputTextFilter(textFilterSpy); // Decompile - decompilerManager.decompile(decompiler, workspace, classToDecompile).get(); + decompilerManager.decompile(decompiler, workspace, classHelloWorld).get(); // Verify each filter was called once verify(bytecodeFilterSpy, times(1)).filter(any(), any(), any()); @@ -94,11 +114,64 @@ void testFiltersUsed() { } } + @Test + void testCaching() { + decompilerManagerConfig.getCacheDecompilations().setValue(true); + JvmDecompiler decompiler = decompilerManager.getJvmDecompiler(CfrDecompiler.NAME); + DecompileResult firstResult = assertDoesNotThrow(() -> decompilerManager.decompile(decompiler, workspace, classHelloWorld).get(1, TimeUnit.DAYS)); + + // Assert that repeated decompiles use the same result (caching, should be handled by abstract base) + // Only the manager will cache results. Using decompilers direcrly will not cache. + assertTrue(decompilerManagerConfig.getCacheDecompilations().getValue(), "Cache config not 'true'"); + DecompileResult newResult = assertDoesNotThrow(() -> decompilerManager.decompile(decompiler, workspace, classHelloWorld).get(1, TimeUnit.SECONDS)); + assertSame(firstResult, newResult, "Decompiler did not cache results"); + + // Change the decompiler hash. The decompiler result should change. + decompiler.getConfig().setHash(-1); + newResult = assertDoesNotThrow(() -> decompilerManager.decompile(decompiler, workspace, classHelloWorld).get(1, TimeUnit.SECONDS)); + assertNotSame(firstResult, newResult, "Decompiler used cached result even though config hash changed"); + + // Verify direct decompiler usage does not cache + DecompileResult direct1 = decompiler.decompile(workspace, classHelloWorld); + DecompileResult direct2 = decompiler.decompile(workspace, classHelloWorld); + assertNotSame(direct1, direct2, "Direct decompiler use cached results unexpectedly"); + } + + @Test + void testFilterHollow() { + String decompilationBefore = assertDoesNotThrow(() -> decompilerManager.decompile(workspace, classHelloWorld).get().getText()); + assertTrue(decompilationBefore.contains("\"Hello world\"")); + + decompilerManagerConfig.getFilterHollow().setValue(true); + + // Hollowing will remove method bodies, so the 'println' call should no longer exist in the output + String decompilationAfter = assertDoesNotThrow(() -> decompilerManager.decompile(workspace, classHelloWorld).get().getText()); + assertFalse(decompilationAfter.contains("\"Hello world\"")); + } + + @Test + void testDisplay() { + for (JvmDecompiler decompiler : decompilerManager.getJvmDecompilers()) { + assertTrue(decompiler.toString().contains(decompiler.getName())); + assertTrue(decompiler.toString().contains(decompiler.getVersion())); + } + } + + @Test + void testComparison() { + JvmDecompiler cfr = decompilerManager.getJvmDecompiler(CfrDecompiler.NAME); + JvmDecompiler pro = decompilerManager.getJvmDecompiler(ProcyonDecompiler.NAME); + assertNotNull(cfr); + assertNotNull(pro); + assertNotEquals(cfr, pro); + assertNotEquals(cfr.hashCode(), pro.hashCode()); + } + private static void runJvmDecompilation(@Nonnull JvmDecompiler decompiler) { try { // Generally, you'd handle results like this, with a when-complete. // The blocking 'get' at the end is just so our test works. - DecompileResult firstResult = decompilerManager.decompile(decompiler, workspace, classToDecompile) + DecompileResult firstResult = decompilerManager.decompile(decompiler, workspace, classHelloWorld) .whenComplete((result, throwable) -> { assertNull(throwable); @@ -107,23 +180,12 @@ private static void runJvmDecompilation(@Nonnull JvmDecompiler decompiler) { assertNotNull(result.getText(), "Decompile result missing text"); assertTrue(result.getText().contains("\"Hello world\""), "Decompilation seems to be wrong"); }) // Block on this thread until we have the value. - .get(1, TimeUnit.DAYS); - - // Assert that repeated decompiles use the same result (caching, should be handled by abstract base) - // Only the manager will cache results. Using decompilers direcrly will not cache. - assertTrue(decompilerManager.getServiceConfig().getCacheDecompilations().getValue(), "Default cache config not 'true'"); - DecompileResult newResult = decompilerManager.decompile(decompiler, workspace, classToDecompile).get(1, TimeUnit.SECONDS); - assertSame(firstResult, newResult, "Decompiler did not cache results"); - - // Change the decompiler hash. The decompiler result should change. - decompiler.getConfig().setHash(-1); - newResult = decompilerManager.decompile(decompiler, workspace, classToDecompile).get(1, TimeUnit.SECONDS); - assertNotSame(firstResult, newResult, "Decompiler used cached result even though config hash changed"); + .get(5, TimeUnit.SECONDS); // Verify direct decompiler usage does not cache - DecompileResult direct1 = decompiler.decompile(workspace, classToDecompile); - DecompileResult direct2 = decompiler.decompile(workspace, classToDecompile); - assertNotSame(direct1, direct2, "Direct decompiler use cached results unexpectedly"); + DecompileResult result = decompiler.decompile(workspace, classHelloWorld); + assertNull(result.getException(), "No exceptions should be reported during decompilation"); + assertNotNull(result.getText(), "Missing decompilation output"); } catch (InterruptedException e) { fail("Decompile was interrupted", e); } catch (ExecutionException e) { @@ -148,5 +210,4 @@ public String filter(@Nonnull Workspace workspace, @Nonnull ClassInfo classInfo, return code; } } - } diff --git a/recaf-core/src/test/java/software/coley/recaf/services/decompile/FallbackDecompilerTest.java b/recaf-core/src/test/java/software/coley/recaf/services/decompile/FallbackDecompilerTest.java new file mode 100644 index 000000000..7c5292646 --- /dev/null +++ b/recaf-core/src/test/java/software/coley/recaf/services/decompile/FallbackDecompilerTest.java @@ -0,0 +1,94 @@ +package software.coley.recaf.services.decompile; + + +import jakarta.annotation.Nonnull; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import software.coley.recaf.services.decompile.fallback.print.ClassPrinter; +import software.coley.recaf.services.text.TextFormatConfig; +import software.coley.recaf.test.TestClassUtils; +import software.coley.recaf.test.dummy.AccessibleFields; +import software.coley.recaf.test.dummy.ClassWithAnnotation; +import software.coley.recaf.test.dummy.ClassWithExceptions; +import software.coley.recaf.test.dummy.ClassWithStaticInit; +import software.coley.recaf.test.dummy.DummyEmptyMap; +import software.coley.recaf.test.dummy.DummyEnum; +import software.coley.recaf.test.dummy.InvisAnnotationImpl; +import software.coley.recaf.util.ClasspathUtil; +import software.coley.recaf.workspace.model.bundle.BasicJvmClassBundle; + +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class FallbackDecompilerTest { + static TextFormatConfig textConfig = new TextFormatConfig(); + + public static Stream penis() { + return ClasspathUtil.getSystemClassSet().stream(); + } + + @Test + void fieldModifiers() { + String decompile = decompile(AccessibleFields.class); + assertTrue(decompile.contains("public static final int CONSTANT_FIELD = 16;")); + assertTrue(decompile.contains("private final int privateFinalField = 8;")); + assertTrue(decompile.contains("protected final int protectedField = 4;")); + assertTrue(decompile.contains("public final int publicField = 2;")); + assertTrue(decompile.contains("final int packageField = 1;")); + } + + @Test + void classAnnotation() { + String decompile = decompile(ClassWithAnnotation.class); + assertTrue(decompile.contains("@AnnotationImpl(value = \"Hello\", policy = @Retention(RetentionPolicy.CLASS))")); + } + + @Test + void annotationClass() { + String decompile = decompile(InvisAnnotationImpl.class); + assertTrue(decompile.contains(""" + @Retention(RetentionPolicy.CLASS) + @Target({ ElementType.TYPE, ElementType.FIELD, ElementType.METHOD }) + public @interface InvisAnnotationImpl + """)); + } + + @Test + void throwsException() { + String decompile = decompile(ClassWithExceptions.class); + assertTrue(decompile.contains("static int readInt(Object input) throws NumberFormatException")); + } + + @Test + void clinit() { + String decompile = decompile(ClassWithStaticInit.class); + assertTrue(decompile.contains("\n static {\n")); + } + + @Test + void enumFields() { + String decompile = decompile(DummyEnum.class); + assertTrue(decompile.contains(" ONE,")); + assertTrue(decompile.contains(" TWO,")); + assertTrue(decompile.contains(" THREE;")); + assertTrue(decompile.contains("private static final /* synthetic */ DummyEnum[] $VALUES;")); + } + + + @Test + @Disabled("Need to implement signature parsing in the fallback decompiler") + void genericClassArgs() { + String decompile = decompile(DummyEmptyMap.class); + assertTrue(decompile.contains("class DummyEmptyMap implements Map {")); + } + + @Nonnull + private static String decompile(@Nonnull Class cls) { + return assertDoesNotThrow(() -> { + BasicJvmClassBundle bundle = TestClassUtils.fromClasses(cls); + return new ClassPrinter(textConfig, bundle.get(cls.getName().replace('.', '/'))).print(); + }); + } +} \ No newline at end of file