[1617] Incorrect resolution of redefined attribute during import

Bug: #1617
Signed-off-by: Arthur Daussy <arthur.daussy@obeo.fr>

Author: adaussy

CHANGELOG.adoc

@@ -56,6 +56,7 @@ Also introduces new `ServiceMethod` helper class to build AQL service call expre
 - https://github.com/eclipse-syson/syson/issues/1611[#1611] [services] Fix an issue where the undo of the deletion of a graphical node in a diagram was not restoring the graphical node correctly.
 - https://github.com/eclipse-syson/syson/issues/1618[#1618] [rest-apis] Fix an issue where the _eAnnotations_ reference coming from the Ecore metamodel was taking into account by REST APIs serialization.
 - https://github.com/eclipse-syson/syson/issues/1621[#1621] [diagrams] Fix an issue where the creation of a _Package_ inside another _Package_ created the _Package_ at the root of the diagram in addition to the inside of the target _Package_.
+- https://github.com/eclipse-syson/syson/issues/1617[#1617] [import] Fix incorrect resolution of _redefined feature_ on `Redefinitions` during textual import.
 
 === Improvements
 

backend/application/syson-application/src/test/java/org/eclipse/syson/application/imports/ImportSysMLModelTest.java

@@ -149,6 +149,64 @@ public void tearDown() {
         TestTransaction.start();
     }
 
