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

Improve error reporting when decorator manager applies a duplicate decorator #711

Open
dselman opened this issue Sep 8, 2023 · 3 comments ยท May be fixed by #815
Open

Improve error reporting when decorator manager applies a duplicate decorator #711

dselman opened this issue Sep 8, 2023 · 3 comments ยท May be fixed by #815

Comments

@dselman
Copy link
Contributor

dselman commented Sep 8, 2023

Feature Request ๐Ÿ›๏ธ

If a decorator command set APPENDS two decorators with the same name the resulting model is invalid.

Use Case

Improve error reporting.

Possible Solution

Fail-fast, when the decorator is being added, with a clearer error message.

Context

Apply this decorator command twice to the model.

      {
        "$class": "[email protected]",
        "type": "APPEND",
        "target": {
          "$class": "[email protected]",
          "namespace": "[email protected]",
          "declaration": "Driver",
          "property": "favoriteColor" // if you change this to 'foo' then validateCommands will throw an error
        },
        "decorator": {
          "$class": "[email protected]",
          "name": "Preference",
          "arguments": []
        }
      }

Currently the error is detected when the new model is created, rather than when it is decorated.

IllegalModelException: Duplicate decorator Preference Line 11 column 3, to line 12 column 1. 
    at Field.validate (/home/runner/AccordProjectConcerto-Decorator-Command-Set/node_modules/@accordproject/concerto-core/lib/introspect/decorated.js:130:23)
    at Field.validate (/home/runner/AccordProjectConcerto-Decorator-Command-Set/node_modules/@accordproject/concerto-core/lib/introspect/property.js:139:15)
    at ConceptDeclaration.validate (/home/runner/AccordProjectConcerto-Decorator-Command-Set/node_modules/@accordproject/concerto-core/lib/introspect/classdeclaration.js:283:23)
    at ModelFile.validate (/home/runner/AccordProjectConcerto-Decorator-Command-Set/node_modules/@accordproject/concerto-core/lib/introspect/modelfile.js:279:30)
    at ModelManager.validateModelFiles (/home/runner/AccordProjectConcerto-Decorator-Command-Set/node_modules/@accordproject/concerto-core/lib/basemodelmanager.js:397:33)
    at ModelManager.fromAst (/home/runner/AccordProjectConcerto-Decorator-Command-Set/node_modules/@accordproject/concerto-core/lib/basemodelmanager.js:754:14)
    at Function.decorateModels (/home/runner/AccordProjectConcerto-Decorator-Command-Set/node_modules/@accordproject/concerto-core/lib/decoratormanager.js:115:25)
    at Object.<anonymous> (/home/runner/AccordProjectConcerto-Decorator-Command-Set/index.js:109:44)

Detailed Description

@vr-varad
Copy link

vr-varad commented Mar 5, 2024

hey @dselman ,I would like to work on this issue could u assign me this issue?

@subhajit20
Copy link
Contributor

Hey @dselman can you review my pr?
And can I also get assigned?

Copy link
Contributor

github-actions bot commented Dec 9, 2024

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants