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..e871e68abebf --- /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 intentional `notcaselabel`, placing the labelled code into a function and then calling that function is more readable. + +## References + +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/). 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 |