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

[Enhancement] Add AllOf validations for KCL jsonschema generations #212

Closed
Peefy opened this issue Jan 15, 2024 · 4 comments · Fixed by #372
Closed

[Enhancement] Add AllOf validations for KCL jsonschema generations #212

Peefy opened this issue Jan 15, 2024 · 4 comments · Fixed by #372
Labels
gen good first issue Good for newcomers help wanted Extra attention is needed tool

Comments

@Peefy
Copy link
Contributor

Peefy commented Jan 15, 2024

Before

  • kcl-go/pkg/tools/gen/types.go
type validation struct {
	Name             string
	Minimum          *float64
	ExclusiveMinimum bool
	Maximum          *float64
	ExclusiveMaximum bool
	MinLength        *int
	MaxLength        *int
	Regex            *regexp.Regexp
	MultiplyOf       *int
	Unique           bool
}
  • After
type validation struct {
	Name             string
	Minimum          *float64
	ExclusiveMinimum bool
	Maximum          *float64
	ExclusiveMaximum bool
	MinLength        *int
	MaxLength        *int
	Regex            *regexp.Regexp
	MultiplyOf       *int
	Unique           bool
        AllOf   []*validation
}
@jakezhu9
Copy link
Contributor

For allOf keyword, a basic use case coule be:

{
  "allOf": [
    { "type": "string" },
    { "minLength": 2 }
  ]
}

In this case, the desgin to add AllOf validations is great. However, things get complicated if it is:

{
  "allOf": [
    {
      "type": "object",
      "properties": {
        "name": { "type": "string" }
      }
    },
    {
      "type": "object",
      "properties": {
        "address": { "type": "string" }
      }
    }
  ]
}

In this case, just adding some validation rules doesn't seem to be enough. Even worse, in some real-world situations, allOf is used with other conditional keywords (not, if, then...) For example in github workflow schema:

 "allOf": [
    {
        "if": {
            "properties": {
                "type": {
                    "const": "boolean"
                }
            },
            "required": [
                "type"
            ]
        },
        "then": {
            "properties": {
                "default": {
                    "type": "boolean"
                }
            }
        },
        "else": {
            "properties": {
                "default": {
                    "type": "string"
                }
            }
        }
    }
    ...

I'm not quite sure if it can all be perfectly converted to KCL. And more design and transformation rules is needed. I'm currently working on improving the import tool (#200) and have made some progress. Maybe later we can have further communication on how to make it better :)

@Peefy
Copy link
Contributor Author

Peefy commented Jan 15, 2024

Great job 👍 For complex validation combinations, my current idea is to aim to connect 5 typical use cases, such as the GitHub workflow JSON schema. For specific implementation, we can first represent the input and output of some typical cases.

Of course, for some corner cases, there may indeed be conversion difficulties that need to be viewed separately.

Here's some references

@shikharish
Copy link

@Peefy Hello, I would like to work on this issue. I am new to the project so can you please give me an overview of the issue.

@EraKin575
Copy link

If this issue is still relevant, I would like to work on it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gen good first issue Good for newcomers help wanted Extra attention is needed tool
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants