Skip to content

QL4QL: Exclude PrintAst like tests from being reported as having missing InlineExpectations. #19104

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

Conversation

michaelnebel
Copy link
Contributor

No description provided.

@michaelnebel michaelnebel marked this pull request as ready for review March 24, 2025 15:40
@Copilot Copilot AI review requested due to automatic review settings March 24, 2025 15:40
@michaelnebel michaelnebel requested a review from a team as a code owner March 24, 2025 15:40
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (8)
  • ql/ql/src/codeql_ql/ast/Yaml.qll: Language not supported
  • ql/ql/src/queries/style/QlRefInlineExpectations.ql: Language not supported
  • ql/ql/test/queries/style/QlRefInlineExpectations/PrintAst.expected: Language not supported
  • ql/ql/test/queries/style/QlRefInlineExpectations/PrintAst.qlref: Language not supported
  • ql/ql/test/queries/style/QlRefInlineExpectations/Test4.expected: Language not supported
  • ql/ql/test/queries/style/QlRefInlineExpectations/Test4.qlref: Language not supported
  • ql/ql/test/queries/style/QlRefInlineExpectations/dummy/PrintAst.expected: Language not supported
  • ql/ql/test/queries/style/QlRefInlineExpectations/dummy/PrintAst.ql: Language not supported

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

Comment on lines +71 to +76
exists(YamlMapping n, YamlScalar value |
n.getDocument() = this and
value.getValue().matches("%PrintAst%")
|
value = n.lookup("query")
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment that this is for .qlref files.
And maybe check that the file-extension is .qlref?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review! That is included in the char pred and description of the class QlRefDocument. Is that acceptable?
However, I do need to add a comment for the predicate.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I didn't notice that context.

LGTM 👍

@erik-krogh erik-krogh merged commit 9d3d3de into github:main Mar 25, 2025
11 checks passed
@@ -64,4 +64,15 @@ class QlRefDocument extends YamlDocument {
value = n.lookup("postprocess").(YamlSequence).getElement(_)
)
}

predicate isPrintAst() {
Copy link
Contributor

Choose a reason for hiding this comment

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

A more ambitious solution would be to resolve the the query pointed to by the .qlref file, and then only flag cases where the query has kind problem or path-problem, but we can leave that for a future improvement.

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.

3 participants