osate / osate2

Open Source AADL2 Tool Environment
http://osate.org
Eclipse Public License 2.0
39 stars 8 forks source link

Annexes are Not Formatted/Have Modified Formatting When Saving an Aadl Resource #1074

Closed philip-alldredge closed 6 years ago

philip-alldredge commented 6 years ago

@lwrage this is the issue I mentioned in my email. It appears I spoke too soon with regards to the root cause of my issue.

This was found when using the graphical editor. When the xtext editor is not open, the graphical editor works with an EMF resource for the AADL model. When it saves the resource, it uses the format option to format the model. However, this option does not seem to format annexes. However, the formatting of the annex is not preserved when any change is made to the AADL model. Additional white space is inserted.

While using the tool, this somehow resulted in a 34MB AADL source file when working with a fairly simple model. Most of the contents of the model was white-space. I am guessing that the white spaces are not added to just the start of the annex but to other parts as well. The result of such a file was an usable text editor and an unusable graphical editor. Example included below.

Before modification:

package test
public
    system top
        annex agree {**assume "c" : true;**};
    end top;

    system implementation top.impl
    end top.impl;

    annex EMV2 {**
error types NewErrorType : type ;
end types ;**};
end test;

After adding a subcomponent:

package test
public
    system top
        annex agree {**                                                      assume "c" : true;**};
    end top;

    system implementation top.impl
        subcomponents
            top_impl_new_subcomponent: system;
    end top.impl;

    annex EMV2 {**                                                                                                                                                           
error types NewErrorType : type ;
end types ;**};
end test;
lwrage commented 6 years ago

I vaguely remember a bug where one space character was added somewhere for each character in the file before the annex start. That blew up quickly...

philip-alldredge commented 6 years ago

This may be just a way to hide the bug, but passing in save options so that the annex will be formatted gets rid of the extra whitespace. I experimented with this by adding "SaveOptions.newBuilder().format().getOptions()" as an extra argument to the serialize() calls in the AnnexUnparser implementations.

However the formatting is sub-optimal because it is formatted as if it was its own document.

lwrage commented 6 years ago

I cannot reproduce this. I create the aadl file for the package, create a structure diagram for top.impl, close the text editor, add a subcomponent in the GE => I get an exception in the agree unparser...

...
Caused by: java.lang.RuntimeException: No Context for AadlPackage'test'.ownedPublicSection->PublicPackageSection'test_public'.ownedClassifier[0]->SystemType'top'.ownedAnnexSubclause[0]->DefaultAnnexSubclause'agree'.parsedAnnexSubclause->AgreeContractSubclause'agree' could be found
    at org.eclipse.xtext.serializer.impl.Serializer.getIContext(Serializer.java:165)
    at org.eclipse.xtext.serializer.impl.Serializer.serialize(Serializer.java:126)
    at org.eclipse.xtext.serializer.impl.Serializer.serialize(Serializer.java:178)
    at org.eclipse.xtext.serializer.impl.Serializer.serialize(Serializer.java:55)
    at com.rockwellcollins.atc.agree.unparsing.AgreeAnnexUnparser.unparseAnnexSubclause(AgreeAnnexUnparser.java:60)
    at org.osate.annexsupport.AnnexUnparserProxy.unparseAnnexSubclause(AnnexUnparserProxy.java:85)
    at org.osate.xtext.aadl2.serializer.Aadl2SemanticSequencer.createSequence(Aadl2SemanticSequencer.xtend:92)

If I remove the agree annex subclause I get an exception about a missing transaction when the emv2 annex is serialized. I guess that's #1063.

lwrage commented 6 years ago

When trying this on the 1063 branch I still get the agree serialization exception but it's only in the log.

philip-alldredge commented 6 years ago

Okay. To reproduce the issue you must be on #1063 and not have the AGREE subclause. The AGREE subclause is a separate issue. I had a solution for that implemented in my AGREE branch since it hasn't been an issue until now. It seems that a recent change(enabling formatting?) is causing the sequencer to be ran which is why #1063 is causing a problem even if an annex wasn't modified. In the past the sequencer hasn't ran unless the annex model was modified.

joeseibel commented 6 years ago

I tested this with EMV2 and set a breakpoint on EMV2AnnexUnparser.unparseAnnexLibrary(). The serializing of the annex was only happening when the text editor was closed. If the text editor was open, then annex serialization would not happen at all.

I think that the spaces are placed in there because the serializer looks to see if there is a node model associated with the objects and tries to serialize them such that they are at the same offset that they were before. This creates a string with many spaces in the beginning followed by the error model text. The Aadl2SemanticSequencer then takes that string and simply places it between {** and **}.

A simple solution is to trim the serialized string in the annex unparser. In the long term, I think it would be better to implement the new formatter for EMV2 and figure out how to call the annex formatter when formatting an AADL file.

joeseibel commented 6 years ago

@lwrage This problem has nothing to do with the indent parameter. indent is not even used in EMV2AnnexUnparser.

joeseibel commented 6 years ago

See osate/ErrorModelV2@31c22c03c8d96989f375dad98f43d787782e2a2f.