[java] New rules about labeled statements

[linusjf]

Proposing several rules:

Rule 1: UnusedLabel

Proposed Rule Name: UnusedLabel

Proposed Category: Best Practices + Quickstart

Description:
Finds labeled statements (https://docs.oracle.com/javase/specs/jls/se25/html/jls-14.html#jls-14.7) whose label is not used.

Code Sample:

class Example {
    void main() {
        lbl1: {                     // violation: Label "lbl1" is not nused
            int x = 1;
            System.out.println(x);
        }
    }
}

UnusedLabel is really uncontroversial and could be included in the quickstart ruleset. Nobody
wants unused code constructs in their code.
#6042 (comment)

---

If a label is declared but not used in the program, it can be considered as dead code and
should therefore be removed.

This will improve maintainability as developers will not wonder what this label is used for.

https://rules.sonarsource.com/java/RSPEC-1065

Possible Properties:
None.

Rule 2: LabeledStatement

Proposed Rule Name: LabeledStatement

Proposed Category: Best Practices

Description:
Labeled statements (https://docs.oracle.com/javase/specs/jls/se25/html/jls-14.html#jls-14.7) break the flow of a program and is difficult to trace the control flow. It is reminiscent of the goto statement. Code can be rewritten to avoid labeled statements.

Quotes:

It's difficult to read code containing breaks to a label. Also, a label can be accidentally
moved, or code inserted at an incorrect location with respect to a label. The compiler is not
able to warn you of these effects since the code remains syntactically valid.

Code that's difficult to read is difficult to maintain. Bugs will inevitably creep in.

Other control structures (break, continue, while, for, etc.) don't suffer from this.

Note that a switch to a label doesn't suffer from these effects either: the structure of a
switch block is well-defined.

The most sensible alternative to breaking out of a nested loop is to recast the code to a
function and use return. You also get the added benefit of being able (potentially) to return
a value back to the caller.

https://stackoverflow.com/questions/33689466/labels-in-java-bad-practice

---

Labels should not be used
Labels are not commonly used in Java, and many developers do not understand how they work.
Moreover, their usage makes the control flow harder to follow, which reduces the code’s
readability.

https://rules.sonarsource.com/java/tag/confusing/RSPEC-1119

---

A break with a label is always tricky, because you'll have to hunt for the label to find out
which statement/block the break escapes from (i.e. the break will continue after the labeled
block/statement).

The worst kind (the one you almost always want to avoid) is a nested loop where you break
somewhere in the middle."

https://softwareengineering.stackexchange.com/questions/185944/java-labels-to-be-or-not-to-be/185945#185945

---

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.

#6042 (comment)

Code Sample:

class Scratch {
    public static void main(String[] args) {
        int x = 1;
        lbl1: while (true) {          // violation: labeled statement on loop (when property allowLoops=false)
            lbl2: if (x == 3) {       // violation: labeled statement
                x++;
                break lbl2;
            }
            lbl3: if (x == 4) {
                break lbl1;
            }
            System.out.println(x);
            x++;
        }
    }
}

Possible Properties:

• allowLoops: boolean. default is true. whether to allow or not labels before loop statements (do, while, for)

[numeralnathan]

I do use labels in some very minor cases where it actually makes the code easier to write and read. I would disable this rule for those specific cases but would leave it enabled otherwise. This way I will think twice before using a label.

[linusjf]

I do use labels in some very minor cases where it actually makes the code easier to write and read. I would disable this rule for those specific cases but would leave it enabled otherwise. This way I will think twice before using a label.

I think this rule was never built into PMD because most java programmers never use labels in their code. And it probably felt like a novice rule. Am I correct?

[linusjf]

https://stackoverflow.com/questions/33689466/labels-in-java-bad-practice

"It's difficult to read code containing breaks to a label. Also, a label can be accidentally moved, or code inserted at an incorrect location with respect to a label. The compiler is not able to warn you of these effects since the code remains syntactically valid.

Code that's difficult to read is difficult to maintain. Bugs will inevitably creep in.

Other control structures (break, continue, while, for, etc.) don't suffer from this.

Note that a switch to a label doesn't suffer from these effects either: the structure of a switch block is well-defined.

The most sensible alternative to breaking out of a nested loop is to recast the code to a function and use return. You also get the added benefit of being able (potentially) to return a value back to the caller."

[numeralnathan]

I think it would be a good idea to write the rule even if very few programmers use labels. If a programmer uses a label, the documentation for the rule will help the programmer make an informed decision. The documentation can give alternative solutions and they may go with those. If they decided that a label is still the best choice, they will then disable the rule for that method (e.g. @SuppressWarnings) and add a comment why they feel using a label is a better approach than the alternatives suggested in the documentation.

[linusjf]

If you're a programmer who doesn't use labels, it doesn't harm to have this rule in your ruleset. That way, when you do use labels or someone on your team does, it is flagged. If the use of label is justified, then you can choose either to disable the rule (assuming PMD supports this in some future release) or omit it or suppress the violation on a case-by-case basis.

[numeralnathan]

assuming PMD supports this in some future release

Uh, I thought PMD already supports this. I can disable a PMD rule for 1 method or class by adding the following.

@SuppressWarnings({"PMD.UseVarargs", "PMD.AvoidThrowingNullPointerException"})

I can disable all PMD rules for 1 line by adding the following.

throw new NullPointerException();     // NOPMD

Maybe this does not affect PMD but affects PMD's Eclipse plugin.

[numeralnathan]

If you're a programmer who doesn't use labels, it doesn't harm to have this rule in your ruleset. That way, when you do use labels or someone on your team does, it is flagged. If the use of label is justified, then you can choose either to disable the rule (assuming PMD supports this in some future release) or omit it or suppress the violation on a case-by-case basis.

Exactly! PMD can introduce a lot of rules including controversial rules. If I think a rule is not good for my project, I disable it for the entire project. If I think a rule is not good for a particular class, method or line, then I disable it in the code. This way PMD can satisfy those that want and those that don't want the rule.

[linusjf]

I do use labels in some very minor cases where it actually makes the code easier to write and read. I would disable this rule for those specific cases but would leave it enabled otherwise. This way I will think twice before using a label.

Do you have any particular scenarios where you'd like to use labels? Are there any properties you can think of?

[linusjf]

> to disable the rule (assuming PMD supports this in some future release) Uh, I thought PMD already supports this. I can disable a PMD rule for 1 method or class by adding the following.... -\> suppress

Via the Xml schema. Perhaps, we could have a property enabled that defaults to true. This way, you can retain all the rules in the rulesets but selectively disable the ones you don't need rather than commenting them out or deleting them.

[numeralnathan]

Do you have any particular scenarios where you'd like to use labels? Are there any properties you can think of?

In one code base of mine, 6 out of 899 Java files use a label. So, you can see I use labels very rarely. In all cases, I either have an outer and inner loop or I have a loop with a switch statement. For all cases, I would disable the PMD rule because the code is significantly easier to write and hence read.

Here is an example. The code is looking for a match. If the inner loop finds a mismatch, then it needs to go back out to the outer loop. I could set a boolean to true before the inner loop, false in the inner loop and then test on the boolean after the inner loop. However, that means another branch with performance implications.

      Outer:
      for (fIndex = fromIndex; fIndex <= len; fIndex++)
      {
         if (get(fIndex) != data[offset])
         {
            continue;
         }

         for (index = 1; index < length; index++)
         {
            if (get(fIndex + index) != data[offset + index])
            {
               continue Outer;
            }
         }

         return fIndex;
      }

In another situation, the parsing code has a loop. In the loop, there is a switch statement to handle different characters. In many case statements, I need to break not just from the switch statement but from the loop as well.

[numeralnathan]

Via the Xml schema.
Perhaps, we could have a property enabled that defaults to true. This way, you can retain all the rules in the rulesets but selectively disable the ones you don't need rather than commenting them out or deleting them.

I do exactly that today. I have an XML configuration file for PMD. Here is a snippet from that configuration file that disables some PMD rules.

    <rule ref="category/java/codestyle.xml">
       <exclude name="LocalVariableCouldBeFinal"/>
       <exclude name="MethodArgumentCouldBeFinal"/> 
       <exclude name="OnlyOneReturn"/>
       ...
    </rule>

[linusjf]

Via the Xml schema.
Perhaps, we could have a property enabled that defaults to true. This way, you can retain all the rules in the rulesets but selectively disable the ones you don't need rather than commenting them out or deleting them.

I do exactly that today. I have an XML configuration file for PMD. Here is a snippet from that configuration file that disables some PMD rules.

<rule ref="category/java/codestyle.xml">
   <exclude name="LocalVariableCouldBeFinal"/>
   <exclude name="MethodArgumentCouldBeFinal"/> 
   <exclude name="OnlyOneReturn"/>
   ...
</rule>

Yes, but my way, you don't need to do an exclude. You simply disable the rule.
Your way works if you're including by ruleset name only and excluding particular rules.
Also, you'd be able to configure each rule fully.

[linusjf]

Do you have any particular scenarios where you'd like to use labels? Are there any properties you can think of?

In one code base of mine, 6 out of 899 Java files use a label. So, you can see I use labels very rarely. In all cases, I either have an outer and inner loop or I have a loop with a switch statement. For all cases, I would disable the PMD rule because the code is significantly easier to write and hence read.

Here is an example. The code is looking for a match. If the inner loop finds a mismatch, then it needs to go back out to the outer loop. I could set a boolean to true before the inner loop, false in the inner loop and then test on the boolean after the inner loop. However, that means another branch with performance implications.

  Outer:
  for (fIndex = fromIndex; fIndex <= len; fIndex++)
  {
     if (get(fIndex) != data[offset])
     {
        continue;
     }

     for (index = 1; index < length; index++)
     {
        if (get(fIndex + index) != data[offset + index])
        {
           continue Outer;
        }
     }

     return fIndex;
  }

In another situation, the parsing code has a loop. In the loop, there is a switch statement to handle different characters. In many case statements, I need to break not just from the switch statement but from the loop as well.

I'd rather you refactor the code. Even though I've never used continue with a label, everything I've read points to it being discouraged---severely.

If you feel the need to use it, go ahead. But this rule will disallow it---definitely.

https://www.programiz.com/java-programming/continue-statement#example-labeled-continue