Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APIs from concerto-core are breaking for aliased models whose types are resolved. #972

Open
sanketshevkar opened this issue Dec 17, 2024 · 0 comments

Comments

@sanketshevkar
Copy link
Member

sanketshevkar commented Dec 17, 2024

Bug Report 🐛

The implementation for the resolution for the previous similar issue is leading to a few APIs in concerto-core and concerto-cto breakiing when they try to refer to the resolved type reference for the imported-aliased declarations.

Main issue is that when a model AST is resolved, we loose the reference to the aliased type names instead have the resolved type name, which was replaced.

Expected Behavior

Resolved model should preserve both the resolved type names and the original type name as specified by the user.

Current Behavior

Resolved model only replaces the type names used by the user with resolved type names (taking into account aliases).

Possible Solution

Steps to Reproduce

  1. Model Decoration
    If we try to decorate these models with given DCS, the process fails with error.
namespace [email protected]

concept BaseNDA {
    o Date executiondate
}

             +

namespace [email protected]

import [email protected].{BaseNDA as NewNDA}

concept BaseNDA extends NewNDA {
    o String person1
}

            +

{
    "commandSet": [
        {
            "type": "UPSERT",
            "target": {
              "namespace": "namespace [email protected]",
              "declaration": "NDA",
              "$class": "[email protected]"
            },
            "decorator": {
              "name": "TestMe",
              "arguments": [],
              "$class": "[email protected]"
            },
            "$class": "[email protected]"
          }
    ]
}

            =

    Error
  1. Printing a resolved model from AST, where an declaration extending an aliased declaration would break
    If we try to load these models, validate, resolve and print these models, the printing step would result in an error.
namespace [email protected]

concept BaseNDA {
    o Date executiondate
}

         +

namespace [email protected]

import [email protected].{BaseNDA as NewNDA}

concept BaseNDA extends NewNDA {
    o String person1
}

        =

Error
  1. Print a resolved model from AST, where a property instantiated using an aliased declaration would change to the base declaration rather than the aliased declaration.
    if we try to load these models, validate, resolve and print these models, the processes are successful but we loose reference to the aliased type which is imported in the property instantiation.
namespace [email protected]

concept BaseNDA {
    o Date executiondate
}

        +

namespace [email protected]

import [email protected].{BaseNDA as NewNDA}

concept NDA {
    o NewNDA newNDA
}

      =

import [email protected].{BaseNDA as NewNDA}

concept NDA {
    o BaseNDA newNDA
}

Context (Environment)

Desktop

  • OS: [e.g. macOS]
  • Browser: [e.g. Chrome, Safari]
  • Version: [e.g. 0.22.15]

Detailed Description

The issue is that resolveMetamodel method (called by ModelManager.toAst(true)) was modified to replace aliased imports with the actual type names inside TypeIdentifiers. This is useful for users processing the AST however it is lossy as it is difficult to recover the name as originally used by the user in the CTO file. In a subsequent call to fromAst the resolved AST is used:

  1. If the resolved type was not imported then a type not found error will occur
  2. If the resolved imported type has the same name as a type in the current namespace then a conflict will occur, leading to undefined behavior
  3. If the resolved imported type is used a super class for a class with the same name, then a conflict will occur
  4. Printer.toCTO has not been updated to recover the aliased name, so the roundtrip from [CTO with aliased import -> Resolved AST -> CTO] will be lossy, with aliased type names replaced with resolved type names

Possible Implementation

ModelManager.getAst(true) should return:

  1. A valid instance of the meta model
  2. Namespaces should be added to TypeIdentifiers so that imports do not need to be inspected
  3. A aliasName property should be added for types that are referred to in this model file by a name that is different from their name in their host namespace
  4. The name property should be updated to be the name of a type in its host namespace

ModelManager.fromAst(true) should return:

  1. A populated ModelManager
  2. If an aliasName property is present on a TypeIdentifier it should be used for the name of a type, otherwise the name property should be used as the type name

Printer.toCTO(ast) should return:

  1. If a TypeIdentifier has an aliasName print that rather that the name property for the type name

Metamodel update

  1. Add an optional aliasName to TypeIdentifier
  2. Republish concerto-metamodel
  3. Update models.accordproject.org

Unit Tests

Alias an import and use wherever a type can be referenced:

  • Super type
  • Property
  • Decorator
  • Map key
  • May value
namespace [email protected]
concept Child {}
scalar MyString extends String
@test(Kid)
namespace [email protected]

@test(Kid)
import [email protected].{MyString as Str, Child as Kid}

@test(Kid)
concept Child extends Kid {
   @test(Kid)
   o Kid kid
   o Str str
}

@test(Kid)
map KidIndex {
   o Str
   @test(Kid)
   o Kid
}

Test that [email protected] CTO file roundtrips through getAst(true) and fromAst(ast) and can be used with decorator command set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant