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

fix: anyOf compare alternatives #32

Open
wants to merge 6 commits into
base: jigx
Choose a base branch
from

Conversation

p-spacek
Copy link
Collaborator

What does this PR do?

improve compare mechanism that should choose correct schema alternatives

What issues does this PR fix or reference?

fixes this issue redhat-developer#684
replces this PR redhat-developer#759

Is it tested? How?

added tests

@p-spacek p-spacek changed the title fix: anyOf compare alternatives fix: anyOf compare alternatives [create PR into redhat after test] Oct 24, 2024
@@ -863,7 +869,7 @@ function validate(
const val = getNodeValue(node);
let enumValueMatch = false;
for (const e of schema.enum) {
if (equals(val, e) || (callFromAutoComplete && isString(val) && isString(e) && val && e.startsWith(val))) {
if (equals(val, e) || isAutoCompleteEqualMaybe(callFromAutoComplete, node, val, e)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

null value is a good match in auto-completion

} else if (compareResult === 0) {
} else if (
compareResult === 0 ||
((node.value === null || node.type === 'null') && node.length === 0) // node with no value can match any schema potentially
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

null yaml value is a good match for sub-schemas, similar to isAutoCompleteEqualMaybe for const and enums

if (
!maxOneMatch &&
!subValidationResult.hasProblems() &&
(!bestMatch.validationResult.hasProblems() || callFromAutoComplete)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(!bestMatch.validationResult.hasProblems() || callFromAutoComplete) didn't solve the previous problem properly
problem was when the order of the subschemas in anyOf changed.
this previous fix allows 'issues' only on the previous bestMatch

}
}
return bestMatch;
}

function mergeValidationMatches(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

only refactor - move to separate function, no functional change

@@ -1289,7 +1289,7 @@ obj:
4,
18,
DiagnosticSeverity.Error,
'yaml-schema: Package',
'yaml-schema: Composer Package',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

test fix

msivasubramaniaan and others added 4 commits March 11, 2025 12:26
Signed-off-by: msivasubramaniaan <[email protected]>
The import from `prettier` includes support for all languages supported
by Prettier. The `prettier/standalone` import only includes the core.
The `prettier` export also points to `prettier/standalone` for the
`browser` export condition.

This means that with this change `yaml-language-server` has to load less
code, leading to faster startup time. And depending on configuration, it
may lead to smaller bundle sizes.

Co-authored-by: Muthurajan Sivasubramanian <[email protected]>
* add function to calculate remainder for floats correctly without rounding errors and fix checking if a number is a multipleOf for floats

* fix formatting

* add unit test

* rm copyright

---------

Co-authored-by: Steinke, Rico <[email protected]>
Co-authored-by: Muthurajan Sivasubramanian <[email protected]>
@p-spacek p-spacek changed the base branch from jigx to main March 12, 2025 13:35
@p-spacek p-spacek changed the base branch from main to jigx March 12, 2025 13:35
@p-spacek p-spacek changed the title fix: anyOf compare alternatives [create PR into redhat after test] fix: anyOf compare alternatives Mar 12, 2025
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