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

Add string predicates startsWith, endsWith and contains #78

Open
Marcono1234 opened this issue Aug 20, 2021 · 2 comments
Open

Add string predicates startsWith, endsWith and contains #78

Marcono1234 opened this issue Aug 20, 2021 · 2 comments
Labels

Comments

@Marcono1234
Copy link
Contributor

Marcono1234 commented Aug 20, 2021

What do you think about adding predicates to the CodeQL type string for determining whether a string has a given prefix or suffix or contains a substring? For example startsWith(string), endsWith(string) and contains(string).
Currently the workarounds are using indexOf(...) = 0 or matches(...%) (which seems to be faster than indexOf, see github/codeql#6479 (comment)). However, these predicates do not convey the intention as clearly, might not be that performant and for matches one must take care not to accidentally use %or _ where the intention was to match them literally.

In the github/codeql repository (at github/codeql@3953331) there are at least:

  • 196 cases where startsWith could be used
    (I searched for the regex matches\("[^%_]*%"\) in CodeQL source files)
  • 72 cases where endsWith could be used
    (I searched for the regex matches\("%[^%_]*"\) in CodeQL source files)
@github-actions github-actions bot added the CLI label Aug 20, 2021
@adityasharad
Copy link
Contributor

adityasharad commented Aug 23, 2021

Thanks for the suggestion!

To give you some insight about what happens under the hood in the CodeQL engine, matches("...%") gets translated into an operation like startsWith("..."), and similarly matches("%...") becomes endsWith("..."). So the engine is already doing that optimisation for you. Using matches in this way is indeed preferable over indexOf, because indexOf has to produce all (character, index) pairs.

This isn't high on our list of priorities, but I agree that either exposing startsWith/endsWith primitives, or updating the docs for matches to clearly describe this case, is a good way to convey the intention.

@Marcono1234 Marcono1234 changed the title Add string predicates startsWith and endsWith Add string predicates startsWith, endsWith and contains Sep 7, 2021
@Marcono1234
Copy link
Contributor Author

A contains(string) predicate would be useful as well, have added it to the description.

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

No branches or pull requests

2 participants