Fix deprecation notices in the backend module by austinderrick · Pull Request #1490 · wintercms/winter

austinderrick · 2026-06-12T18:06:20Z

Summary

Fixes two families of deprecation notices in the backend module, found while auditing PHP 8.4/8.5 readiness (follow-up to #1489):

1. TranscodeFilter is missing the return types declared by php_user_filter. Loading the class raises three deprecations on PHP 8.1+:

Deprecated: Return type of ...TranscodeFilter::filter($in, $out, &$consumed, $closing) should either be compatible with php_user_filter::filter($in, $out, &$consumed, bool $closing): int, or the #[\ReturnTypeWillChange] attribute should be used...
Deprecated: Return type of ...TranscodeFilter::onCreate() should either be compatible with php_user_filter::onCreate(): bool, ...
Deprecated: Return type of ...TranscodeFilter::onClose() should either be compatible with php_user_filter::onClose(): void, ...

Since the repo's PHP floor is 8.1, this adds the real return types (: int, : bool, : void) rather than #[\ReturnTypeWillChange]. The existing bodies already return the right types (PSFS_PASS_ON, booleans, nothing).

2. displayAs() passes a null render mode into strtolower(). Lists::makeListColumn() and Filter::makeFilterScope() pass $config['type'] ?? null into displayAs(), and FormField, ListColumn, and FilterScope each do strtolower($type) ?: $this->type, so any column/scope/field config without an explicit type raises:

Deprecated: strtolower(): Passing null to parameter #1 ($string) of type string is deprecated

Fixed by typing the parameter natively (?string $type) in all three classes and coalescing inside (strtolower($type ?? '')), keeping identical semantics: null and '' both fall back to the existing type. Real argument types rather than a docblock-only change, per the direction from the storm#229 review. All in-repo callers already pass string-or-null (Form::makeFormField() validates the type before calling), and untyped subclass overrides remain legal (widening).

Verification

Reproduced all four deprecations on a clean develop checkout under PHP 8.5.5 with a small harness (load TranscodeFilter, call each displayAs(null)); after this change the same harness emits none. modules/backend PHPUnit suite shows an identical result before and after the change on the same machine (the handful of locally-failing tests are environment-related and untouched by this diff: FileUploadScoping, NavigationManager, WidgetManager).

Summary by CodeRabbit

Bug Fixes
- Prevented missing “type” configs from causing rendering issues by defaulting to sensible values (e.g., text for fields/columns and group for filter scopes).
Refactor
- Strengthened type-safety across backend display helpers by requiring non-nullable string inputs.
- Added explicit return/parameter typing for filter lifecycle methods to make behavior more predictable.

coderabbitai · 2026-06-12T18:06:36Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fb83249e-dcef-4bb3-ac9b-de349474dc2b

📥 Commits

Reviewing files that changed from the base of the PR and between b545a05 and e714ec3.

📒 Files selected for processing (6)

modules/backend/classes/FilterScope.php
modules/backend/classes/FormField.php
modules/backend/classes/ListColumn.php
modules/backend/widgets/Filter.php
modules/backend/widgets/Form.php
modules/backend/widgets/Lists.php

✅ Files skipped from review due to trivial changes (1)

modules/backend/widgets/Filter.php

Walkthrough

This PR updates backend method signatures and default type handling. TranscodeFilter now declares explicit return types and a typed closing parameter. FilterScope, FormField, and ListColumn now require string type arguments in displayAs(). Filter, Form, and Lists now default missing type values to strings, and Form::makeFormField() now rejects non-string resolved types.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: fixing backend PHP deprecation notices.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

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

coderabbitai

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@modules/backend/behaviors/importexportcontroller/TranscodeFilter.php`:
- Line 20: The filter method signature in TranscodeFilter::filter must match the
php_user_filter parent by adding the missing bool type hint for the fourth
parameter; update the method declaration from filter($in, $out, &$consumed,
$closing): int to include bool $closing so it reads filter($in, $out,
&$consumed, bool $closing): int, ensuring the parameter type aligns with the
parent and any callers remain compatible.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dae7d38b-7200-4ba8-969d-ac7595131222

📥 Commits

Reviewing files that changed from the base of the PR and between 7c28580 and 7684a13.

📒 Files selected for processing (4)

modules/backend/behaviors/importexportcontroller/TranscodeFilter.php
modules/backend/classes/FilterScope.php
modules/backend/classes/FormField.php
modules/backend/classes/ListColumn.php

🚧 Files skipped from review as they are similar to previous changes (2)

modules/backend/classes/ListColumn.php
modules/backend/classes/FilterScope.php

TranscodeFilter was missing the return types declared (tentatively) by php_user_filter, and the three displayAs() implementations passed a null render mode straight into strtolower(). Both raise deprecation notices on modern PHP.

Per review (option 2): FilterScope, FormField and ListColumn displayAs() now require a non-null string $type, and the Filter/Form/Lists builders default a missing config 'type' to the class default (group/text) instead of passing null. Callers that pass null now fail fast rather than being silently coerced to the default.

LukeTowers · 2026-06-27T06:38:00Z

    {
        $label = $config['label'] ?? null;
-        $scopeType = $config['type'] ?? null;
+        $scopeType = $config['type'] ?? 'group';


Where was this default being inferred previously?

LukeTowers · 2026-06-27T06:38:06Z

        else {
-            $fieldType = $config['type'] ?? null;
-            if (!is_string($fieldType) && $fieldType !== null) {
+            $fieldType = $config['type'] ?? 'text';


Where was this default being inferred previously?

LukeTowers · 2026-06-27T06:38:11Z

        }

-        $columnType = $config['type'] ?? null;
+        $columnType = $config['type'] ?? 'text';


Where was this default being inferred previously?

LukeTowers · 2026-06-27T06:39:32Z

-    public function displayAs($type, $config = [])
+    public function displayAs(string $type, $config = [])
    {
        $this->type = strtolower($type) ?: $this->type;


Does this only fallback to $this->type now when calling displayAs('')?

austinderrick force-pushed the fix/php85-deprecations branch from 7c28580 to 7684a13 Compare June 12, 2026 18:20

coderabbitai Bot reviewed Jun 12, 2026

View reviewed changes

Comment thread modules/backend/behaviors/importexportcontroller/TranscodeFilter.php Outdated

Fix deprecation notices in backend module

b545a05

TranscodeFilter was missing the return types declared (tentatively) by php_user_filter, and the three displayAs() implementations passed a null render mode straight into strtolower(). Both raise deprecation notices on modern PHP.

austinderrick force-pushed the fix/php85-deprecations branch from 7684a13 to b545a05 Compare June 12, 2026 18:35

LukeTowers requested changes Jun 20, 2026

View reviewed changes

Comment thread modules/backend/classes/FilterScope.php Outdated

austinderrick requested a review from LukeTowers June 24, 2026 22:35

LukeTowers reviewed Jun 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix deprecation notices in the backend module#1490

Fix deprecation notices in the backend module#1490
austinderrick wants to merge 2 commits into
wintercms:developfrom
austinderrick:fix/php85-deprecations

austinderrick commented Jun 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 12, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

LukeTowers Jun 27, 2026

Uh oh!

LukeTowers Jun 27, 2026

Uh oh!

LukeTowers Jun 27, 2026

Uh oh!

LukeTowers Jun 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

austinderrick commented Jun 12, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LukeTowers Jun 27, 2026

Choose a reason for hiding this comment

Uh oh!

LukeTowers Jun 27, 2026

Choose a reason for hiding this comment

Uh oh!

LukeTowers Jun 27, 2026

Choose a reason for hiding this comment

Uh oh!

LukeTowers Jun 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

austinderrick commented Jun 12, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 12, 2026 •

edited

Loading