Issue 18955: Problems with OCL definition of Package::makesVisible (uml2-rtf) Source: No Magic, Inc. (Mr. Tomas Juknevicius, tomas.juknevicius(at)nomagic.com) Nature: Uncategorized Issue Severity: Summary: Looking at the UML2.5 specification draft (I have the document UML25AfterBallot9.pdf - not sure if this is the newest one) I see problems with definition of Package::makesVisible - which is expressed in OCL.: makesVisible(el : NamedElement) : Boolean The query makesVisible() defines whether a Package makes an element visible outside itself. Elements with no visibility and elements with public visibility are made visible. pre: member->includes(el) body: ownedMember->includes(el) or (elementImport->select(ei|ei.importedElement = VisibilityKind::public)->collect(importedElement.oclAsType(NamedElement))->includes(el)) or (packageImport->select(visibility = VisibilityKind::public)->collect(importedPackage.member->includes(el))->notEmpty()) Actually those problems carry on from the previous versions of UML; but since in previous versions even the OCL syntax was wrong (carried over from the pre-OCL2.0 times) I assumed this section is old/abandoned and did not pay much attention to it. But now with UML2.5 somebody took it seriously to update the syntax of the OCLs (kudos for that brave soul :-) ), so we have an updated variant. But while the raw syntax problems were fixed, semantic problems were carried form the old revision verbatim. If we are updating OCLs anyway, I think it would be a good time to also correct those. So here goes: -------------------------------------------------------------------------------- Problem #1 the following comparison is nonsensical (the case handling ElementImports, line #2 of the body): ei.importedElement = VisibilityKind::public The OCL here tries to compare the model element (at the end of ElementImport relationship) with the enumeration literal - VisibilityKind::public, which is not what we want I think this passage should be restated as follows: ei.visibility= VisibilityKind::public i.e. we want to test whether element import has visibility set to public, just as in the other case - with package imports - one line below. Also the whole case handling element imports could be rewritten to simplify it: elementImport->exists(ei|ei.visibility = VisibilityKind::public and ei.importedElement = el) This does not change the semantics, but is much better readable/understandable: we are iterating through all (public) element imports checking whether imported element matches the element el. -------------------------------------------------------------------------------- Problem #2 the case handling package imports (line #3 of the body) is also borked: packageImport->select(visibility = VisibilityKind::public)->collect(importedPackage.member->includes(el))->notEmpty() Here the first part of the expression is OK; we take all package import relationships and filter them - accept only public ones: packageImport->select(visibility = VisibilityKind::public) But the next part again makes no sense ...->collect(importedPackage.member->includes(el))->notEmpty() here expression part importedPackage.member->includes(el) produces a boolean - whether element el is included among the members of the package being imported. So the result of the expression part ...->collect(importedPackage.member->includes(el))... is a collection of booleans (of the form: {false, false, true, false, true} ), where each boolean signifies whether element is among the members of each particular imported package. Then it makes no sense to test that for emptiness: ->notEmpty() this produces true if there is at least one item (does not matter true, or false) in that bag of booleans. So that part produces true if there is at least 1 public package import ( it does not matter what package is imported). I think this passage should be restated as follows: packageImport->select(visibility = VisibilityKind::public)->exists(importedPackage.member->includes(el)) I.e. we are iterating through all (public) package imports and checking whether element el appears among members of at least one of the imported packages. -------------------------------------------------------------------------------- So the final OCL of makesVisible could be (also getting rid of some unnecessary parentheses, and further simplification): pre: member->includes(el) body: ownedMember->includes(el) or elementImport->exists(ei|ei.visibility = VisibilityKind::public and ei.importedElement = el) or packageImport->exists(pi|pi.visibility = VisibilityKind::public and pi.importedPackage.member->includes(el)) Resolution: Revised Text: Actions taken: September 24, 2013: received issue Discussion: End of Annotations:===== rus-Scanned: OK From: Ed Seidewitz To: Nerijus Jankevicius CC: "uml25-ftf@omg.org" Subject: RE: Problems with OCL definition of Package::makesVisible Thread-Topic: Problems with OCL definition of Package::makesVisible Thread-Index: AQHOuj0jcZCy0Y/dWUqnHYCs5Tmrb5nYagQQ Date: Thu, 26 Sep 2013 19:38:31 +0000 Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [63.117.77.253] X-Virus-Scanned: amavisd-new at omg.org Yes, this is a mess. It is a shame we didn.t fix it in UML 2.5. There is, in fact, a further problem: the OCL treats all ownedMembers of a package as being made visible, not just the ones with no visibility or public visibility! So, I believe the OCL should be: pre: member->includes(el) body: ownedMember->includes(el) and (el.visibility = null or el.viisbility = VisibilityKind::public) or elementImport->exists(ei|ei.visibility = VisibilityKind::public and ei.importedElement = el) or packageImport->exists(pi|pi.visibility = VisibilityKind::public and pi.importedPackage.makesVisible(el))) -- Ed From: Nerijus Jankevicius [mailto:nerijus@nomagic.com] Sent: Wednesday, September 25, 2013 6:17 PM To: uml25-ftf@omg.org Subject: Fwd: Problems with OCL definition of Package::makesVisible resending, cause he was filtered by OMG server. From: T J Date: September 24, 2013 12:30:02 PM EDT To: issues@omg.org, uml2-rtf@omg.org Cc: tomasjkn@nomagic.com, nerijus@nomagic.com Subject: Problems with OCL definition of Package::makesVisible Hello dear UML developers, Looking at the UML2.5 specification draft (I have the document UML25AfterBallot9.pdf - not sure if this is the newest one) I see problems with definition of Package::makesVisible - which is expressed in OCL.: makesVisible(el : NamedElement) : Boolean The query makesVisible() defines whether a Package makes an element visible outside itself. Elements with no visibility and elements with public visibility are made visible. pre: member->includes(el) body: ownedMember->includes(el) or (elementImport->select(ei|ei.importedElement = VisibilityKind::public)->collect(importedElement.oclAsType(NamedElement))->includes(el)) or (packageImport->select(visibility = VisibilityKind::public)->collect(importedPackage.member->includes(el))->notEmpty()) Actually those problems carry on from the previous versions of UML; but since in previous versions even the OCL syntax was wrong (carried over from the pre-OCL2.0 times) I assumed this section is old/abandoned and did not pay much attention to it. But now with UML2.5 somebody took it seriously to update the syntax of the OCLs (kudos for that brave soul :-) ), so we have an updated variant. But while the raw syntax problems were fixed, semantic problems were carried form the old revision verbatim. If we are updating OCLs anyway, I think it would be a good time to also correct those. So here goes: -------------------------------------------------------------------------------- Problem #1 the following comparison is nonsensical (the case handling ElementImports, line #2 of the body): ei.importedElement = VisibilityKind::public The OCL here tries to compare the model element (at the end of ElementImport relationship) with the enumeration literal - VisibilityKind::public, which is not what we want I think this passage should be restated as follows: ei.visibility= VisibilityKind::public i.e. we want to test whether element import has visibility set to public, just as in the other case - with package imports - one line below. Also the whole case handling element imports could be rewritten to simplify it: elementImport->exists(ei|ei.visibility = VisibilityKind::public and ei.importedElement = el) This does not change the semantics, but is much better readable/understandable: we are iterating through all (public) element imports checking whether imported element matches the element el. -------------------------------------------------------------------------------- Problem #2 the case handling package imports (line #3 of the body) is also borked: packageImport->select(visibility = VisibilityKind::public)->collect(importedPackage.member->includes(el))->notEmpty() Here the first part of the expression is OK; we take all package import relationships and filter them - accept only public ones: packageImport->select(visibility = VisibilityKind::public) But the next part again makes no sense ...->collect(importedPackage.member->includes(el))->notEmpty() here expression part importedPackage.member->includes(el) produces a boolean - whether element el is included among the members of the package being imported. So the result of the expression part ...->collect(importedPackage.member->includes(el))... is a collection of booleans (of the form: {false, false, true, false, true} ), where each boolean signifies whether element is among the members of each particular imported package. Then it makes no sense to test that for emptiness: ->notEmpty() this produces true if there is at least one item (does not matter true, or false) in that bag of booleans. So that part produces true if there is at least 1 public package import ( it does not matter what package is imported). I think this passage should be restated as follows: packageImport->select(visibility = VisibilityKind::public)->exists(importedPackage.member->includes(el)) I.e. we are iterating through all (public) package imports and checking whether element el appears among members of at least one of the imported packages. -------------------------------------------------------------------------------- So the final OCL of makesVisible could be (also getting rid of some unnecessary parentheses, and further simplification): pre: member->includes(el) body: ownedMember->includes(el) or elementImport->exists(ei|ei.visibility = VisibilityKind::public and ei.importedElement = el) or packageImport->exists(pi|pi.visibility = VisibilityKind::public and pi.importedPackage.member->includes(el)) Sincerely, -- Tomas Juknevicius System Analyst No Magic Europe Savanoriu 363 - IV fl., LT-49425, Kaunas, Lithuania Phone: +370-37-324032; Fax: +370-37-320670 e-mail: Tomas.Juknevicius@nomagic.com WWW: http://www.magicdraw.com, http://www.nomagic.com