Merge branch 'GP-205-dragonmacher-structure-edit-action-fix' into Ghidra_9.2

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/EditComponentAction.java

@@ -16,6 +16,7 @@
 package ghidra.app.plugin.core.compositeeditor;
 
 import docking.ActionContext;
+import ghidra.app.plugin.core.datamgr.util.DataTypeUtils;
 import ghidra.app.services.DataTypeManagerService;
 import ghidra.program.model.data.*;
 import ghidra.program.model.data.Enum;
@@ -43,29 +44,43 @@ public class EditComponentAction extends CompositeEditorTableAction {
 	@Override
 	public void actionPerformed(ActionContext context) {
 		int row = model.getRow();
-		if (row < model.getNumComponents()) {
+		if (row >= model.getNumComponents()) {
-			DataTypeComponent comp = model.getComponent(row);
+			requestTableFocus();
-			DataType dt = DataTypeHelper.getBaseType(comp.getDataType());
+			return;
-			if ((dt instanceof Structure) || (dt instanceof Union) || (dt instanceof Enum)) {
+		}
-				DataTypeManager dtm = model.getOriginalDataTypeManager();
+
-				if (dtm != null) {
+		DataTypeComponent comp = model.getComponent(row);
-					dt = dtm.getDataType(dt.getDataTypePath());
+		DataType clickedType = comp.getDataType();
-					if (dt != null) {
+		DataType dt = DataTypeUtils.getBaseDataType(clickedType);
-						this.dtmService.edit(dt);
+		boolean isEditableType =
-						return;
+			(dt instanceof Structure) || (dt instanceof Union) || (dt instanceof Enum);
-					}
+		if (isEditableType) {
-				}
+			edit(dt, clickedType.getName());
-				String name =
+		}
-					(dt != null) ? dt.getDisplayName() : comp.getDataType().getDisplayName();
+		else {
-				model.setStatus("Can't edit \"" + name + "\".");
+			model.setStatus("Can only edit a structure, union or enum.");
-			}
-			else {
-				model.setStatus("Can only edit a structure, union or enum.");
-			}
 		}
 		requestTableFocus();
 	}
 
+	private void edit(DataType dt, String clickedName) {
+
+		DataTypeManager dtm = model.getOriginalDataTypeManager();
+		if (dtm == null) {
+			// shouldn't happen
+			model.setStatus("No Data Type Manager found for '" + clickedName + "'");
+			return;
+		}
+
+		DataType actualType = dtm.getDataType(dt.getDataTypePath());
+		if (actualType == null) {
+			model.setStatus("Can't edit '" + dt.getDisplayName() + "' - type not found.");
+			return;
+		}
+
+		dtmService.edit(actualType);
+	}
+
 	@Override
 	public void adjustEnablement() {
 		setEnabled(model.isEditComponentAllowed());

Ghidra/Features/Base/src/test.slow/java/docking/ComponentProviderActionsTest.java

@@ -271,7 +271,7 @@ public class ComponentProviderActionsTest extends AbstractGhidraHeadedIntegratio
 
 		try {
 			setErrorsExpected(true);
-			runSwingWithExceptions(this::showProvider, true);
+			runSwingWithException(this::showProvider);
 			setErrorsExpected(false);
 			fail();
 		}
@@ -289,7 +289,7 @@ public class ComponentProviderActionsTest extends AbstractGhidraHeadedIntegratio
 
 		try {
 			setErrorsExpected(true);
-			runSwingWithExceptions(() -> provider.setIcon(null), true);
+			runSwingWithException(() -> provider.setIcon(null));
 			setErrorsExpected(false);
 			fail("Expected an exception passing a null icon when specifying a toolbar action");
 		}

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions2Test.java

@@ -166,7 +166,7 @@ public class StructureEditorUnlockedActions2Test
 		assertEquals(getDataType(4), dt3);
 
 		invoke(action);
-		dialog = env.waitForDialogComponent(NumberInputDialog.class, 1000);
+		dialog = waitForDialogComponent(NumberInputDialog.class);
 		assertNotNull(dialog);
 		okInput(dialog, 2);
 		dialog = null;
@@ -180,7 +180,7 @@ public class StructureEditorUnlockedActions2Test
 
 		setSelection(new int[] { 2 });
 		invoke(action);
-		dialog = env.waitForDialogComponent(NumberInputDialog.class, 1000);
+		dialog = waitForDialogComponent(NumberInputDialog.class);
 		assertNotNull(dialog);
 		okInput(dialog, 2);
 		dialog = null;
@@ -223,7 +223,7 @@ public class StructureEditorUnlockedActions2Test
 		assertEquals(getDataType(1), dt1);
 		assertEquals(getDataType(2), dt2);
 		assertEquals(getDataType(3), dt3);
-		assertTrue(!"".equals(model.getStatus()));
+		assertNotEquals("", model.getStatus());
 	}
 
 	@Test

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions3Test.java

@@ -45,7 +45,7 @@ public class StructureEditorUnlockedActions3Test
 		DataType dt7 = getDataType(7);// SimpleUnion
 
 		invoke(duplicateMultipleAction);
-		dialog = env.waitForDialogComponent(NumberInputDialog.class, 1000);
+		dialog = waitForDialogComponent(NumberInputDialog.class);
 		assertNotNull(dialog);
 		okInput(dialog, 2);
 		dialog = null;
@@ -102,7 +102,7 @@ public class StructureEditorUnlockedActions3Test
 	public void testEditFieldOnBlankLine() throws Exception {
 		init(emptyStructure, pgmTestCat);
 
-		assertTrue(!model.isEditingField());
+		assertFalse(model.isEditingField());
 		triggerActionKey(getTable(), editFieldAction);
 		assertTrue(model.isEditingField());
 		assertEquals(0, model.getRow());
@@ -116,7 +116,7 @@ public class StructureEditorUnlockedActions3Test
 		init(complexStructure, pgmTestCat);
 
 		setSelection(new int[] { 3 });
-		assertTrue(!model.isEditingField());
+		assertFalse(model.isEditingField());
 		invoke(editFieldAction);
 		JTable table = getTable();
 		Container component = (Container) table.getEditorComponent();
@@ -140,10 +140,10 @@ public class StructureEditorUnlockedActions3Test
 
 		DataTypeComponent dtc = model.getComponent(3);
 		assertNotNull(dtc);
-		assertTrue(!dtc.isBitFieldComponent());
+		assertFalse(dtc.isBitFieldComponent());
 
 		setSelection(new int[] { 3 });
-		assertTrue(!model.isEditingField());
+		assertFalse(model.isEditingField());
 		invoke(editFieldAction);
 		JTable table = getTable();
 		Container component = (Container) table.getEditorComponent();
@@ -156,7 +156,7 @@ public class StructureEditorUnlockedActions3Test
 
 		waitForSwing();
 
-		assertTrue(!model.isEditingField());
+		assertFalse(model.isEditingField());
 		assertEquals(3, model.getRow());
 		assertNotEditingField();
 
@@ -192,7 +192,7 @@ public class StructureEditorUnlockedActions3Test
 
 		int num = model.getNumComponents();
 		setSelection(new int[] { 3 });
-		assertTrue(!getDataType(3).isEquivalent(dt));
+		assertFalse(getDataType(3).isEquivalent(dt));
 		invoke(fav);// replacing dword with byte followed by 3 undefineds
 		assertEquals(num + 3, model.getNumComponents());
 		assertTrue(getDataType(3).isEquivalent(dt));

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions4Test.java

@@ -25,8 +25,7 @@ import org.junit.Test;
 
 import docking.widgets.dialogs.NumberInputDialog;
 import ghidra.program.model.data.*;
-import ghidra.util.InvalidNameException;
+import ghidra.util.exception.UsrException;
-import ghidra.util.exception.*;
 import ghidra.util.task.TaskMonitor;
 
 public class StructureEditorUnlockedActions4Test
@@ -76,7 +75,7 @@ public class StructureEditorUnlockedActions4Test
 
 		// Make array of 3 pointers
 		invoke(arrayAction);
-		dialog = env.waitForDialogComponent(NumberInputDialog.class, 1000);
+		dialog = waitForDialogComponent(NumberInputDialog.class);
 		assertNotNull(dialog);
 		okInput(dialog, 3);
 		dialog = null;
@@ -103,25 +102,12 @@ public class StructureEditorUnlockedActions4Test
 	}
 
 	@Test
-	public void testDuplicateAction() throws Exception {
+	public void testDuplicateAction() throws Throwable {
 		init(complexStructure, pgmTestCat);
-		runSwing(() -> {
+		runSwingWithException(() -> {
-			try {
+			model.setComponentName(1, "comp1");
-				model.setComponentName(1, "comp1");
+			model.setComponentComment(1, "comment 1");
-				model.setComponentComment(1, "comment 1");
+			model.clearComponent(2);
-				model.clearComponent(2);
-			}
-			catch (InvalidInputException e) {
-				failWithException("Unexpected error", e);
-			}
-			catch (InvalidNameException e) {
-				// TODO Auto-generated catch block
-				e.printStackTrace();
-			}
-			catch (DuplicateNameException e) {
-				// TODO Auto-generated catch block
-				e.printStackTrace();
-			}
 		});
 		int len = model.getLength();
 		int num = model.getNumComponents();
@@ -150,7 +136,7 @@ public class StructureEditorUnlockedActions4Test
 
 	@Test
 	public void testEditComponentAction() throws Exception {
-		//		init(complexStructure, pgmTestCat);
+
 		runSwing(() -> {
 			installProvider(new StructureEditorProvider(plugin, complexStructure, false));
 			model = provider.getModel();
@@ -159,12 +145,46 @@ public class StructureEditorUnlockedActions4Test
 		getActions();
 
 		assertEquals("", model.getStatus());
-		setSelection(new int[] { 21 });
+		setSelection(new int[] { 21 }); // 'simpleStructure'
 		String complexSubTitle = getProviderSubTitle(complexStructure);
 		String simpleSubTitle = getProviderSubTitle(simpleStructure);
 		assertTrue("Couldn't find editor = " + complexSubTitle,
 			isProviderShown(tool.getToolFrame(), "Structure Editor", complexSubTitle));
-		assertTrue(!isProviderShown(tool.getToolFrame(), "Structure Editor", simpleSubTitle));
+		assertFalse(isProviderShown(tool.getToolFrame(), "Structure Editor", simpleSubTitle));
+
+		invoke(editComponentAction);
+		assertEquals("", model.getStatus());
+		assertTrue("Couldn't find editor = " + complexSubTitle,
+			isProviderShown(tool.getToolFrame(), "Structure Editor", complexSubTitle));
+		assertTrue("Couldn't find editor = " + simpleSubTitle,
+			isProviderShown(tool.getToolFrame(), "Structure Editor", simpleSubTitle));
+
+		runSwing(() -> provider.closeComponent());
+	}
+
+	@Test
+	public void testEditComponentAction_ComplexStructure() throws Exception {
+
+		//
+		// Test that the Edit Component action will work when the type is multi-layered, like
+		// a pointer to a pointer to a structure
+		//
+
+		runSwing(() -> {
+			installProvider(new StructureEditorProvider(plugin, complexStructure, false));
+			model = provider.getModel();
+		});
+		waitForSwing();
+		getActions();
+
+		assertEquals("", model.getStatus());
+		setSelection(new int[] { 20 }); // 'simpleStructureTypedef * *[2][3]'		
+		String complexSubTitle = getProviderSubTitle(complexStructure);
+		String simpleSubTitle = getProviderSubTitle(simpleStructure);
+		assertTrue("Couldn't find editor = " + complexSubTitle,
+			isProviderShown(tool.getToolFrame(), "Structure Editor", complexSubTitle));
+		assertFalse(isProviderShown(tool.getToolFrame(), "Structure Editor", simpleSubTitle));
+
 		invoke(editComponentAction);
 		assertEquals("", model.getStatus());
 		assertTrue("Couldn't find editor = " + complexSubTitle,
@@ -194,58 +214,6 @@ public class StructureEditorUnlockedActions4Test
 		assertEquals(num + 13, model.getNumComponents());
 	}
 
-	//	public void testCancelPointerOnFixedDt() throws Exception {
-	//      // FUTURE
-	//		init(complexStructure,  pgmTestCat);
-	//		NumberInputDialog dialog;
-	//		int num = model.getNumComponents();
-	//
-	//		setSelection(new int[] {2});
-	//		DataType dt2 = getDataType(2);
-	//		assertTrue(getDataType(2).isEquivalent(new WordDataType()));
-	//		invoke(pointerAction);
-	//		dialog = (NumberInputDialog)env.waitForDialog(NumberInputDialog.class, 1000);
-	//		assertNotNull(dialog);
-	//		cancelInput(dialog);
-	//		dialog.dispose();
-	//		dialog = null;
-	//		assertEquals(num, model.getNumComponents());
-	//		assertEquals("word", getDataType(2).getDisplayName());
-	//		assertTrue(getDataType(2).isEquivalent(dt2));
-	//		assertEquals(4, model.getComponent(2).getLength());
-	//	}
-
-	//	@Test
-	//	public void testCreatePointerOnArray() throws Exception {
-	//		init(complexStructure, pgmTestCat);
-	//		int num = model.getNumComponents();
-	//
-	//		setSelection(new int[] { 14 });
-	//		DataType dt14 = getDataType(14);
-	//		assertEquals("byte[7]", dt14.getDisplayName());
-	//		invoke(pointerAction);
-	//		assertEquals(num + 3, model.getNumComponents());
-	//		assertEquals("byte[7] *", getDataType(14).getDisplayName());
-	//		assertTrue(((Pointer) getDataType(14)).getDataType().isEquivalent(dt14));
-	//		assertEquals(4, getDataType(14).getLength());
-	//		assertEquals(4, model.getComponent(14).getLength());
-	//	}
-	//
-	//	@Test
-	//	public void testCreatePointerOnTypedef() throws Exception {
-	//		init(complexStructure, pgmTestCat);
-	//		int num = model.getNumComponents();
-	//
-	//		setSelection(new int[] { 19 });
-	//		DataType dt19 = getDataType(19);
-	//		assertEquals("simpleStructureTypedef", dt19.getDisplayName());
-	//		invoke(pointerAction);
-	//		assertEquals(num + 25, model.getNumComponents());
-	//		assertEquals("simpleStructureTypedef *", getDataType(19).getDisplayName());
-	//		assertTrue(((Pointer) getDataType(19)).getDataType().isEquivalent(dt19));
-	//		assertEquals(4, model.getComponent(19).getLength());
-	//	}
-
 	@Test
 	public void testApplyComponentChange() throws Exception {
 		init(complexStructure, pgmTestCat);
@@ -262,7 +230,7 @@ public class StructureEditorUnlockedActions4Test
 		});
 		DataType viewCopy = model.viewComposite.clone(null);
 
-		assertTrue(!complexStructure.isEquivalent(model.viewComposite));
+		assertFalse(complexStructure.isEquivalent(model.viewComposite));
 		assertTrue(viewCopy.isEquivalent(model.viewComposite));
 		invoke(applyAction);
 		assertTrue(viewCopy.isEquivalent(complexStructure));

Ghidra/Framework/Generic/src/main/java/generic/test/AbstractGenericTest.java

@@ -55,6 +55,7 @@ import sun.awt.AppContext;
 import utilities.util.FileUtilities;
 import utilities.util.reflection.ReflectionUtilities;
 import utility.application.ApplicationLayout;
+import utility.function.ExceptionalCallback;
 
 public abstract class AbstractGenericTest extends AbstractGTest {
 
@@ -1115,23 +1116,32 @@ public abstract class AbstractGenericTest extends AbstractGTest {
 	}
 
 	/**
-	 * Call this version of {@link #runSwing(Runnable)} when you expect your runnable to throw
+	 * Call this version of {@link #runSwing(Runnable)} when you expect your runnable <b>may</b> 
-	 * an exception 
+	 * throw exceptions
-	 * @param runnable the runnable
+	 * 
-	 * @param wait true signals to wait for the Swing operation to finish
+	 * @param callback the runnable code snippet to call
-	 * @throws Throwable any exception that is thrown on the Swing thread
+	 * @throws Exception any exception that is thrown on the Swing thread
 	 */
-	public static void runSwingWithExceptions(Runnable runnable, boolean wait) throws Throwable {
+	public static <E extends Exception> void runSwingWithException(ExceptionalCallback<E> callback)
+			throws Exception {
 
 		if (Swing.isSwingThread()) {
 			throw new AssertException("Unexpectedly called from the Swing thread");
 		}
 
-		ExceptionHandlingRunner exceptionHandlingRunner = new ExceptionHandlingRunner(runnable);
+		ExceptionHandlingRunner exceptionHandlingRunner = new ExceptionHandlingRunner(callback);
 		Throwable throwable = exceptionHandlingRunner.getException();
-		if (throwable != null) {
+		if (throwable == null) {
-			throw throwable;
+			return;
 		}
+
+		if (throwable instanceof Exception) {
+			// this is what the client expected
+			throw (Exception) throwable;
+		}
+
+		// a runtime exception; re-throw
+		throw new AssertException(throwable);
 	}
 
 	public static void runSwing(Runnable runnable, boolean wait) {
@@ -1170,11 +1180,18 @@ public abstract class AbstractGenericTest extends AbstractGTest {
 	}
 
 	protected static class ExceptionHandlingRunner {
-		private final Runnable delegateRunnable;
+		private final ExceptionalCallback<? extends Exception> delegateCallback;
 		private Throwable exception;
 
 		ExceptionHandlingRunner(Runnable delegateRunnable) {
-			this.delegateRunnable = delegateRunnable;
+			this.delegateCallback = () -> {
+				delegateRunnable.run();
+			};
+			run();
+		}
+
+		ExceptionHandlingRunner(ExceptionalCallback<? extends Exception> delegateCallback) {
+			this.delegateCallback = delegateCallback;
 			run();
 		}
 
@@ -1213,7 +1230,7 @@ public abstract class AbstractGenericTest extends AbstractGTest {
 
 			Runnable swingExceptionCatcher = () -> {
 				try {
-					delegateRunnable.run();
+					delegateCallback.call();
 				}
 				catch (Throwable t) {
 					exception = t;