PackageDeclarationCS irregularities

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 251349 | | Status | CLOSED FIXED | | Importance | P3 normal | | Reported | Oct 20, 2008 02:07 EDT | | Modified | May 27, 2011 02:40 EDT | | Reporter | Ed Willink |

Description

A multi-package OCL module is parsed but not analyzed.

The packageDeclarationCSm rule creates an inverse order CST daisy chain.

PackageDeclarationCS.packageDeclarationCS has a missing containment.

The analyzer has no support for multiple packages.

It would be helpful to introduce a TopLevelCS node to contain the PackageDeclarationCS objects in natural order, and to support a 1:1 root AST node (Resource) to root CST node (TopLevelCS) mapping.

It would be helpful to introduce a topLevelCS action to analyze the multiple packages.

This should give no API incompatibilities.

Is it ok to fix this as part of Bug 242236?

Better: can you add TopLevelCS, so that the UML tooling is correct. I have no idea how to edit OCLCST.uml - it looks quite unmanageable.

[eclipse-ocl-bot]

By Christian Damus on Nov 30, 2008 21:52

Created attachment 119108 (attachment deleted)\ Top-level OCLDocumentCS

Attached a patch that introduces a top-level OCLDocumentCS construct to collect the PackageDeclarationCS elements into a conventional containment property.

The parser grammar is not changed, because producing this new construct as the goal of an OCL parse would constitute a breaking API change. However, the infrastructure should make it easy for an extended grammar to add this.

Please, let me know how this works for you.

[eclipse-ocl-bot]

By Ed Willink on Dec 01, 2008 03:12

Great, I can delete 3 packages, 8 classes, 1 Model ...

But:

OCLDocumentCS needs to extend CSTNode otherwise it is the only CST node that isn't a CSTNode.

There is an unnecessary cast in: packageDeclarationCS((PackageDeclarationCS) decl, constraints);

The OCLAnalyzer.parseOCLDocument method doesn't help or hinder me.

For consistency a new analyzer method could have a signature such as:

    XXXAnalyzer.defineTopLevelCS(topLevelCS, environment);

so that the environment can provide the TopLevel CST node prior to and independently of populating it.

My derived OCLFileAnalyzer method is:

public boolean analyze(OCLTopLevelEnvironment environment) {\
    this.topLevelEnvironment = environment;\
    List<Constraint> constraints = new ArrayList<Constraint>();\
    for (PackageDeclarationCS pkgdecl : environment.getCSTNode().getPackageDeclarations())\
        packageDeclarationCS(pkgdecl, constraints);\
    if (!constraints.isEmpty())\
        environment.getASTNode().getContents().addAll(constraints);\
    return true;\
}

from which the last few lines could become:

public boolean analyze(OCLTopLevelEnvironment environment) {\
    this.topLevelEnvironment = environment;\
            oclDocumentCS(environment.getCSTNode(), environment);\
    return true;\
}

[eclipse-ocl-bot]

By Christian Damus on Dec 01, 2008 09:41

(In reply to comment #2)

Great, I can delete 3 packages, 8 classes, 1 Model ...

But:

OCLDocumentCS needs to extend CSTNode otherwise it is the only CST node that isn't a CSTNode.

Good point. I had thought that it might be omitted, as this isn't really an element of the concrete syntax (it's just an artificial container), but it's not a significant distinction. It's nicer for switch-based visitors and other purposes to let it be a CSTNode.

There is an unnecessary cast in: packageDeclarationCS((PackageDeclarationCS) decl, constraints);

Yeah. I do a warnings sweep before I commit. It's a little more challenging since the latest JDT build introduced spurious raw type warnings on the instanceof operator. I must raise a bug on that if there isn't already one.

The OCLAnalyzer.parseOCLDocument method doesn't help or hinder me.

That's fine. OCL has more clients than just QVT Declarative that need to be considered, and this will help at least one that I know of. Actually, the patch doesn't have a later change that I made, to update parseOCLDocument(...) to call documentCS(...).

For consistency a new analyzer method could have a signature such as:

XXXAnalyzer.defineTopLevelCS(topLevelCS, environment);

so that the environment can provide the TopLevel CST node prior to and independently of populating it.

For consistency with what existing API? I don't see anything like that. The parser can create a root OCLDocumentCS and the AbstractOCLAnalyzer::documentCS(...) method can process it in the analyzer's root environment. It's consistent with the existing packageDeclarationCS(...) method.

Actually, if anything, an alternative packageDeclarationCS(...) should be provided that accepts an environment, like the contextDeclCS(...), for use in the analysis of an OCLDocumentCS. I'll attach a new patch that does this.

[eclipse-ocl-bot]

By Christian Damus on Dec 01, 2008 09:47

Created attachment 119146 Updated CST and API

Attached an updated patch that adds the OCLDocumentCS generalization relationship to CSTNode and adds a packageDeclarationCS(...) overload that accepts an Environment, for use with the top-level document CS.

:notepad_spiral: oclDocumentCS.patch

[eclipse-ocl-bot]

By Christian Damus on Dec 02, 2008 07:01

Committed the revised patch to HEAD (1.3 branch).

[eclipse-ocl-bot]

By Christian Damus on Dec 04, 2008 10:35

Fix available in HEAD: 1.3.0.I200812021600.

[eclipse-ocl-bot]

By Ed Willink on May 27, 2011 02:38

Closing after over a year in verified state.

[eclipse-ocl-bot]

By Ed Willink on May 27, 2011 02:40

Closing after over a year in verified state.