Fix AST Viewer #949
Fix AST Viewer #949
Conversation
The previous synthetic query suite was not finding the ast query because the `qlpack` directive in a query suite only matches queries from the default suite, which `printAST.ql` is not part of. This changes to using `from` and `queries` directives. Also, adds an integration test to ensure we find the queries using different CLIs. However, this only tests using the latest `main` from the codeql repository. I wonder if we should start testing using different versions of the repo.
|
We will probably need another release for this. Also, I expect that some of the integration tests will fail, but not sure which ones. Let's see how far back we can still support print asts with the new formulation. |
| @@ -68,4 +70,16 @@ describe('Use cli', function() { | |||
| const queryInfo: QueryInfoByLanguage = await cli.resolveQueryByLanguage(getOnDiskWorkspaceFolders(), Uri.file(queryPath)); | |||
| expect((Object.keys(queryInfo.byLanguage))[0]).to.eql('javascript'); | |||
| }); | |||
|
|
|||
| it.only('should resolve printAST queries', async function() { | |||
There was a problem hiding this comment.
Should we make this loop over all languages? The filenames may differ.
There was a problem hiding this comment.
Good idea. I will change that.
| skipIfNoCodeQL(this); | ||
|
|
||
| const result = await resolveQueries(cli, { | ||
| dbschemePack: 'codeql/javascript-all', |
There was a problem hiding this comment.
How do these tests get the query repo checkout? If we're splitting over multiple CLI versions, we should use the corresponding codeql-cli/version tags from the query repos, as well as latest main for the last released CLI.
There was a problem hiding this comment.
As discussed, the integration tests checks out the expected codeql version for each CLI version.
Go is not yet supported since we do not include the go submodule in the integration tests.
This is old enough that we don't need to support it.
aeisenberg
force-pushed
the
aeisenberg/print-ast-fix
branch
from
99c018f
to
024d3e6
Compare
|
Works for all languages in Also, not yet tested are go and ruby since these languages exist in other repos that are not yet being downloaded by the integration tests. |
extensions/ql-vscode/CHANGELOG.md
Outdated
| @@ -3,6 +3,7 @@ | |||
| ## [UNRELEASED] | |||
|
|
|||
| - Fix bug where a query is sometimes run before the file is saved. [#947](https://github.com/github/vscode-codeql/pull/947) | |||
| - Fix contextual queries that are broken due to a malformed synthetic query suite. [#949](https://github.com/github/vscode-codeql/pull/949) | |||
There was a problem hiding this comment.
Mention the AST viewer here.
aeisenberg
force-pushed
the
aeisenberg/print-ast-fix
branch
from
2c64701
to
5b4479f
Compare
The previous synthetic query suite was not finding the ast query because
the
qlpackdirective in a query suite only matches queries from thedefault suite, which
printAST.qlis not part of.This changes to using
fromandqueriesdirectives.Also, adds an integration test to ensure we find the queries using
different CLIs. However, this only tests using the latest
mainfromthe codeql repository. I wonder if we should start testing using
different versions of the repo.
Replace this with a description of the changes your pull request makes.
Checklist
@github/docs-content-codeqlhas been cc'd in all issues for UI or other user-facing changes made by this pull request.