google / google/error-prone

DuplicateBranches should not warn if there are different comments

Open
#4,734 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
7.2k
Forks
820
Avg merge
5h 9m
Merged PRs (30d)
50

Description

In some cases we have code where the new `DuplicateBranches` check triggers but the code was intentional because the same thing needs to be done for different reasons, and there are comments explaining this. In a few more cases the code is unfinished and the duplication is only temporary, marked with a TODO, and to be replaced by different code later. In these cases it would be possible to remove the duplication and explain the situation in a comment, but I think that actually having the branching code makes it more clear and less error prone compared to having to write down what each branch would be in a textual comment. Furthermore, the branching code benefits from refactorings being applied automatically, whereas comments need to be updated manually.

### Examples:

#### Preferred:
```java
public static void main(String[] args) {
if (args.length == 0) {
// nothing we can do here
return;
} else {
// TODO: add proper argument handling here
return;
}
}
```
#### Less optimal:
```java
public static void main(String[] args) {
// TODO we cannot handle empty args, but need to add proper argument handling args.length > 0
return;
}
```

#### Preferred:
```java
public static void main(String[] args) {
if (args.length == 0) {
// nothing we can do here
return;
} else if (args.length == 1) {
// some other reason why the following is intended
return;
} else {
...
}
}
```
#### Less optimal:
```java
public static void main(String[] args) {
if (args.length < 0) {
// either args is empty, then there is nothing we can do here,
// or args has one element, then some other reason why the following is intended
return;
} else {
...
}
}
```

In both cases the actual code can be more precise than a textual description (especially if the conditions get more complex) and would benefit from refactorings such as renamings of `args`.

Thus I would suggest to change `DuplicateBranches` such that it does not warn about cases where (different) comments exist in each of the branches. `@SuppressWarnings` is unfortunately not a good workaround because one would have to label the whole method with it.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.