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

Create a linter for Concerto #814

Open
jamieshorten opened this issue Mar 6, 2024 · 16 comments
Open

Create a linter for Concerto #814

jamieshorten opened this issue Mar 6, 2024 · 16 comments
Labels
Hacktoberfest by DigitalOcean and DEV Type: Feature Request πŸ›οΈ New feature or request

Comments

@jamieshorten
Copy link
Collaborator

Feature Request πŸ›οΈ

Create a linter to allow users to specify rules for how their Concerto models are structured

Use Case

It would be useful to have a tool to allow users to do some / all of the following:

  1. Specify the naming of declarations. E.g. all names of scalars should be in camel case.
  2. Specify the naming of properties, enum cases e.t.c
  3. Specify which language features can be used. E.g. disallow maps, disallow forward references in regex validators.
  4. Enforce the use of certain features. E.g. all string properties should have a length validator.
  5. Enforce the use of @Term decorators on all declarations and properties e.t.c

Context

  1. We have tools and applications that support different subsets of the language. It would be useful to enforce a common subset of functionality.
  2. It would be useful to enforce a style guide around naming and best practices.
@vr-varad
Copy link

vr-varad commented Mar 9, 2024

Hi @sanketshevkar, I'm eager to work on the pull request. Could you please assign it to me? Thanks!

@sanketshevkar
Copy link
Member

hey @vr-varad
This issue is one of the ideas for GSoC'24. We suggest you to visit https://summerofcode.withgoogle.com/how-it-works
to get more details.

@sanketshevkar
Copy link
Member

Please use this issue for discussion or doubts regarding this project idea.

@subhajit20
Copy link
Contributor

hey @jamieshorten @sanketshevkar does this need to convert a concert model file into a AST JSON format and then the linter will check one by one all the classes, properties, etc and validate with certain rules?
Am I in the right direction?

@sanketshevkar
Copy link
Member

Yes! Conversion of Concerto model to JSON AST already exists. But the linting part is missing.

@subhajit20
Copy link
Contributor

Yes! Conversion of Concerto model to JSON AST already exists. But the linting part is missing.

Can you point out where the AST conversation code is written ?

@sanketshevkar
Copy link
Member

You need to add file to the modelManager, that will give you a modelFile object, which has a getAST method, which would return the JSON AST.

@dselman is there any better way for this?

@dselman
Copy link
Sponsor Contributor

dselman commented Mar 14, 2024

No, that's the way to do it if you want to ensure the AST is valid and consistent (references resolve).

@dselman
Copy link
Sponsor Contributor

dselman commented Mar 14, 2024

We can use Spectral as the framework for defining our own rules over the Concerto AST (JSON):
https://github.com/stoplightio/spectral

Here are two sample rules:

rules:
  declaration-pascal-case:
    description: Declaration names should use Pascal case.
    message: "Declaration '{{value}}' should use Pascal case."
    severity: warn
    given: $.declarations[*].name
    then:
      function: casing
      functionOptions:
        type: pascal
  enum-value-macro-case:
    description: Enum values should use Macro case.
    message: "Enum '{{value}}' should use Macro case."
    severity: warn
    given: $.declarations[?(@.$class=="[email protected]")].properties[*].name
    then:
      function: casing
      functionOptions:
        type: macro

When used with this CTO:

namespace [email protected]

enum Gender {
    o Male
    o Female
    o Other
}

concept person {
    o String name
}

To create the AST:

concerto parse --model test.cto --excludeLineLocations --output test.json

test.json:

{
  "$class": "[email protected]",
  "decorators": [],
  "namespace": "[email protected]",
  "imports": [],
  "declarations": [
    {
      "$class": "[email protected]",
      "name": "Gender",
      "properties": [
        {
          "$class": "[email protected]",
          "name": "Male"
        },
        {
          "$class": "[email protected]",
          "name": "Female"
        },
        {
          "$class": "[email protected]",
          "name": "Other"
        }
      ]
    },
    {
      "$class": "[email protected]",
      "name": "person",
      "isAbstract": false,
      "properties": [
        {
          "$class": "[email protected]",
          "name": "name",
          "isArray": false,
          "isOptional": false
        }
      ]
    }
  ]
}

Produces the output:

spectral lint test.json

/Users/dan.selman/dev/concerto-spectral/test.json
 13:19  warning  enum-value-macro-case    Enum 'Male' should use Macro case.            declarations[0].properties[0].name
 17:19  warning  enum-value-macro-case    Enum 'Female' should use Macro case.          declarations[0].properties[1].name
 21:19  warning  enum-value-macro-case    Enum 'Other' should use Macro case.           declarations[0].properties[2].name
 27:15  warning  declaration-pascal-case  Declaration 'person' should use Pascal case.  declarations[1].name

@subhajit20
Copy link
Contributor

subhajit20 commented Mar 14, 2024

We can use Spectral as the framework for defining our own rules over the Concerto AST (JSON): https://github.com/stoplightio/spectral

Here are two sample rules:

rules:
  declaration-pascal-case:
    description: Declaration names should use Pascal case.
    message: "Declaration '{{value}}' should use Pascal case."
    severity: warn
    given: $.declarations[*].name
    then:
      function: casing
      functionOptions:
        type: pascal
  enum-value-macro-case:
    description: Enum values should use Macro case.
    message: "Enum '{{value}}' should use Macro case."
    severity: warn
    given: $.declarations[?(@.$class=="[email protected]")].properties[*].name
    then:
      function: casing
      functionOptions:
        type: macro

When used with this CTO:

namespace [email protected]

