Make isRegistryCompatible() and isOrderingCompatible() public#734
Conversation
📝 WalkthroughWalkthroughTwo methods in ChangesCIDSystemInfo Compatibility Method Visibility
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
validation-model/src/main/java/org/verapdf/gf/model/impl/pd/font/GFPDType0Font.java (1)
198-218:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd null guards before exposing these methods as public API.
Both methods can throw
NullPointerExceptionifgetCIDSystemInfo()orgetCMap()returns null, since they chain method calls directly without null checks. This is inconsistent with the other similar public methods in this class (e.g.,getCIDFontRegistry(),getCMapOrdering()) which properly guard against null before accessing these objects.Now that these methods are part of the public API, external callers may not be aware of this precondition.
🛡️ Suggested fix to add null guards
public boolean isRegistryCompatible() { + PDCIDSystemInfo cidSystemInfo = ((org.verapdf.pd.font.PDType0Font) this.pdFont).getCIDSystemInfo(); + org.verapdf.pd.font.cmap.PDCMap cmap = ((org.verapdf.pd.font.PDType0Font) this.pdFont).getCMap(); + if (cidSystemInfo == null || cmap == null) { + LOGGER.log(Level.FINE, "CIDSystemInfo or CMap is missing"); + return false; + } - String fontRegistry = ((org.verapdf.pd.font.PDType0Font) this.pdFont).getCIDSystemInfo() - .getStringKey(ASAtom.REGISTRY); - String cMapRegistry = ((org.verapdf.pd.font.PDType0Font) this.pdFont).getCMap().getRegistry(); + String fontRegistry = cidSystemInfo.getStringKey(ASAtom.REGISTRY); + String cMapRegistry = cmap.getRegistry(); if (fontRegistry == null) { LOGGER.log(Level.FINE, "CIDSystemInfo dictionary doesn't contain Registry entry."); return false; } return fontRegistry.equals(cMapRegistry); } public boolean isOrderingCompatible() { + PDCIDSystemInfo cidSystemInfo = ((org.verapdf.pd.font.PDType0Font) this.pdFont).getCIDSystemInfo(); + org.verapdf.pd.font.cmap.PDCMap cmap = ((org.verapdf.pd.font.PDType0Font) this.pdFont).getCMap(); + if (cidSystemInfo == null || cmap == null) { + LOGGER.log(Level.FINE, "CIDSystemInfo or CMap is missing"); + return false; + } - String fontOrdering = ((org.verapdf.pd.font.PDType0Font) this.pdFont).getCIDSystemInfo() - .getStringKey(ASAtom.ORDERING); - String cMapOrdering = ((org.verapdf.pd.font.PDType0Font) this.pdFont).getCMap().getOrdering(); + String fontOrdering = cidSystemInfo.getStringKey(ASAtom.ORDERING); + String cMapOrdering = cmap.getOrdering(); if (fontOrdering == null) { LOGGER.log(Level.FINE, "CIDSystemInfo dictionary doesn't contain Ordering entry."); return false; } return fontOrdering.equals(cMapOrdering); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@validation-model/src/main/java/org/verapdf/gf/model/impl/pd/font/GFPDType0Font.java` around lines 198 - 218, Add null guards to the isRegistryCompatible() and isOrderingCompatible() methods to prevent NullPointerException when getCIDSystemInfo() or getCMap() return null. Check if getCIDSystemInfo() is null before calling getStringKey() on it, and check if getCMap() is null before calling getRegistry() or getOrdering() on it. Return false in these null cases, consistent with how null checks are handled for fontRegistry and fontOrdering within each method.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@validation-model/src/main/java/org/verapdf/gf/model/impl/pd/font/GFPDType0Font.java`:
- Around line 198-218: Add null guards to the isRegistryCompatible() and
isOrderingCompatible() methods to prevent NullPointerException when
getCIDSystemInfo() or getCMap() return null. Check if getCIDSystemInfo() is null
before calling getStringKey() on it, and check if getCMap() is null before
calling getRegistry() or getOrdering() on it. Return false in these null cases,
consistent with how null checks are handled for fontRegistry and fontOrdering
within each method.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6a6414b6-3c76-4e58-bfa2-548dc6a31d06
📒 Files selected for processing (1)
validation-model/src/main/java/org/verapdf/gf/model/impl/pd/font/GFPDType0Font.java
Summary by CodeRabbit