[java] Fix #2928: New Rules UnusedLabel and LabeledStatement

[UncleOwen]

Describe the PR

Two new rules as discussed below and summarized on #2928.

Related issues

• Fix [java] New rules about labeled statements #2928

Ready?

• Added unit tests for fixed bug/feature
• Passing all unit tests
• Complete build ./mvnw clean verify passes (checked automatically by github actions)
• Added (in-code) documentation (if needed)

[pmd-actions-helper]

Documentation Preview

Compared to main:
This changeset changes 6 violations,
introduces 91 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 0 errors and 0 configuration errors.

Regression Tester Report

(comment created at 2025-10-29 13:42:00+00:00 for 9ca1336)

[adangel]

Thanks for the PR!

I've read through the (unfortunately very, very messy) comments in #2928. And it actually suggests two or three rules:

1. Detecting break/continue statements that use labels -> goal: avoid
2. Detecting labels that are unused -> goal: remove
3. Detecting any labels (used / unused) -> goal: avoid

For the rule names, maybe we can find something that does "name the underlying problem" (https://docs.pmd-code.org/latest/pmd_devdocs_major_rule_guidelines.html).

For the first rule (this PR), maybe instead of "AvoidLoopLabels" simply just "LabelInLoops". In the rule description, we can then say "Using labels in loops should be avoided as it makes the control flow difficult to understand...". I know, we have a couple of rules that start with an order ("Avoid...", "Use...", "OverrideBoth...").
What we actually detect with this rule are break/continue statements with labels. So, the rule should be named maybe "BreakOrContinueWithLabel"?

The second rule could be named "UnusedLabel". An XPath expression, that detect such unused labels, looks like this:

for $label in //LabeledStatement
  return if (not($label/
      (ancestor::ConstructorDeclaration|ancestor::MethodDeclaration|ancestor::Initializer)/
      Block//(BreakStatement|ContinueStatement)[@Label = $label/@Label])) then $label else ()

The third rule could be named "LabeledStatement" - like the AST node. The XPath is simply //LabeledStatement.

The second and third rule could be combined with a property (unusedOnly=true/false):

if ($unusedOnly) then
  for $label in //LabeledStatement
    return if (not($label/
        (ancestor::ConstructorDeclaration|ancestor::MethodDeclaration|ancestor::Initializer)/
        Block//(BreakStatement|ContinueStatement)[@Label = $label/@Label])) then $label else ()
else
  //LabeledStatement

However, I like the simple name "UnusedLabel".

What do you (and others) think about the names?
Once we agree on the names and which rules to implement, we can update the original issue to reflect our latest findings.

Btw. I'm going to rebase your PR/branch...

[UncleOwen]

Slight twist on your "LabelInLoops" suggestion: "LabeledLoops". And maybe that is, what the rule should be looking for?

[oowekyala]

I think there is some overlap between the three rules you propose.

• UnusedLabel is really uncontroversial and could be included in the quickstart ruleset. Nobody wants unused code constructs in their code.
• If we can assume that, then the other rules will only ever have to deal with used labels, i.e., labels that have at least a break or continue that target them. This means we don't need a rule that targets the break/continue and a separate rule that targets the labeled statement. I think we should just keep one that targets the labeled statement (and ignore unused labels, the same way some rules ignore unused variables to avoid duplicate violations).

Also I think labels on loops are actually the only legit use case for labels, even though they can be applied to any statement. I think it would be better to have a rule LabeledStatement that by default flags all labels except those applied to loops, and that has a property to also flag labels of loops.

Your XPath expression for detecting unused labels could be simplified, because usages of a label must be descendants of that labeled statement:

//LabeledStatement[let $label := @Label return
      not( (.//BreakStatement | .//ContinueStatement)[@Label = $label] )
]

[adangel]

@UncleOwen @oowekyala I've updated the description in #2928 and updated this PR. I think, this one is ready to be merge, wdyt?