Skip to content

QL: allow getURL as an acronym#10605

Merged
erik-krogh merged 2 commits into
github:mainfrom
erik-krogh:allow-getURL
Sep 28, 2022
Merged

QL: allow getURL as an acronym#10605
erik-krogh merged 2 commits into
github:mainfrom
erik-krogh:allow-getURL

Conversation

@erik-krogh
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
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