Affects PMD Version: 5.5.1...6.55.0
Rule: SingularField
Description:
I have the following JAX-RS event listener that uses a Prometheus Summary to measure the time of all non-404 responses. It violates the java/design/SingularField rule.
Code Sample demonstrating the issue:
// package, imports
@Provider
public class LatencyRequestEventListener implements RequestEventListener {
private static final Summary SUMMARY_LATENCY_SECONDS = …;
private final ResourceInfo resourceInfo;
private Summary.Timer timer; // BREAKS HERE
LatencyRequestEventListener(final ResourceInfo resourceInfo) {
this.resourceInfo = resourceInfo;
}
@Override
public void onEvent(final RequestEvent event) {
switch (event.getType()) {
case RESOURCE_METHOD_START:
final ResourceMethod matchedResourceMethod = event.getUriInfo().getMatchedResourceMethod();
if (matchedResourceMethod == null) {
return;
}
timer = SUMMARY_LATENCY_SECONDS // ASSIGN FIELD
.labels(
matchedResourceMethod.getHttpMethod()
, resourceInfo.getResourceClass().getSimpleName() + "." + resourceInfo.getResourceMethod().getName()
)
.startTimer();
break; // Don't keep going (both cases cannot be reached in the same call)
case FINISHED:
if (timer != null) {
timer.observeDuration(); // READ FIELD
}
}
}
}
Expected outcome:
PMD reports a violation at line 25, but that's wrong. That's a false positive.
This results in the following error:
/path/to/project/…/listeners/LatencyRequestEventListener.java:25: Perhaps 'timer' could be replaced by a local variable.
timer can't be a local variable, so I don't think it's correct to report this rule violated.
I understand that the rule is violated if a field is only accessed by a single method. I also acknowledge that having the same method be called twice to first assign a field (the RESOURCE_METHOD_START case) and then read the field (FINISHED) may not be a good design. However, that's how the JAX-RS people designed the RequestEventListener class.
If I pull the code for the RESOURCE_METHOD_START case into a separate method, it passes with no problem since multiple methods access the field.
Is this a bug, or am I doing something wrong?
EDIT: As a funky side note, pulling the configuration of the Timer into a method makes the code legal, despite still only referencing the timer field in onEvent(…):
private static Summary.Child configureTimer(
final Summary summary,
final String httpMethod
, final String handlerName
) {
return summary
.labels(httpMethod, handlerName);
}
@Override
public void onEvent(final RequestEvent event) {
switch (event.getType()) {
case RESOURCE_METHOD_START:
final ResourceMethod matchedResourceMethod = event.getUriInfo().getMatchedResourceMethod();
if (matchedResourceMethod != null) {
timer = configureTimer(
SUMMARY_LATENCY_SECONDS,
matchedResourceMethod.getHttpMethod()
, resourceInfo.getResourceClass().getSimpleName() + "." + resourceInfo.getResourceMethod().getName()
).startTimer();
}
break;
case FINISHED:
if (timer != null) {
timer.observeDuration();
}
}
}
RetroSearch is an open source project built by @garambo | Open a GitHub Issue
Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo
HTML:
3.2
| Encoding:
UTF-8
| Version:
0.7.4