Skip to content

QL: allow getURL as an acronym #10605

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

Merged
merged 2 commits into from
Sep 28, 2022
Merged

Conversation

erik-krogh
Copy link
Contributor

getURL is a language feature, so it should be allowed as an upper-case acronym.

@erik-krogh erik-krogh marked this pull request as ready for review September 28, 2022 11:15
@erik-krogh erik-krogh requested a review from a team as a code owner September 28, 2022 11:15
nickrolfe
nickrolfe previously approved these changes Sep 28, 2022
Copy link
Contributor

@nickrolfe nickrolfe left a comment

Choose a reason for hiding this comment

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

Makes sense.

Copy link

@kaeluka kaeluka left a comment

Choose a reason for hiding this comment

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

Strictly speaking, that change means that users can write their own getURL methods (right?), but this implementation still seems to strike a good balance between precision and complexity.

@MathiasVP
Copy link
Contributor

Strictly speaking, that change means that users can write their own getURL methods (right?), but this implementation still seems to strike a good balance between precision and complexity.

People can write their own getURL predicate, yes. See https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/#providing-urls.

@erik-krogh
Copy link
Contributor Author

Strictly speaking, that change means that users can write their own getURL methods (right?), but this implementation still seems to strike a good balance between precision and complexity.

Yes, and that was always the case, now they just don't get a warning when they do it.

The getURL predicate is special at the language level.
So sometimes it's the right name to use, even if the name doesn't follow our style-guide.

I got some FPs due to that in another PR, so that's why I'm doing this PR.

@erik-krogh erik-krogh merged commit e0c68c3 into github:main Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants