Skip to content

Misc fixes - #288

Merged
plokhotnyuk merged 3 commits into
NeverBlink-OSS:mainfrom
plokhotnyuk:fix-validation-bugs
Oct 16, 2026
Merged

plokhotnyuk merged 3 commits into
NeverBlink-OSS:mainfrom
plokhotnyuk:fix-validation-bugs

Conversation

@plokhotnyuk

@plokhotnyuk plokhotnyuk commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Report non-ASCII element names as NonAsciiName

Why: Case.isAllowedAscii checked c >= ' ' || c <= '~', which is true for every character.
So NonAsciiName was never reported. A name like wąż🐍 got only a NonStandardSeparator
warning, while the issue type and the comment in checkNaming say a non-ASCII name is an error.

What changed:

  • isAllowedAscii uses &&, so it accepts only printable ASCII (' ' to '~').
  • checkNaming no longer skips surrogates when listing non-standard separators. Names that reach
    that check are all ASCII now.
  • The wąż🐍 test in SchemaViewSpec now expects NonAsciiName. New CaseSpec test for
    isAllowedAscii.

User-visible change: a schema with a non-ASCII element name now fails validate (exit code 1).
NonAsciiName is an error, not a fatal issue, so generators still run.

Give slot_usage precedence when merging map metaslots

Why: slot derivation applies the class slot_usage first and then the slot definition.
combineWith keeps the first value for single-value metaslots, so slot_usage has precedence
there. But combineMap used v1.concat(v2), so for a key in both maps the slot definition had
precedence. For example:

slots:
  s:
    annotations: {a: 1}
classes:
  C:
    slots: [s]
    slot_usage:
      s:
        annotations: {a: 2}

The derived slot C.s had a: 1, while a description from the same slot_usage was kept. This
affected annotations, extensions, alt_descriptions, local_names and type_mappings.

What changed:

  • combineMap is now v2.concat(v1), so for a key in both maps the value from the first map
    (the more specific one) is kept. For small maps, which keep insertion order, the keys of the
    second map now come first, so generate linkml may list such map entries in a different order.
  • New CombiningSpec, and a SlotDerivationSpec test for an annotation set in both places.

Evaluate the receiver of orElseFast once

Why: the FastUtils Scaladoc says the *Fast methods work exactly like the standard
Option methods. But orElseFast was if (opt eq None) fallback else opt, and opt is an
inline parameter, so the expression it stands for ran twice when it had a value. For example,
sv.resolve(...).orElseFast(...) in SchemaValidator resolved the same reference twice. That is
wasted work, and it gives wrong results if the expression has side effects.

What changed:

  • orElseFast stores the option in a local val before the eq None check.
  • New FastUtilsSpec that checks every *Fast method against the Option method it replaces,
    for both Some and None. It also counts how many times the receiver and the argument are
    evaluated: the receiver once, the argument only when the standard method would use it.

@plokhotnyuk
plokhotnyuk force-pushed the fix-validation-bugs branch 3 times, most recently from a1d03a0 to 1123f0a Compare October 14, 2026 14:27

@niegrzybkowski niegrzybkowski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove the CURIE/prefixology things. They will not happen anymore after we fix curies properly.

@plokhotnyuk plokhotnyuk changed the title Fix NonAsciiName, slot_usage map merging and undeclared prefix handling in generators Misc fixes Oct 14, 2026
@plokhotnyuk
plokhotnyuk force-pushed the fix-validation-bugs branch 2 times, most recently from e891a5a to 342fa08 Compare October 16, 2026 09:30
Why:
`Case.isAllowedAscii` checked `c >= ' ' || c <= '~'`, which is true for every
character. So the `NonAsciiName` error was never reported. A name like `wąż🐍`
got only a `NonStandardSeparator` warning, while the issue type and the
comment in `checkNaming` say a non-ASCII name is an error.

What changed:
- `Case.isAllowedAscii` uses `&&`, so it accepts only printable ASCII
  (' ' to '~').
- `checkNaming` no longer skips surrogates when listing non-standard
  separators. Names that reach that check are all ASCII now, so there are no
  surrogates to skip.
- The `wąż🐍` test in `SchemaViewSpec` now expects `NonAsciiName` and no
  `NonStandardSeparator`. New `CaseSpec` test for `isAllowedAscii`.
Why:
Slot derivation applies the class `slot_usage` first and then the slot
definition. `combineWith` keeps the first value for scalar metaslots
(`combineFallback`), so `slot_usage` has precedence over the slot. But
`combineMap` used `v1.concat(v2)`, so for a key in both maps the value from
the slot definition had precedence. For example, a slot with
`annotations: {a: 1}` and a class with `slot_usage: {s: {annotations: {a: 2}}}`
derived `a: 1`, while `description` from the same `slot_usage` was kept. This
affected `annotations`, `extensions`, `alt_descriptions`, `local_names` and
`type_mappings`.

What changed:
- `combineMap` keeps the value from the first map for a key in both maps,
  and adds only the keys missing from it, as the Scaladoc now says.
- New `CombiningSpec` for `combineMap`, and a `SlotDerivationSpec` test for an
  annotation set in both the slot and `slot_usage`.
Why:
The FastUtils Scaladoc says the *Fast methods work exactly like the
standard Option methods. But orElseFast was
`if (opt eq None) fallback else opt`, and opt is an inline parameter,
so the expression it stands for ran twice when it had a value. For
example, `sv.resolve(...).orElseFast(...)` in SchemaValidator resolved
the same reference twice.

What changed:
- orElseFast stores the option in a local val before the `eq None`
  check.
- New FastUtilsSpec that checks every *Fast method against the Option
  method it replaces, for both Some and None. It also counts how many
  times the receiver and the argument are evaluated: the receiver once,
  the argument only when the standard method would use it.
@plokhotnyuk
plokhotnyuk merged commit e5cdd31 into NeverBlink-OSS:main Oct 16, 2026
9 checks passed
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.

2 participants