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

Implement an analysis check that statically validates a string literal pattern argument to find, matches, and sub is a valid regex. #253

Open
peterhuene opened this issue Nov 12, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@peterhuene
Copy link
Collaborator

peterhuene commented Nov 12, 2024

If the pattern argument to the find, matches, and sub WDL standard library functions are string literals, we can easily check to see that the pattern compiles as a regex and, if not, emit a diagnostic. It's quite common to use a string literal for the pattern too.

I'm leaning towards doing that in a lint rather than analysis, although a lint warning seems wrong as it could lead to a runtime evaluation error.

@peterhuene
Copy link
Collaborator Author

Another reason I don't want this in analysis itself is that compiling regular expressions can be expensive depending on their complexity; as we must compile it for evaluation, I don't want to compile it twice either (once for analysis and then once again for evaluation).

@a-frantz
Copy link
Member

This makes more sense in analysis to me. IMO evaluating it twice is an acceptable redundancy. Main reason for me is that I would want it emitted during a run of sprocket check when lints aren't active.

@peterhuene
Copy link
Collaborator Author

peterhuene commented Nov 12, 2024

I guess we could implement a cache in each analyzed document where you can ask it for an already-compiled regex for a particular expression (which would need to be a string literal expression).

Ok, makes sense to stick this in analysis then.

@peterhuene peterhuene changed the title Implement a lint that statically validates a string literal pattern argument to find, matches, and sub is a valid regex. Implement an analysis check that statically validates a string literal pattern argument to find, matches, and sub is a valid regex. Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants