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

Regression in renovate regex comprehension #518

Closed
Callek opened this issue Jan 24, 2025 · 3 comments
Closed

Regression in renovate regex comprehension #518

Callek opened this issue Jan 24, 2025 · 3 comments
Labels
bug Something isn't working pre-commit-hooks Regarding the hook config or generation regular-expressions Related to the regex engines and modes

Comments

@Callek
Copy link

Callek commented Jan 24, 2025

Similar to #516 and with a similar workaround I can't update to the newest (0.31.0) check-jsonschema via pre-commit due to this regression in renovate.

Example renovate.json

{
  "$schema": "https://docs.renovatebot.com/renovate-schema.json",
  "customManagers": [
    {
      "description": "Update rust toolchain declarations of dtolnay/rust-toolchain",
      "customType": "regex",
      "fileMatch": [
        "\\.github/(?:workflows|actions)/.+\\.ya?ml$",
        "(^|/)action\\.ya?ml$"
      ],
      "matchStrings": [
          "# renovate: rust-toolchain\\s*toolchain\\:\\s*(?<currentValue>\\d+\\.\\d+(\\.\\d+)?)"
      ],
      "depNameTemplate": "rust",
      "packageNameTemplate": "rust-lang/rust",
      "datasourceTemplate": "github-tags"
    }
  ]
}

Verification that its fixed with nonunicode

$ check-jsonschema --version
check-jsonschema, version 0.31.0
$ check-jsonschema --builtin-schema renovate renovate.json 
Schema validation errors were encountered.
  renovate.json::$.customManagers[0].matchStrings[0]: '# renovate: rust-toolchain\\s*toolchain\\:\\s*(?<currentValue>\\d+\\.\\d+(\\.\\d+)?)' is not a 'regex'
$ check-jsonschema --builtin-schema renovate --regex-variant nonunicode renovate.json 
ok -- validation done

Environment Details

  • MacOS Sequoia
  • Bash + Python3.12

(Though I don't think the environment matters in this case)

@sirosen
Copy link
Member

sirosen commented Jan 25, 2025

Thanks for filling this!

I'm starting to regret not doing more research before choosing Unicode mode as the default. With these issues cropping up, I'm doubting that decision a bit.

To keep churn in the project to a minimum, I'll most likely mark the renovate hook as nonunicode in the next release (possibly today, if I can get some time).

I want to look at why that pattern fails though -- it's not obvious to me why the Unicode mode regex engine fails it. Maybe something about the capture group?

@sirosen sirosen added bug Something isn't working pre-commit-hooks Regarding the hook config or generation labels Jan 25, 2025
@sirosen
Copy link
Member

sirosen commented Jan 25, 2025

I just checked using my browser and the issue with that regex is that there's an improper escape, \:.
Nonunicode mode will, indeed, accept that expression because it is less strict.

This might be evidence of the 0.31.0 update catching a (trivial) mistake in your usage? But if the regex works in renovate then they're probably not evaluating regexes in Unicode mode. So I think that (since renovate is implemented in JS), I'll proceed with the change in hook config, to try to match their implementation.

@Callek
Copy link
Author

Callek commented Jan 25, 2025

the issue with that regex is that there's an improper escape, :

Good catch, but i agree with your planned outcome anyway.

sirosen added a commit that referenced this issue Jan 30, 2025
@sirosen sirosen added the regular-expressions Related to the regex engines and modes label Jan 30, 2025
@sirosen sirosen closed this as completed Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pre-commit-hooks Regarding the hook config or generation regular-expressions Related to the regex engines and modes
Projects
None yet
Development

No branches or pull requests

2 participants