+    @Test
+    @DisplayName("Given of model with redefinition depending on inherited memberships computation, WHEN importing the model, THEN redefined feature should resolve properly using inherited memberships")
+    public void checkRedefinedFeatureToInheritedFields() throws IOException {
+        var input = """
+                private import ISQBase::mass;
+                #MP part p2 {
+                    attribute :>> mass = 2.0;
+                }
+                private import Metaobjects::SemanticMetadata;
+                part p {
+                    attribute mass;
+                }
+                metadata def MP :> SemanticMetadata {
+                    :>> baseType = p meta SysML::Systems::PartUsage;
+                    :> annotatedElement : SysML::Systems::PartUsage;
+                }
+                part p3 :> p1::p2::p0 {
+                    attribute :>> mass = 2.0;
+                }
+                package p1 {
+                    package p2 {
+                        part p0 :> p;
+                    }
+                }
+                part p {
+                    attribute mass;
+                }
+                """;
+
+        this.checker.checkImportedModel(resource -> {
+            Optional<AttributeUsage> pMass = EMFUtils.allContainedObjectOfType(resource, AttributeUsage.class)
+                    .filter(partUsage -> "p::mass".equals(partUsage.getQualifiedName()))
+                    .findFirst();
+
+            assertThat(pMass)
+                    .isPresent();
+
+            // Checks that p2::mass redefined p::mass and not ISQBase::mass because of the use of the semantic MetadataDefinition that implicitly makes p2 subset p
+            Optional<AttributeUsage> p2Mass = EMFUtils.allContainedObjectOfType(resource, AttributeUsage.class)
+                    .filter(partUsage -> "p2::mass".equals(partUsage.getQualifiedName()))
+                    .findFirst();
+            assertThat(p2Mass)
+                    .isPresent();
+
+            assertThat(p2Mass.get()).matches(attributeUsage -> attributeUsage.redefines(pMass.get()));
+
+            // Checks that p2::mass redefined p::mass and not ISQBase::mass because p3 subsets p1::p2::p0 and p0 subsets p
+            Optional<AttributeUsage> p3Mass = EMFUtils.allContainedObjectOfType(resource, AttributeUsage.class)
+                    .filter(partUsage -> "p3::mass".equals(partUsage.getQualifiedName()))
+                    .findFirst();
+            assertThat(p2Mass)
+                    .isPresent();
+
+            assertThat(p2Mass.get()).matches(attributeUsage -> attributeUsage.redefines(pMass.get()));
+
+        }).check(input);
+    }
+
     @Test
     @DisplayName("GIVEN a model with a FlowUsage using a FeatureChain, WHEN importing the model, THEN the source and the target of the FlowUsage should be properly resolved")
     public void checkFlowUsageWithFeatureChain() throws IOException {

backend/application/syson-sysml-import/src/main/java/org/eclipse/syson/sysml/parser/AstTreeParser.java

@@ -19,12 +19,18 @@
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Map.Entry;
+import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
 
 import org.eclipse.emf.ecore.EObject;
 import org.eclipse.emf.ecore.EReference;
+import org.eclipse.emf.ecore.InternalEObject;
 import org.eclipse.syson.sysml.Element;
+import org.eclipse.syson.sysml.Import;
+import org.eclipse.syson.sysml.Redefinition;
+import org.eclipse.syson.sysml.Specialization;
 import org.eclipse.syson.sysml.helper.EMFUtils;
 import org.eclipse.syson.sysml.parser.translation.EClassifierTranslator;
 import org.eclipse.syson.sysml.utils.LogNameProvider;
@@ -157,6 +163,83 @@ private void handleNodeValue(final EObject eObject, String key, JsonNode value)
     }
 
     public void resolveAllReference(List<ProxiedReference> unresolvedReferences) {
+        /*
+         * We need to order some of the proxies to resolve. For example, redefinition resolution should be handled once
+         * all specializations have been handled. Indeed, the name resolution of a redefined feature on Redefinition can
+         * depend on either the Subclassification or Subsetting of its owning Namespace (that might be implied through
+         * the use of semantic MetadataUsage) or the type of the owning namespace</li> </ul></p>
+         */
+        Map<ReferenceType, List<ProxiedReference>> partitionedUnresolvedReferences = unresolvedReferences.stream().collect(Collectors.groupingBy(this::getProxyClassification));
+
+        // First resolve imports
+        List<ProxiedReference> unresolvedProxiesAfterImport = this.doResolveAllReference(partitionedUnresolvedReferences, ReferenceType.IMPORT,
+                "Import resolution phase", List.of());
+
+        // Then resolve unclassified proxies and the previous unresolved proxies
+        List<ProxiedReference> unresolvedProxiesAfterUnqualified = this.doResolveAllReference(partitionedUnresolvedReferences, ReferenceType.UNCLASSIFIED,
+                "Non qualified proxy resolution phase", unresolvedProxiesAfterImport);
+
+        // Then resolve specializations that are not redefinitions since redefinition needs to access inherited memberships
+        List<ProxiedReference> unresolvedAfterSpecialization = this.doResolveAllReference(partitionedUnresolvedReferences, ReferenceType.OTHER_SPECIALIZATION,
+                "Non redefinition specialization proxy resolution phase",
+                unresolvedProxiesAfterUnqualified);
+
+        // Then resolve base_type redefinition proxies stored MetadataDefinition since they may impact implicit inherited members
+        List<ProxiedReference> unresolvedAfterBaseTypeRedefinition = this.doResolveAllReference(partitionedUnresolvedReferences, ReferenceType.BASE_TYPE_METADATA_DEFINITION_REDEFINITION,
+                "Semantic MetadataData base_type redefinition proxy resolution phase", unresolvedAfterSpecialization);
+
+        // At the end resolve redefinition
+        List<ProxiedReference> unresolvedAfterRedefinition = this.doResolveAllReference(partitionedUnresolvedReferences, ReferenceType.OTHER_REDEFINITION, "Other redefinition proxy resolution phase",
+                unresolvedAfterBaseTypeRedefinition);
+
+        if (!unresolvedAfterRedefinition.isEmpty()) {
+            this.handleUnresolvableProxies(unresolvedAfterRedefinition);
+        }
+    }
+
+    private ReferenceType getProxyClassification(ProxiedReference proxyReference) {
+        EObject owner = proxyReference.owner();
+        InternalEObject targetProxy = proxyReference.targetProxy();
+        final ReferenceType result;
+        if (owner instanceof Redefinition && targetProxy.toString().contains("baseType")) {
+            result = ReferenceType.BASE_TYPE_METADATA_DEFINITION_REDEFINITION;
+        } else if (owner instanceof Import) {
+            result = ReferenceType.IMPORT;
+        } else if (owner instanceof Redefinition) {
+            result = ReferenceType.OTHER_REDEFINITION;
+        } else if (owner instanceof Specialization) {
+            result = ReferenceType.OTHER_SPECIALIZATION;
+        } else {
+            result = ReferenceType.UNCLASSIFIED;
+        }
+        return result;
+    }
+
+    /**
+     * Runs a resolution phase of a given class of proxy
+     *
+     * @param classifiedProxies
+     *         all the proxy classified
+     * @param proxyClass
+     *         the type of proxy to resolve
+     * @param resolutionPhaseName
+     *         a name of resolution phase
+     * @param yetNotResolved
+     *         A list of unresolved proxy from the previous phases
+     * @return all proxies that have not been resolved
+     */
+    private List<ProxiedReference> doResolveAllReference(Map<ReferenceType, List<ProxiedReference>> classifiedProxies, ReferenceType proxyClass, String resolutionPhaseName,
+            List<ProxiedReference> yetNotResolved) {
+        List<ProxiedReference> toResolve = classifiedProxies.get(proxyClass);
+        if (toResolve != null && !toResolve.isEmpty()) {
+            List<ProxiedReference> tmpToResolve = new ArrayList<>(toResolve);
+            tmpToResolve.addAll(yetNotResolved);
+            return this.doResolveAllReference(tmpToResolve, resolutionPhaseName);
+        }
+        return yetNotResolved;
+    }
+
+    private List<ProxiedReference> doResolveAllReference(List<ProxiedReference> unresolvedReferences, String resolutionPhase) {
         // Sorts proxies to resolve by breadth-first search strategy. This way we resolve first the import
         // generally located at top level part of the model. Those proxies are often used by lowest part of the
         // model to resolve their link
@@ -172,18 +255,19 @@ public void resolveAllReference(List<ProxiedReference> unresolvedReferences) {
             List<ProxiedReference> proxiedReferences = unresolvedReferencesTmp.stream()
                     .filter(proxiedReference -> !this.proxyResolver.resolveProxy(proxiedReference))
                     .toList();
-            LOGGER.info(MessageFormat.format("{0} remaining proxies to resolve after {1} try", Integer.toString(proxiedReferences.size()), Integer.toString(tryNb)));
+            LOGGER.info(MessageFormat.format("{2} : {0} remaining proxies to resolve after {1} try", Integer.toString(proxiedReferences.size()), Integer.toString(tryNb), resolutionPhase));
             if (proxiedReferences.equals(unresolvedReferencesTmp)) {
-                this.printResolutionError(proxiedReferences);
                 break;
             } else {
                 unresolvedReferencesTmp = proxiedReferences;
             }
         }
+
+        return unresolvedReferencesTmp;
     }
 
