fix(migrate): an untyped property follows its tool's kind marker; unmarked, it is an attribute, or a part when composite

changes/unreleased/untyped-property-kind.fixed.md

@@ -0,0 +1 @@
+- **An untyped v1 property is no longer always a `ref`.** The SysML v1 migrator wrote every property without a type as `ref x;`, ignoring MagicDraw's «ValueProperty», «PartProperty», «SharedProperty», «ReferenceProperty» and «ConstraintProperty» markers and then writing the marker as an applied stereotype. An untyped property now takes the usage its marker names (`attribute`, `part`, `ref`, `constraint`), with the marker omitted as it is on typed properties; without a marker it is a `part` when its aggregation is composite and an `attribute` otherwise, reported approximated. A property whose default is an individual stays a `ref` typed by it. A slot of such an untyped `part` holding an individual, which crashed the migration, is written as `individual part :>> x : 'the individual';`, checked against every type the part inherits when it redefines, subsets or shadows typed ones.

docs/project/sysml-v1-transformation-census-baseline.json

@@ -248,7 +248,7 @@
       },
       "uml:Property": {
         "pssm": 25,
-        "fixtures": 468
+        "fixtures": 475
       },
       "uml:Property[qualifier]": {
         "pssm": 0,
@@ -14643,14 +14643,16 @@
         "filter"
       ],
       "ocl": "sha256:99f06b2a46c64e98963563afeee2a1b8cb76a43e5fae2ee1af153c247800e6fc",
-      "status": "faithful",
+      "status": "approximate",
       "implementation": [
-        "internal/translate/migrate/migrate.go:featureKeyword"
+        "internal/translate/migrate/migrate.go:featureKeyword",
+        "internal/translate/migrate/migrate.go:untypedKeyword"
       ],
       "tests": [
-        "tests/migrate/properties_common_test.go:TestUntypedPropertiesAreReferencesOutsideConstraintDefinitions"
+        "tests/migrate/properties_common_test.go:TestUntypedPropertiesAreAttributesInAndOutsideConstraintDefinitions",
+        "tests/migrate/relations_test.go:TestPropertyKindMarkersAreNotWritten"
       ],
-      "reason": "an untyped property is written as a reference usage; the scope counts every uml:Property, so it is an upper bound",
+      "reason": "an untyped property is written as the usage its tool's property-kind marker names, as a `ref` typed by its default individual, else as a `part` when composite and an `attribute` otherwise, rather than always as a reference usage; the scope counts every uml:Property, so it is an upper bound",
       "scope": [
         "uml:Property"
       ]

docs/reference/sysml-v1-migration.md

