From a2ff045a7a6723faf35ab0d97977ae4e552c9b85 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 13 Jun 2025 16:53:47 +0100 Subject: [PATCH 1/3] Update tags for high precision quality queries --- go/ql/src/InconsistentCode/ConstantLengthComparison.ql | 5 ++++- .../src/InconsistentCode/InconsistentLoopOrientation.ql | 4 +++- go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql | 5 ++--- go/ql/src/InconsistentCode/MissingErrorCheck.ql | 8 ++++---- go/ql/src/InconsistentCode/MistypedExponentiation.ql | 5 ++++- .../src/InconsistentCode/UnhandledCloseWritableHandle.ql | 9 ++++----- .../InconsistentCode/WhitespaceContradictsPrecedence.ql | 5 +++-- go/ql/src/InconsistentCode/WrappedErrorAlwaysNil.ql | 7 +++---- go/ql/src/RedundantCode/CompareIdenticalValues.ql | 4 +++- go/ql/src/RedundantCode/DeadStoreOfField.ql | 4 +++- go/ql/src/RedundantCode/DeadStoreOfLocal.ql | 4 +++- go/ql/src/RedundantCode/DuplicateBranches.ql | 3 ++- go/ql/src/RedundantCode/DuplicateCondition.ql | 3 ++- go/ql/src/RedundantCode/DuplicateSwitchCase.ql | 3 ++- go/ql/src/RedundantCode/ExprHasNoEffect.ql | 3 ++- go/ql/src/RedundantCode/ImpossibleInterfaceNilCheck.ql | 5 ++++- go/ql/src/RedundantCode/NegativeLengthCheck.ql | 6 ++++-- go/ql/src/RedundantCode/RedundantExpr.ql | 4 +++- go/ql/src/RedundantCode/RedundantRecover.ql | 5 +++-- go/ql/src/RedundantCode/SelfAssignment.ql | 4 +++- go/ql/src/RedundantCode/ShiftOutOfRange.ql | 4 +++- go/ql/src/RedundantCode/UnreachableStatement.ql | 3 ++- 22 files changed, 66 insertions(+), 37 deletions(-) diff --git a/go/ql/src/InconsistentCode/ConstantLengthComparison.ql b/go/ql/src/InconsistentCode/ConstantLengthComparison.ql index c60e093650fc..d0bcec7a89cb 100644 --- a/go/ql/src/InconsistentCode/ConstantLengthComparison.ql +++ b/go/ql/src/InconsistentCode/ConstantLengthComparison.ql @@ -5,7 +5,10 @@ * @kind problem * @problem.severity warning * @id go/constant-length-comparison - * @tags correctness + * @tags quality + * reliability + * correctness + * external/cwe/cwe-129 * @precision high */ diff --git a/go/ql/src/InconsistentCode/InconsistentLoopOrientation.ql b/go/ql/src/InconsistentCode/InconsistentLoopOrientation.ql index fa5051ed5c72..c200ea010b22 100644 --- a/go/ql/src/InconsistentCode/InconsistentLoopOrientation.ql +++ b/go/ql/src/InconsistentCode/InconsistentLoopOrientation.ql @@ -8,7 +8,9 @@ * @kind problem * @problem.severity error * @id go/inconsistent-loop-direction - * @tags correctness + * @tags quality + * reliability + * correctness * external/cwe/cwe-835 * @precision very-high */ diff --git a/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql b/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql index 436eb8a8fe51..176e34bc9bbb 100644 --- a/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql +++ b/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql @@ -5,10 +5,9 @@ * @kind problem * @problem.severity error * @id go/index-out-of-bounds - * @tags reliability + * @tags quality + * reliability * correctness - * logic - * quality * external/cwe/cwe-193 * @precision high */ diff --git a/go/ql/src/InconsistentCode/MissingErrorCheck.ql b/go/ql/src/InconsistentCode/MissingErrorCheck.ql index 9acd7e136022..8e277c6ae747 100644 --- a/go/ql/src/InconsistentCode/MissingErrorCheck.ql +++ b/go/ql/src/InconsistentCode/MissingErrorCheck.ql @@ -5,10 +5,10 @@ * @kind problem * @problem.severity warning * @id go/missing-error-check - * @tags reliability - * correctness - * logic - * quality + * @tags quality + * reliability + * error-handling + * external/cwe/cwe-252 * @precision high */ diff --git a/go/ql/src/InconsistentCode/MistypedExponentiation.ql b/go/ql/src/InconsistentCode/MistypedExponentiation.ql index 989f536f3a55..b445a713ce6f 100644 --- a/go/ql/src/InconsistentCode/MistypedExponentiation.ql +++ b/go/ql/src/InconsistentCode/MistypedExponentiation.ql @@ -4,7 +4,10 @@ * @kind problem * @problem.severity warning * @id go/mistyped-exponentiation - * @tags correctness + * @tags quality + * reliability + * correctness + * external/cwe/cwe-480 * @precision high */ diff --git a/go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql b/go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql index 051e4644cc7c..d3210c48011e 100644 --- a/go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql +++ b/go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql @@ -7,11 +7,10 @@ * @problem.severity warning * @precision high * @id go/unhandled-writable-file-close - * @tags maintainability - * correctness - * call - * defer - * quality + * @tags quality + * reliability + * error-handling + * external/cwe/cwe-252 */ import go diff --git a/go/ql/src/InconsistentCode/WhitespaceContradictsPrecedence.ql b/go/ql/src/InconsistentCode/WhitespaceContradictsPrecedence.ql index f2303cf08a61..7e2846cf6b2f 100644 --- a/go/ql/src/InconsistentCode/WhitespaceContradictsPrecedence.ql +++ b/go/ql/src/InconsistentCode/WhitespaceContradictsPrecedence.ql @@ -1,11 +1,12 @@ /** * @name Whitespace contradicts operator precedence * @description Nested expressions where the formatting contradicts the grouping enforced by operator precedence - * are difficult to read and may even indicate a bug. + * are difficult to read and may indicate a bug. * @kind problem * @problem.severity warning * @id go/whitespace-contradicts-precedence - * @tags maintainability + * @tags quality + * reliability * correctness * external/cwe/cwe-783 * @precision very-high diff --git a/go/ql/src/InconsistentCode/WrappedErrorAlwaysNil.ql b/go/ql/src/InconsistentCode/WrappedErrorAlwaysNil.ql index fac236c7f036..8c6fcab7d282 100644 --- a/go/ql/src/InconsistentCode/WrappedErrorAlwaysNil.ql +++ b/go/ql/src/InconsistentCode/WrappedErrorAlwaysNil.ql @@ -4,10 +4,9 @@ * @kind problem * @problem.severity warning * @id go/unexpected-nil-value - * @tags reliability - * correctness - * logic - * quality + * @tags quality + * reliability + * error-handling * @precision high */ diff --git a/go/ql/src/RedundantCode/CompareIdenticalValues.ql b/go/ql/src/RedundantCode/CompareIdenticalValues.ql index cd4d9d6dbfa2..43be53387351 100644 --- a/go/ql/src/RedundantCode/CompareIdenticalValues.ql +++ b/go/ql/src/RedundantCode/CompareIdenticalValues.ql @@ -5,7 +5,9 @@ * @kind problem * @problem.severity warning * @id go/comparison-of-identical-expressions - * @tags correctness + * @tags quality + * reliability + * correctness * external/cwe/cwe-570 * external/cwe/cwe-571 * @precision very-high diff --git a/go/ql/src/RedundantCode/DeadStoreOfField.ql b/go/ql/src/RedundantCode/DeadStoreOfField.ql index edc1d62cb00d..be3a77d3ac78 100644 --- a/go/ql/src/RedundantCode/DeadStoreOfField.ql +++ b/go/ql/src/RedundantCode/DeadStoreOfField.ql @@ -4,7 +4,9 @@ * @kind problem * @problem.severity warning * @id go/useless-assignment-to-field - * @tags maintainability + * @tags quality + * maintainability + * useless-code * external/cwe/cwe-563 * @precision very-high */ diff --git a/go/ql/src/RedundantCode/DeadStoreOfLocal.ql b/go/ql/src/RedundantCode/DeadStoreOfLocal.ql index d6e7351a76d7..3e3642f92db1 100644 --- a/go/ql/src/RedundantCode/DeadStoreOfLocal.ql +++ b/go/ql/src/RedundantCode/DeadStoreOfLocal.ql @@ -5,7 +5,9 @@ * @kind problem * @problem.severity warning * @id go/useless-assignment-to-local - * @tags maintainability + * @tags quality + * maintainability + * useless-code * external/cwe/cwe-563 * @precision very-high */ diff --git a/go/ql/src/RedundantCode/DuplicateBranches.ql b/go/ql/src/RedundantCode/DuplicateBranches.ql index c6aa7523e28e..589aa55246cd 100644 --- a/go/ql/src/RedundantCode/DuplicateBranches.ql +++ b/go/ql/src/RedundantCode/DuplicateBranches.ql @@ -6,7 +6,8 @@ * @problem.severity warning * @precision very-high * @id go/duplicate-branches - * @tags maintainability + * @tags quality + * reliability * correctness * external/cwe/cwe-561 */ diff --git a/go/ql/src/RedundantCode/DuplicateCondition.ql b/go/ql/src/RedundantCode/DuplicateCondition.ql index fb031044e47d..e0ea97980438 100644 --- a/go/ql/src/RedundantCode/DuplicateCondition.ql +++ b/go/ql/src/RedundantCode/DuplicateCondition.ql @@ -5,7 +5,8 @@ * @kind problem * @problem.severity error * @id go/duplicate-condition - * @tags maintainability + * @tags quality + * reliability * correctness * external/cwe/cwe-561 * @precision very-high diff --git a/go/ql/src/RedundantCode/DuplicateSwitchCase.ql b/go/ql/src/RedundantCode/DuplicateSwitchCase.ql index 3096f3bef942..b10ed6a794c0 100644 --- a/go/ql/src/RedundantCode/DuplicateSwitchCase.ql +++ b/go/ql/src/RedundantCode/DuplicateSwitchCase.ql @@ -5,7 +5,8 @@ * @kind problem * @problem.severity error * @id go/duplicate-switch-case - * @tags maintainability + * @tags quality + * reliability * correctness * external/cwe/cwe-561 * @precision very-high diff --git a/go/ql/src/RedundantCode/ExprHasNoEffect.ql b/go/ql/src/RedundantCode/ExprHasNoEffect.ql index 5b722cfdbf7b..ba879054faf5 100644 --- a/go/ql/src/RedundantCode/ExprHasNoEffect.ql +++ b/go/ql/src/RedundantCode/ExprHasNoEffect.ql @@ -5,7 +5,8 @@ * @kind problem * @problem.severity warning * @id go/useless-expression - * @tags maintainability + * @tags quality + * reliability * correctness * external/cwe/cwe-480 * external/cwe/cwe-561 diff --git a/go/ql/src/RedundantCode/ImpossibleInterfaceNilCheck.ql b/go/ql/src/RedundantCode/ImpossibleInterfaceNilCheck.ql index 8fe764ee88fa..c5aeb287358a 100644 --- a/go/ql/src/RedundantCode/ImpossibleInterfaceNilCheck.ql +++ b/go/ql/src/RedundantCode/ImpossibleInterfaceNilCheck.ql @@ -4,7 +4,10 @@ * @kind problem * @problem.severity warning * @id go/impossible-interface-nil-check - * @tags correctness + * @tags quality + * reliability + * correctness + * external/cwe/cwe-570 * @precision high */ diff --git a/go/ql/src/RedundantCode/NegativeLengthCheck.ql b/go/ql/src/RedundantCode/NegativeLengthCheck.ql index adac6fe78d97..443ec37154fd 100644 --- a/go/ql/src/RedundantCode/NegativeLengthCheck.ql +++ b/go/ql/src/RedundantCode/NegativeLengthCheck.ql @@ -8,8 +8,10 @@ * @problem.severity warning * @precision very-high * @id go/negative-length-check - * @tags correctness - * quality + * @tags quality + * reliability + * correctness + * external/cwe/cwe-571 */ import go diff --git a/go/ql/src/RedundantCode/RedundantExpr.ql b/go/ql/src/RedundantCode/RedundantExpr.ql index c4b0ea912f53..49cc06125e34 100644 --- a/go/ql/src/RedundantCode/RedundantExpr.ql +++ b/go/ql/src/RedundantCode/RedundantExpr.ql @@ -6,7 +6,9 @@ * @kind problem * @problem.severity warning * @id go/redundant-operation - * @tags correctness + * @tags quality + * reliability + * correctness * external/cwe/cwe-480 * external/cwe/cwe-561 * @precision very-high diff --git a/go/ql/src/RedundantCode/RedundantRecover.ql b/go/ql/src/RedundantCode/RedundantRecover.ql index 08fc06727e5c..40d415257e0a 100644 --- a/go/ql/src/RedundantCode/RedundantRecover.ql +++ b/go/ql/src/RedundantCode/RedundantRecover.ql @@ -6,9 +6,10 @@ * @kind problem * @problem.severity warning * @id go/redundant-recover - * @tags maintainability + * @tags quality + * reliability * correctness - * quality + * external/cwe/cwe-248 * @precision high */ diff --git a/go/ql/src/RedundantCode/SelfAssignment.ql b/go/ql/src/RedundantCode/SelfAssignment.ql index ca1c96147515..2b4701f2f7d1 100644 --- a/go/ql/src/RedundantCode/SelfAssignment.ql +++ b/go/ql/src/RedundantCode/SelfAssignment.ql @@ -4,7 +4,9 @@ * @kind problem * @problem.severity warning * @id go/redundant-assignment - * @tags correctness + * @tags quality + * reliability + * correctness * external/cwe/cwe-480 * external/cwe/cwe-561 * @precision high diff --git a/go/ql/src/RedundantCode/ShiftOutOfRange.ql b/go/ql/src/RedundantCode/ShiftOutOfRange.ql index 275cae3bbebd..942a9cdbdab0 100644 --- a/go/ql/src/RedundantCode/ShiftOutOfRange.ql +++ b/go/ql/src/RedundantCode/ShiftOutOfRange.ql @@ -6,7 +6,9 @@ * @problem.severity warning * @id go/shift-out-of-range * @precision very-high - * @tags correctness + * @tags quality + * reliability + * correctness * external/cwe/cwe-197 */ diff --git a/go/ql/src/RedundantCode/UnreachableStatement.ql b/go/ql/src/RedundantCode/UnreachableStatement.ql index e67b3c4915f1..c177705a86f4 100644 --- a/go/ql/src/RedundantCode/UnreachableStatement.ql +++ b/go/ql/src/RedundantCode/UnreachableStatement.ql @@ -5,7 +5,8 @@ * @kind problem * @problem.severity warning * @id go/unreachable-statement - * @tags maintainability + * @tags quality + * reliability * correctness * external/cwe/cwe-561 * @precision very-high From a26610a05cb284b17e98ce1bef478201f32afdae Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 13 Jun 2025 17:07:21 +0100 Subject: [PATCH 2/3] Add change note --- .../2025-06-13-add-tags-to-quality-queries.md | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 go/ql/src/change-notes/2025-06-13-add-tags-to-quality-queries.md diff --git a/go/ql/src/change-notes/2025-06-13-add-tags-to-quality-queries.md b/go/ql/src/change-notes/2025-06-13-add-tags-to-quality-queries.md new file mode 100644 index 000000000000..9233cb05e809 --- /dev/null +++ b/go/ql/src/change-notes/2025-06-13-add-tags-to-quality-queries.md @@ -0,0 +1,22 @@ +--- +category: queryMetadata +--- +* The tag `quality` has been added to multiple Go quality queries for consistency. They have all been given a tag for one of the two top-level categories `reliability` or `maintainability`, and a tag for a sub-category. See [Query file metadata and alert message style guide](https://github.com/github/codeql/blob/main/docs/query-metadata-style-guide.md#quality-query-sub-category-tags) for more information about these categories. +* The tag `external/cwe/cwe-129` has been added to `go/constant-length-comparison`. +* The tag `external/cwe/cwe-193` has been added to `go/index-out-of-bounds`. +* The tag `external/cwe/cwe-197` has been added to `go/shift-out-of-range`. +* The tag `external/cwe/cwe-248` has been added to `go/redundant-recover`. +* The tag `external/cwe/cwe-252` has been added to `go/missing-error-check` and `go/unhandled-writable-file-close`. +* The tag `external/cwe/cwe-480` has been added to `go/mistyped-exponentiation`. +* The tag `external/cwe/cwe-570` has been added to `go/impossible-interface-nil-check` and `go/comparison-of-identical-expressions`. +* The tag `external/cwe/cwe-571` has been added to `go/negative-length-check` and `go/comparison-of-identical-expressions`. +* The tag `external/cwe/cwe-783` has been added to `go/whitespace-contradicts-precedence`. +* The tag `external/cwe/cwe-835` has been added to `go/inconsistent-loop-direction`. +* The tag `error-handling` has been added to `go/missing-error-check`, `go/unhandled-writable-file-close`, and `go/unexpected-nil-value`. +* The tag `useless-code` has been added to `go/useless-assignment-to-field`, `go/useless-assignment-to-local`, `go/useless-expression`, and `go/unreachable-statement`. +* The tag `logic` has been removed from `go/index-out-of-bounds` and `go/unexpected-nil-value`. +* The tags `call` and `defer` have been removed from `go/unhandled-writable-file-close`. +* The tags `correctness` and `quality` have been reordered in `go/missing-error-check` and `go/unhandled-writable-file-close`. +* The tag `maintainability` has been changed to `reliability` for `go/unhandled-writable-file-close`. +* The tag order has been standardized to have `quality` first, followed by the top-level category (`reliability` or `maintainability`), then sub-category tags, and finally CWE tags. +* The description text has been updated in `go/whitespace-contradicts-precedence` to change "may even indicate" to "may indicate". From ebd917600d9672c3b712eda6edb9c83c90492c12 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Mon, 16 Jun 2025 10:04:32 +0100 Subject: [PATCH 3/3] Update quality suite integration test --- .../go-code-quality-extended.qls.expected | 16 ++++++++++++++++ .../query-suite/go-code-quality.qls.expected | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/go/ql/integration-tests/query-suite/go-code-quality-extended.qls.expected b/go/ql/integration-tests/query-suite/go-code-quality-extended.qls.expected index 236c285ece05..7eb72f2abc21 100644 --- a/go/ql/integration-tests/query-suite/go-code-quality-extended.qls.expected +++ b/go/ql/integration-tests/query-suite/go-code-quality-extended.qls.expected @@ -1,6 +1,22 @@ +ql/go/ql/src/InconsistentCode/ConstantLengthComparison.ql +ql/go/ql/src/InconsistentCode/InconsistentLoopOrientation.ql ql/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql ql/go/ql/src/InconsistentCode/MissingErrorCheck.ql +ql/go/ql/src/InconsistentCode/MistypedExponentiation.ql ql/go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql +ql/go/ql/src/InconsistentCode/WhitespaceContradictsPrecedence.ql ql/go/ql/src/InconsistentCode/WrappedErrorAlwaysNil.ql +ql/go/ql/src/RedundantCode/CompareIdenticalValues.ql +ql/go/ql/src/RedundantCode/DeadStoreOfField.ql +ql/go/ql/src/RedundantCode/DeadStoreOfLocal.ql +ql/go/ql/src/RedundantCode/DuplicateBranches.ql +ql/go/ql/src/RedundantCode/DuplicateCondition.ql +ql/go/ql/src/RedundantCode/DuplicateSwitchCase.ql +ql/go/ql/src/RedundantCode/ExprHasNoEffect.ql +ql/go/ql/src/RedundantCode/ImpossibleInterfaceNilCheck.ql ql/go/ql/src/RedundantCode/NegativeLengthCheck.ql +ql/go/ql/src/RedundantCode/RedundantExpr.ql ql/go/ql/src/RedundantCode/RedundantRecover.ql +ql/go/ql/src/RedundantCode/SelfAssignment.ql +ql/go/ql/src/RedundantCode/ShiftOutOfRange.ql +ql/go/ql/src/RedundantCode/UnreachableStatement.ql diff --git a/go/ql/integration-tests/query-suite/go-code-quality.qls.expected b/go/ql/integration-tests/query-suite/go-code-quality.qls.expected index 236c285ece05..7eb72f2abc21 100644 --- a/go/ql/integration-tests/query-suite/go-code-quality.qls.expected +++ b/go/ql/integration-tests/query-suite/go-code-quality.qls.expected @@ -1,6 +1,22 @@ +ql/go/ql/src/InconsistentCode/ConstantLengthComparison.ql +ql/go/ql/src/InconsistentCode/InconsistentLoopOrientation.ql ql/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql ql/go/ql/src/InconsistentCode/MissingErrorCheck.ql +ql/go/ql/src/InconsistentCode/MistypedExponentiation.ql ql/go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql +ql/go/ql/src/InconsistentCode/WhitespaceContradictsPrecedence.ql ql/go/ql/src/InconsistentCode/WrappedErrorAlwaysNil.ql +ql/go/ql/src/RedundantCode/CompareIdenticalValues.ql +ql/go/ql/src/RedundantCode/DeadStoreOfField.ql +ql/go/ql/src/RedundantCode/DeadStoreOfLocal.ql +ql/go/ql/src/RedundantCode/DuplicateBranches.ql +ql/go/ql/src/RedundantCode/DuplicateCondition.ql +ql/go/ql/src/RedundantCode/DuplicateSwitchCase.ql +ql/go/ql/src/RedundantCode/ExprHasNoEffect.ql +ql/go/ql/src/RedundantCode/ImpossibleInterfaceNilCheck.ql ql/go/ql/src/RedundantCode/NegativeLengthCheck.ql +ql/go/ql/src/RedundantCode/RedundantExpr.ql ql/go/ql/src/RedundantCode/RedundantRecover.ql +ql/go/ql/src/RedundantCode/SelfAssignment.ql +ql/go/ql/src/RedundantCode/ShiftOutOfRange.ql +ql/go/ql/src/RedundantCode/UnreachableStatement.ql