Skip to content

fixed Enum issue on schema.enum value#1083

Closed
msivasubramaniaan wants to merge 2 commits intoredhat-developer:mainfrom
msivasubramaniaan:fix-enum-value-on-schema
Closed

fixed Enum issue on schema.enum value#1083
msivasubramaniaan wants to merge 2 commits intoredhat-developer:mainfrom
msivasubramaniaan:fix-enum-value-on-schema

Conversation

@msivasubramaniaan
Copy link
Contributor

What does this PR do?

This PR fix the schema.enum value on array element

What issues does this PR fix or reference?

#1078

Is it tested? How?

Yes. Added test case

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@datho7561
Copy link
Contributor

This causes a regression:

In VS Code, add this to settings.json:

{
  "yaml.schemas": {
    "https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/v1.32.1-standalone-strict/all.json": "*.k8s.yaml"
  }
}

Then, create a file my-pod.k8s.yaml:

apiVersion: v1
kind: Pod
metadata:
  resourceVersion: test

Pod is listed as invalid, but it is suggested as an autocomplete option and it should be valid

Copy link
Contributor

@Kosta-Github Kosta-Github left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, the root of this issue is actually this location:

case 'null':
case 'string':
case 'number':
return node.value;
case 'boolean':
return node.source;

which returns a boolean value as a string ("true" or "false"), since node.source is of type string.

I am not sure, why it is that way and returning node.value (which would be of the right type boolean) as is done above for null, number, and string?

let enumValueMatch = false;
for (const e of schema.enum) {
if (val === e || isAutoCompleteEqualMaybe(callFromAutoComplete, node, val, e)) {
if (equals(val, schema.enum, node.type) || isAutoCompleteEqualMaybe(callFromAutoComplete, node, val, e)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is wrong and should be:

Suggested change
if (equals(val, schema.enum, node.type) || isAutoCompleteEqualMaybe(callFromAutoComplete, node, val, e)) {
if (equals(val, e, node.type) || isAutoCompleteEqualMaybe(callFromAutoComplete, node, val, e)) {

}
}
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this array handling for a boolean is not the right thing to add here.
Iterating over the enum values should be done at the caller site (see my other comment).

@Kosta-Github
Copy link
Contributor

I still think, this is the more appropriate fix: #1080
and all the CI tests were green...

@coveralls
Copy link

Coverage Status

coverage: 83.926% (+0.05%) from 83.873%
when pulling 172b9b8 on msivasubramaniaan:fix-enum-value-on-schema
into c94fa43 on redhat-developer:main.

@msivasubramaniaan msivasubramaniaan deleted the fix-enum-value-on-schema branch June 12, 2025 16:28
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

Successfully merging this pull request may close these issues.

4 participants