Skip to content

Investigate additional suppression cleanup opportunities #635

Open

Description

@david-waltermire

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

Activity

  1. coderabbitai commented on Jan 11, 2026

    @coderabbitai

    📝 CodeRabbit Plan Mode

    Generate an implementation plan and prompts that you can use with your favorite coding agent.

    • Create Plan
    Examples

    🔗 Related PRs

    #137 - Ensure SARIF results are valid to SARIF JSON schema [merged]
    #512 - Remove unnecessary empty finalize() methods from abstract classes [merged]
    #545 - Fix sequence initialization deadlock and add metapath tests [merged]
    #549 - Prepare for release: PMD fixes and code cleanup [merged]
    #576 - docs: add Metaschema authoring skills and Metapath reference [merged]


    🧪 Issue enrichment is currently in open beta.

    You can configure auto-planning by selecting labels in the issue_enrichment configuration.

    To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

    issue_enrichment:
      auto_enrich:
        enabled: false

    💬 Have feedback or questions? Drop into our discord!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions