From 570e7528a7eb084ee202551f42fafe31e7d37f25 Mon Sep 17 00:00:00 2001 From: Skylot Date: Sun, 14 Nov 2021 13:05:22 +0000 Subject: [PATCH] fix(gui): use correct definition position on jump after code reload (#1273) --- .../src/main/java/jadx/api/JadxDecompiler.java | 12 ++++++++---- .../src/main/java/jadx/gui/treemodel/JClass.java | 10 ++++++++-- jadx-gui/src/main/java/jadx/gui/ui/MainWindow.java | 3 +-- jadx-gui/src/main/java/jadx/gui/ui/TabbedPane.java | 5 ++++- .../main/java/jadx/gui/ui/codearea/CodeArea.java | 5 ++--- .../main/java/jadx/gui/ui/dialog/RenameDialog.java | 10 +++------- .../main/java/jadx/gui/ui/dialog/UsageDialog.java | 2 +- .../src/main/java/jadx/gui/utils/JNodeCache.java | 14 ++++++++++++-- .../src/main/java/jadx/gui/utils/JumpPosition.java | 11 +---------- 9 files changed, 40 insertions(+), 32 deletions(-) diff --git a/jadx-core/src/main/java/jadx/api/JadxDecompiler.java b/jadx-core/src/main/java/jadx/api/JadxDecompiler.java index 3e9678ab5..2717fd88e 100644 --- a/jadx-core/src/main/java/jadx/api/JadxDecompiler.java +++ b/jadx-core/src/main/java/jadx/api/JadxDecompiler.java @@ -419,7 +419,11 @@ public final class JadxDecompiler implements Closeable { * Get JavaClass by ClassNode without loading and decompilation */ JavaClass convertClassNode(ClassNode cls) { - return classesMap.computeIfAbsent(cls, node -> { + return classesMap.compute(cls, (node, prevJavaCls) -> { + if (prevJavaCls != null && prevJavaCls.getClassNode() == cls) { + // keep previous variable + return prevJavaCls; + } if (cls.isInner()) { return new JavaClass(cls, convertClassNode(cls.getParentClass())); } @@ -431,7 +435,7 @@ public final class JadxDecompiler implements Closeable { @ApiStatus.Internal public JavaClass getJavaClassByNode(ClassNode cls) { JavaClass javaClass = classesMap.get(cls); - if (javaClass != null) { + if (javaClass != null && javaClass.getClassNode() == cls) { return javaClass; } // load parent class if inner @@ -461,7 +465,7 @@ public final class JadxDecompiler implements Closeable { @Nullable private JavaMethod getJavaMethodByNode(MethodNode mth) { JavaMethod javaMethod = methodsMap.get(mth); - if (javaMethod != null) { + if (javaMethod != null && javaMethod.getMethodNode() == mth) { return javaMethod; } if (mth.contains(AFlag.DONT_GENERATE)) { @@ -486,7 +490,7 @@ public final class JadxDecompiler implements Closeable { @Nullable private JavaField getJavaFieldByNode(FieldNode fld) { JavaField javaField = fieldsMap.get(fld); - if (javaField != null) { + if (javaField != null && javaField.getFieldNode() == fld) { return javaField; } // parent class not loaded yet diff --git a/jadx-gui/src/main/java/jadx/gui/treemodel/JClass.java b/jadx-gui/src/main/java/jadx/gui/treemodel/JClass.java index 09cd87427..394c366c6 100644 --- a/jadx-gui/src/main/java/jadx/gui/treemodel/JClass.java +++ b/jadx-gui/src/main/java/jadx/gui/treemodel/JClass.java @@ -17,6 +17,7 @@ import jadx.core.dex.info.AccessInfo; import jadx.gui.ui.TabbedPane; import jadx.gui.ui.codearea.ClassCodeContentPanel; import jadx.gui.ui.panel.ContentPanel; +import jadx.gui.utils.CacheObject; import jadx.gui.utils.NLS; import jadx.gui.utils.UiUtils; @@ -71,13 +72,18 @@ public class JClass extends JLoadableNode implements Comparable { update(); } - public synchronized void reload() { + public synchronized void reload(CacheObject cache) { + cache.getNodeCache().removeWholeClass(cls); + cache.getIndexService().remove(cls); cls.reload(); loaded = true; update(); + cache.getIndexService().indexCls(cls); } - public synchronized void unload() { + public synchronized void unload(CacheObject cache) { + cache.getNodeCache().removeWholeClass(cls); + cache.getIndexService().remove(cls); cls.unload(); loaded = false; } diff --git a/jadx-gui/src/main/java/jadx/gui/ui/MainWindow.java b/jadx-gui/src/main/java/jadx/gui/ui/MainWindow.java index ce1b49024..f92a56fa0 100644 --- a/jadx-gui/src/main/java/jadx/gui/ui/MainWindow.java +++ b/jadx-gui/src/main/java/jadx/gui/ui/MainWindow.java @@ -254,8 +254,7 @@ public class MainWindow extends JFrame { NLS.str("error_dialog.title"), JOptionPane.ERROR_MESSAGE); return; } - JNode node = cacheObject.getNodeCache().makeFrom(javaNode); - tabbedPane.codeJump(new JumpPosition(node.getRootClass(), node.getLine(), JumpPosition.getDefPos(node))); + tabbedPane.codeJump(cacheObject.getNodeCache().makeFrom(javaNode)); } } diff --git a/jadx-gui/src/main/java/jadx/gui/ui/TabbedPane.java b/jadx-gui/src/main/java/jadx/gui/ui/TabbedPane.java index 4bfe54fe6..0e9282701 100644 --- a/jadx-gui/src/main/java/jadx/gui/ui/TabbedPane.java +++ b/jadx-gui/src/main/java/jadx/gui/ui/TabbedPane.java @@ -218,8 +218,11 @@ public class TabbedPane extends JTabbedPane { } } + /** + * Jump to node definition + */ public void codeJump(JNode node) { - codeJump(new JumpPosition(node)); + codeJump(new JumpPosition(Objects.requireNonNull(node))); } public void codeJump(JumpPosition pos) { diff --git a/jadx-gui/src/main/java/jadx/gui/ui/codearea/CodeArea.java b/jadx-gui/src/main/java/jadx/gui/ui/codearea/CodeArea.java index 70eefab36..81f424f76 100644 --- a/jadx-gui/src/main/java/jadx/gui/ui/codearea/CodeArea.java +++ b/jadx-gui/src/main/java/jadx/gui/ui/codearea/CodeArea.java @@ -171,7 +171,7 @@ public final class CodeArea extends AbstractCodeArea { return null; } JNode jNode = convertJavaNode(foundNode); - return new JumpPosition(jNode.getRootClass(), pos.getLine(), JumpPosition.getDefPos(jNode)); + return new JumpPosition(jNode.getRootClass(), pos); } private JNode convertJavaNode(JavaNode javaNode) { @@ -252,8 +252,7 @@ public final class CodeArea extends AbstractCodeArea { CaretPositionFix caretFix = new CaretPositionFix(this); caretFix.save(); - cls.reload(); - getMainWindow().getCacheObject().getIndexService().refreshIndex(cls.getCls()); + cls.reload(getMainWindow().getCacheObject()); ClassCodeContentPanel codeContentPanel = (ClassCodeContentPanel) this.contentPanel; codeContentPanel.getTabbedPane().refresh(cls); diff --git a/jadx-gui/src/main/java/jadx/gui/ui/dialog/RenameDialog.java b/jadx-gui/src/main/java/jadx/gui/ui/dialog/RenameDialog.java index c87bed366..deff31b23 100644 --- a/jadx-gui/src/main/java/jadx/gui/ui/dialog/RenameDialog.java +++ b/jadx-gui/src/main/java/jadx/gui/ui/dialog/RenameDialog.java @@ -42,7 +42,6 @@ import jadx.core.dex.nodes.RootNode; import jadx.core.dex.visitors.rename.RenameVisitor; import jadx.core.utils.Utils; import jadx.core.utils.exceptions.JadxRuntimeException; -import jadx.gui.jobs.IndexService; import jadx.gui.jobs.TaskStatus; import jadx.gui.settings.JadxProject; import jadx.gui.treemodel.JClass; @@ -222,14 +221,12 @@ public class RenameDialog extends JDialog { } private void refreshClasses(Set updatedTopClasses) { - IndexService indexService = cache.getIndexService(); if (updatedTopClasses.size() < 10) { // small batch => reload LOG.debug("Classes to reload: {}", updatedTopClasses.size()); for (JClass cls : updatedTopClasses) { try { - cls.reload(); - indexService.refreshIndex(cls.getCls()); + cls.reload(cache); } catch (Exception e) { LOG.error("Failed to reload class: {}", cls.getFullName(), e); } @@ -237,11 +234,10 @@ public class RenameDialog extends JDialog { } else { // big batch => unload LOG.debug("Classes to unload: {}", updatedTopClasses.size()); - indexService.setComplete(false); + cache.getIndexService().setComplete(false); for (JClass cls : updatedTopClasses) { try { - cls.unload(); - indexService.remove(cls.getCls()); + cls.unload(cache); } catch (Exception e) { LOG.error("Failed to unload class: {}", cls.getFullName(), e); } diff --git a/jadx-gui/src/main/java/jadx/gui/ui/dialog/UsageDialog.java b/jadx-gui/src/main/java/jadx/gui/ui/dialog/UsageDialog.java index 23e3ef8a8..bc5bd2fba 100644 --- a/jadx-gui/src/main/java/jadx/gui/ui/dialog/UsageDialog.java +++ b/jadx-gui/src/main/java/jadx/gui/ui/dialog/UsageDialog.java @@ -45,7 +45,6 @@ public class UsageDialog extends CommonSearchDialog { @Override protected void openInit() { - usageList = new ArrayList<>(); mainWindow.getBackgroundExecutor().execute(NLS.str("progress.load"), this::collectUsageData, (status) -> { @@ -59,6 +58,7 @@ public class UsageDialog extends CommonSearchDialog { } private void collectUsageData() { + usageList = new ArrayList<>(); node.getJavaNode().getUseIn() .stream() .map(JavaNode::getTopParentClass) diff --git a/jadx-gui/src/main/java/jadx/gui/utils/JNodeCache.java b/jadx-gui/src/main/java/jadx/gui/utils/JNodeCache.java index b4166414a..06342bf83 100644 --- a/jadx-gui/src/main/java/jadx/gui/utils/JNodeCache.java +++ b/jadx-gui/src/main/java/jadx/gui/utils/JNodeCache.java @@ -25,7 +25,7 @@ public class JNodeCache { } // don't use 'computeIfAbsent' method here, it this cause 'Recursive update' exception JNode jNode = cache.get(javaNode); - if (jNode == null) { + if (jNode == null || jNode.getJavaNode() != javaNode) { jNode = convert(javaNode); cache.put(javaNode, jNode); } @@ -37,13 +37,23 @@ public class JNodeCache { return null; } JClass jCls = (JClass) cache.get(javaCls); - if (jCls == null) { + if (jCls == null || jCls.getCls() != javaCls) { jCls = convert(javaCls); cache.put(javaCls, jCls); } return jCls; } + public void remove(JavaNode javaNode) { + cache.remove(javaNode); + } + + public void removeWholeClass(JavaClass javaCls) { + remove(javaCls); + javaCls.getMethods().forEach(this::remove); + javaCls.getFields().forEach(this::remove); + } + private JClass convert(JavaClass cls) { JavaClass parentCls = cls.getDeclaringClass(); if (parentCls == cls) { diff --git a/jadx-gui/src/main/java/jadx/gui/utils/JumpPosition.java b/jadx-gui/src/main/java/jadx/gui/utils/JumpPosition.java index 383a74aaa..dbc8c8f02 100644 --- a/jadx-gui/src/main/java/jadx/gui/utils/JumpPosition.java +++ b/jadx-gui/src/main/java/jadx/gui/utils/JumpPosition.java @@ -3,7 +3,6 @@ package jadx.gui.utils; import java.util.Objects; import jadx.api.CodePosition; -import jadx.api.JavaNode; import jadx.gui.treemodel.JNode; public class JumpPosition { @@ -41,14 +40,6 @@ public class JumpPosition { return line; } - public static int getDefPos(JNode node) { - JavaNode javaNode = node.getJavaNode(); - if (javaNode == null) { - return -1; - } - return javaNode.getDefPos(); - } - @Override public boolean equals(Object obj) { if (this == obj) { @@ -58,7 +49,7 @@ public class JumpPosition { return false; } JumpPosition position = (JumpPosition) obj; - return line == position.line && node.equals(position.node) && pos == position.pos; + return line == position.line && pos == position.pos && node.equals(position.node); } @Override