From 845cb14a1f33841d2444ff9567b661633b03bab7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 16 Jan 2019 12:23:57 +0100 Subject: [PATCH] Highlight name occurences of methods --- .../fxdesigner/MainDesignerController.java | 24 +++++++++++++------ .../fxdesigner/NodeInfoPanelController.java | 4 +++- .../fxdesigner/SourceEditorController.java | 7 +++--- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java index 662f9a1d94..5941a3d853 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java @@ -217,9 +217,9 @@ public class MainDesignerController extends AbstractController { * Executed when the user selects a node in a treeView or listView. */ public void onNodeItemSelected(Node selectedValue) { - nodeInfoPanelController.setFocusNode(selectedValue); - sourceEditorController.setFocusNode(selectedValue); - sourceEditorController.focusNodeInTreeView(selectedValue); + // doing that in parallel speeds it up + Platform.runLater(()-> nodeInfoPanelController.setFocusNode(selectedValue)); + Platform.runLater(()-> sourceEditorController.setFocusNode(selectedValue)); } @@ -231,19 +231,29 @@ public class MainDesignerController extends AbstractController { public void onNameDeclarationSelected(NameDeclaration declaration) { Platform.runLater(() -> { + // TODO highlight usages of regular node selection and move that logic to nodeInfoPanelController.setFocusNode + // In fact I think the current symbol table is too low level for that. You + // can map a NameDeclaration to its node but not the reverse... sourceEditorController.clearNameOccurences(); List occurrences = declaration.getNode().getScope().getDeclarations().get(declaration); + if (occurrences == null) { + // For MethodNameDeclaration the scope is the method scope, which is not the scope it is declared + // in but the scope it declares! That means that getDeclarations().get(declaration) returns null + // and no name occurrences are found. We thus look in the parent, but ultimately the name occurrence + // finder is broken since it can't find e.g. the use of a method in another scope. Plus in case of + // overloads both overloads are reported to have a usage. + // Plus this is some serious law of Demeter breaking there... + occurrences = declaration.getNode().getScope().getParent().getDeclarations().get(declaration); + } + if (occurrences != null) { sourceEditorController.highlightNameOccurrences(occurrences); } }); - Platform.runLater(() -> { - sourceEditorController.setFocusNode(declaration.getNode()); - sourceEditorController.focusNodeInTreeView(declaration.getNode()); - }); + Platform.runLater(() -> sourceEditorController.setFocusNode(declaration.getNode())); } /** diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java index bd0f321b81..11c91d0123 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java @@ -17,6 +17,8 @@ import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.xpath.Attribute; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.symboltable.NameDeclaration; +import net.sourceforge.pmd.lang.symboltable.NameOccurrence; +import net.sourceforge.pmd.lang.symboltable.ScopedNode; import net.sourceforge.pmd.util.fxdesigner.model.MetricEvaluator; import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; import net.sourceforge.pmd.util.fxdesigner.util.AbstractController; @@ -152,7 +154,7 @@ public class NodeInfoPanelController extends AbstractController { scopeHierarchyTreeView.setRoot(rootScope); if (focusScopeView && previousSelection != null) { - // Try to find the node that was previously selected and focus it in the new ascendant hierarchy + // Try to find the node that was previously selected and focus it in the new ascendant hierarchy. // Otherwise, when you select a node in the scope tree, since focus of the app is shifted to that // node, the scope hierarchy is reset and you lose the selection - even though obviously the node // you selected is in its own scope hierarchy so it looks buggy. diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java index 4b76e41638..d7a0170339 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java @@ -51,7 +51,6 @@ import net.sourceforge.pmd.util.fxdesigner.util.controls.TreeViewWrapper; import javafx.application.Platform; import javafx.css.PseudoClass; import javafx.fxml.FXML; -import javafx.scene.control.Label; import javafx.scene.control.MenuButton; import javafx.scene.control.RadioMenuItem; import javafx.scene.control.SelectionModel; @@ -249,7 +248,7 @@ public class SourceEditorController extends AbstractController { /** Clears the name occurences. */ public void clearNameOccurences() { - codeEditorArea.clearStyleLayer(StyleLayerIds.ERROR); + codeEditorArea.clearStyleLayer(StyleLayerIds.NAME_OCCURENCE); } @@ -268,6 +267,8 @@ public class SourceEditorController extends AbstractController { return; } + Platform.runLater(() -> focusNodeInTreeView(node)); + codeEditorArea.styleNodes(node == null ? emptyList() : singleton(node), StyleLayerIds.FOCUS, true); if (node != null) { @@ -324,7 +325,7 @@ public class SourceEditorController extends AbstractController { } - public void focusNodeInTreeView(Node node) { + private void focusNodeInTreeView(Node node) { SelectionModel> selectionModel = astTreeView.getSelectionModel(); // node is different from the old one