Skip type-incompatible imports in ModelAssembler.resolveImports

resolveImports resolves fragment <imports> 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.

Skip such an import with a warning instead: an import that resolves to an
incompatible element is a contribution error, not a reason to abort
assembly. The reference is left unset and every other fragment still
applies. Also skip, rather than NPE, an unresolved import on a
many-valued feature. Add a regression test covering the type mismatch.

Fixes #4148

Author: vogella

bundles/org.eclipse.e4.ui.workbench/META-INF/MANIFEST.MF

@@ -1,7 +1,7 @@
 Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-SymbolicName: org.eclipse.e4.ui.workbench;singleton:=true
-Bundle-Version: 1.18.300.qualifier
+Bundle-Version: 1.18.400.qualifier
 Bundle-Name: %pluginName
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin

bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelAssembler.java

@@ -775,7 +775,20 @@ public void resolveImports(List<MApplicationElement> imports, List<MApplicationE
 						element = null;
 					}
 					commands.add(() -> {
+						if (element != null && !feature.getEType().isInstance(element)) {
+							// An import that resolves to a different-typed element (e.g. two
+							// elements sharing an elementId) is a contribution error, not a
+							// reason to abort assembling the whole application model.
+							warn("Skipping import for feature {} on {}: resolved element {} is not assignable to the feature type", //$NON-NLS-1$
+									feature.getName(), target, element.getElementId());
+							return;
+						}
 						if (feature.isMany()) {
+							if (element == null) {
+								warn("Skipping unresolved import for many-valued feature {} on {}", //$NON-NLS-1$
+										feature.getName(), target);
+								return;
+							}
 							error("""
 									Replacing in {}.
 									Feature={}.

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.assertNotEquals;
 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.MCommand;
 import org.eclipse.e4.ui.model.application.impl.ApplicationFactoryImpl;
 import org.eclipse.e4.ui.model.application.ui.MUIElement;
 import org.eclipse.e4.ui.model.application.ui.advanced.MArea;
@@ -340,6 +342,54 @@ 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 (two
+	 * elements sharing an elementId) is skipped with a warning instead of aborting
+	 * model assembly with a {@link ClassCastException}. See
+	 * <a href="https://github.com/eclipse-platform/eclipse.platform.ui/issues/4148">issue
+	 * 4148</a>.
+	 */
+	@Test
+	public void testImports_typeIncompatibleElement() throws Exception {
+		List<MApplicationElement> imports = new ArrayList<>();
+		List<MApplicationElement> addedElements = new ArrayList<>();
+
+		final String sharedElementId = "testImports_typeIncompatible_id";
+
+		// The fragment imports a window with the shared id ...
+		MTrimmedWindow importWindow = modelService.createModelElement(MTrimmedWindow.class);
+		importWindow.setElementId(sharedElementId);
+		MModelFragments fragment = MFragmentFactory.INSTANCE.createModelFragments();
+		fragment.getImports().add(importWindow);
+		imports.add(importWindow);
+
+		// ... but the application only contains a command (a different type) that
+		// shares that id, so the id-based lookup resolves to a type-incompatible
+		// element for the MUIElement-typed placeholder reference.
+		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;
+
+		// Must not throw: the incompatible resolution is skipped, the reference is
+		// left untouched, and every other fragment still applies.
+		assembler.resolveImports(imports, addedElements);
+
+		assertNotEquals(collidingCommand, placeholder.getRef());
+		assertEquals(importWindow, placeholder.getRef());
+
+		boolean completed = countDownLatch.await(COUNTDOWN_TIMEOUT, TimeUnit.MILLISECONDS);
+		assertTrue(completed, "Timeout - no event received");
+		assertEquals(1, logMessages.size());
+		assertTrue(logMessages.poll().startsWith("Skipping import for feature"));
+	}
+
 	/**
 	 * Make sure that all fragments and imports are resolved before the
 	 * post-processors are run. For reference, see