@@ -149,7 +149,9 @@ ties each of its mapping classes to the code that carries it out (or records why
 | InstanceSpecification naming no classifier, under a `SimulationConfig`'s `resultLocation`, whose slots are of features of one lineage of blocks ending in the configuration's target classifier or a general of it (a simulation tool's result snapshot) | the `individual part def` of the most special of those blocks, with its slots; the note says which owner classified it and for which configuration | mapped |
 | InstanceSpecification naming no classifier, anywhere else, or under a `resultLocation` with slots of features of blocks that are no one lineage or none the target is of | comment | **unmapped** — nothing classifies it; under a `resultLocation` the note says which owners its slots have and why they type no snapshot |
 | Slot contradicting its feature (more values than the multiplicity allows, a repeated value of a feature written unique — declared so in v1, or written without its `nonunique` by the rule above — a feature of a classifier the instance is not written to specialize, an instance that is not of the property's type or of its default individual, a value outside the document) | comment | **unmapped** |
-| Slot of a port, or of an untyped property | comment (no individual can type a port; a `ref` without a type takes none) | **unmapped** |
+| Slot of a port | comment (no individual can type a port) | **unmapped** |
+| Property without a type | the usage MagicDraw's marker names — «ValueProperty» `attribute x;`, «PartProperty» `part x;`, «SharedProperty» or «ReferenceProperty» `ref x;`, «ConstraintProperty» `constraint x;` — the marker itself not written; one whose default is an individual, `ref x : 'the individual';` | mapped |
+| Property without a type or marker | `part x;` when its aggregation is composite, `attribute x;` otherwise | approximated |
 | Property whose default is an InstanceSpecification of a block | the individual added to the usage's types, or its only type when the property is untyped; no `default` (a definition is not a v2 value). A port, a usage of another kind than the individual, or a usage whose type the individual is not an instance of, keeps its types and the default is a comment | approximated |
 | Literal default on a value type with no scalar base (a structured value type, an enumeration) | comment | approximated |
 | Real literal on an `Integer`/`Natural` feature, numeric string on a scalar feature | converted to the feature's scalar | mapped |

internal/translate/migrate/behavior_members_internal_test.go

@@ -71,14 +71,14 @@ func TestBehaviorMembersUnwritten(t *testing.T) {
 			   <ownedPort xmi:type="uml:Port" xmi:id="_q" name="q"/>
 			 </packagedElement>`,
 			[]string{"_q"}, []string{"_r", "_p"}, 1,
-			[]string{"action def Run {\n    view Rules {\n        expose Run;\n        expose p;\n        render Views::asTextualNotation;\n    }\n    /* not migrated: Port 'q' — owned by a Activity, whose v2 body is its parameters and flow, not a place for a Port */\n    ref p;\n    assert constraint keep { true }\n}"}},
+			[]string{"action def Run {\n    view Rules {\n        expose Run;\n        expose p;\n        render Views::asTextualNotation;\n    }\n    /* not migrated: Port 'q' — owned by a Activity, whose v2 body is its parameters and flow, not a place for a Port */\n    attribute p;\n    assert constraint keep { true }\n}"}},
 		{"a state machine writes its constraint and attribute, and has no place for a port",
 			`<packagedElement xmi:type="uml:StateMachine" xmi:id="_b" name="Modes">` + rule("_r", "keep") + `
 			   <ownedAttribute xmi:type="uml:Property" xmi:id="_p" name="p"/>
 			   <ownedPort xmi:type="uml:Port" xmi:id="_q" name="q"/>
 			 </packagedElement>`,
 			[]string{"_q"}, []string{"_r", "_p"}, 1,
-			[]string{"state def Modes {\n    view Rules {\n        expose Modes;\n        expose p;\n        render Views::asTextualNotation;\n    }\n    /* not migrated: Port 'q' — owned by a StateMachine, whose v2 body is its parameters and states, not a place for a Port */\n    assert constraint keep { true }\n    ref p;"}},
+			[]string{"state def Modes {\n    view Rules {\n        expose Modes;\n        expose p;\n        render Views::asTextualNotation;\n    }\n    /* not migrated: Port 'q' — owned by a StateMachine, whose v2 body is its parameters and states, not a place for a Port */\n    assert constraint keep { true }\n    attribute p;"}},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			shown := append(append([]string{"_b"}, tc.unwritten...), tc.written...)

internal/translate/migrate/diagrams_internal_test.go

@@ -108,7 +108,7 @@ func TestDiagramViews(t *testing.T) {
 			[]string{"view Pumps {\n        expose Pump;\n        expose Sys::Pump::rate;\n        render Views::asTreeDiagram;\n    }"}, Mapped, ""},
 		{"a diagram of a block is written in its part def",
 			``, diagram("_d", "Pump IBD", "_pump", "SysML Internal Block Diagram", "_rate"),
-			[]string{"part def Pump {\n        ref rate;\n        view 'Pump IBD' {\n            expose rate;\n            render Views::asInterconnectionDiagram;\n        }\n    }"}, Mapped, ""},
+			[]string{"part def Pump {\n        attribute rate;\n        view 'Pump IBD' {\n            expose rate;\n            render Views::asInterconnectionDiagram;\n        }\n    }"}, Mapped, ""},
 		{"a diagram naming no owner is written where its extension is held",
 			``, diagram("_d", "Loose", "", "Class Diagram", "_pump"),
 			[]string{"view Loose {\n    expose Sys::Pump;\n    render Views::asTreeDiagram;\n}"}, Approximated, "the diagram names no owner; written at the top level"},
@@ -183,7 +183,7 @@ func TestDiagramViews(t *testing.T) {
 			   </ownedBehavior>
 			 </packagedElement>`,
 			diagram("_d", "Valves", "_sys", "SysML Block Definition Diagram", "_status"),
-			[]string{"action def Open {\n        ref status;", "view Valves {\n        expose Valve::Open::status;\n        render Views::asTreeDiagram;\n    }"}, Mapped, ""},
+			[]string{"action def Open {\n        attribute status;", "view Valves {\n        expose Valve::Open::status;\n        render Views::asTreeDiagram;\n    }"}, Mapped, ""},
 		{"a diagram of a method activity named like one of its members is numbered, the member keeping its name",
 			`<packagedElement xmi:type="uml:Class" xmi:id="_valve" name="Valve">
 			   <ownedOperation xmi:type="uml:Operation" xmi:id="_open" name="Open" method="_opening"/>
@@ -195,7 +195,7 @@ func TestDiagramViews(t *testing.T) {
 			   </ownedBehavior>
 			 </packagedElement>`,
 			diagram("_d1", "status", "_opening", "SysML Activity Diagram", "_status") + diagram("_d", "turn", "_opening", "SysML Activity Diagram", "_turn"),
-			[]string{"action def Open {\n        view 'status 2' {\n            expose Valve::Open::status;\n            render Views::asTextualNotation;", "view 'turn 2' : StandardViewDefinitions::ActionFlowView {\n            expose Open;\n            render Views::asInterconnectionDiagram;", "ref status;", "action turn {"}, Approximated,
+			[]string{"action def Open {\n        view 'status 2' {\n            expose Valve::Open::status;\n            render Views::asTextualNotation;", "view 'turn 2' : StandardViewDefinitions::ActionFlowView {\n            expose Open;\n            render Views::asInterconnectionDiagram;", "attribute status;", "action turn {"}, Approximated,
 			"written as turn 2"},
 		{"a shown association end is exposed under the name its connection def declares",
 			`<packagedElement xmi:type="uml:Class" xmi:id="_tank" name="Tank">

internal/translate/migrate/migrate.go

@@ -1891,7 +1891,7 @@ func (m *migration) valueSlot(e, slot, f *sysmlv1.Element, dir string) ([]string
 // instanceSlot resolves a slot holding instances: one redefines the feature
 // typed by its individual; several each subset it under a redefinition counting them.
 func (m *migration) instanceSlot(e, slot, f *sysmlv1.Element, kw, prefix string) ([]string, string, bool) {
-	t := m.model.Ref(f, "type")
+	types := m.effectiveTypes(f)
 	var refs []string
 	for _, v := range slot.Owned("value") {
 		if v.Type != "InstanceValue" {
@@ -1913,8 +1913,11 @@ func (m *migration) instanceSlot(e, slot, f *sysmlv1.Element, kw, prefix string)
 		if kind == catNone || !individualTypes(kind, kw) {
 			return nil, slotValueSubject + describe(inst) + " is an " + individualKeyword(kind) + ", which cannot type " + article(kw) + kw, false
 		}
-		if !m.instanceOf(classifiers, t) {
-			return nil, slotValueSubject + describe(inst) + " is not an instance of " + qualifiedName(t) + ", the type of " + f.Name, false
+		// A property with no type, its own or inherited, takes any individual of its kind.
+		for _, t := range types {
+			if !m.instanceOf(classifiers, t) {
+				return nil, slotValueSubject + describe(inst) + " is not an instance of " + qualifiedName(t) + ", the type of " + f.Name, false
+			}
 		}
 		// The default individual types the property, so a slot can only repeat it.
 		if d, _ := m.typingIndividual(f, kw); d != nil && d != inst {
@@ -1948,6 +1951,39 @@ func (m *migration) instanceSlot(e, slot, f *sysmlv1.Element, kw, prefix string)
 	return lines, "", true
 }
 
+// effectiveTypes are the types a property's usage must conform to: its own,
+// else those of every written property it redefines, subsets or shadows by name.
+func (m *migration) effectiveTypes(f *sysmlv1.Element) []*sysmlv1.Element {
+	var types []*sysmlv1.Element
+	seen := map[*sysmlv1.Element]bool{}
+	var walk func(*sysmlv1.Element)
+	walk = func(p *sysmlv1.Element) {
+		if p == nil || seen[p] {
+			return
+		}
+		seen[p] = true
+		if t := m.model.Ref(p, "type"); t != nil {
+			if !seen[t] {
+				seen[t] = true
+				types = append(types, t)
+			}
+			return
+		}
+		for _, role := range []string{"redefinedProperty", "subsettedProperty"} {
+			for _, r := range m.model.Refs(p, role) {
+				if m.written(r) {
+					walk(r)
+				}
+			}
+		}
+		if r, redefinable := m.shadowed(p); redefinable {
+			walk(r)
+		}
+	}
+	walk(f)
+	return types
+}
+
 // article is the indefinite article before a word: "an item", "a part".
 func article(word string) string {
 	if strings.ContainsRune("aeiou", rune(word[0])) {
@@ -2244,7 +2280,7 @@ func (m *migration) featureKeyword(p *sysmlv1.Element, owner category) (keyword,
 		return "port", "", ""
 	}
 	if t == nil {
-		return "ref", "", "the untyped property is written as a reference usage"
+		return m.untypedKeyword(p)
 	}
 	if owner == catUseCase && t.Type == "Actor" && m.written(t) {
 		return "actor", "", ""
@@ -2288,6 +2324,22 @@ func (m *migration) featureKeyword(p *sysmlv1.Element, owner category) (keyword,
 	return kw, "", note
 }
 
+// untypedKeyword is the usage an untyped property is written as: the kind its
+// tool's marker gives it, a ref its default individual can type, else a part
+// when composite and an attribute otherwise.
+func (m *migration) untypedKeyword(p *sysmlv1.Element) (keyword, prefix, note string) {
+	if kind := m.markedPropertyKind(p); kind != "" {
+		return kind, "", ""
+	}
+	if m.defaultIndividual(p) != nil {
+		return "ref", "", ""
+	}
+	if p.Attrs["aggregation"] == "composite" {
+		return "part", "", "the untyped composite property is written as a part"
+	}
+	return "attribute", "", "the untyped property is written as an attribute"
+}
+
 // typeKeyword is the usage keyword a property takes from its type alone,
 // before its owner and aggregation weigh in.
 func (m *migration) typeKeyword(t *sysmlv1.Element) (keyword, note string) {
@@ -4238,9 +4290,8 @@ var propertyKindMarkers = map[string]string{
 // isPropertyKindMarker recognises MagicDraw's marker of a property's kind, which
 // the usage's keyword already says; a same-named stereotype from elsewhere is kept.
 func (m *migration) isPropertyKindMarker(e *sysmlv1.Element, s *sysmlv1.Stereotype) bool {
-	kind, ok := propertyKindMarkers[s.Name]
-	if !ok || len(s.Tags) > 0 || e.Type != "Property" || e.Parent == nil ||
-		!isMagicDrawCustomization(s.Namespace) {
+	kind := propertyKindMarker(e, s)
+	if kind == "" || e.Parent == nil {
 		return false
 	}
 	owner, _ := m.classify(e.Parent)
@@ -4251,6 +4302,26 @@ func (m *migration) isPropertyKindMarker(e *sysmlv1.Element, s *sysmlv1.Stereoty
 	return usageKind(kw, prefix) == kind
 }
 
+// propertyKindMarker is the kind of usage MagicDraw's marker s says property e
+// is, or empty when s is not such a marker.
+func propertyKindMarker(e *sysmlv1.Element, s *sysmlv1.Stereotype) string {
+	kind, ok := propertyKindMarkers[s.Name]
+	if !ok || len(s.Tags) > 0 || e.Type != "Property" || !isMagicDrawCustomization(s.Namespace) {
+		return ""
+	}
+	return kind
+}
+
+// markedPropertyKind is the kind MagicDraw's marker on property e gives it, if any.
+func (m *migration) markedPropertyKind(e *sysmlv1.Element) string {
+	for _, s := range e.Stereotypes {
+		if kind := propertyKindMarker(e, s); kind != "" {
+			return kind
+		}
+	}
+	return ""
+}
+
 // usageKind is the kind of property a usage keyword and its `ref ` prefix express.
 func usageKind(kw, prefix string) string {
 	switch {

tests/migrate/properties_common_test.go

@@ -29,7 +29,7 @@ func TestPropertyCommonPartsAreWrittenOnTheUsage(t *testing.T) {
 	}
 }
 
-func TestUntypedPropertiesAreReferencesOutsideConstraintDefinitions(t *testing.T) {
+func TestUntypedPropertiesAreAttributesInAndOutsideConstraintDefinitions(t *testing.T) {
 	r := migrateDocument(t, `
     <packagedElement xmi:type="uml:Class" xmi:id="_owner" name="Owner">
       <ownedAttribute xmi:type="uml:Property" xmi:id="_plain" name="plain"/>
@@ -39,6 +39,6 @@ func TestUntypedPropertiesAreReferencesOutsideConstraintDefinitions(t *testing.T
     </packagedElement>`,
 		`<sysml:ConstraintBlock xmi:id="_constraintStereotype" base_Class="_constraint"/>`)
 	wantClean(t, "property_untyped.sysml", r)
-	wantLine(t, r.Notation, "ref plain;")
+	wantLine(t, r.Notation, "attribute plain;")
 	wantLine(t, r.Notation, "in attribute parameter[1];")
 }

tests/migrate/relations_test.go

@@ -377,10 +377,10 @@ func TestUnwritableFeaturePartsAreDroppedWithNotes(t *testing.T) {
     </packagedElement>`, `
   <sysml:Block xmi:id="_s2" base_Class="_h"/>`)
 	wantLine(t, r.Notation, "ref intro;")
-	wantLine(t, r.Notation, "ref grid;")
-	wantLine(t, r.Notation, "ref many[0..*];")
-	wantLine(t, r.Notation, "ref margin {")
-	wantLine(t, r.Notation, `ref exposure default = "4";`)
+	wantLine(t, r.Notation, "attribute grid;")
+	wantLine(t, r.Notation, "attribute many[0..*];")
+	wantLine(t, r.Notation, "attribute margin {")
+	wantLine(t, r.Notation, `attribute exposure default = "4";`)
 	if strings.Contains(string(r.Notation), "= NaN") {
 		t.Errorf("a non-finite real was written:\n%s", r.Notation)
 	}
@@ -426,9 +426,9 @@ func TestClashingSiblingNamesAreDistinguished(t *testing.T) {
   <sysml:Block xmi:id="_s3" base_Class="_c"/>
   <sysml:Block xmi:id="_s4" base_Class="_c2"/>
   <sysml:Block xmi:id="_s5" base_Class="_u"/>`)
-	wantLine(t, r.Notation, "ref unnamed1;")
-	wantLine(t, r.Notation, "ref 'unnamed1 2';")
-	wantLine(t, r.Notation, "ref q :>> 'unnamed1 2';")
+	wantLine(t, r.Notation, "attribute unnamed1;")
+	wantLine(t, r.Notation, "attribute 'unnamed1 2';")
+	wantLine(t, r.Notation, "attribute q :>> 'unnamed1 2';")
 	wantLine(t, r.Notation, "part def 'C 2'")
 	wantLine(t, r.Notation, "part x : 'C 2';")
 	for id, want := range map[string]string{
@@ -501,8 +501,8 @@ func TestExternalSpecializationsAreNotWritten(t *testing.T) {
       </slot>
     </packagedElement>`, `
   <sysml:Block xmi:id="_s1" base_Class="_h"/>`)
-	wantLine(t, r.Notation, "ref a;")
-	wantLine(t, r.Notation, "ref b;")
+	wantLine(t, r.Notation, "attribute a;")
+	wantLine(t, r.Notation, "attribute b;")
 	if strings.Contains(string(r.Notation), "_ext") || strings.Contains(string(r.Notation), "unnamed") {
 		t.Errorf("notation refers to an external feature:\n%s", r.Notation)
 	}
@@ -1310,10 +1310,19 @@ func TestConstraintParametersStoredAsPortsAreInParameters(t *testing.T) {
 
 // MagicDraw's tagless property-kind markers say what the usage keyword says,
 // so they are not written; a same-named stereotype from any other profile, a
-// marker carrying a tag, and one on a property of another kind are kept.
+// marker carrying a tag, and one on a property of another kind are kept. An
+// untyped property takes its marker's kind; without one it is an attribute,
+// or a part when composite.
 func TestPropertyKindMarkersAreNotWritten(t *testing.T) {
 	r := migrateFixtureFile(t, "property_markers")
 	for _, line := range []string{
+		"attribute load;",
+		"part trailer;",
+		"ref depot;",
+		"ref driver;",
+		"constraint rule;",
+		"attribute color;",
+		"part chassis;",
 		"attribute mass : ScalarValues::Real;",
 		"attribute mode : Mode;",
 		"part engine : Engine;",
@@ -1341,11 +1350,14 @@ func TestPropertyKindMarkersAreNotWritten(t *testing.T) {
 			t.Errorf("«%s» written %d times, want %d", name, n, want)
 		}
 	}
-	for _, id := range []string{"_mass", "_mode", "_engine", "_lead", "_limit", "_ms_v", "_ms_wheel", "_ms_hub", "_ms_k", "_odd", "_axle", "_cabin", "_serial"} {
+	for _, id := range []string{"_mass", "_mode", "_engine", "_lead", "_limit", "_ms_v", "_ms_wheel", "_ms_hub", "_ms_k", "_odd", "_axle", "_cabin", "_serial",
+		"_load", "_trailer", "_depot", "_driver", "_rule"} {
 		if es := entriesFor(r, id); len(es) != 1 || es[0].Verdict != migrate.Mapped {
 			t.Errorf("%s entries = %+v", id, es)
 		}
 	}
+	wantNote(t, r, "_color", migrate.Approximated, "the untyped property is written as an attribute")
+	wantNote(t, r, "_chassis", migrate.Approximated, "the untyped composite property is written as a part")
 	if es := entriesFor(r, "_spare"); len(es) != 1 || es[0].Verdict != migrate.Approximated || !strings.Contains(es[0].Note, "shared aggregation") {
 		t.Errorf("_spare entries = %+v", es)
 	}