From 3375345594865fdebfacc434cdcfb5825b0854bf Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Thu, 2 Jul 2026 19:12:02 +0200 Subject: [PATCH] Skip type-incompatible imports in ModelAssembler.resolveImports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveImports resolves fragment by elementId via ModelUtils.findElementById, which returns the first match regardless of type, and then eSets it onto the target reference. When the model contains a different-typed element sharing the imported id, the eSet fails with a ClassCastException and aborts assembly of the whole application model. For example ResourceHandlerTest fails in the I-builds with "BindingTableImpl cannot be cast to MBindingContext", because BindingToModelProcessor.createTable() gives each generated BindingTable the binding-context id as its own elementId, so a fragment importing that BindingContext resolves to the same-id BindingTable. Treat such an import as unresolved and log a warning instead: an import that resolves to an incompatible element is a contribution error, not a reason to abort assembly. A single-valued reference is cleared as for any unresolved import, and an unresolved import in a many-valued reference is now removed from the list instead of failing with an NPE. Fixes https://github.com/eclipse-platform/eclipse.platform.ui/issues/4148 Assisted-by: multiple AI agents and layers of automated tooling 🤖 --- .../ui/internal/workbench/ModelAssembler.java | 21 +++++-- .../tests/workbench/ModelAssemblerTests.java | 62 +++++++++++++++++++ 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelAssembler.java b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelAssembler.java index ada9e92096e..5098514f136 100644 --- a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelAssembler.java +++ b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelAssembler.java @@ -835,22 +835,33 @@ public void resolveImports(List imports, List { + MApplicationElement resolved = element; + if (resolved != null && !feature.getEType().isInstance(resolved)) { + // a different-typed element sharing the id must not abort model assembly + warn("Could not resolve import for {}: incompatible with feature {} of {}", //$NON-NLS-1$ + resolved.getElementId(), feature.getName(), target); + resolved = null; + } if (feature.isMany()) { + @SuppressWarnings("unchecked") + List l = (List) target.eGet(feature); + if (resolved == null) { + l.remove(importObject); + return; + } error(""" Replacing in {}. Feature={}. InternalElement={} contributed by {}. ImportObject={} - """, target, feature.getName(), element.getElementId(), element.getContributorURI(), //$NON-NLS-1$ + """, target, feature.getName(), resolved.getElementId(), resolved.getContributorURI(), //$NON-NLS-1$ importObject); - @SuppressWarnings("unchecked") - List l = (List) target.eGet(feature); int index = l.indexOf(importObject); if (index >= 0) { - l.set(index, element); + l.set(index, resolved); } } else { - target.eSet(feature, element); + target.eSet(feature, resolved); } }); } diff --git a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/ModelAssemblerTests.java b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/ModelAssemblerTests.java index b8e63c77815..117ff703671 100644 --- a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/ModelAssemblerTests.java +++ b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/ModelAssemblerTests.java @@ -17,6 +17,7 @@ package org.eclipse.e4.ui.tests.workbench; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import jakarta.annotation.PostConstruct; @@ -44,6 +45,7 @@ import org.eclipse.e4.ui.internal.workbench.swt.E4Application; import org.eclipse.e4.ui.model.application.MApplication; import org.eclipse.e4.ui.model.application.MApplicationElement; +import org.eclipse.e4.ui.model.application.commands.MBindingContext; import org.eclipse.e4.ui.model.application.commands.MCommand; import org.eclipse.e4.ui.model.application.commands.MHandler; import org.eclipse.e4.ui.model.application.impl.ApplicationFactoryImpl; @@ -561,6 +563,66 @@ public void testImports_noImportElementId() throws Exception { assertEquals("Could not resolve import for null", logMessages.poll()); } + /** Tests that an import resolving to an element of an incompatible type is dropped. */ + @Test + public void testImports_typeIncompatibleElement() throws Exception { + List imports = new ArrayList<>(); + List addedElements = new ArrayList<>(); + + final String sharedElementId = "testImports_typeIncompatible_id"; + MTrimmedWindow importWindow = modelService.createModelElement(MTrimmedWindow.class); + importWindow.setElementId(sharedElementId); + MModelFragments fragment = MFragmentFactory.INSTANCE.createModelFragments(); + fragment.getImports().add(importWindow); + imports.add(importWindow); + MCommand collidingCommand = modelService.createModelElement(MCommand.class); + collidingCommand.setElementId(sharedElementId); + application.getCommands().add(collidingCommand); + + MPlaceholder placeholder = modelService.createModelElement(MPlaceholder.class); + placeholder.setRef(importWindow); + addedElements.add(placeholder); + + CountDownLatch countDownLatch = new CountDownLatch(1); + this.logListener.countDownLatch = countDownLatch; + + assembler.resolveImports(imports, addedElements); + assertNull(placeholder.getRef()); + + boolean completed = countDownLatch.await(COUNTDOWN_TIMEOUT, TimeUnit.MILLISECONDS); + assertTrue(completed, "Timeout - no event received"); + assertEquals(1, logMessages.size()); + assertTrue(logMessages.poll().startsWith("Could not resolve import for " + sharedElementId + ": incompatible")); + } + + /** Tests that an unresolved import in a many-valued reference is removed. */ + @Test + public void testImports_unresolvedInManyValuedFeature() throws Exception { + List imports = new ArrayList<>(); + List addedElements = new ArrayList<>(); + + MBindingContext importContext = modelService.createModelElement(MBindingContext.class); + importContext.setElementId("testImports_unresolvedMany_context"); + MModelFragments fragment = MFragmentFactory.INSTANCE.createModelFragments(); + fragment.getImports().add(importContext); + imports.add(importContext); + + MPart part = modelService.createModelElement(MPart.class); + part.getBindingContexts().add(importContext); + addedElements.add(part); + + CountDownLatch countDownLatch = new CountDownLatch(1); + this.logListener.countDownLatch = countDownLatch; + + assembler.resolveImports(imports, addedElements); + assertTrue(part.getBindingContexts().isEmpty()); + + boolean completed = countDownLatch.await(COUNTDOWN_TIMEOUT, TimeUnit.MILLISECONDS); + assertTrue(completed, "Timeout - no event received"); + assertEquals(1, logMessages.size()); + assertEquals("Could not resolve import for testImports_unresolvedMany_context", logMessages.poll()); + } + /** * Make sure that all fragments and imports are resolved before the * post-processors are run. For reference, see