From cce17743bb0e88808e02fd4b25753992daed13f8 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 30 Jun 2025 14:12:36 +0200 Subject: [PATCH 01/10] Ql4Ql: Re-factor the ql/mising-security-metadata query. --- ql/ql/src/codeql_ql/ast/Ast.qll | 26 ++++++++-- .../queries/style/MissingSecurityMetadata.ql | 49 +++++++------------ .../MissingSecurityMetadata.expected | 2 +- 3 files changed, 40 insertions(+), 37 deletions(-) diff --git a/ql/ql/src/codeql_ql/ast/Ast.qll b/ql/ql/src/codeql_ql/ast/Ast.qll index 89bdf14d4b2a..5713e21592b8 100644 --- a/ql/ql/src/codeql_ql/ast/Ast.qll +++ b/ql/ql/src/codeql_ql/ast/Ast.qll @@ -202,25 +202,43 @@ class QueryDoc extends QLDoc { override string getAPrimaryQlClass() { result = "QueryDoc" } - /** Gets the @kind for the query */ + /** Gets the @kind for the query. */ string getQueryKind() { result = this.getContents().regexpCapture("(?s).*@kind ([\\w-]+)\\s.*", 1) } - /** Gets the @name for the query */ + /** Gets the @name for the query. */ string getQueryName() { result = this.getContents().regexpCapture("(?s).*@name (.+?)(?=\\n).*", 1) } - /** Gets the id part (without language) of the @id */ + /** Gets the id part (without language) of the @id. */ string getQueryId() { result = this.getContents().regexpCapture("(?s).*@id (\\w+)/([\\w\\-/]+)\\s.*", 2) } - /** Gets the language of the @id */ + /** Gets the language of the @id. */ string getQueryLanguage() { result = this.getContents().regexpCapture("(?s).*@id (\\w+)/([\\w\\-/]+)\\s.*", 1) } + + /** Gets the @precision for the query. */ + string getQueryPrecision() { + result = this.getContents().regexpCapture("(?s).*@precision ([\\w\\-]+)\\s.*", 1) + } + + /** Gets the @security-severity for the query. */ + string getQuerySecuritySeverity() { + result = this.getContents().regexpCapture("(?s).*@security\\-severity ([\\d\\.]+)\\s.*", 1) + } + + /** Gets the individual @tags for the query. */ + string getQueryTags() { + exists(string tags | tags = this.getContents().regexpCapture("(?s).*@tags ([^@]+)", 1) | + result = tags.splitAt("*").trim() and + result.regexpMatch("[\\w\\s\\-]+") + ) + } } class BlockComment extends TBlockComment, Comment { diff --git a/ql/ql/src/queries/style/MissingSecurityMetadata.ql b/ql/ql/src/queries/style/MissingSecurityMetadata.ql index 10f50fb3f990..fea75d36302c 100644 --- a/ql/ql/src/queries/style/MissingSecurityMetadata.ql +++ b/ql/ql/src/queries/style/MissingSecurityMetadata.ql @@ -10,45 +10,30 @@ import ql -predicate missingSecuritySeverity(QLDoc doc) { - exists(string s | s = doc.getContents() | - exists(string securityTag | securityTag = s.splitAt("@") | - securityTag.matches("tags%security%") - ) and - exists(string precisionTag | precisionTag = s.splitAt("@") | - precisionTag.matches("precision %") - ) and - not exists(string securitySeverity | securitySeverity = s.splitAt("@") | - securitySeverity.matches("security-severity %") - ) - ) +private predicate unInterestingLocation(File f) { + f.getRelativePath().matches("%/" + ["experimental", "examples", "test"] + "/%") } -predicate missingSecurityTag(QLDoc doc) { - exists(string s | s = doc.getContents() | - exists(string securitySeverity | securitySeverity = s.splitAt("@") | - securitySeverity.matches("security-severity %") - ) and - exists(string precisionTag | precisionTag = s.splitAt("@") | - precisionTag.matches("precision %") - ) and - not exists(string securityTag | securityTag = s.splitAt("@") | - securityTag.matches("tags%security%") - ) - ) +predicate missingSecuritySeverity(QueryDoc doc) { + doc.getQueryTags() = "security" and + exists(doc.getQueryPrecision()) and + not exists(doc.getQuerySecuritySeverity()) +} + +predicate missingSecurityTag(QueryDoc doc) { + exists(doc.getQuerySecuritySeverity()) and + exists(doc.getQueryPrecision()) and + not doc.getQueryTags() = "security" } -from TopLevel t, string msg +from TopLevel t, QueryDoc doc, string msg where - t.getLocation().getFile().getBaseName().matches("%.ql") and - not t.getLocation() - .getFile() - .getRelativePath() - .matches("%/" + ["experimental", "examples", "test"] + "/%") and + doc = t.getQLDoc() and + not unInterestingLocation(t.getLocation().getFile()) and ( - missingSecuritySeverity(t.getQLDoc()) and + missingSecuritySeverity(doc) and msg = "This query file is missing a `@security-severity` tag." or - missingSecurityTag(t.getQLDoc()) and msg = "This query file is missing a `@tag security`." + missingSecurityTag(doc) and msg = "This query file is missing a `@tags security`." ) select t, msg diff --git a/ql/ql/test/queries/style/MissingSecurityMetadata/MissingSecurityMetadata.expected b/ql/ql/test/queries/style/MissingSecurityMetadata/MissingSecurityMetadata.expected index 28421838ae37..af2fbd54acb0 100644 --- a/ql/ql/test/queries/style/MissingSecurityMetadata/MissingSecurityMetadata.expected +++ b/ql/ql/test/queries/style/MissingSecurityMetadata/MissingSecurityMetadata.expected @@ -1,2 +1,2 @@ -| testcases/BadNoSecurity.ql:1:1:16:9 | TopLevel | This query file is missing a `@tag security`. | +| testcases/BadNoSecurity.ql:1:1:16:9 | TopLevel | This query file is missing a `@tags security`. | | testcases/BadNoSeverity.ql:1:1:16:9 | TopLevel | This query file is missing a `@security-severity` tag. | From c46b528c0505cc1c3237b4af0490f16f14b65f89 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 30 Jun 2025 16:20:50 +0200 Subject: [PATCH 02/10] Ql4Ql: Add some quality tag testcases. --- ...QualityMaintainabilityWrongToplevel.expected | 0 .../BadQualityMaintainabilityWrongToplevel.ql | 17 +++++++++++++++++ .../BadQualityMultipleTopLevel.expected | 0 .../testcases/BadQualityMultipleTopLevel.ql | 17 +++++++++++++++++ .../testcases/BadQualityNoToplevel.expected | 0 .../testcases/BadQualityNoToplevel.ql | 16 ++++++++++++++++ .../BadQualityReliabilityWrongToplevel.expected | 0 .../BadQualityReliabilityWrongToplevel.ql | 17 +++++++++++++++++ .../testcases/GoodNotQuality.expected | 0 .../testcases/GoodNotQuality.ql | 16 ++++++++++++++++ .../GoodQualityMaintainability.expected | 0 .../testcases/GoodQualityMaintainability.ql | 17 +++++++++++++++++ .../GoodQualityMaintainabilityWithSub.expected | 0 .../GoodQualityMaintainabilityWithSub.ql | 17 +++++++++++++++++ .../testcases/GoodQualityReliability.expected | 0 .../testcases/GoodQualityReliability.ql | 16 ++++++++++++++++ .../GoodQualityReliabilityWithSub.expected | 0 .../testcases/GoodQualityReliabilityWithSub.ql | 17 +++++++++++++++++ 18 files changed, 150 insertions(+) create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMaintainabilityWrongToplevel.expected create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMaintainabilityWrongToplevel.ql create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMultipleTopLevel.expected create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMultipleTopLevel.ql create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityNoToplevel.expected create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityNoToplevel.ql create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityReliabilityWrongToplevel.expected create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityReliabilityWrongToplevel.ql create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodNotQuality.expected create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodNotQuality.ql create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainability.expected create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainability.ql create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithSub.expected create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithSub.ql create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliability.expected create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliability.ql create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithSub.expected create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithSub.ql diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMaintainabilityWrongToplevel.expected b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMaintainabilityWrongToplevel.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMaintainabilityWrongToplevel.ql b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMaintainabilityWrongToplevel.ql new file mode 100644 index 000000000000..3dd18771f959 --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMaintainabilityWrongToplevel.ql @@ -0,0 +1,17 @@ +/** + * @name Some query + * @description Some description + * @kind problem + * @problem.severity warning + * @precision very-high + * @id ql/quality-query-test + * @tags quality + * maintainability + * error-handling + */ + +import ql + +from Class c +where none() +select c, "" diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMultipleTopLevel.expected b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMultipleTopLevel.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMultipleTopLevel.ql b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMultipleTopLevel.ql new file mode 100644 index 000000000000..a9a7b48b76c7 --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityMultipleTopLevel.ql @@ -0,0 +1,17 @@ +/** + * @name Some query + * @description Some description + * @kind problem + * @problem.severity warning + * @precision very-high + * @id ql/quality-query-test + * @tags quality + * maintainability + * reliability + */ + +import ql + +from Class c +where none() +select c, "" diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityNoToplevel.expected b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityNoToplevel.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityNoToplevel.ql b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityNoToplevel.ql new file mode 100644 index 000000000000..ad2ab5c1fb57 --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityNoToplevel.ql @@ -0,0 +1,16 @@ +/** + * @name Some query + * @description Some description + * @kind problem + * @problem.severity warning + * @precision very-high + * @id ql/quality-query-test + * @tags quality + * someothertag + */ + +import ql + +from Class c +where none() +select c, "" diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityReliabilityWrongToplevel.expected b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityReliabilityWrongToplevel.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityReliabilityWrongToplevel.ql b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityReliabilityWrongToplevel.ql new file mode 100644 index 000000000000..53e84fb8a196 --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/BadQualityReliabilityWrongToplevel.ql @@ -0,0 +1,17 @@ +/** + * @name Some query + * @description Some description + * @kind problem + * @problem.severity warning + * @precision very-high + * @id ql/quality-query-test + * @tags quality + * reliability + * readability + */ + +import ql + +from Class c +where none() +select c, "" diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodNotQuality.expected b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodNotQuality.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodNotQuality.ql b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodNotQuality.ql new file mode 100644 index 000000000000..60b722918317 --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodNotQuality.ql @@ -0,0 +1,16 @@ +/** + * @name Some query + * @description Some description + * @kind problem + * @problem.severity warning + * @security-severity 10.0 + * @precision very-high + * @id ql/quality-query-test + * @tags security + */ + +import ql + +from Class c +where none() +select c, "" diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainability.expected b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainability.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainability.ql b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainability.ql new file mode 100644 index 000000000000..9e152b90d458 --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainability.ql @@ -0,0 +1,17 @@ +/** + * @name Some query + * @description Some description + * @kind problem + * @problem.severity warning + * @security-severity 10.0 + * @precision very-high + * @id ql/quality-query-test + * @tags quality + * maintainability + */ + +import ql + +from Class c +where none() +select c, "" diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithSub.expected b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithSub.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithSub.ql b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithSub.ql new file mode 100644 index 000000000000..7d70c8564033 --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithSub.ql @@ -0,0 +1,17 @@ +/** + * @name Some query + * @description Some description + * @kind problem + * @problem.severity warning + * @precision very-high + * @id ql/quality-query-test + * @tags quality + * maintainability + * readability + */ + +import ql + +from Class c +where none() +select c, "" diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliability.expected b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliability.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliability.ql b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliability.ql new file mode 100644 index 000000000000..f3979922b0d8 --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliability.ql @@ -0,0 +1,16 @@ +/** + * @name Some query + * @description Some description + * @kind problem + * @problem.severity warning + * @precision very-high + * @id ql/quality-query-test + * @tags quality + * reliability + */ + +import ql + +from Class c +where none() +select c, "" diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithSub.expected b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithSub.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithSub.ql b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithSub.ql new file mode 100644 index 000000000000..ec9c4136e862 --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithSub.ql @@ -0,0 +1,17 @@ +/** + * @name Some query + * @description Some description + * @kind problem + * @problem.severity warning + * @precision very-high + * @id ql/quality-query-test + * @tags quality + * reliability + * correctness + */ + +import ql + +from Class c +where none() +select c, "" From e00b5351a42dfe4b3d60e32796b86f53a1b2a3bf Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 30 Jun 2025 16:43:20 +0200 Subject: [PATCH 03/10] Ql4Ql: Add a check for quality tag consistency. --- .../queries/style/MissingQualityMetadata.ql | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 ql/ql/src/queries/style/MissingQualityMetadata.ql diff --git a/ql/ql/src/queries/style/MissingQualityMetadata.ql b/ql/ql/src/queries/style/MissingQualityMetadata.ql new file mode 100644 index 000000000000..a01b2028e572 --- /dev/null +++ b/ql/ql/src/queries/style/MissingQualityMetadata.ql @@ -0,0 +1,51 @@ +/** + * @name Missing quality metadata + * @description Quality queries should have exactly one top-level category and if sub-categories are used, the appropriate top-level category should be used. + * @kind problem + * @problem.severity warning + * @precision very-high + * @id ql/missing-quality-metadata + * @tags correctness + */ + +import ql + +private predicate unInterestingLocation(File f) { + f.getRelativePath().matches("%/" + ["experimental", "examples", "test"] + "/%") +} + +private predicate hasQualityTag(QueryDoc doc) { doc.getQueryTags() = "quality" } + +private predicate incorrectTopLevelCategorisation(QueryDoc doc) { + count(string s | s = doc.getQueryTags() and s = ["maintainability", "reliability"]) != 1 +} + +private predicate reliabilitySubCategory(QueryDoc doc) { + doc.getQueryTags() = ["correctness", "performance", "concurrency", "error-handling"] +} + +private predicate maintainabilitySubCategory(QueryDoc doc) { + doc.getQueryTags() = ["readability", "useless-code", "complexity"] +} + +from TopLevel t, QueryDoc doc, string msg +where + doc = t.getQLDoc() and + not unInterestingLocation(t.getLocation().getFile()) and + hasQualityTag(doc) and + ( + incorrectTopLevelCategorisation(doc) and + msg = + "This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`." + or + maintainabilitySubCategory(doc) and + not doc.getQueryTags() = "maintainability" and + msg = + "This query file has a sub-category of maintainability but is missing the `@tags maintainability` tag." + or + reliabilitySubCategory(doc) and + not doc.getQueryTags() = "reliability" and + msg = + "This query file has a sub-category of reliability but is missing the `@tags reliability` tag." + ) +select t, msg From 60a1d02357f423ad777132a70a4d03dd6ab50934 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 30 Jun 2025 16:44:29 +0200 Subject: [PATCH 04/10] Ql4Ql: Add MissingQualityMetadata test. --- .../MissingQualityMetadata/MissingQualityMetadata.expected | 4 ++++ .../style/MissingQualityMetadata/MissingQualityMetadata.qlref | 1 + 2 files changed, 5 insertions(+) create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.qlref diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected b/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected new file mode 100644 index 000000000000..6eabd28445bd --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected @@ -0,0 +1,4 @@ +| testcases/BadQualityMaintainabilityWrongToplevel.ql:1:1:17:13 | TopLevel | This query file has a sub-category of reliability but is missing the `@tags reliability` tag. | +| testcases/BadQualityMultipleTopLevel.ql:1:1:17:13 | TopLevel | This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`. | +| testcases/BadQualityNoToplevel.ql:1:1:16:13 | TopLevel | This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`. | +| testcases/BadQualityReliabilityWrongToplevel.ql:1:1:17:13 | TopLevel | This query file has a sub-category of maintainability but is missing the `@tags maintainability` tag. | diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.qlref b/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.qlref new file mode 100644 index 000000000000..6d7eb26bedeb --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.qlref @@ -0,0 +1 @@ +queries/style/MissingQualityMetadata.ql From af1c4e0896095fa28dddc05bcc4a14d64e00124d Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 1 Jul 2025 09:42:20 +0200 Subject: [PATCH 05/10] Ql4Ql: Share the definition of TestFile between multiple tests. --- ql/ql/src/codeql/files/FileSystem.qll | 5 +++++ ql/ql/src/queries/style/MissingQualityMetadata.ql | 6 +----- ql/ql/src/queries/style/MissingSecurityMetadata.ql | 6 +----- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/ql/ql/src/codeql/files/FileSystem.qll b/ql/ql/src/codeql/files/FileSystem.qll index 5a219f3b7f0c..7f64ed00b030 100644 --- a/ql/ql/src/codeql/files/FileSystem.qll +++ b/ql/ql/src/codeql/files/FileSystem.qll @@ -61,3 +61,8 @@ class File extends Container, Impl::File { /** Holds if this file was extracted from ordinary source code. */ predicate fromSource() { any() } } + +/** A test file. */ +class TestFile extends File { + TestFile() { this.getRelativePath().matches("%/" + ["experimental", "examples", "test"] + "/%") } +} diff --git a/ql/ql/src/queries/style/MissingQualityMetadata.ql b/ql/ql/src/queries/style/MissingQualityMetadata.ql index a01b2028e572..ceed39cf717e 100644 --- a/ql/ql/src/queries/style/MissingQualityMetadata.ql +++ b/ql/ql/src/queries/style/MissingQualityMetadata.ql @@ -10,10 +10,6 @@ import ql -private predicate unInterestingLocation(File f) { - f.getRelativePath().matches("%/" + ["experimental", "examples", "test"] + "/%") -} - private predicate hasQualityTag(QueryDoc doc) { doc.getQueryTags() = "quality" } private predicate incorrectTopLevelCategorisation(QueryDoc doc) { @@ -31,7 +27,7 @@ private predicate maintainabilitySubCategory(QueryDoc doc) { from TopLevel t, QueryDoc doc, string msg where doc = t.getQLDoc() and - not unInterestingLocation(t.getLocation().getFile()) and + not t.getLocation().getFile() instanceof TestFile and hasQualityTag(doc) and ( incorrectTopLevelCategorisation(doc) and diff --git a/ql/ql/src/queries/style/MissingSecurityMetadata.ql b/ql/ql/src/queries/style/MissingSecurityMetadata.ql index fea75d36302c..1618bed02ead 100644 --- a/ql/ql/src/queries/style/MissingSecurityMetadata.ql +++ b/ql/ql/src/queries/style/MissingSecurityMetadata.ql @@ -10,10 +10,6 @@ import ql -private predicate unInterestingLocation(File f) { - f.getRelativePath().matches("%/" + ["experimental", "examples", "test"] + "/%") -} - predicate missingSecuritySeverity(QueryDoc doc) { doc.getQueryTags() = "security" and exists(doc.getQueryPrecision()) and @@ -29,7 +25,7 @@ predicate missingSecurityTag(QueryDoc doc) { from TopLevel t, QueryDoc doc, string msg where doc = t.getQLDoc() and - not unInterestingLocation(t.getLocation().getFile()) and + not t.getLocation().getFile() instanceof TestFile and ( missingSecuritySeverity(doc) and msg = "This query file is missing a `@security-severity` tag." From f58064e119500396b54182d5fe91050f5350571e Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 2 Jul 2025 12:01:36 +0200 Subject: [PATCH 06/10] Ql4Ql: Address review comments. --- ql/ql/src/codeql_ql/ast/Ast.qll | 4 ++-- .../queries/style/MissingQualityMetadata.ql | 18 +++++++++--------- .../queries/style/MissingSecurityMetadata.ql | 8 ++++---- .../MissingQualityMetadata.expected | 8 ++++---- .../MissingSecurityMetadata.expected | 4 ++-- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/ql/ql/src/codeql_ql/ast/Ast.qll b/ql/ql/src/codeql_ql/ast/Ast.qll index 5713e21592b8..a7c3709ff22b 100644 --- a/ql/ql/src/codeql_ql/ast/Ast.qll +++ b/ql/ql/src/codeql_ql/ast/Ast.qll @@ -232,8 +232,8 @@ class QueryDoc extends QLDoc { result = this.getContents().regexpCapture("(?s).*@security\\-severity ([\\d\\.]+)\\s.*", 1) } - /** Gets the individual @tags for the query. */ - string getQueryTags() { + /** Gets the individual @tags for the query, if any. */ + string getAQueryTag() { exists(string tags | tags = this.getContents().regexpCapture("(?s).*@tags ([^@]+)", 1) | result = tags.splitAt("*").trim() and result.regexpMatch("[\\w\\s\\-]+") diff --git a/ql/ql/src/queries/style/MissingQualityMetadata.ql b/ql/ql/src/queries/style/MissingQualityMetadata.ql index ceed39cf717e..547590c01ee6 100644 --- a/ql/ql/src/queries/style/MissingQualityMetadata.ql +++ b/ql/ql/src/queries/style/MissingQualityMetadata.ql @@ -10,18 +10,18 @@ import ql -private predicate hasQualityTag(QueryDoc doc) { doc.getQueryTags() = "quality" } +private predicate hasQualityTag(QueryDoc doc) { doc.getAQueryTag() = "quality" } -private predicate incorrectTopLevelCategorisation(QueryDoc doc) { - count(string s | s = doc.getQueryTags() and s = ["maintainability", "reliability"]) != 1 +private predicate correctTopLevelCategorisation(QueryDoc doc) { + strictcount(string s | s = doc.getAQueryTag() and s = ["maintainability", "reliability"]) = 1 } private predicate reliabilitySubCategory(QueryDoc doc) { - doc.getQueryTags() = ["correctness", "performance", "concurrency", "error-handling"] + doc.getAQueryTag() = ["correctness", "performance", "concurrency", "error-handling"] } private predicate maintainabilitySubCategory(QueryDoc doc) { - doc.getQueryTags() = ["readability", "useless-code", "complexity"] + doc.getAQueryTag() = ["readability", "useless-code", "complexity"] } from TopLevel t, QueryDoc doc, string msg @@ -30,18 +30,18 @@ where not t.getLocation().getFile() instanceof TestFile and hasQualityTag(doc) and ( - incorrectTopLevelCategorisation(doc) and + not correctTopLevelCategorisation(doc) and msg = "This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`." or maintainabilitySubCategory(doc) and - not doc.getQueryTags() = "maintainability" and + not doc.getAQueryTag() = "maintainability" and msg = "This query file has a sub-category of maintainability but is missing the `@tags maintainability` tag." or reliabilitySubCategory(doc) and - not doc.getQueryTags() = "reliability" and + not doc.getAQueryTag() = "reliability" and msg = "This query file has a sub-category of reliability but is missing the `@tags reliability` tag." ) -select t, msg +select doc, msg diff --git a/ql/ql/src/queries/style/MissingSecurityMetadata.ql b/ql/ql/src/queries/style/MissingSecurityMetadata.ql index 1618bed02ead..5ab2cd98bbe5 100644 --- a/ql/ql/src/queries/style/MissingSecurityMetadata.ql +++ b/ql/ql/src/queries/style/MissingSecurityMetadata.ql @@ -1,6 +1,6 @@ /** * @name Missing security metadata - * @description Security queries should have both a `@tag security` and a `@security-severity` tag. + * @description Security queries should have both a `@tags security` and a `@security-severity` tag. * @kind problem * @problem.severity warning * @precision very-high @@ -11,7 +11,7 @@ import ql predicate missingSecuritySeverity(QueryDoc doc) { - doc.getQueryTags() = "security" and + doc.getAQueryTag() = "security" and exists(doc.getQueryPrecision()) and not exists(doc.getQuerySecuritySeverity()) } @@ -19,7 +19,7 @@ predicate missingSecuritySeverity(QueryDoc doc) { predicate missingSecurityTag(QueryDoc doc) { exists(doc.getQuerySecuritySeverity()) and exists(doc.getQueryPrecision()) and - not doc.getQueryTags() = "security" + not doc.getAQueryTag() = "security" } from TopLevel t, QueryDoc doc, string msg @@ -32,4 +32,4 @@ where or missingSecurityTag(doc) and msg = "This query file is missing a `@tags security`." ) -select t, msg +select doc, msg diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected b/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected index 6eabd28445bd..ec4939b9c4e9 100644 --- a/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected +++ b/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected @@ -1,4 +1,4 @@ -| testcases/BadQualityMaintainabilityWrongToplevel.ql:1:1:17:13 | TopLevel | This query file has a sub-category of reliability but is missing the `@tags reliability` tag. | -| testcases/BadQualityMultipleTopLevel.ql:1:1:17:13 | TopLevel | This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`. | -| testcases/BadQualityNoToplevel.ql:1:1:16:13 | TopLevel | This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`. | -| testcases/BadQualityReliabilityWrongToplevel.ql:1:1:17:13 | TopLevel | This query file has a sub-category of maintainability but is missing the `@tags maintainability` tag. | +| testcases/BadQualityMaintainabilityWrongToplevel.ql:1:1:11:3 | QueryDoc | This query file has a sub-category of reliability but is missing the `@tags reliability` tag. | +| testcases/BadQualityMultipleTopLevel.ql:1:1:11:3 | QueryDoc | This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`. | +| testcases/BadQualityNoToplevel.ql:1:1:10:3 | QueryDoc | This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`. | +| testcases/BadQualityReliabilityWrongToplevel.ql:1:1:11:3 | QueryDoc | This query file has a sub-category of maintainability but is missing the `@tags maintainability` tag. | diff --git a/ql/ql/test/queries/style/MissingSecurityMetadata/MissingSecurityMetadata.expected b/ql/ql/test/queries/style/MissingSecurityMetadata/MissingSecurityMetadata.expected index af2fbd54acb0..bc241f3f0b4f 100644 --- a/ql/ql/test/queries/style/MissingSecurityMetadata/MissingSecurityMetadata.expected +++ b/ql/ql/test/queries/style/MissingSecurityMetadata/MissingSecurityMetadata.expected @@ -1,2 +1,2 @@ -| testcases/BadNoSecurity.ql:1:1:16:9 | TopLevel | This query file is missing a `@tags security`. | -| testcases/BadNoSeverity.ql:1:1:16:9 | TopLevel | This query file is missing a `@security-severity` tag. | +| testcases/BadNoSecurity.ql:1:1:10:3 | QueryDoc | This query file is missing a `@tags security`. | +| testcases/BadNoSeverity.ql:1:1:10:3 | QueryDoc | This query file is missing a `@security-severity` tag. | From b79e2dd0baaa1c68f9507aeb1f37f9132ffd82dc Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 3 Jul 2025 11:13:57 +0200 Subject: [PATCH 07/10] Ql4Ql: Add some more quality tag testcases. --- .../MissingQualityMetadata.expected | 2 ++ ...QualityMaintainabilityWithCrossSub.expected | 0 .../GoodQualityMaintainabilityWithCrossSub.ql | 18 ++++++++++++++++++ ...GoodQualityReliabilityWithCrossSub.expected | 0 .../GoodQualityReliabilityWithCrossSub.ql | 18 ++++++++++++++++++ 5 files changed, 38 insertions(+) create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithCrossSub.expected create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithCrossSub.ql create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithCrossSub.expected create mode 100644 ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithCrossSub.ql diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected b/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected index ec4939b9c4e9..9ee4bd78576e 100644 --- a/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected +++ b/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected @@ -2,3 +2,5 @@ | testcases/BadQualityMultipleTopLevel.ql:1:1:11:3 | QueryDoc | This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`. | | testcases/BadQualityNoToplevel.ql:1:1:10:3 | QueryDoc | This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`. | | testcases/BadQualityReliabilityWrongToplevel.ql:1:1:11:3 | QueryDoc | This query file has a sub-category of maintainability but is missing the `@tags maintainability` tag. | +| testcases/GoodQualityMaintainabilityWithCrossSub.ql:1:1:12:3 | QueryDoc | This query file has a sub-category of reliability but is missing the `@tags reliability` tag. | +| testcases/GoodQualityReliabilityWithCrossSub.ql:1:1:12:3 | QueryDoc | This query file has a sub-category of maintainability but is missing the `@tags maintainability` tag. | diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithCrossSub.expected b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithCrossSub.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithCrossSub.ql b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithCrossSub.ql new file mode 100644 index 000000000000..fe1f511abfff --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityMaintainabilityWithCrossSub.ql @@ -0,0 +1,18 @@ +/** + * @name Some query + * @description Some description + * @kind problem + * @problem.severity warning + * @precision very-high + * @id ql/quality-query-test + * @tags quality + * maintainability + * readability + * correctness + */ + +import ql + +from Class c +where none() +select c, "" diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithCrossSub.expected b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithCrossSub.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithCrossSub.ql b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithCrossSub.ql new file mode 100644 index 000000000000..78594e8f9c3c --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/testcases/GoodQualityReliabilityWithCrossSub.ql @@ -0,0 +1,18 @@ +/** + * @name Some query + * @description Some description + * @kind problem + * @problem.severity warning + * @precision very-high + * @id ql/quality-query-test + * @tags quality + * reliability + * correctness + * readability + */ + +import ql + +from Class c +where none() +select c, "" From f810e17d9ee6956fe74bac548853aa3fa806e345 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 3 Jul 2025 11:26:11 +0200 Subject: [PATCH 08/10] Ql4Ql: Address review comments and update expected test output. --- .../queries/style/MissingQualityMetadata.ql | 23 +++++++++++-------- .../MissingQualityMetadata.expected | 6 ++--- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/ql/ql/src/queries/style/MissingQualityMetadata.ql b/ql/ql/src/queries/style/MissingQualityMetadata.ql index 547590c01ee6..88c877186340 100644 --- a/ql/ql/src/queries/style/MissingQualityMetadata.ql +++ b/ql/ql/src/queries/style/MissingQualityMetadata.ql @@ -34,14 +34,19 @@ where msg = "This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`." or - maintainabilitySubCategory(doc) and - not doc.getAQueryTag() = "maintainability" and - msg = - "This query file has a sub-category of maintainability but is missing the `@tags maintainability` tag." - or - reliabilitySubCategory(doc) and - not doc.getAQueryTag() = "reliability" and - msg = - "This query file has a sub-category of reliability but is missing the `@tags reliability` tag." + correctTopLevelCategorisation(doc) and + ( + doc.getAQueryTag() = "reliability" and + not reliabilitySubCategory(doc) and + maintainabilitySubCategory(doc) and + msg = + "This query file has a sub-category of maintainability but has the `@tags reliability` tag." + or + doc.getAQueryTag() = "maintainability" and + not maintainabilitySubCategory(doc) and + reliabilitySubCategory(doc) and + msg = + "This query file has a sub-category of reliability but has the `@tags maintainability` tag." + ) ) select doc, msg diff --git a/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected b/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected index 9ee4bd78576e..7904870bdf64 100644 --- a/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected +++ b/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected @@ -1,6 +1,4 @@ -| testcases/BadQualityMaintainabilityWrongToplevel.ql:1:1:11:3 | QueryDoc | This query file has a sub-category of reliability but is missing the `@tags reliability` tag. | +| testcases/BadQualityMaintainabilityWrongToplevel.ql:1:1:11:3 | QueryDoc | This query file has a sub-category of reliability but has the `@tags maintainability` tag. | | testcases/BadQualityMultipleTopLevel.ql:1:1:11:3 | QueryDoc | This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`. | | testcases/BadQualityNoToplevel.ql:1:1:10:3 | QueryDoc | This query file has incorrect top-level categorisation. It should have exactly one top-level category, either `@tags maintainability` or `@tags reliability`. | -| testcases/BadQualityReliabilityWrongToplevel.ql:1:1:11:3 | QueryDoc | This query file has a sub-category of maintainability but is missing the `@tags maintainability` tag. | -| testcases/GoodQualityMaintainabilityWithCrossSub.ql:1:1:12:3 | QueryDoc | This query file has a sub-category of reliability but is missing the `@tags reliability` tag. | -| testcases/GoodQualityReliabilityWithCrossSub.ql:1:1:12:3 | QueryDoc | This query file has a sub-category of maintainability but is missing the `@tags maintainability` tag. | +| testcases/BadQualityReliabilityWrongToplevel.ql:1:1:11:3 | QueryDoc | This query file has a sub-category of maintainability but has the `@tags reliability` tag. | From aefd941135731333afe4081819ad668f867d4798 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 3 Jul 2025 11:48:56 +0200 Subject: [PATCH 09/10] Java/Javascript: Fix violations. --- java/ql/src/Language Abuse/TypeVariableHidesType.ql | 4 ++-- javascript/ql/src/Quality/UnhandledErrorInStreamPipeline.ql | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/java/ql/src/Language Abuse/TypeVariableHidesType.ql b/java/ql/src/Language Abuse/TypeVariableHidesType.ql index 81da0e9703e6..42d0a7bea2bf 100644 --- a/java/ql/src/Language Abuse/TypeVariableHidesType.ql +++ b/java/ql/src/Language Abuse/TypeVariableHidesType.ql @@ -6,10 +6,10 @@ * @problem.severity warning * @precision medium * @id java/type-variable-hides-type - * @tags reliability + * @tags quality + * maintainability * readability * types - * quality */ import java diff --git a/javascript/ql/src/Quality/UnhandledErrorInStreamPipeline.ql b/javascript/ql/src/Quality/UnhandledErrorInStreamPipeline.ql index a6142a2e6e73..6500cd157778 100644 --- a/javascript/ql/src/Quality/UnhandledErrorInStreamPipeline.ql +++ b/javascript/ql/src/Quality/UnhandledErrorInStreamPipeline.ql @@ -6,7 +6,7 @@ * @problem.severity warning * @precision high * @tags quality - * maintainability + * reliability * error-handling * frameworks/nodejs */ From 11c4a638bc63f6ddc5861831967afa2073636672 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 3 Jul 2025 12:19:41 +0200 Subject: [PATCH 10/10] Quality tags: Clarify the quality sub-category tagging policy. --- docs/query-metadata-style-guide.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/query-metadata-style-guide.md b/docs/query-metadata-style-guide.md index 18fa5d1880f5..f6cc6f883fc1 100644 --- a/docs/query-metadata-style-guide.md +++ b/docs/query-metadata-style-guide.md @@ -157,7 +157,7 @@ Each code quality related query should have **one** of these two "top-level" cat * `@tags maintainability`–for queries that detect patterns that make it harder for developers to make changes to the code. * `@tags reliability`–for queries that detect issues that affect whether the code will perform as expected during execution. -In addition to the "top-level" categories, we will also add sub-categories to further group code quality related queries: +In addition to the "top-level" categories, we may also add sub-categories to further group code quality related queries: * `@tags maintainability`–for queries that detect patterns that make it harder for developers to make changes to the code. * `@tags readability`–for queries that detect confusing patterns that make it harder for developers to read the code. @@ -171,6 +171,7 @@ In addition to the "top-level" categories, we will also add sub-categories to fu * `@tags concurrency`-for queries that detect concurrency related issues such as race conditions, deadlocks, thread safety, etc * `@tags error-handling`-for queries that detect issues related to unsafe error handling such as uncaught exceptions, etc +You may use sub-categories from both top-level categories on the same query. However, if you only use sub-categories from a single top-level category, then you must also tag the query with that top-level category. There are also more specific `@tags` that can be added. See, the following pages for examples of the low-level tags: