Investigate additional suppression cleanup opportunities #635
Description
Summary
During PR #634 (PMD warning suppressions cleanup), CodeRabbit identified additional cleanup opportunities that were deferred as they require deeper investigation or refactoring.
Deferred Items
1. FnAvg.java - Add explanatory comment for null return
Location: core/src/main/java/dev/metaschema/core/metapath/function/library/FnAvg.java (lines 90-92)
Issue: Now that the NOPMD is gone, consider adding a brief non-suppression comment explaining that null encodes the empty sequence for fn:avg (and that ISequence.of(null) yields empty), to preserve intent for future readers.
2. AbstractMarkupWriter.java - Narrow unchecked suppression scope
Location: core/src/main/java/dev/metaschema/core/datatype/markup/flexmark/impl/AbstractMarkupWriter.java (lines 265-273)
Issue: The @SuppressWarnings("unchecked") annotation could be scoped more narrowly to just the cast rather than the entire method:
// Instead of method-level suppression:
@SuppressWarnings("unchecked")
private void writeHtml(Node node) throws E {
...
throw (E) ex.getCause();
}
// Prefer cast-level suppression:
private void writeHtml(Node node) throws E {
...
@SuppressWarnings("unchecked")
E cause = (E) ex.getCause();
throw cause;
}3. ConstraintSupport.java - Investigate remaining null suppressions
Location: databind/src/main/java/dev/metaschema/databind/model/impl/ConstraintSupport.java (lines 41, 79)
Issue: Both parse methods retain @SuppressWarnings("null") annotations. Since Java annotation methods cannot return null (they return default values), these suppressions might be unnecessary. Verify whether these suppressions are still needed by checking SpotBugs behavior with annotation array methods.
4. AbstractSchemaGenerator.java - Resource ownership pattern
Location: schemagen/src/main/java/dev/metaschema/schemagen/AbstractSchemaGenerator.java (lines 86-101)
Issue: The @SuppressWarnings("resource") suppression masks a potential ownership/leak mismatch. The schemaWriter is marked @Owning and implements AutoCloseable, but is never closed.
Suggested fix: Wrap the caller-owned out in a non-closing FilterWriter, then use try-with-resources:
Writer nonClosingOut = new FilterWriter(out) {
@Override
public void close() throws IOException {
flush();
}
};
try (T schemaWriter = newWriter(nonClosingOut)) {
S generationState = newGenerationState(metaschema, schemaWriter, configuration);
generateSchema(generationState);
generationState.flushWriter();
}This requires confirmation that T.close() would close out (hence the intentional non-close behavior).
Related
- PR refactor: remove unnecessary PMD warning suppressions #634 - refactor: remove unnecessary PMD warning suppressions