From a578f44af4a19f9eff6f1bdf28f66295867884ed Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 10 Apr 2025 14:40:59 +0200 Subject: [PATCH 1/2] QL4QL: Restrict `ql/qlref-inline-expectations` to `(path-)problem` queries --- ql/ql/src/codeql/files/FileSystem.qll | 2 + ql/ql/src/codeql_ql/ast/Yaml.qll | 71 +++++++++++++++++++ .../queries/style/QlRefInlineExpectations.ql | 6 +- .../ProblemQuery.expected | 0 .../QlRefInlineExpectations/ProblemQuery.ql | 13 ++++ .../QlRefInlineExpectations.expected | 3 +- .../style/QlRefInlineExpectations/Test1.qlref | 2 +- .../style/QlRefInlineExpectations/Test2.qlref | 2 +- .../style/QlRefInlineExpectations/Test3.qlref | 2 +- 9 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 ql/ql/test/queries/style/QlRefInlineExpectations/ProblemQuery.expected create mode 100644 ql/ql/test/queries/style/QlRefInlineExpectations/ProblemQuery.ql diff --git a/ql/ql/src/codeql/files/FileSystem.qll b/ql/ql/src/codeql/files/FileSystem.qll index 738241a402f1..5a219f3b7f0c 100644 --- a/ql/ql/src/codeql/files/FileSystem.qll +++ b/ql/ql/src/codeql/files/FileSystem.qll @@ -30,6 +30,8 @@ class Container = Impl::Container; class Folder = Impl::Folder; +module Folder = Impl::Folder; + /** A file. */ class File extends Container, Impl::File { /** Gets a token in this file. */ diff --git a/ql/ql/src/codeql_ql/ast/Yaml.qll b/ql/ql/src/codeql_ql/ast/Yaml.qll index d5e80365ff4a..403640c1f841 100644 --- a/ql/ql/src/codeql_ql/ast/Yaml.qll +++ b/ql/ql/src/codeql_ql/ast/Yaml.qll @@ -6,6 +6,7 @@ */ private import codeql.yaml.Yaml as LibYaml +private import codeql.files.FileSystem private module YamlSig implements LibYaml::InputSig { import codeql.Locations @@ -49,6 +50,58 @@ private module YamlSig implements LibYaml::InputSig { import LibYaml::Make +/** A `qlpack.yml` document. */ +class QlPackDocument extends YamlDocument { + QlPackDocument() { this.getFile().getBaseName() = ["qlpack.yml", "qlpack.test.yml"] } + + /** Gets the name of this QL pack. */ + string getPackName() { + exists(YamlMapping n | + n.getDocument() = this and + result = n.lookup("name").(YamlScalar).getValue() + ) + } + + private string getADependencyName() { + exists(YamlMapping n, YamlScalar key | + n.getDocument() = this and + n.lookup("dependencies").(YamlMapping).maps(key, _) and + result = key.getValue() + ) + } + + /** Gets a dependency of this QL pack. */ + QlPackDocument getADependency() { result.getPackName() = this.getADependencyName() } + + private Folder getRootFolder() { result = this.getFile().getParentContainer() } + + /** Gets a folder inside this QL pack. */ + pragma[nomagic] + Folder getAFolder() { + result = this.getRootFolder() + or + exists(Folder mid | + mid = this.getAFolder() and + result.getParentContainer() = mid and + not result = any(QlPackDocument other).getRootFolder() + ) + } +} + +private predicate shouldAppend(QlRefDocument qlref, Folder f, string relativePath) { + relativePath = qlref.getRelativeQueryPath() and + ( + exists(QlPackDocument pack | + pack.getAFolder() = qlref.getFile().getParentContainer() and + f = [pack, pack.getADependency()].getFile().getParentContainer() + ) + or + f = qlref.getFile().getParentContainer() + ) +} + +private predicate shouldAppend(Folder f, string relativePath) { shouldAppend(_, f, relativePath) } + /** A `.qlref` YAML document. */ class QlRefDocument extends YamlDocument { QlRefDocument() { this.getFile().getExtension() = "qlref" } @@ -65,6 +118,24 @@ class QlRefDocument extends YamlDocument { ) } + /** Gets the relative path of the query in this `.qlref` file. */ + string getRelativeQueryPath() { + exists(YamlMapping n | n.getDocument() = this | + result = n.lookup("query").(YamlScalar).getValue() + ) + or + not exists(YamlMapping n | n.getDocument() = this) and + result = this.eval().(YamlScalar).getValue() + } + + /** Gets the query file referenced in this `.qlref` file. */ + File getQueryFile() { + exists(Folder f, string relativePath | + shouldAppend(this, f, relativePath) and + result = Folder::Append::append(f, relativePath) + ) + } + predicate isPrintAst() { this.getFile().getStem() = "PrintAst" or diff --git a/ql/ql/src/queries/style/QlRefInlineExpectations.ql b/ql/ql/src/queries/style/QlRefInlineExpectations.ql index 66c139f683f6..7f47f2d06c8d 100644 --- a/ql/ql/src/queries/style/QlRefInlineExpectations.ql +++ b/ql/ql/src/queries/style/QlRefInlineExpectations.ql @@ -10,8 +10,10 @@ import ql import codeql_ql.ast.Yaml -from QlRefDocument f +from QlRefDocument f, TopLevel t, QueryDoc doc where not f.usesInlineExpectations() and - not f.isPrintAst() + t.getFile() = f.getQueryFile() and + doc = t.getQLDoc() and + doc.getQueryKind() in ["problem", "path-problem"] select f, "Query test does not use inline test expectations." diff --git a/ql/ql/test/queries/style/QlRefInlineExpectations/ProblemQuery.expected b/ql/ql/test/queries/style/QlRefInlineExpectations/ProblemQuery.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/ql/ql/test/queries/style/QlRefInlineExpectations/ProblemQuery.ql b/ql/ql/test/queries/style/QlRefInlineExpectations/ProblemQuery.ql new file mode 100644 index 000000000000..fe4867113f37 --- /dev/null +++ b/ql/ql/test/queries/style/QlRefInlineExpectations/ProblemQuery.ql @@ -0,0 +1,13 @@ +/** + * @name Problem query + * @description Description of the problem + * @kind problem + * @problem.severity warning + * @id ql/problem-query + */ + +import ql + +from VarDecl decl +where none() +select decl, "Problem" diff --git a/ql/ql/test/queries/style/QlRefInlineExpectations/QlRefInlineExpectations.expected b/ql/ql/test/queries/style/QlRefInlineExpectations/QlRefInlineExpectations.expected index 7dd57a3ef860..9605589e514e 100644 --- a/ql/ql/test/queries/style/QlRefInlineExpectations/QlRefInlineExpectations.expected +++ b/ql/ql/test/queries/style/QlRefInlineExpectations/QlRefInlineExpectations.expected @@ -1,2 +1 @@ -| QlRefInlineExpectations.qlref:1:1:1:40 | queries ... ions.ql | Query test does not use inline test expectations. | -| Test3.qlref:1:1:1:39 | query: ... ists.ql | Query test does not use inline test expectations. | +| Test3.qlref:1:1:1:22 | query: ... uery.ql | Query test does not use inline test expectations. | diff --git a/ql/ql/test/queries/style/QlRefInlineExpectations/Test1.qlref b/ql/ql/test/queries/style/QlRefInlineExpectations/Test1.qlref index 03dd6f50713b..07255415589e 100644 --- a/ql/ql/test/queries/style/QlRefInlineExpectations/Test1.qlref +++ b/ql/ql/test/queries/style/QlRefInlineExpectations/Test1.qlref @@ -1,2 +1,2 @@ -query: queries/style/OmittableExists.ql +query: ProblemQuery.ql postprocess: utils/test/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/ql/ql/test/queries/style/QlRefInlineExpectations/Test2.qlref b/ql/ql/test/queries/style/QlRefInlineExpectations/Test2.qlref index 414c61f1e9cc..a0c03488dae0 100644 --- a/ql/ql/test/queries/style/QlRefInlineExpectations/Test2.qlref +++ b/ql/ql/test/queries/style/QlRefInlineExpectations/Test2.qlref @@ -1,3 +1,3 @@ -query: queries/style/OmittableExists.ql +query: ProblemQuery.ql postprocess: - utils/test/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/ql/ql/test/queries/style/QlRefInlineExpectations/Test3.qlref b/ql/ql/test/queries/style/QlRefInlineExpectations/Test3.qlref index 496512e7b452..5582a96837a3 100644 --- a/ql/ql/test/queries/style/QlRefInlineExpectations/Test3.qlref +++ b/ql/ql/test/queries/style/QlRefInlineExpectations/Test3.qlref @@ -1 +1 @@ -query: queries/style/OmittableExists.ql \ No newline at end of file +query: ProblemQuery.ql \ No newline at end of file From 40390d1adacda812a1a3f87b2a5fc727c4c1d8c1 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Tue, 22 Apr 2025 15:08:39 +0200 Subject: [PATCH 2/2] Address review comment --- ql/ql/src/codeql_ql/ast/Yaml.qll | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ql/ql/src/codeql_ql/ast/Yaml.qll b/ql/ql/src/codeql_ql/ast/Yaml.qll index 403640c1f841..e88f2a0c2aab 100644 --- a/ql/ql/src/codeql_ql/ast/Yaml.qll +++ b/ql/ql/src/codeql_ql/ast/Yaml.qll @@ -88,6 +88,16 @@ class QlPackDocument extends YamlDocument { } } +/** + * Holds if `qlref` is a `.qlref` YAML document referencing a query + * at relative path `relativePath`, and `f` is a candidate folder + * in which to lookup the referenced query. + * + * `f` is either: + * - the root of the QL pack containing `qlref`, + * - the root of a QL pack that is a dependency of the QL pack containing `qlref`, or + * - the folder containing `qlref`. + */ private predicate shouldAppend(QlRefDocument qlref, Folder f, string relativePath) { relativePath = qlref.getRelativeQueryPath() and (