-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Ql4ql: Quality query tagging. #19931
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
base: main
Are you sure you want to change the base?
Conversation
70d3b6a
to
eb14420
Compare
eb14420
to
ce5f119
Compare
private predicate hasQualityTag(QueryDoc doc) { doc.getQueryTags() = "quality" } | ||
|
||
private predicate incorrectTopLevelCategorisation(QueryDoc doc) { | ||
count(string s | s = doc.getQueryTags() and s = ["maintainability", "reliability"]) != 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May as well use strictcount
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh wait, we can't - the predicate won't hold in the case there is no top level tag (if the count is 0
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well ok, I can invert the logic 😄
ql/ql/src/codeql_ql/ast/Ast.qll
Outdated
} | ||
|
||
/** Gets the individual @tags for the query. */ | ||
string getQueryTags() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getAQueryTag
would be a better name.
ce5f119
to
179793e
Compare
In this PR we introduce a Ql4Ql check for correct use of quality query tags as documented here.
According to the documentation
quality
) should have at least a top level categorisation.Furthermore, we also fix the existing violations.
The failing java integration tests are unrelated.