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: 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 */ 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/codeql_ql/ast/Ast.qll b/ql/ql/src/codeql_ql/ast/Ast.qll index 89bdf14d4b2a..a7c3709ff22b 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, if any. */ + string getAQueryTag() { + 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/MissingQualityMetadata.ql b/ql/ql/src/queries/style/MissingQualityMetadata.ql new file mode 100644 index 000000000000..88c877186340 --- /dev/null +++ b/ql/ql/src/queries/style/MissingQualityMetadata.ql @@ -0,0 +1,52 @@ +/** + * @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 hasQualityTag(QueryDoc doc) { doc.getAQueryTag() = "quality" } + +private predicate correctTopLevelCategorisation(QueryDoc doc) { + strictcount(string s | s = doc.getAQueryTag() and s = ["maintainability", "reliability"]) = 1 +} + +private predicate reliabilitySubCategory(QueryDoc doc) { + doc.getAQueryTag() = ["correctness", "performance", "concurrency", "error-handling"] +} + +private predicate maintainabilitySubCategory(QueryDoc doc) { + doc.getAQueryTag() = ["readability", "useless-code", "complexity"] +} + +from TopLevel t, QueryDoc doc, string msg +where + doc = t.getQLDoc() and + not t.getLocation().getFile() instanceof TestFile and + hasQualityTag(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 + 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/src/queries/style/MissingSecurityMetadata.ql b/ql/ql/src/queries/style/MissingSecurityMetadata.ql index 10f50fb3f990..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 @@ -10,45 +10,26 @@ 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 %") - ) - ) +predicate missingSecuritySeverity(QueryDoc doc) { + doc.getAQueryTag() = "security" and + exists(doc.getQueryPrecision()) and + not exists(doc.getQuerySecuritySeverity()) } -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 missingSecurityTag(QueryDoc doc) { + exists(doc.getQuerySecuritySeverity()) and + exists(doc.getQueryPrecision()) and + not doc.getAQueryTag() = "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 t.getLocation().getFile() instanceof TestFile 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 +select doc, msg 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..7904870bdf64 --- /dev/null +++ b/ql/ql/test/queries/style/MissingQualityMetadata/MissingQualityMetadata.expected @@ -0,0 +1,4 @@ +| 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 has the `@tags reliability` 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 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/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/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/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, "" 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, "" diff --git a/ql/ql/test/queries/style/MissingSecurityMetadata/MissingSecurityMetadata.expected b/ql/ql/test/queries/style/MissingSecurityMetadata/MissingSecurityMetadata.expected index 28421838ae37..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 `@tag 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. |