Systems-Modeling / SysML-v2-Pilot-Implementation

Proof-of-concept pilot implementation of the SysML v2 textual notation and visualization
GNU Lesser General Public License v3.0
114 stars 23 forks source link

ST6RI-770 Computing the derived property View::exposedElement causes an exception #565

Closed seidewitz closed 1 month ago

seidewitz commented 1 month ago

This PR fixes a bug that was introduced in PR #554 (Implement invocation delegates for operations).

The Import_importedMembers_InvocationDelegate handles the implementation of the Import::importedMembers operation for all concrete specializations of the abstract metaclass Import. Unfortunately, the class Import_importedMembers_InvocationDelegate was also left abstract in PR #554, which means it could not be instantiated when the operation was invoked. The bug presented itself in a failure to compute the derived value for the ViewUsage::exposedElement property, whose implementation calls importedMembers on Expose relationships, which are kinds of Import relationships (this happens, e.g., when doing a %publish or %view in Jupyter on a view usage with an expose).

This PR includes the following changes:

  1. Makes Import_importedMembers_InvocationDelegate class concrete, which resolves the reported bug.
  2. Slightly refactors the SysML grammar productions for Expose to avoid a possible ParseException on an ill-formed expose declaration.
  3. Updates the compiler and JRE settings for various Eclipse projects to ensure they are all consistent with Java 17.

Note. Since the 2024-04 release has already been tagged, the changes from this PR will be included in an incremental 2024-04.1 release.

himi commented 1 month ago

I'm testing this but I could not reproduce this problem with your test case:

package Test {
    package P {
        public part p1;
    }

    view v {
        expose P::*;
    }
}

was parsed without an error: Package Test (3f7207c9-87e8-4eca-9253-56572e8c47ad) However,

package Test {
    package P {
        public part p1;
    }

    view v {
        expose P::*::*;
    }
}

caused an exception you mentioned.

org.eclipse.xtext.parser.ParseException: java.lang.IllegalArgumentException: Cannot create instance of abstract class 'Expose'
    at org.eclipse.xtext.parser.antlr.AbstractAntlrParser.doParse(AbstractAntlrParser.java:106)
    at org.eclipse.xtext.parser.antlr.AbstractAntlrParser.parse(AbstractAntlrParser.java:85)
    at org.eclipse.xtext.parser.antlr.AbstractAntlrParser.doParse(AbstractAntlrParser.java:63)
    at org.eclipse.xtext.parser.AbstractParser.parse(AbstractParser.java:34)
    at org.eclipse.xtext.resource.XtextResource.doLoad(XtextResource.java:178)
    at org.eclipse.xtext.linking.lazy.LazyLinkingResource.doLoad(LazyLinkingResource.java:114)
    at org.eclipse.xtext.resource.XtextResource.reparse(XtextResource.java:215)
    at org.omg.sysml.interactive.SysMLInteractive.parse(SysMLInteractive.java:169)
    at org.omg.sysml.interactive.SysMLInteractive.process(SysMLInteractive.java:209)
    at org.omg.sysml.interactive.SysMLInteractive.process(SysMLInteractive.java:203)
    at org.omg.sysml.jupyter.kernel.SysMLKernel.eval(SysMLKernel.java:102)
    at io.github.spencerpark.jupyter.kernel.BaseKernel.handleExecuteRequest(BaseKernel.java:334)
    at io.github.spencerpark.jupyter.channels.ShellChannel.lambda$bind$0(ShellChannel.java:64)
    at io.github.spencerpark.jupyter.channels.Loop.lambda$new$0(Loop.java:21)
    at io.github.spencerpark.jupyter.channels.Loop.run(Loop.java:78)

So let me suggest to change the xpect test case.

seidewitz commented 1 month ago

That's a different exception. expose P::*::*; is not legal syntax. Though that should really be reported as an error, not an exception.

The bug was not in parsing your first example. It was in calling the importedMembership operation on an Expose relationship or computing the exposedElement derived property. This cannot be tested in Xpect, because this property is not used in any validation. I added a JUnit test to org.omg.sysml.interactive.test.DerivedPropertyTest to test it programmatically.

The bug was discovered by trying to %publish the model from Jupyter, because, in this case, all derived properties are computed. If you try this in the current JupyterHub deployment, you will see the exception. It will also happen if you try to do a %view on the view usage. If you install a local Jupyter instance from this branch, the exception will not occur.

himi commented 1 month ago

So is that the exception you mentioned? Screenshot 2024-05-09 at 5 17 08 PM

seidewitz commented 1 month ago

Yes. The exception trace and steps to reproduce are in the Jira bug report, but I didn't all of that into the PR description.

himi commented 1 month ago

I checked it in ST6RI-770 branch and confirmed it worked: Screenshot 2024-05-09 at 5 29 56 PM

However, as you implied, the parsing problem is not solved because it's different issue. I hope it should be fixed as well because exception is not a correct result for syntax error. But I think it's not urgent.

himi commented 1 month ago

Just note that %publish was worked without an error:

Screenshot 2024-05-09 at 5 34 13 PM

seidewitz commented 1 month ago

The parse exception seems to be a problem with the ANTLR grammar generated by Xtext. The odd thing is that the problem does not happen for Import, which has almost the same syntax. I slightly refactored the productions for Expose so they were exactly like Import, and the problem went away. So, I am going to go ahead and include this update in the PR, too.

himi commented 1 month ago

Thank you. I confirmed that showed an error message instead of throwing an exception: Screenshot 2024-05-09 at 6 14 29 PM