enum Gender {
    o Male
    o Female
    o Other
}

concept person {
    o String name
}

To create the AST:

concerto parse --model test.cto --excludeLineLocations --output test.json

test.json:

{
  "$class": "[email protected]",
  "decorators": [],
  "namespace": "[email protected]",
  "imports": [],
  "declarations": [
    {
      "$class": "[email protected]",
      "name": "Gender",
      "properties": [
        {
          "$class": "[email protected]",
          "name": "Male"
        },
        {
          "$class": "[email protected]",
          "name": "Female"
        },
        {
          "$class": "[email protected]",
          "name": "Other"
        }
      ]
    },
    {
      "$class": "[email protected]",
      "name": "person",
      "isAbstract": false,
      "properties": [
        {
          "$class": "[email protected]",
          "name": "name",
          "isArray": false,
          "isOptional": false
        }
      ]
    }
  ]
}

Produces the output:

spectral lint test.json

/Users/dan.selman/dev/concerto-spectral/test.json
 13:19  warning  enum-value-macro-case    Enum 'Male' should use Macro case.            declarations[0].properties[0].name
 17:19  warning  enum-value-macro-case    Enum 'Female' should use Macro case.          declarations[0].properties[1].name
 21:19  warning  enum-value-macro-case    Enum 'Other' should use Macro case.           declarations[0].properties[2].name
 27:15  warning  declaration-pascal-case  Declaration 'person' should use Pascal case.  declarations[1].name

I have gone through the readme section and I am having one question that -
Do we need to create a separate package for example concerto-lint which will use spectral framework where we can define our rulesets in the form of JSON or. YML and then we will pass the folder name via command line like concerto-lint test.json and it will validate all the json file and log all the modifications needed for the cto model ?

@apoorvsxna
Copy link
Contributor

Hi. I've been looking into this project's requirements and am interested in working on it. This is my current understanding of how the linter would work for a general user, please correct me if I'm wrong-

  1. Create concerto model.
  2. Open up terminal and parse model using the linter cli.
  3. Linter converts model to AST, and checks each element for rule violations (pre-specified spectral rules)
  4. Linter reports rule violating elements as output.

@sanketshevkar
Copy link
Member

Yes, that's correct! For the following point

Linter converts model to AST, and checks each element for rule violations (pre-specified spectral rules)

We already have methods in Concerto that can get you the AST, rule checking is something that you'll have to build.

@sanketshevkar
Copy link
Member

sanketshevkar commented Mar 19, 2024

Do we need to create a separate package for example concerto-lint which will use spectral framework where we can define our rulesets in the form of JSON or. YML and then we will pass the folder name via command line like concerto-lint test.json and it will validate all the json file and log all the modifications needed for the cto model ?

Something like that. It seems like a good approach to start with. I haven't played much around with spectral yet, so please take this with a grain of salt. On a high level it seems like a good approach to me.

@subhajit20
Copy link
Contributor

subhajit20 commented Mar 20, 2024

Do we need to create a separate package for example concerto-lint which will use spectral framework where we can define our rulesets in the form of JSON or. YML and then we will pass the folder name via command line like concerto-lint test.json and it will validate all the json file and log all the modifications needed for the cto model ?

Something like that. It seems like a good approach to start with. I haven't played much around with spectral yet, so please take this with a grain of salt. On a high level it seems like a good approach to me.

Hey I have tried both options like cli and javascript ruleset both worked fine

  • javascript ruleset

`async function lintConcertoModel() {
const data = fs.readFileSync('./test.cto','utf8');
const model_manager = new ModelManager()
const modelFile = model_manager.addCTOModel(data,'test.cto');
const ast = modelFile.getAst();

const spectral = new Spectral();
spectral.setRuleset({
    rules: {
        "concept-name-capitalized": {
          description:"Concept names should start with a capital letter.",
          given: "$.declarations[*].name",
          message: "Declaration '{{value}}' should use Pascal case.",
          then: {
            field:"name",
            function: pattern,
            functionOptions:{
                match:"^[A-Z]"
            }
          },
        },
    }
});

spectral.run(ast).then((e)=>{
    console.log(e)
})
.catch((err)=>{

    console.log(err)
})

// return results;

}
`

  • Output
    [ { code: 'concept-name-capitalized', message: "Declaration 'person' should use Pascal case.", path: [ 'declarations', '0', 'name' ], severity: 1, range: { start: [Object], end: [Object] } }, { code: 'concept-name-capitalized', message: "Declaration 'subhajit' should use Pascal case.", path: [ 'declarations', '1', 'name' ], severity: 1, range: { start: [Object], end: [Object] } } ]

@dselman dselman added Hacktoberfest by DigitalOcean and DEV and removed Difficulty: Challenging Google Summer of Code β˜€οΈ Project idea for GSoC labels Sep 27, 2024
@kairveeehh
Copy link

hey @dselman as this issue had been under gsoc'24 but still seems to be open , could you let me know about it if its available to work and what were the complications faced with the previous flow that was listed in the comments?
thanks

@dselman
Copy link
Sponsor Contributor

dselman commented Oct 8, 2024

hey @dselman as this issue had been under gsoc'24 but still seems to be open , could you let me know about it if its available to work and what were the complications faced with the previous flow that was listed in the comments? thanks

Yes, the issue is still open. There wasn't any major issue with the approach described, it was just deprioritised for GSoC. I still think it would be a valuable contribution, and by leveraging Spectral should not be too costly or difficult to implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest by DigitalOcean and DEV Type: Feature Request πŸ›οΈ New feature or request
Projects
None yet
Development

No branches or pull requests

8 participants