Merge remote-tracking branch

'origin/GP-6326-dragonmacher-symbol-tree-convert-ns-bug--SQUASHED'
(Closes #8869)

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/actions/ConvertToClassAction.java

@@ -4,9 +4,9 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -71,20 +71,22 @@ public class ConvertToClassAction extends SymbolTreeContextAction {
 
 		Symbol symbol = ((SymbolNode) node).getSymbol();
 		Namespace namespace = (Namespace) symbol.getObject();
-		if (namespace != null) {
+		if (namespace == null) {
-			String name = namespace.getName();
+			return;
-			convertToClass(program, namespace);
-			program.flushEvents();
-
-			GTreeNode classesNode = root.getChild(SymbolCategory.CLASS_CATEGORY.getName());
-			if (classesNode != null) {
-				context.getSymbolTree().startEditing(classesNode, name);
-			}
-			else {
-				Msg.showInfo(this, null, "Classes Filtered Out of View",
-					"New class node is filtered out of view");
-			}
 		}
+
+		String name = namespace.getName();
+		convertToClass(program, namespace);
+		program.flushEvents();
+
+		GTreeNode classesNode = root.getChild(SymbolCategory.CLASS_CATEGORY.getName());
+		if (classesNode == null) {
+			Msg.showInfo(this, null, "Classes Filtered Out of View",
+				"New class node is filtered out of view");
+			return;
+		}
+
+		tree.startEditing(classesNode, name);
 	}
 
 	private static void convertToClass(Program program, Namespace ns) {

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/actions/CreateClassAction.java

@@ -4,9 +4,9 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -64,19 +64,21 @@ public class CreateClassAction extends SymbolTreeContextAction {
 		if (object instanceof ClassCategoryNode) {
 			return true;
 		}
-		else if (object instanceof SymbolNode) {
+		if (!(object instanceof SymbolNode symbolNode)) {
-			SymbolNode symbolNode = (SymbolNode) object;
+			return false;
-			Symbol symbol = symbolNode.getSymbol();
-			SymbolType symbolType = symbol.getSymbolType();
-			if (symbolType == SymbolType.NAMESPACE) {
-				// allow SymbolType to perform additional checks
-				Namespace parentNamespace = (Namespace) symbol.getObject();
-				return SymbolType.CLASS.isValidParent(context.getProgram(), parentNamespace,
-					Address.NO_ADDRESS, parentNamespace.isExternal());
-			}
-			return (symbolType == SymbolType.CLASS || symbolType == SymbolType.LIBRARY);
 		}
-		return false;
+
+		Symbol symbol = symbolNode.getSymbol();
+		SymbolType symbolType = symbol.getSymbolType();
+		if (symbolType == SymbolType.NAMESPACE) {
+			Namespace parent = (Namespace) symbol.getObject();
+			if (parent == null) {
+				return false; // the symbol has been deleted, but the tree has not updated
+			}
+			return SymbolType.CLASS.isValidParent(context.getProgram(), parent,
+				Address.NO_ADDRESS, parent.isExternal());
+		}
+		return (symbolType == SymbolType.CLASS || symbolType == SymbolType.LIBRARY);
 	}
 
 	private void createNewClass(SymbolTreeActionContext context) {
@@ -85,7 +87,6 @@ public class CreateClassAction extends SymbolTreeContextAction {
 		Program program = context.getProgram();
 		Namespace parent = program.getGlobalNamespace();
 		GTreeNode node = (GTreeNode) selectionPaths[0].getLastPathComponent();
-
 		if (node instanceof SymbolNode) {
 			Symbol symbol = ((SymbolNode) node).getSymbol();
 			parent = (Namespace) symbol.getObject();
@@ -99,14 +100,16 @@ public class CreateClassAction extends SymbolTreeContextAction {
 			// error occurred
 			return;
 		}
+
 		program.flushEvents();
 		context.getSymbolTree().startEditing(node, newClassName);
 	}
 
 	private String createClass(Program program, Namespace parent) {
-		String newClassName = "NewClass";
+
-		int transactionID = program.startTransaction("Create Class");
+		return program.withTransaction("Create Class", () -> {
-		try {
+
+			String newClassName = "NewClass";
 			SymbolTable symbolTable = program.getSymbolTable();
 			int oneUp = 0;
 			Namespace namespace = null;
@@ -123,11 +126,8 @@ public class CreateClassAction extends SymbolTreeContextAction {
 					return null;
 				}
 			}
-		}
-		finally {
-			program.endTransaction(transactionID, true);
-		}
 
-		return newClassName;
+			return newClassName;
+		});
 	}
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/ClassCategoryNode.java

@@ -146,17 +146,18 @@ public class ClassCategoryNode extends SymbolCategoryNode {
 		}
 
 		SymbolNode key = SymbolNode.createKeyNode(symbol, oldName, program);
-		Namespace parentNs = symbol.getParentNamespace();
+
-		if (parentNs == globalNamespace) {
+		// See if the class lives on this Classes node. This can happen when the class is the child
-			// no need to search for the class in the tree; the class only lives at the top
+		// of a non-class namespace.
-			GTreeNode symbolNode = findNode(this, key, false, monitor);
+		GTreeNode symbolNode = findNode(this, key, false, monitor);
-			if (symbolNode != null) {
+		if (symbolNode != null) {
-				removeNode(symbolNode);
+			removeNode(symbolNode);
-			}
 			return;
 		}
 
-		// set getAllClassNodes() for a description of the map
+		// We could not find the node. See if it is under another class node.
+		// (See getAllClassNodes() for a description of the map.)
+		Namespace parentNs = symbol.getParentNamespace();
 		Map<GTreeNode, List<Namespace>> classNodes = getAllClassNodes(symbol, parentNs, monitor);
 		removeSymbol(key, classNodes, monitor);
 	}
@@ -175,12 +176,12 @@ public class ClassCategoryNode extends SymbolCategoryNode {
 			// parent for the given symbol
 			GTreeNode classNode = entry.getKey();
 			List<Namespace> parentPath = entry.getValue();
-			GTreeNode symbolParent =
+			GTreeNode symbolParent = getNamespaceNode(classNode, parentPath, false, monitor);
-				getNamespaceNode(classNode, parentPath, false, monitor);
+			if (symbolParent == null) {
-			GTreeNode symbolNode = findNode(symbolParent, key, false, monitor);
+				continue;
-			if (symbolParent != null) {
-				symbolParent.removeNode(symbolNode);
 			}
+			GTreeNode symbolNode = findNode(symbolParent, key, false, monitor);
+			symbolParent.removeNode(symbolNode);
 		}
 	}
 

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/symboltree/SymbolTreePlugin2Test.java

@@ -316,6 +316,55 @@ public class SymbolTreePlugin2Test extends AbstractGhidraHeadedIntegrationTest {
 		waitForCondition(tree::isEditing);
 	}
 
+	@Test
+	public void testClassNestedUnderNonClassNamespace_RenameClass() throws Exception {
+
+		/*
+		
+		 	The Classes folder flattens classes so every class appears at the top level.  Because
+		 	users can expand classes, top level classes may also appear nested under other classes.
+		 	
+		 	Classes
+		 		Class1
+		 		
+		 	Namespaces		 	
+		 		FooNs
+		 			Class1
+		 */
+
+		Namespace fooNs = createNamespace("FooNs");
+		GhidraClass class1 = createClass(fooNs, "Class1");
+
+		expandClasses();
+		expandNamesapces();
+
+		//@formatter:off
+		assertNamespaceNodes(
+			"FooNs::Class1"
+		);
+		//@formatter:on
+
+		//@formatter:off
+		assertClassNodes(
+			"Class1"
+		);
+		//@formatter:on
+
+		renameSymbol(class1.getSymbol(), "Class1.renamed");
+
+		//@formatter:off
+		assertNamespaceNodes(
+			"FooNs::Class1.renamed"
+		);
+		//@formatter:on
+
+		//@formatter:off
+		assertClassNodes(
+			"Class1.renamed"
+		);
+		//@formatter:on
+	}
+
 	@Test
 	public void testClassCategoryDuplicates_NestedClass_RenameLabel() throws Exception {
 
@@ -733,7 +782,8 @@ public class SymbolTreePlugin2Test extends AbstractGhidraHeadedIntegrationTest {
 		convertToClassAction = getAction(plugin, "Convert to Class");
 		assertNotNull(convertToClassAction);
 
-		navigateIncomingAction = (ToggleDockingAction) getAction(plugin, NavigateOnIncomingAction.NAME);
+		navigateIncomingAction =
+			(ToggleDockingAction) getAction(plugin, NavigateOnIncomingAction.NAME);
 		assertNotNull(navigateIncomingAction);
 	}
 

Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/internal/GTreeModel.java

@@ -111,6 +111,10 @@ public class GTreeModel implements TreeModel {
 
 	@Override
 	public void valueForPathChanged(TreePath path, Object newValue) {
+		if (path == null) {
+			// this can happen when we try to edit a node and it doesn't work due to filtering
+			return;
+		}
 		GTreeNode node = (GTreeNode) path.getLastPathComponent();
 		node.valueChanged(newValue);
 	}

Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/tasks/GTreeStartEditingTask.java

@@ -31,12 +31,12 @@ import ghidra.util.task.TaskMonitor;
 public class GTreeStartEditingTask extends GTreeTask {
 
 	private final GTreeNode modelParent;
-	private final GTreeNode editNode;
+	private final GTreeNode modelEditNode;
 
 	public GTreeStartEditingTask(GTree gTree, JTree jTree, GTreeNode editNode) {
 		super(gTree);
 		this.modelParent = tree.getModelNode(editNode.getParent());
-		this.editNode = editNode;
+		this.modelEditNode = tree.getModelNode(editNode);
 	}
 
 	@Override
@@ -55,13 +55,16 @@ public class GTreeStartEditingTask extends GTreeTask {
 	}
 
 	private void edit() {
-		TreePath path = editNode.getTreePath();
+
+		GTreeNode viewEditNode = tree.getViewNode(modelEditNode);
+
+		TreePath path = viewEditNode.getTreePath();
 		CellEditor cellEditor = tree.getCellEditor();
 		cellEditor.addCellEditorListener(new CellEditorListener() {
 			@Override
 			public void editingCanceled(ChangeEvent e) {
 				cellEditor.removeCellEditorListener(this);
-				tree.setSelectedNode(editNode); // reselect the node on cancel
+				tree.setSelectedNode(viewEditNode); // reselect the node on cancel
 			}
 
 			@Override
@@ -71,7 +74,7 @@ public class GTreeStartEditingTask extends GTreeTask {
 
 				// NOTE: there may be cases where this node search fails to correctly
 				// identify the renamed node when name and node class is insufficient to match.
-				Class<?> nodeClass = editNode.getClass();
+				Class<?> nodeClass = viewEditNode.getClass();
 				Predicate<GTreeNode> nodeMatches = n -> {
 					return nodeClass == n.getClass() && n.getName().equals(newName);
 				};
@@ -82,7 +85,7 @@ public class GTreeStartEditingTask extends GTreeTask {
 			}
 		});
 
-		tree.setNodeEditable(editNode);
+		tree.setNodeEditable(viewEditNode);
 		jTree.startEditingAtPath(path);
 
 	}