[Carbon32]

The original method does not check if the provided output is a Mask or not, the intended behavior is for it to accept either None or a Mask so I've set up a small check to make sure the provided output is a Mask.

Fixes #4015

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: pygame-community/pygame-ce
• Branch: main
• File: .coderabbit.yaml
• Commit: c9988ea
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: d0c4848d-fca9-465a-80fa-c8bae45dc18f

Commits

Reviewing files that changed from the base of the PR and between 536215f and e5a3bd8.

Files selected for processing: 1

• test/mask_test.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

Mask.convolve now checks that a supplied output is a Mask instance or subclass. Other values raise TypeError with the message "output must be of type Mask".

Changes: Mask convolve output validation

Layer / File(s)	Summary
Validate the output argument src_c/mask.c, test/mask_test.py	mask_convolve rejects non-Mask output values with TypeError before using them. The test checks rejection of the value 1.

Priority: Normal

Estimated code review effort: 1 (Trivial) | ~4 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: starbuck5

Merge Risk: Minimal · up to e5a3b

The requested invalid-output protection is implemented without an identified merge-blocking risk.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: adding a type check to Mask.convolve().
Description check		The description explains the missing output type check, the intended accepted values, and the issue addressed. It is directly related to the changeset.
Linked Issues check		Issue #4015 requires Mask.convolve() to reject non-None values that are not Mask objects before dereferencing them. src_c/mask.c now checks the supplied output and raises TypeError for inv…
Out of Scope Changes check		The changes are limited to the Mask.convolve() type check and its regression test. Both changes directly support issue #4015. No unrelated change is shown.

Full details: Linked Issues check

Explanation

Issue #4015 requires Mask.convolve() to reject non-None values that are not Mask objects before dereferencing them. src_c/mask.c now checks the supplied output and raises TypeError for invalid values. test/mask_test.py covers output=1. This prevents the reported type-confusion path.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[oddbookworm]

I would like to see some regression testing for this added to the mask test suite

[Carbon32]

I would like to see some regression testing for this added to the mask test suite

Should be done, there is a test that ensures the functionality functions as intended.