-    private void printResolutionError(List<ProxiedReference> proxiedReferences) {
-        for (ProxiedReference pr : proxiedReferences) {
+    private void handleUnresolvableProxies(List<ProxiedReference> unresolvableProxies) {
+        for (ProxiedReference pr : unresolvableProxies) {
             String msg = MessageFormat.format("Unable to resolve name ''{1}'' for reference ''{2}'' on element ''{0}''", this.logNameProvider.getName(pr.owner()),
                     pr.targetProxy().eProxyURI().fragment(),
                     pr.reference().getName());
@@ -233,4 +317,30 @@ public int compare(EObject owner1, EObject owner2) {
             return signum;
         }
     }
+
+    /**
+     * Classification of proxy to resolve.
+     */
+    private enum ReferenceType {
+        /**
+         * A proxy related to imports.
+         */
+        IMPORT,
+        /**
+         * All proxies that do not belong to any other categories.
+         */
+        UNCLASSIFIED,
+        /**
+         * Proxy of the "base_type" redefinition feature of the "SemanticMetadata" used in MetadataDefinition.
+         */
+        BASE_TYPE_METADATA_DEFINITION_REDEFINITION,
+        /**
+         * All other redefinitions except the one stored in BASE_TYPE_METADATA_DEFINITION_REDEFINITION.
+         */
+        OTHER_REDEFINITION,
+        /**
+         * All specializations except the Redefinition.
+         */
+        OTHER_SPECIALIZATION
+    }
 }

doc/content/modules/user-manual/pages/release-notes/2025.12.0.adoc

@@ -40,6 +40,27 @@ requirement <'SN'> {
 - Fix an issue where the undo of the deletion of a graphical node in a diagram was not restoring the graphical node correctly.
 - Fix an issue where the _eAnnotations_ reference coming from the Ecore metamodel was taking into account by REST APIs serialization.
 - In diagrams, fix an issue where the creation of a _Package_ inside another _Package_ created the _Package_ at the root of the diagram in addition to the inside of the target _Package_.
+- Fix invalid resolution during textual import of _redefined Feature_ on `Redefinition`.
+This problem appears in cases of a name conflict between a global available item and an item accessible via _inherited membership_ such as in the following model:
+
+```
+private import ISQBase::mass; // Gives global access to "mass"
+
+#MP part p2 { // Implicit subsetting "p" using a semantic MetadataDefinition
+	attribute :>> mass = 2.0; // Should redefine "p::mass" and not "ISQBase::mass"
+}
+
+private import Metaobjects::SemanticMetadata;
+
+part p {
+	attribute mass;
+}
+
+metadata def MP :> SemanticMetadata {
+    :>> baseType = p meta SysML::PartUsage;
+    :> annotatedElement : SysML::PartUsage;
+}
+```
 
 == Improvements