From 664a54ac2c492cecabf056b9cc2b878b69814881 Mon Sep 17 00:00:00 2001 From: Mauro Baluda Date: Tue, 24 Mar 2026 15:49:15 +0100 Subject: [PATCH 1/7] Add query validation guidelines to copilot review instructions Added guidelines for validating query style in code reviews. --- .github/copilot-instructions.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 64d87a5339..0c6e5a5ff5 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -58,3 +58,22 @@ When reviewing tests, it is critical to: - Check that the locations do not refer to files in the standard library, as these have issues in GitHub's Code Scanning UI and complicate our compiler compatibility tests. - Consider the "test coverage" of the query, are each of its logical statements effectively exercised individually, collectively? The test should neither be overly bloated nor under specified. - Consider the edge cases of the language itself, will the analysis work in non-trivial cases, are all relevant language concepts tested here? This doesn't need to be exhaustive, but it should be thoughfully thorough. + +### Validating Query style + +The following list describes the required style guides for a query that **must** be validated during the code-review process. + +A query **must** include: + +- A use of the `isExcluded` predicate on the element reported as the primary location. This predicate ensures that we have a central mechanism for excluding results. This predicate may also be used on other elements relevant to the alert, but only if a suppression on that element should also cause alerts on the current element to be suppressed. +- A well formatted alert message: + - The message should be a complete standalone sentence, with punctuation and a full stop. + - The message should refer to this particular instance of the problem, rather than repeating the generic rule. e.g. "Call to banned function x." instead of "Do not use function x." + - Code elements should be placed in 'single quotes', unless they are formatted as links. + - Avoid value judgments such as "dubious" and "suspicious", and focus on factual statements about the problem. + - If possible, avoid constant alert messages. Either add placeholders and links (using $@), or concatenate element names to the alert message. Non-constant messages make it easier to find particular results, and links to other program elements can help provide additional context to help a developer understand the results. Examples: + - Instead of `Call to banned function.` prefer `Call to banned function foobar.`. + - Instead of `Return value from call is unused.` prefer `Return value from call to function [x] is unused.`, where `[x]` is a link to the function itself. + - Do not try to explain the solution in the message; instead that should be provided in the help for the query. + +All public predicates, classes, modules and files should be documented with QLDoc. All QLDoc should follow the [QLDoc style guide](https://github.com/github/codeql/blob/main/docs/qldoc-style-guide.md). From 83cf49c4141323a9a19d9f8fe817692dfb1c69fd Mon Sep 17 00:00:00 2001 From: Mauro Baluda Date: Tue, 24 Mar 2026 16:19:46 +0100 Subject: [PATCH 2/7] Revise QLDoc style guide and add examples Updated QLDoc style guide with detailed requirements for documentation, including general, language, and specific item requirements. Added examples for predicates, classes, and modules. --- .github/copilot-instructions.md | 184 +++++++++++++++++++++++++++++++- 1 file changed, 182 insertions(+), 2 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 0c6e5a5ff5..6a146b05b3 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -59,7 +59,7 @@ When reviewing tests, it is critical to: - Consider the "test coverage" of the query, are each of its logical statements effectively exercised individually, collectively? The test should neither be overly bloated nor under specified. - Consider the edge cases of the language itself, will the analysis work in non-trivial cases, are all relevant language concepts tested here? This doesn't need to be exhaustive, but it should be thoughfully thorough. -### Validating Query style +## Validating Query style The following list describes the required style guides for a query that **must** be validated during the code-review process. @@ -76,4 +76,184 @@ A query **must** include: - Instead of `Return value from call is unused.` prefer `Return value from call to function [x] is unused.`, where `[x]` is a link to the function itself. - Do not try to explain the solution in the message; instead that should be provided in the help for the query. -All public predicates, classes, modules and files should be documented with QLDoc. All QLDoc should follow the [QLDoc style guide](https://github.com/github/codeql/blob/main/docs/qldoc-style-guide.md). +All public predicates, classes, modules and files should be documented with QLDoc. All QLDoc should follow the following QLDoc style guide: + +### General QLDoc requirements + +1. Documentation must adhere to the [QLDoc specification](https://codeql.github.com/docs/ql-language-reference/ql-language-specification/#qldoc). +1. Documentation comments should be appropriate for users of the code. +1. Documentation for maintainers of the code must use normal comments. +1. Use `/** ... */` for documentation, even for single line comments. + - For single-line documentation, the `/**` and `*/` are written on the same line as the comment. + - For multi-line documentation, the `/**` and `*/` are written on separate lines. There is a `*` preceding each comment line, aligned on the first `*`. +1. Use code formatting (backticks) within comments for code from the source language, and also for QL code (for example, names of classes, predicates, and variables). +1. Give explanatory examples of code in the target language, enclosed in ```` ``` ```` or `` ` ``. + + +### Language requirements + +1. Use American English. +1. Use full sentences, with capital letters and periods, except for the initial sentence of the comment, which may be fragmentary as described below. +1. Use simple sentence structures and avoid complex or academic language. +1. Avoid colloquialisms and contractions. +1. Use words that are in common usage. + + +### Requirements for specific items + +1. Public declarations must be documented. +1. Non-public declarations should be documented. +1. Declarations in query files should be documented. +1. Library files (`.qll` files) should have a documentation comment at the top of the file. +1. Query files, except for tests, must have a QLDoc query documentation comment at the top of the file. + +### QLDoc for predicates + +1. Refer to all predicate parameters in the predicate documentation. +1. Reference names, such as types and parameters, using backticks `` ` ``. +1. Give examples of code in the target language, enclosed in ```` ``` ```` or `` ` ``. +1. Predicates that override a single predicate don't need QLDoc, as they will inherit it. + +#### Predicates without result + +1. Use a third-person verb phrase of the form ``Holds if `arg` has .`` +1. Avoid: + - `/** Whether ... */` + - `/**" Relates ... */` + - Question forms: + - ``/** Is `x` a foo? */`` + - ``/** Does `x` have a bar? */`` + +##### Example + +```ql +/** + * Holds if the qualifier of this call has type `qualifierType`. + * `isExactType` indicates whether the type is exact, that is, whether + * the qualifier is guaranteed not to be a subtype of `qualifierType`. + */ +``` + +#### Predicates with result + +1. Use a third-person verb phrase of the form `Gets (a|the) .` +1. Use "if any" if the item is usually unique but might be missing. For example +`Gets the body of this method, if any.` +1. If the predicate has more complex behaviour, for example multiple arguments are conceptually "outputs", it can be described like a predicate without a result. For example +``Holds if `result` is a child of this expression.`` +1. Avoid: + - `Get a ...` + - `The ...` + - `Results in ...` + - Any use of `return` + +##### Example +```ql +/** + * Gets the expression denoting the super class of this class, + * or nothing if this is an interface or a class without an `extends` clause. + */ +``` + +#### Deprecated predicates + +The documentation for deprecated predicates should be updated to emphasize the deprecation and specify what predicate to use as an alternative. +Insert a sentence of the form `DEPRECATED: Use instead.` at the start of the QLDoc comment. + +##### Example + +```ql +/** DEPRECATED: Use `getAnExpr()` instead. */ +deprecated Expr getInitializer() +``` + +#### Internal predicates + +Some predicates are internal-only declarations that cannot be made private. The documentation for internal predicates should begin with `INTERNAL: Do not use.` + +##### Example + +```ql +/** + * INTERNAL: Do not use. + */ +``` + +#### Special predicates + +Certain special predicates should be documented consistently. + +- Always document `toString` as + + ```ql + /** Gets a textual representation of this element. */ + string toString() { ... } + ``` + +- Always document `hasLocationInfo` as + + ```ql + /** + * Holds if this element is at the specified location. + * The location spans column `startcolumn` of line `startline` to + * column `endcolumn` of line `endline` in file `filepath`. + * For more information, see + * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/). + */ + + predicate hasLocationInfo(string filepath, int startline, int startcolumn, int endline, int endcolumn) { ... } + ``` + +### QLDoc for classes + +1. Document classes using a noun phrase of the form `A that .` +1. Use "that", not "which". +1. Refer to member elements in the singular. +1. Where a class denotes a generic concept with subclasses, list those subclasses. + +##### Example + +```ql +/** + * A delegate declaration, for example + * ``` + * delegate void Logger(string text); + * ``` + */ +class Delegate extends ... +``` + +```ql +/** + * An element that can be called. + * + * Either a method (`Method`), a constructor (`Constructor`), a destructor + * (`Destructor`), an operator (`Operator`), an accessor (`Accessor`), + * an anonymous function (`AnonymousFunctionExpr`), or a local function + * (`LocalFunction`). + */ +class Callable extends ... +``` + +### QLDoc for modules + +Modules should be documented using a third-person verb phrase of the form `Provides .` + +##### Example + +```ql +/** Provides logic for determining constant expressions. */ +``` +```ql +/** Provides classes representing the control flow graph within functions. */ +``` + +### Special variables + +When referring to `this`, you may either refer to it as `` `this` `` or `this `. For example: +- ``Holds if `this` is static.`` +- `Holds if this method is static.` + +When referring to `result`, you may either refer to it as `` `result` `` or as `the result`. For example: +- ``Holds if `result` is a child of this expression.`` +- `Holds if the result is a child of this expression.` From 26402a3101059bcf48116f013d0508bd92ebfc33 Mon Sep 17 00:00:00 2001 From: Mauro Baluda Date: Tue, 24 Mar 2026 16:35:04 +0100 Subject: [PATCH 3/7] Update .github/copilot-instructions.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 6a146b05b3..323856e032 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -67,7 +67,7 @@ A query **must** include: - A use of the `isExcluded` predicate on the element reported as the primary location. This predicate ensures that we have a central mechanism for excluding results. This predicate may also be used on other elements relevant to the alert, but only if a suppression on that element should also cause alerts on the current element to be suppressed. - A well formatted alert message: - - The message should be a complete standalone sentence, with punctuation and a full stop. + - The message should be a complete standalone sentence, with punctuation and a period. - The message should refer to this particular instance of the problem, rather than repeating the generic rule. e.g. "Call to banned function x." instead of "Do not use function x." - Code elements should be placed in 'single quotes', unless they are formatted as links. - Avoid value judgments such as "dubious" and "suspicious", and focus on factual statements about the problem. From b65b58fad9b960fcb8229ebc8496770c8157dec6 Mon Sep 17 00:00:00 2001 From: Mauro Baluda Date: Tue, 24 Mar 2026 16:35:25 +0100 Subject: [PATCH 4/7] Update .github/copilot-instructions.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 323856e032..dd3a0a919d 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -119,7 +119,7 @@ All public predicates, classes, modules and files should be documented with QLDo 1. Use a third-person verb phrase of the form ``Holds if `arg` has .`` 1. Avoid: - `/** Whether ... */` - - `/**" Relates ... */` + - `/** Relates ... */` - Question forms: - ``/** Is `x` a foo? */`` - ``/** Does `x` have a bar? */`` From 53640724d0daea1681a11b98b8994b65643c006f Mon Sep 17 00:00:00 2001 From: Mauro Baluda Date: Tue, 24 Mar 2026 16:35:55 +0100 Subject: [PATCH 5/7] Update .github/copilot-instructions.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index dd3a0a919d..888eb3b082 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -138,9 +138,9 @@ All public predicates, classes, modules and files should be documented with QLDo 1. Use a third-person verb phrase of the form `Gets (a|the) .` 1. Use "if any" if the item is usually unique but might be missing. For example -`Gets the body of this method, if any.` + `Gets the body of this method, if any.` 1. If the predicate has more complex behaviour, for example multiple arguments are conceptually "outputs", it can be described like a predicate without a result. For example -``Holds if `result` is a child of this expression.`` + ``Holds if `result` is a child of this expression.`` 1. Avoid: - `Get a ...` - `The ...` From 0c298598a26998b15b1b3940c2e4901adbe985f7 Mon Sep 17 00:00:00 2001 From: Mauro Baluda Date: Tue, 24 Mar 2026 16:36:07 +0100 Subject: [PATCH 6/7] Update .github/copilot-instructions.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 888eb3b082..47d15f7b6b 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -59,7 +59,7 @@ When reviewing tests, it is critical to: - Consider the "test coverage" of the query, are each of its logical statements effectively exercised individually, collectively? The test should neither be overly bloated nor under specified. - Consider the edge cases of the language itself, will the analysis work in non-trivial cases, are all relevant language concepts tested here? This doesn't need to be exhaustive, but it should be thoughfully thorough. -## Validating Query style +## Validating Query Style The following list describes the required style guides for a query that **must** be validated during the code-review process. From fb0af0797641016565935e28e40e58a92f599051 Mon Sep 17 00:00:00 2001 From: Mauro Baluda Date: Wed, 25 Mar 2026 10:21:55 +0100 Subject: [PATCH 7/7] Update CodeQL guidelines and QLDoc requirements Added guidelines for CodeQL source files and QLDoc requirements. --- .github/copilot-instructions.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 47d15f7b6b..47da64c8ab 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -75,6 +75,8 @@ A query **must** include: - Instead of `Call to banned function.` prefer `Call to banned function foobar.`. - Instead of `Return value from call is unused.` prefer `Return value from call to function [x] is unused.`, where `[x]` is a link to the function itself. - Do not try to explain the solution in the message; instead that should be provided in the help for the query. + +All lines in CodeQL source files and test files should be kept to a maximum of 100 characters. All public predicates, classes, modules and files should be documented with QLDoc. All QLDoc should follow the following QLDoc style guide: