Skip to content

Boxed 3.4.1: Sonar low-risk fixes, test coverage, and NO_MOBS placement option#127

Open
tastybento wants to merge 12 commits into
masterfrom
develop
Open

Boxed 3.4.1: Sonar low-risk fixes, test coverage, and NO_MOBS placement option#127
tastybento wants to merge 12 commits into
masterfrom
develop

Conversation

@tastybento
Copy link
Copy Markdown
Member

Boxed 3.4.1

Version bump to 3.4.1, a sweep of low-risk SonarCloud issues, a substantial increase in test coverage, and re-enabling the NO_MOBS placement option.

Each change was made in a focused, individually revertable commit, and mvn clean package is green (167 tests, 0 failures).

Version

  • Bump build.version 3.4.0 → 3.4.1.

SonarCloud low-risk cleanups (one commit per rule)

  • S1128 – removed unused imports.
  • S1130 – removed redundant throws declarations in tests (kept CommonTestSetup.setUp() since subclass overrides legitimately throw).
  • S5786 – gave the 5 test classes and their 118 @Test methods package visibility (lifecycle overrides stay public).
  • S6068 – removed useless Mockito eq() matchers where all args were eq().
  • S1612 – lambdas → method references (LivingEntity.class::isInstance, List::isEmpty).
  • S6201instanceof pattern matching in BoxedChunkGenerator.
  • S1488 / S2293 / S116 / S100 – return-directly, diamond operator, and the final_statefinalState rename (GSON key pinned with @SerializedName so stored jigsaw data still deserialises).

A few flagged items were deliberately left alone with rationale: the S1874 deprecated-API swaps (behaviour-sensitive, in untested generator code), one ambiguous toLegacyText method reference, the S2637 false positive, and the S1135 "complete this TODO" items.

Test coverage (targeting 0%-covered areas from SonarCloud)

  • Data objectsBoxedStructureBlock (GSON), IslandStructures, ToBePlacedStructures + StructureRecord, IslandAdvancements, BoxedJigsawBlock.
  • Generator/biome logicAbstractBoxedChunkGenerator.repeatCalc (seed-region tiling math) and AbstractCopyBiomeProvider.getBiome (stored lookup, default fallback, missing-chunk warning, coordinate wrapping).
  • AdminPlaceStructureCommandcanExecute argument validation and tabComplete across all argument positions.

NO_MOBS placement option

While adding command tests I found the 7-argument NO_MOBS form of /boxadmin place was unreachable — the args.size() > 6 guard rejected it before the NO_MOBS branch could run, so admins could not place a structure with its built-in mobs (bastion piglins, village animals/iron golem, …) suppressed, even though the downstream place() / processJigsaw pipeline already honoured the flag.

  • Relaxed the guard to > 7.
  • Reset the noMobs field at the start of canExecute so it can't leak between placements.

🤖 Generated with Claude Code

tastybento and others added 12 commits May 30, 2026 18:35
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removes three unused imports flagged by SonarCloud:
- TrialSpawnerConfiguration in BoxedBlockPopulator
- AbstractMetaData in NewAreaListener
- static anyInt in AdvancementListenerTest

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removes 15 thrown-exception declarations that the method bodies cannot
actually throw, in AdvancementsManagerTest and EnderPearlListenerTest.
CommonTestSetup.setUp() keeps 'throws Exception' because subclass
overrides legitimately throw checked exceptions and the override
contract requires it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removes the redundant public modifier from the 5 test classes and their
118 @test methods. setUp()/tearDown() keep public because they override
the public lifecycle methods of CommonTestSetup across package boundaries.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Where every argument was wrapped in eq(), pass the raw values directly:
addAdvancement() in AdvancementListenerTest and two getIslandAt() stubs
in EnderPearlListenerTest. Mixed-matcher calls keep eq() as required.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- BoxedChunkGenerator: LivingEntity.class::isInstance
- NewAreaListener: List::isEmpty in removeIf

The third flagged lambda (CommonTestSetup toLegacyText) is left as-is:
BaseComponent has both an instance toLegacyText() and a static
toLegacyText(BaseComponent...), so a method reference is ambiguous.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…201)

Replaces three instanceof-and-cast pairs (Tameable, ChestedHorse,
Ageable) with pattern variables. Behaviour is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- AdminPlaceStructureCommand: return the air block state directly (S1488)
- NewAreaListener: use the diamond operator for the Pair (S2293)
- BoxedJigsawBlock: rename final_state -> finalState and getFinal_state ->
  getFinalState (S116/S100), pinning the GSON key with @SerializedName so
  the Minecraft jigsaw 'final_state' data still deserialises. Caller in
  NewAreaListener updated. Added BoxedJigsawBlockTest covering the mapping.

S2637 (line 96) is left as-is: it flags getIslandAt(getLocation()), but
Location from getLocation() is @nonnull, so it is a false positive.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Covers the previously 0%-covered data objects:
- BoxedStructureBlock: GSON deserialisation of all field types + toString
- IslandStructures: structure/nether maps, add helpers, lazy null re-init
- ToBePlacedStructures: uniqueId, lazy map, StructureRecord accessors/equality
- IslandAdvancements: uniqueId and advancements list

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- RepeatCalcTest: covers AbstractBoxedChunkGenerator.repeatCalc, the
  function that tiles the captured seed region across the game world
  (identity within [-size,size), wrapping, range invariant, varied sizes).
- CopyBiomeProviderTest: covers AbstractCopyBiomeProvider.getBiome via the
  overworld BoxedBiomeGenerator - stored biome lookup, default fallback,
  missing-chunk warning, and coordinate wrapping into the seed region.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Covers the previously 0%-covered admin /place command:
- canExecute argument validation: undo shortcut, wrong-world, unknown
  structure, coordinate parsing (~, integers, non-integers), rotation and
  mirror parsing, and the size>6 rejection guard.
- tabComplete suggestions for each argument position.
- setup() permission/label/only-player.

Documents that the 7-arg NO_MOBS form is unreachable (the size>6 guard
rejects it before the NO_MOBS block) by asserting the actual behaviour.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 7-argument NO_MOBS form was unreachable: the argument-count guard
rejected any 7th argument before the NO_MOBS branch could run, so admins
could not place a structure with its built-in mobs suppressed.

- Relax the guard from size > 6 to size > 7 so the NO_MOBS argument is
  accepted (the downstream place()/processJigsaw pipeline already honours
  the flag via structures.yml).
- Reset the noMobs field at the start of canExecute alongside rotation and
  mirror so the setting does not leak into subsequent placements.

Tests updated: NO_MOBS is now accepted, an unknown 7th arg is rejected
with the 'unknown' message, and 8 args is the new over-limit case.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 1, 2026

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant