HDDS-14923. Enable UselessStringValueOf, UseStringBufferForStringAppends in PMD#10007
HDDS-14923. Enable UselessStringValueOf, UseStringBufferForStringAppends in PMD#10007ptlrs wants to merge 2 commits intoapache:masterfrom
Conversation
Gargi-jais11
left a comment
There was a problem hiding this comment.
Thanks @ptlrs for the patch. Please resolve the conflicts in your branch and find the inline comments.
| * @param nextLevel | ||
| * @return subpath | ||
| */ | ||
| @SuppressWarnings("PMD.UseStringBufferForStringAppends") |
There was a problem hiding this comment.
Why to do suppression over here? If we are applying this as a rule we should refactor this as well as the change is not very complex.
public static String buildSubpath(String path, String nextLevel) {
StringBuilder sb = new StringBuilder();
if (!path.startsWith(OM_KEY_PREFIX)) {
sb.append(OM_KEY_PREFIX);
}
sb.append(path);
String normalized = removeTrailingSlashIfNeeded(sb.toString());
if (nextLevel == null) {
return normalized;
}
return new StringBuilder(normalized)
.append(OM_KEY_PREFIX)
.append(nextLevel)
.toString();
}
There was a problem hiding this comment.
The first StringBuilder is not needed.
String subpath = !path.startsWith(OM_KEY_PREFIX)
? OM_KEY_PREFIX + path
: path;| private static final int PATH_INDENT = 27; | ||
|
|
||
| @Override | ||
| @SuppressWarnings(value = "PMD.UseStringBufferForStringAppends") |
There was a problem hiding this comment.
I think no need of suppression here as well. Just need to change these below lines 166 - 170 in this method:
String subPath = subPathDU.path("path").asText("");
// differentiate key from other types
if (!subPathDU.path("isKey").asBoolean(false)) {
subPath += OM_KEY_PREFIX;
}
adoroszlai
left a comment
There was a problem hiding this comment.
Thanks @ptlrs for the patch.
| if (name == null) { | ||
| name = "_" + RDB_CHECKPOINT_DIR_PREFIX + currentTime; | ||
| } | ||
| checkpointDir += name; | ||
| checkpointDir.append(name); |
There was a problem hiding this comment.
String concatenation can be changed to use the outer StringBuilder:
if (name == null) {
checkpointDir.append('_')
.append(RDB_CHECKPOINT_DIR_PREFIX)
.append(currentTime);
} else {
checkpointDir.append(name);
}| boolean topologyAware) { | ||
| String key = pipeline.getId().getId().toString() + pipeline.getType(); | ||
| StringBuilder key = new StringBuilder() | ||
| .append(pipeline.getId().getId().toString()) |
There was a problem hiding this comment.
nit: unnecessary toString()
| .append(pipeline.getId().getId().toString()) | |
| .append(pipeline.getId().getId()) |
What changes were proposed in this pull request?
This PR enables new PMD rules:
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14923
How was this patch tested?
CI: https://github.com/ptlrs/ozone/actions/runs/23722077147