From fb4a885be121a373641770986619166473928d8f Mon Sep 17 00:00:00 2001 From: Vest Date: Fri, 26 Jun 2026 14:49:26 +0200 Subject: [PATCH] =?UTF-8?q?spotbugs:=20fix=20PZLA,=20SI,=20DB=5FDUP,=20MC?= =?UTF-8?q?=5FOVERRIDABLE=20=E2=80=94=204=20small=20residuals?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Region: reorder static fields so ordinalCount precedes the final NONE constant, and drop the redundant '= 0' initializer. Today the '= 0' assignment runs *after* the NONE constructor reads ordinalCount++, which works only because Java zero-inits ints. If anyone added another public-static Region constant later it would collide with NONE on ordinal 0. Fixes SI_INSTANCE_BEFORE_FINALS_ASSIGNED. - HitPointFacet.rollHP: name HP_STANDARD as its own case (it is the documented primary mode and the default loaded by SettingsHandler), and turn 'default' into an explicit errorPrint-and-fall-back for unknown HP roll methods. Matches the cdom/facet convention for invalid-internal-state (cf. ArmorClassFacet's 'Invalid ACType'). Drops the silent duplication that DB_DUP flagged while preserving the safety net for corrupted config. - FactSetParser.unparse: suppress PZLA_PREFER_ZERO_LENGTH_ARRAYS. null on the error path is deliberate: it distinguishes 'unparse errored' from 'no output' (empty array). Same convention as TemplateFeatToken.unparse. - CDOMObject.clone: suppress MC_OVERRIDABLE_METHOD_CALL_IN_CLONE on the ownBonuses() call. PCClass overrides ownBonuses to recurse into its PCClassLevel children; making it final would break that. The override only reads the clone's BONUS list, which super.clone() already populated. SpotBugs total: 77 -> 71. --- code/src/java/pcgen/cdom/base/CDOMObject.java | 6 ++++++ .../pcgen/cdom/content/factset/FactSetParser.java | 5 +++++ code/src/java/pcgen/cdom/enumeration/Region.java | 15 +++++++++------ code/src/java/pcgen/cdom/facet/HitPointFacet.java | 7 ++++++- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/code/src/java/pcgen/cdom/base/CDOMObject.java b/code/src/java/pcgen/cdom/base/CDOMObject.java index ed4b98ec6f3..fc128b623d7 100644 --- a/code/src/java/pcgen/cdom/base/CDOMObject.java +++ b/code/src/java/pcgen/cdom/base/CDOMObject.java @@ -66,6 +66,8 @@ import pcgen.core.analysis.BonusActivation; import pcgen.core.bonus.BonusObj; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + public abstract class CDOMObject extends ConcretePrereqObject implements Cloneable, BonusContainer, Loadable, Reducible, PCGenScoped, VarHolder, VarContainer @@ -1032,6 +1034,10 @@ public final void overlayCDOMObject(CDOMObject cdo) } } + @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CLONE", + justification = "ownBonuses is intentionally overridable: PCClass overrides it to " + + "recursively re-own bonuses on its PCClassLevel children. The override only " + + "reads the clone's BONUS list, which super.clone() already populated.") @Override public CDOMObject clone() throws CloneNotSupportedException { diff --git a/code/src/java/pcgen/cdom/content/factset/FactSetParser.java b/code/src/java/pcgen/cdom/content/factset/FactSetParser.java index 9b65f3fab9f..22fc1af4506 100644 --- a/code/src/java/pcgen/cdom/content/factset/FactSetParser.java +++ b/code/src/java/pcgen/cdom/content/factset/FactSetParser.java @@ -33,6 +33,8 @@ import pcgen.rules.context.LoadContext; import pcgen.rules.persistence.token.AbstractTokenWithSeparator; import pcgen.rules.persistence.token.CDOMSecondaryToken; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import pcgen.rules.persistence.token.ParseResult; /** @@ -120,6 +122,9 @@ public String getParentToken() return "FACTSET"; } + @SuppressFBWarnings(value = "PZLA_PREFER_ZERO_LENGTH_ARRAYS", + justification = "null signals 'unparse errored'; empty array signals 'no output'. " + + "Same convention as TemplateFeatToken.unparse.") @Override public String[] unparse(LoadContext context, T obj) { diff --git a/code/src/java/pcgen/cdom/enumeration/Region.java b/code/src/java/pcgen/cdom/enumeration/Region.java index 8995713ea64..76255b91025 100644 --- a/code/src/java/pcgen/cdom/enumeration/Region.java +++ b/code/src/java/pcgen/cdom/enumeration/Region.java @@ -35,11 +35,6 @@ */ public final class Region implements TypeSafeConstant, Comparable { - /** - * A "None" region for universal use. - */ - public static final Region NONE = new Region(Constants.NONE); - /** * This Map contains the mappings from Strings to the Type Safe Constant */ @@ -48,7 +43,15 @@ public final class Region implements TypeSafeConstant, Comparable /** * This is used to provide a unique ordinal to each constant in this class */ - private static int ordinalCount = 0; + private static int ordinalCount; + + /** + * A "None" region for universal use. + * + * Declared after the mutable counters above so the constructor sees a + * defined `ordinalCount` — silences SpotBugs SI_INSTANCE_BEFORE_FINALS_ASSIGNED. + */ + public static final Region NONE = new Region(Constants.NONE); /** * The name of this Constant diff --git a/code/src/java/pcgen/cdom/facet/HitPointFacet.java b/code/src/java/pcgen/cdom/facet/HitPointFacet.java index 4f49b8af533..f8a56aa9eed 100644 --- a/code/src/java/pcgen/cdom/facet/HitPointFacet.java +++ b/code/src/java/pcgen/cdom/facet/HitPointFacet.java @@ -36,6 +36,7 @@ import pcgen.core.PCTemplate; import pcgen.core.PlayerCharacter; import pcgen.core.SettingsHandler; +import pcgen.util.Logging; /** * HitPointFacet stores information about hit points for a Player Character. @@ -92,7 +93,11 @@ private static int rollHP(final int min, final int max, final int totalLevel) case Constants.HP_PERCENTAGE -> roll = (min - 1) + (int) ((SettingsHandler.getHPPercent() * ((max - min) + 1)) / 100.0); case Constants.HP_AVERAGE_ROUNDED_UP -> roll = (int) Math.ceil((min + max) / 2.0); case Constants.HP_STANDARD -> roll = Math.abs(RandomUtil.getRandomInt((max - min) + 1)) + min; - default -> roll = Math.abs(RandomUtil.getRandomInt((max - min) + 1)) + min; + default -> { + Logging.errorPrint("Unknown HP roll method " + + SettingsHandler.getHPRollMethod() + "; falling back to HP_STANDARD"); + roll = Math.abs(RandomUtil.getRandomInt((max - min) + 1)) + min; + } } return roll;