Skip to content
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 53 additions & 23 deletions docs/query-metadata-style-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,29 +121,11 @@ Alert queries (`@kind problem` or `path-problem`) support two further properties

## Query tags `@tags`

The `@tags` property is used to define categories that the query relates to. Each alert query should belong to one (or more, if necessary) of the following four top-level categories:
The `@tags` property is used to define the high level category of problem that the query relates to. Each alert query should belong to one of the following two top-level categories, with additional sub-categories:

* `@tags correctness`–for queries that detect incorrect program behavior.
* `@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.
### High level category `@tags`
* `@tags security`–for queries that detect security weaknesses. See below for further information.

There are also more specific `@tags` that can be added. See, the following pages for examples of the low-level tags:

* [C/C++ queries](https://codeql.github.com/codeql-query-help/cpp/)
* [C# queries](https://codeql.github.com/codeql-query-help/csharp/)
* [Go queries](https://codeql.github.com/codeql-query-help/go/)
* [Java queries](https://codeql.github.com/codeql-query-help/java/)
* [JavaScript queries](https://codeql.github.com/codeql-query-help/javascript/)
* [Python queries](https://codeql.github.com/codeql-query-help/python/)

Metric queries (`@kind metric`) may have the `summary` tag. If SARIF output is used, the results of these queries can be found at `run[].properties.metricResults`.

If necessary, you can also define your own low-level tags to categorize the queries specific to your project or organization. When creating your own tags, you should:

* Use all lower-case letters, including for acronyms and proper nouns, with no spaces. All characters apart from * and @ are accepted.
* Use a forward slash / to indicate a hierarchical relationship between tags if necessary. For example, a query with tag `foo/bar` is also interpreted as also having tag `foo`, but not `bar`.
* Use a single-word `@tags` name. Multiple words, separated with hyphens, can be used for clarity if necessary.
* `@tags quality`–for queries that detect code quality issues. See below for further information.

#### Security query `@tags`

Expand All @@ -160,13 +142,61 @@ When you tag a query like this, the associated CWE pages from [MITRE.org](https:
> [!NOTE]
> The automatic addition of CWE reference links works only if the qhelp file already contains a `<references>` section.

#### Metric/summary `@tags`
#### Quality query sub-category `@tags`

Code Scanning may use tags to identify queries with specific meanings across languages. Currently, there is only one such tag: `lines-of-code`. The sum of the results for queries with this tag that return a single number column ([example for JavaScript](https://github.com/github/codeql/blob/c47d680d65f09a851e41d4edad58ffa7486b5431/java/ql/src/Metrics/Summaries/LinesOfCode.ql)) is interpreted by Code Scanning as the lines of code under the source root present in the database. Each language should have exactly one query of this form.
* `@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.

Software quality doesn't have as universally-agreed categorization method as security issues like CWE, so we will do our own categorization instead of using tags like CWE.

We'll use two "top-level" categories of quality queries, with sub-categories beneath:

* `@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.
* `@tags useless-code`-for queries that detect functions that are never used and other instances of unused code
* `@tags complexity`-for queries that detect patterns in the code that lead to unnecesary complexity such as unclear control flow, or high cyclomatic complexity
Comment thread
jonjanego marked this conversation as resolved.


* `@tags reliability`–for queries that detect issues that affect whether the code will perform as expected during execution.
* `@tags correctness`–for queries that detect incorrect program behavior or couse result in unintended outcomes.
* `@tags performance`-for queries that detect code that could impact performance through inefficient algorithms, unnecessary computation, etc
* `@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
Comment thread
jonjanego marked this conversation as resolved.


There are also more specific `@tags` that can be added. See, the following pages for examples of the low-level tags:

* [C/C++ queries](https://codeql.github.com/codeql-query-help/cpp/)
* [C# queries](https://codeql.github.com/codeql-query-help/csharp/)
* [Go queries](https://codeql.github.com/codeql-query-help/go/)
* [Java queries](https://codeql.github.com/codeql-query-help/java/)
* [JavaScript queries](https://codeql.github.com/codeql-query-help/javascript/)
* [Python queries](https://codeql.github.com/codeql-query-help/python/)

> [!NOTE]
> There is a limit of 10 tags per query!

### Severities

Maintainers are expected to add a `@security-severity` tag to security relevant queries that will be run on Code Scanning. There is a documented internal process for generating these `@security-severity` values.

TODO: should we have a severity value for quality queries?
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a non-security problem.severity field, which can be error/warning/recommendation/none.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A while ago we intended severity to be the new spelling of problem.severity (and @kind alert to replace @kind problem). This document is even older than that and we never got around to switching existing queries to the new spelling but should we change this to those or do we want to go back on those changes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @alexet - ah... i did not know that history. I'm open to whatever the team thinks is best. @yo-h and @adityasharad , do you have opinions of what to do here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also did not know that history, so I have been adding queries with @problem.severity and @kind {path-}problem for the past 2+ years. 😅 It looks like those changes were made in 2021 (https://github.com/github/semmle-code/pull/39133, https://github.com/github/semmle-code/pull/39132), but when I joined in 2022, I was directed to this metadata style guide and metadata-for-codeql-queries which both still only listed problem.severity and {path-}problem as options.

should we change this to those or do we want to go back on those changes

Searching in the codeql repo shows only a handful of queries using @severity (mostly model-generator related queries) and no queries using @kind path-alert or @kind alert. Since it's been four years since these changes were made, and they haven't been used much, maybe it makes sense to go back on them? Or at least not prioritize them now?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the CLI recognises those variants, but we never changed the guidance and existing practices :) I am fine keeping the aliases around so we can change over later to the newer names, but no need to rewrite any existing code.


### Metric/summary `@tags`

Code Scanning may use tags to identify queries with specific meanings across languages. Currently, there is only one such tag: `lines-of-code`. The sum of the results for queries with this tag that return a single number column ([example for JavaScript](https://github.com/github/codeql/blob/c47d680d65f09a851e41d4edad58ffa7486b5431/java/ql/src/Metrics/Summaries/LinesOfCode.ql)) is interpreted by Code Scanning as the lines of code under the source root present in the database. Each language should have exactly one query of this form.

Metric queries (`@kind metric`) may have the `summary` tag. If SARIF output is used, the results of these queries can be found at `run[].properties.metricResults`.


### Customizing tags

If necessary, you can also define your own low-level tags to categorize the queries specific to your project or organization, but if possible, please try to follow the above standards (or propose changes to this style guide). When creating your own tags, you should:

* Use all lower-case letters, including for acronyms and proper nouns, with no spaces. All characters apart from * and @ are accepted.
* Use a forward slash / to indicate a hierarchical relationship between tags if necessary. For example, a query with tag `foo/bar` is also interpreted as also having tag `foo`, but not `bar`.
* Use a single-word `@tags` name. Multiple words, separated with hyphens, can be used for clarity if necessary.

## QL area

### Alert messages
Expand Down