diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/EditComponentAction.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/EditComponentAction.java index 89ede6f036..aa0f7494e5 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/EditComponentAction.java +++ b/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()) { - DataTypeComponent comp = model.getComponent(row); - DataType dt = DataTypeHelper.getBaseType(comp.getDataType()); - if ((dt instanceof Structure) || (dt instanceof Union) || (dt instanceof Enum)) { - DataTypeManager dtm = model.getOriginalDataTypeManager(); - if (dtm != null) { - dt = dtm.getDataType(dt.getDataTypePath()); - if (dt != null) { - this.dtmService.edit(dt); - return; - } - } - String name = - (dt != null) ? dt.getDisplayName() : comp.getDataType().getDisplayName(); - model.setStatus("Can't edit \"" + name + "\"."); - } - else { - model.setStatus("Can only edit a structure, union or enum."); - } + if (row >= model.getNumComponents()) { + requestTableFocus(); + return; + } + + DataTypeComponent comp = model.getComponent(row); + DataType clickedType = comp.getDataType(); + DataType dt = DataTypeUtils.getBaseDataType(clickedType); + boolean isEditableType = + (dt instanceof Structure) || (dt instanceof Union) || (dt instanceof Enum); + if (isEditableType) { + edit(dt, clickedType.getName()); + } + 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()); diff --git a/Ghidra/Features/Base/src/test.slow/java/docking/ComponentProviderActionsTest.java b/Ghidra/Features/Base/src/test.slow/java/docking/ComponentProviderActionsTest.java index 01db055015..4f57794763 100644 --- a/Ghidra/Features/Base/src/test.slow/java/docking/ComponentProviderActionsTest.java +++ b/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"); } diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions2Test.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions2Test.java index 4d397d7d72..ea4dd50efe 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions2Test.java +++ b/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 diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions3Test.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions3Test.java index 57128ade09..c91b9d021d 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions3Test.java +++ b/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)); diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions4Test.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions4Test.java index fcdc33323c..0eee6ce58e 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions4Test.java +++ b/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.*; +import ghidra.util.exception.UsrException; 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(() -> { - try { - model.setComponentName(1, "comp1"); - model.setComponentComment(1, "comment 1"); - 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(); - } + runSwingWithException(() -> { + model.setComponentName(1, "comp1"); + model.setComponentComment(1, "comment 1"); + model.clearComponent(2); }); 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)); diff --git a/Ghidra/Framework/Generic/src/main/java/generic/test/AbstractGenericTest.java b/Ghidra/Framework/Generic/src/main/java/generic/test/AbstractGenericTest.java index 30c5b66be3..3552574e71 100644 --- a/Ghidra/Framework/Generic/src/main/java/generic/test/AbstractGenericTest.java +++ b/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 - * an exception - * @param runnable the runnable - * @param wait true signals to wait for the Swing operation to finish - * @throws Throwable any exception that is thrown on the Swing thread + * Call this version of {@link #runSwing(Runnable)} when you expect your runnable may + * throw exceptions + * + * @param callback the runnable code snippet to call + * @throws Exception any exception that is thrown on the Swing thread */ - public static void runSwingWithExceptions(Runnable runnable, boolean wait) throws Throwable { + public static void runSwingWithException(ExceptionalCallback 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) { - throw throwable; + if (throwable == null) { + 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 delegateCallback; private Throwable exception; ExceptionHandlingRunner(Runnable delegateRunnable) { - this.delegateRunnable = delegateRunnable; + this.delegateCallback = () -> { + delegateRunnable.run(); + }; + run(); + } + + ExceptionHandlingRunner(ExceptionalCallback 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;