Repository navigation
fix(migrate): keep sequence lifelines off inherited occurrence member names - #2132
Open
devin-ai-integration[bot] wants to merge 9 commits into
Open
devin-ai-integration[bot] wants to merge 9 commits into
devin-ai-integration[bot] wants to merge 9 commits into
Conversation
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Contributor
Author
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Contributor
Author
There was a problem hiding this comment.
Devin Review found 2 findings. Devin automatically fixed all 2 findings.
Fixed by Devin (2)
- Renamed roots strand sequence member references — Real: a root renamed to OccurrencesModel stopped the member rewrite, leaving
...Sequence::portionOfLife(type error). Root and member spellings are now computed together from the source references. Regression: TestSequenceExpressionThroughRenamedRoot. - Typed local reads retain inherited names — Real:
{in x : Sequence; x.'self'.amount}kept 'self' (unresolved member). Typed locals are now followed through their declared type and member spans are rewritten, without renaming the local itself. Regression: TestSequenceExpressionTypedLocalMember.
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
A SysML v1 interaction migrated as an occurrence def could declare a lifeline usage named after a member every occurrence inherits, e.g.
ref part self : X;, which hidesOccurrence::selfand drawswarning[name-conflict]: Duplicate of inherited member name 'self' from Occurrence.Where
selfcomes from: the name is the source's own, not one the migrator made up. The Interaction owns auml:Propertyliterally namedself(private, typed by a block, no aggregation, no stereotype), and an unnameduml:Lifelinerepresentsit.scenario.lifelinetakes the represented property's name throughnameFor. The interaction-scoped name setscenario.usedstarted empty, so nothing reserved the inherited names. Activity and state bodies already reserve theirs (inheritedActionNames,inheritedStateNamesSet).Fix:
scenarionow seeds its name set with every member name ofOccurrences::Occurrenceand its supertypes, read from the bundled standard library once and cached (inheritedOccurrenceNames). This covers both the occurrence-def and the occurrence-usage form. A clashing lifeline, message, step, parameter or interaction-use name is then renamed by the existingfreshInrule (self→self2). The represented property's migrated name follows its lifeline, so message ends (endRef) andDiagramLayout::Layout about …references all write the same name. The migration report gives the rename as "since an occurrence inherits a member named self".Expressions that read a renamed interaction member resolve against the source name and are rewritten to the migrated one, so a default such as
portionOfLife + 1keeps working once its parameter becomesportionOfLife2. This covers qualified paths, forward references to later parameters, and reads through a lifeline's represented property. The cached name set is keyed by the active library index, so a library override in the same process reserves its own names.Specification basis
self. In UML 2.5.1 §17.3.4,selfhas a meaning only as a Lifeline's name keyword, where it stands for the object of the classifier that owns the interaction. These lifelines are unnamed, and the property is typed by the context's general classifier rather than by the context itself.Occurrence::selfis the occurrence itself: here, the interaction's own occurrence (KerML 1.0, Occurrences library). It is not a block instance. So pointing the lifeline at the inheritedself, or redefining it, would change the meaning. Keeping the participant as its own usage under a distinct name keeps the source semantics.How it was verified
tests/migrate/sequence_names_test.gobuilds a block-owned interaction with a propertyself, an unnamed lifeline representing it, a second lifeline and a message. On develop it fails (sequence declares the inherited name self, plus thename-conflictwarning). With the fix it passes, and it checks the message end and the Layout reference.interaction_names_internal_test.gocovers the derived set, suffixing, and that the cached set is never mutated. Furthersequence_names_test.gocases cover expressions that read renamed parameters and lifeline properties: defaults, forward and qualified references, and message arguments.sequence_library_names_test.gomigrates under the bundled library and then two overrides in one process.-migrate sysml -strict, then-validate):name-conflict, 57bound-feature-types, 3subsetting-multiplicity.bound-feature-types, 3subsetting-multiplicity..sysmldiff is exactly 17 lines:ref part self→ref part self2.-layout(MTIP): the same count movement and 34 changed lines, which are the 17 declarations plus their 17Layout about self→Layout about self2.go build ./...,go vet ./...,gofmt,make lintand the fullgo test ./...(corpora provisioned) all pass.TestPSSMSuiteMigrationpasses, and no migration baseline moved.Checklist
make testandmake lintpass locallychanges/unreleased/<slug>.<section>.md, not as an edit toCHANGELOG.mdmake docs-countsrun if a gate count moved (compliance rows need nothing: the census is counted at docs build)F4,K5) in the body, docs, or changelogLink to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/31010346b7e149fdaf7a6cdf176173cf
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/31010346b7e149fdaf7a6cdf176173cf?variant=devin
Requested by: @HuiJun
🟢 Devin Review found nothing to fix · View findings · Follow Devin's work