Content-Length: 8231 | pFad | http://github.com/github/codeql/pull/19998.patch

thub.com From 5f7d746266e64e664264afb5c372ee9cf9b41362 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Tue, 8 Jul 2025 14:53:39 +0200 Subject: [PATCH 1/2] Java: Add query to detect non-case labels in switch statements --- .../java-code-quality-extended.qls.expected | 1 + .../java-code-quality.qls.expected | 1 + java/ql/src/Language Abuse/LabelInSwitch.md | 33 +++++++++++++++++++ java/ql/src/Language Abuse/LabelInSwitch.ql | 25 ++++++++++++++ .../LabelInSwitch/LabelInSwitch.expected | 2 ++ .../LabelInSwitch/LabelInSwitch.qlref | 2 ++ .../test/query-tests/LabelInSwitch/Test.java | 21 ++++++++++++ 7 files changed, 85 insertions(+) create mode 100644 java/ql/src/Language Abuse/LabelInSwitch.md create mode 100644 java/ql/src/Language Abuse/LabelInSwitch.ql create mode 100644 java/ql/test/query-tests/LabelInSwitch/LabelInSwitch.expected create mode 100644 java/ql/test/query-tests/LabelInSwitch/LabelInSwitch.qlref create mode 100644 java/ql/test/query-tests/LabelInSwitch/Test.java diff --git a/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected b/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected index 5c82bd5e3498..7de884f0081c 100644 --- a/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected +++ b/java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected @@ -6,6 +6,7 @@ ql/java/ql/src/Compatibility/JDK9/UnderscoreIdentifier.ql ql/java/ql/src/DeadCode/UselessParameter.ql ql/java/ql/src/Language Abuse/EmptyMethod.ql ql/java/ql/src/Language Abuse/IterableIterator.ql +ql/java/ql/src/Language Abuse/LabelInSwitch.ql ql/java/ql/src/Language Abuse/TypeVariableHidesType.ql ql/java/ql/src/Language Abuse/UselessNullCheck.ql ql/java/ql/src/Language Abuse/UselessTypeTest.ql diff --git a/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected b/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected index e558cf3ffc48..c85479a29b09 100644 --- a/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected +++ b/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected @@ -5,6 +5,7 @@ ql/java/ql/src/Compatibility/JDK9/JdkInternalAccess.ql ql/java/ql/src/Compatibility/JDK9/UnderscoreIdentifier.ql ql/java/ql/src/DeadCode/UselessParameter.ql ql/java/ql/src/Language Abuse/IterableIterator.ql +ql/java/ql/src/Language Abuse/LabelInSwitch.ql ql/java/ql/src/Language Abuse/UselessNullCheck.ql ql/java/ql/src/Language Abuse/UselessTypeTest.ql ql/java/ql/src/Language Abuse/WrappedIterator.ql diff --git a/java/ql/src/Language Abuse/LabelInSwitch.md b/java/ql/src/Language Abuse/LabelInSwitch.md new file mode 100644 index 000000000000..d8b39e08d151 --- /dev/null +++ b/java/ql/src/Language Abuse/LabelInSwitch.md @@ -0,0 +1,33 @@ +## Overview + +Java allows to freely mix `case` labels and ordinary statement labels in the body of +a `switch` statement. However, this is confusing to read and may be the result of a typo. + +## Recommendation + +Examine the non-`case` labels to see whether they were meant to be `case` labels. If not, consider placing the non-`case` label headed code into a function, and use a function call inline in the `switch` body instead. + +## Example + +```java +public class Test { + void test_noncase_label_in_switch(int p) { + switch (p) { + case 1: // Compliant + case2: // Non-compliant, likely a typo + break; + case 3: + notcaselabel: // Non-compliant, confusing to read + for (;;) { + break notcaselabel; + } + } + } +} +``` + +In the example, `case2` is most likely a typo and should be fixed. For the intensional `notcaselabel`, placing the labelled code into a function and then calling that function is more readable. + +## References + +Similar to the JS CodeQL query - [js/label-in-switch](https://codeql.github.com/codeql-query-help/javascript/js-label-in-switch/). diff --git a/java/ql/src/Language Abuse/LabelInSwitch.ql b/java/ql/src/Language Abuse/LabelInSwitch.ql new file mode 100644 index 000000000000..56245df80331 --- /dev/null +++ b/java/ql/src/Language Abuse/LabelInSwitch.ql @@ -0,0 +1,25 @@ +/** + * @id java/label-in-switch + * @name Non-case label in switch statement + * @description A non-case label appearing in a switch statement + * is confusing to read or may even indicate a bug. + * @previous-id java/label-in-case + * @kind problem + * @precision very-high + * @problem.severity recommendation + * @tags quality + * maintainability + * readability + */ + +import java + +from LabeledStmt l, SwitchStmt s, string alert +where + l = s.getAStmt+() and + if exists(JumpStmt jump | jump.getTargetLabel() = l) + then alert = "Confusing non-case label in switch statement." + else + alert = + "Possibly erroneous non-case label in switch statement. The case keyword might be missing." +select l, alert diff --git a/java/ql/test/query-tests/LabelInSwitch/LabelInSwitch.expected b/java/ql/test/query-tests/LabelInSwitch/LabelInSwitch.expected new file mode 100644 index 000000000000..40b1bf38242d --- /dev/null +++ b/java/ql/test/query-tests/LabelInSwitch/LabelInSwitch.expected @@ -0,0 +1,2 @@ +| Test.java:14:17:14:31 | : ... | Possibly erroneous non-case label in switch statement. The case keyword might be missing. | +| Test.java:17:17:17:39 | : ... | Confusing non-case label in switch statement. | diff --git a/java/ql/test/query-tests/LabelInSwitch/LabelInSwitch.qlref b/java/ql/test/query-tests/LabelInSwitch/LabelInSwitch.qlref new file mode 100644 index 000000000000..99d3cd52e915 --- /dev/null +++ b/java/ql/test/query-tests/LabelInSwitch/LabelInSwitch.qlref @@ -0,0 +1,2 @@ +query: Language Abuse/LabelInSwitch.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/java/ql/test/query-tests/LabelInSwitch/Test.java b/java/ql/test/query-tests/LabelInSwitch/Test.java new file mode 100644 index 000000000000..de4d22889660 --- /dev/null +++ b/java/ql/test/query-tests/LabelInSwitch/Test.java @@ -0,0 +1,21 @@ +public class Test { + void test_case_label_only_in_switch(int p) { + switch (p) { + case 1: + case 2: + break; + } + notcaselabelnotinswitch: for (;;) {} + } + + void test_noncase_label_in_switch(int p) { + switch (p) { + case 1: + notcaselabel1:; // $ Alert | Possibly erroneous non-case label in switch statement. The case keyword might be missing. + break; + case 2: + notcaselabel2: for (;;) { break notcaselabel2; } // $ Alert | Confusing non-case label in switch statement. + break; + } + } +} From 5edb60ea04eba92465377d78d0f766ffe7cd13b5 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 10 Jul 2025 09:43:08 +0200 Subject: [PATCH 2/2] Improve query documentation --- java/ql/src/Language Abuse/LabelInSwitch.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/ql/src/Language Abuse/LabelInSwitch.md b/java/ql/src/Language Abuse/LabelInSwitch.md index d8b39e08d151..e871e68abebf 100644 --- a/java/ql/src/Language Abuse/LabelInSwitch.md +++ b/java/ql/src/Language Abuse/LabelInSwitch.md @@ -26,8 +26,8 @@ public class Test { } ``` -In the example, `case2` is most likely a typo and should be fixed. For the intensional `notcaselabel`, placing the labelled code into a function and then calling that function is more readable. +In the example, `case2` is most likely a typo and should be fixed. For the intentional `notcaselabel`, placing the labelled code into a function and then calling that function is more readable. ## References -Similar to the JS CodeQL query - [js/label-in-switch](https://codeql.github.com/codeql-query-help/javascript/js-label-in-switch/). +CodeQL query help for JavaScript and TypeScript - [Non-case label in switch statement](https://codeql.github.com/codeql-query-help/javascript/js-label-in-switch/).

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!