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

Mixing escaped parentheses and non-capturing groups in substitution rule leads to an error #804

Closed
1 task done
teners opened this issue Apr 9, 2024 · 1 comment · Fixed by #813
Closed
1 task done

Comments

@teners
Copy link
Contributor

teners commented Apr 9, 2024

Check for existing issues

  • Completed

Environment

3.3.0 in Vale Studio

Describe the bug / provide steps to reproduce it

Using regexes with non-capturing groups together with escaped parenthesis in substitution rules leads to an error.

Expected behaviour: regular expression is considered valid.

Current behaviour: error capture group not supported; use '(?:' instead of '('.

Package example:

extends: substitution
message: "Use '%s' instead of '%s'."
link: "https://example.com/styleguide/style/wordlist/"
level: error
ignorecase: false
action:
  name: replace
swap:
 '(?:\()(?:api|Api)(?:\))?': API

(I realize that escaped parentheses in this particular regex are redundant — I've actually got more difficult case, but I've prepared this simpler one for the issue demonstration)

In Vale Studio: https://studio.vale.sh/s/e49686ef79df096fefe7886dce4fb9e1

In regex101: https://regex101.com/r/6G21Pd/1

Seems that the issue is in these lines: ./internal/check/substitution.go#L69-L70.

I've made an attempt at fixing it: https://github.com/teners/vale/commit/54203033a56581489828ddc92079be7e51da7366, if that's the right way I can submit a PR.

@jdkato
Copy link
Member

jdkato commented Apr 19, 2024

Thanks for looking into this -- your solution looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants