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

Require type for oneOf mutual exclusion #5426

Merged
merged 2 commits into from
Feb 17, 2025
Merged

Conversation

Namnamseo
Copy link
Contributor

@Namnamseo Namnamseo commented Feb 17, 2025

Hello!

The following config:

linters-settings:
  custom:
    foo:
      path: "bin/linter/myplugin.so"

is considered broken under current JSON schema for config:

{
  "custom": {
    "type": "object",
    "patternProperties": {
      "^.*$": {
        "type": "object",
        "additionalProperties": false,
        "properties": {
          "type": {
            "enum": [ "module", "goplugin" ],
            "default": "goplugin"
          },
          "path": {
            "type": "string",
            "examples": [ "/path/to/example.so" ]
          }, /* ... */
        },
        "oneOf": [
          {
            "properties": {
              "type": { "enum": [ "module" ] }
            }
          },
          {
            "required": [ "path" ]
          }
        ]
      }
    }
  }
}

for two reasons:

  • In JSON Schema, oneOf requires exactly one of each options. When both sub-schemas match the object, the validation fails.
  • In JSON Schema, all properties are deemed optional unless explicitly stated to be "required".

The given YAML:

  • matches the first sub-schema, which states: "If it has a type property, it should be one among ["module"].". Since it doesn't have that property, the statement is (vacuously) true.
  • matches the second sub-schema, which states: "It should have a "path" property.". It does.

I think the oneOf clause was there to do a mutual exclusion between "module" and "goplugin" type. Since the default is considered the latter, the "module" case should explicitly require for the property to be present.
This PR adds the requirement, and adds some test cases for that.


Since I'm new to the code base, I don't know which files among jsonschema/*.json needs to change; please point me if I'm wrong.

Copy link

boring-cyborg bot commented Feb 17, 2025

Hey, thank you for opening your first Pull Request !

@CLAassistant
Copy link

CLAassistant commented Feb 17, 2025

CLA assistant check
All committers have signed the CLA.

@ldez ldez added bug Something isn't working area: JSON schema labels Feb 17, 2025
@ldez ldez added this to the unreleased milestone Feb 17, 2025
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez merged commit 3d28c57 into golangci:master Feb 17, 2025
18 checks passed
alexandear pushed a commit to alexandear/golangci-lint that referenced this pull request Feb 18, 2025
@ldez ldez modified the milestones: unreleased, v1.64 Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: JSON schema bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants