Reject a negative padding and a non-positive size

[matt-edmondson]

Fixes #115

The defect

Arguments.Validate() checked only padding's upper bound:

if (Padding >= Size / 2)

Which is not a bound at all for a negative number — -5 >= 64 is false. EffectivePadding then hands the value back unchanged, the content is resized to finalSize - padding * 2 (larger than the canvas), and ImageSharp's Pad never shrinks an image. The output PNG comes out bigger than the documented min(squareSize, size), quietly, with exit code 0.

That breaks the invariant README.md and CLAUDE.md both state outright: padding insets the content, it never changes the canvas.

The change

Two checks, and a small restructure so each complaint is the right one:

if (Size <= 0)
{
	errors.Add("Size must be greater than zero.");
}

if (Padding < 0)
{
	errors.Add("Padding must not be negative.");
}
else if (Size > 0 && Padding >= Size / 2)
{
	errors.Add("Padding must be less than half the size of the image.");
}

Size gets its own check, which the issue raises as worth double-checking and which turns out to matter. It was only ever caught incidentally, by the padding comparison derived from it — and that stops being a net the moment padding is negative too: -20 >= -10 / 2 is false, so --size -10 --padding -20 passed validation completely untouched. Where it did catch something, it lied about it: --size 0 reported "Padding must be less than half the size of the image."

The two padding messages are exclusive. "Less than half the size of the image" is not the complaint about a negative number, and half of an invalid size is not a bound worth quoting. ValidateReportsEveryProblemAtOnce still reports every distinct problem at once; this only stops one problem being reported twice in two voices.

Rejected, not clamped. EffectivePadding already clamps padding downward against the effective canvas, so clamping a negative value up to 0 would have been the locally consistent move. It is the wrong one: the downward clamp exists because Validate cannot know finalSize yet, which is a genuine limit of where the check sits. A negative padding needs no such knowledge — it is unusable at any canvas size — so it belongs at the gate, where the caller finds out rather than silently getting something else.

Tests

test	covers
ValidateRejectsNegativePadding	the reported defect; --size 128 --padding -5
ValidateRejectsASizeThatIsNotPositive	--size 0, and that it now says so rather than blaming padding
ValidateRejectsANegativeSizeAndPaddingTogether	the combination that passed validation entirely, and that both problems are reported

Proved failing without the fix. Reverting only IconHelper/Arguments.cs and keeping the tests takes the suite from 8 failures to 11 — the three new ones, nothing else moving:

failed ValidateRejectsANegativeSizeAndPaddingTogether (28ms)
failed ValidateRejectsASizeThatIsNotPositive (28ms)
failed ValidateRejectsNegativePadding (35ms)
  total: 76   failed: 11   succeeded: 65

The existing padding tests — ValidateAcceptsPaddingBelowHalfTheSize, ValidateRejectsPaddingEqualToHalfTheSize, ValidateUsesIntegerDivisionForOddSizes, ValidateReportsEveryProblemAtOnce — pass unchanged, so the upper bound and its exact message are untouched.

Verification

• dotnet build IconHelper.sln -c Release — succeeded, 0 warnings, 0 errors (analyzers run as errors here)
• dotnet test IconHelper.sln -c Release — 76 total, 68 passed, 8 failed

The 8 failures are GoldMasterTests and are not from this change: unmodified main gives 73 total, 65 passed, the same 8 failed. The gold master fixtures under IconHelper.Test/GoldMaster/Input/ are Git LFS objects, git-lfs is not installed in this container, and a shallow clone leaves them as pointer files that ImageSharp cannot decode. CI, which materializes LFS content, should see all 76 green. The same note is on #117.

Docs

README.md's padding section gains two sentences stating that a negative --padding is rejected rather than clamped, and why. It sits directly under the "the output is always finalSize square whatever the padding" claim, which this makes true rather than aspirational.

Relationship to #117

Independent, and branched separately from main — #117 is the .new.png path-versus-name fix in IconHelper.cs, this is validation in Arguments.cs. No shared files, so they can merge in either order.

🤖 Generated with Claude Code

https://claude.ai/code/session_012betVHk3gFcj5RYkEe4vrm

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud