fix: Update type comparisons to strict equality for CustomAttributeTy… by JohnVillalovos · Pull Request #974 · LibreBooking/app

JohnVillalovos · 2026-02-06T23:53:08Z

…pes and CustomAttributeCategory

fixes phpstan issues for wrong type comparisson

…pes and CustomAttributeCategory fixes phpstan issues for wrong type comparisson

Copilot

Pull request overview

This PR aims to resolve PHPStan issues around incorrect/mixed-type comparisons for custom attribute types/categories by moving comparisons toward strict equality (often with explicit int casts) and updating some related annotations/baseline entries.

Changes:

Replaces several ==/!= comparisons with stricter comparisons for CustomAttributeTypes / CustomAttributeCategory (often via (int) casts).
Updates ScheduleResourceFilter annotations from Attribute to LBAttribute, and removes related items from the PHPStan baseline.
Expands AttributeSaveController request validation to accept CustomAttributeTypes::DATETIME and updates the validation error message accordingly.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
phpstan-baseline.neon	Removes baseline ignores for ScheduleResourceFilter undefined-method findings.
lib/Application/Schedule/ScheduleResourceFilter.php	Updates attribute-related PHPDoc types and adjusts type comparisons.
lib/Application/Reservation/Validation/CustomAttributeValidationRule.php	Makes secondary-category comparisons strict via `(int)` casting and `===`.
lib/Application/Attributes/AttributeService.php	Tightens secondary-category comparisons in attribute availability checks.
WebServices/Controllers/AttributeSaveController.php	Validates type/category IDs with strict comparisons; adds DATETIME as a valid type.
Pages/Admin/ManageResourcesPage.php	Uses strict comparisons for attribute type handling in filtering SQL.
Pages/Admin/ManageAttributesPage.php	Adjusts interface docblocks for type/category getters/setters.
Domain/CustomAttribute.php	Tightens category checks to be more type-safe (with one remaining loose comparison).
Domain/Access/AttributeFilter.php	Uses strict comparisons for attribute-type branching when building SQL.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T23:59:37Z

Pages/Admin/ManageAttributesPage.php


    /**
-     * @return CustomAttributeCategory|int|null
+     * @return CustomAttributeCategory|null


GetSecondaryCategory() is a form value ($this->GetForm(...)) and may be a string/int. Declaring the return type as CustomAttributeCategory|null is misleading because CustomAttributeCategory is not an enum/object type here. Prefer int|null (or int|CustomAttributeCategory|null if matching existing conventions).

Suggested change

* @return CustomAttributeCategory|null

* @return int|null

Copilot · 2026-02-06T23:59:38Z

WebServices/Controllers/AttributeSaveController.php

@@ -197,15 +199,15 @@ private function ValidateRequest($request)
            );
        }

-        if ($request->type == CustomAttributeTypes::SELECT_LIST && empty($request->possibleValues)) {
+        if ((int)$request->type === CustomAttributeTypes::SELECT_LIST && empty($request->possibleValues)) {
            $errors[] = 'possibleValues is required when the type is a select list';
        }

-        if ($request->type != CustomAttributeTypes::SELECT_LIST && !empty($request->possibleValues)) {
+        if ((int)$request->type !== CustomAttributeTypes::SELECT_LIST && !empty($request->possibleValues)) {
            $errors[] = 'possibleValues is only valid when the type is a select list';
        }

-        if ($request->categoryId == CustomAttributeCategory::RESERVATION && !empty($request->appliesToIds)) {
+        if ((int)$request->categoryId === CustomAttributeCategory::RESERVATION && !empty($request->appliesToIds)) {


ValidateRequest() repeatedly casts $request->type/$request->categoryId inline. Consider casting once into local $type / $categoryId variables and reusing them (and optionally passing the casted ints into CustomAttribute::Create/CustomAttribute), to reduce duplication and keep types consistent after validation.

Copilot · 2026-02-06T23:59:38Z

lib/Application/Attributes/AttributeService.php

+        if ((int)$secondaryCategory === CustomAttributeCategory::RESOURCE || (int)$secondaryCategory === CustomAttributeCategory::RESOURCE_TYPE) {
+            if ((int)$secondaryCategory === CustomAttributeCategory::RESOURCE) {
                $applies = array_intersect($attribute->SecondaryEntityIds(), $requestedResourceIds);


Within AvailableForResource(), you updated the initial category checks to strict comparisons, but the subsequent RESOURCE_TYPE branch still uses a loose == comparison. For consistency and to avoid mixed-type comparison warnings, make the RESOURCE_TYPE check strict as well (casting once if needed).

Copilot · 2026-02-06T23:59:38Z

WebServices/Controllers/AttributeSaveController.php

+        if ((int)$request->type !== CustomAttributeTypes::CHECKBOX &&
+                (int)$request->type !== CustomAttributeTypes::MULTI_LINE_TEXTBOX &&
+                (int)$request->type !== CustomAttributeTypes::SELECT_LIST &&
+                (int)$request->type !== CustomAttributeTypes::SINGLE_LINE_TEXTBOX &&
+                (int)$request->type !== CustomAttributeTypes::DATETIME


ValidateRequest() now treats CustomAttributeTypes::DATETIME as a valid type. There are existing unit tests for AttributeSaveController, but none assert that DATETIME requests pass validation (and/or that invalid types are rejected). Add a focused test case to cover this new allowed value so regressions are caught.

Copilot · 2026-02-06T23:59:38Z

lib/Application/Schedule/ScheduleResourceFilter.php

@@ -185,7 +185,7 @@ private function AttributeValueMatches($attribute, $value)
            return false;
        }


In AttributeValueMatches, the PHPDoc says $value is LBAttribute, but the implementation explicitly allows null (if ($value == null)). Update the docblock to LBAttribute|null (and consider using strict null check) so static analysis matches actual behavior.

Copilot · 2026-02-06T23:59:39Z

lib/Application/Schedule/ScheduleResourceFilter.php

        }

-        if ($value->Type() == CustomAttributeTypes::SINGLE_LINE_TEXTBOX || $value->Type() == CustomAttributeTypes::MULTI_LINE_TEXTBOX) {
+        if ((int)$value->Type() == CustomAttributeTypes::SINGLE_LINE_TEXTBOX || (int)$value->Type() == CustomAttributeTypes::MULTI_LINE_TEXTBOX) {


This comparison still uses == even after casting to int. To fully address “strict equality” (and avoid phpstan mixed-type warnings), use === for these type comparisons (or assign the casted type to a local variable once and compare strictly).

Suggested change

if ((int)$value->Type() == CustomAttributeTypes::SINGLE_LINE_TEXTBOX || (int)$value->Type() == CustomAttributeTypes::MULTI_LINE_TEXTBOX) {

$type = (int)$value->Type();

if ($type === CustomAttributeTypes::SINGLE_LINE_TEXTBOX || $type === CustomAttributeTypes::MULTI_LINE_TEXTBOX) {

Copilot · 2026-02-06T23:59:39Z

Domain/CustomAttribute.php

    public function WithSecondaryEntities($category, $entityIds, $entityDescriptions = null)
    {
-        if ($this->category != CustomAttributeCategory::RESERVATION) {
+        if ((int)$this->category != CustomAttributeCategory::RESERVATION) {


WithSecondaryEntities uses (int)$this->category != ... which is still a non-strict comparison. For consistency with the rest of this PR (and to avoid loose-comparison surprises), change this to strict !== (keeping the cast if needed).

Suggested change

if ((int)$this->category != CustomAttributeCategory::RESERVATION) {

if ((int)$this->category !== CustomAttributeCategory::RESERVATION) {

Copilot · 2026-02-06T23:59:39Z

Pages/Admin/ManageAttributesPage.php

    /**
     * @abstract
-     * return int|CustomAttributeTypes
+     * return CustomAttributeTypes
     */
    public function GetType();

    /**
-     * return int|CustomAttributeCategory
+     * return CustomAttributeCategory
     */
    public function GetCategory();


These interface docblocks use plain return ... text instead of @return, so they likely won’t be interpreted by PHPStan/IDEs as type information. If the intent is to document types, switch to proper @return tags and use int (or int|CustomAttributeTypes / int|CustomAttributeCategory) since these are classes of constants rather than real value object/enum types.

fix: Update type comparisons to strict equality for CustomAttributeTy…

b75ba03

…pes and CustomAttributeCategory fixes phpstan issues for wrong type comparisson

Copilot AI review requested due to automatic review settings February 6, 2026 23:53

Copilot started reviewing on behalf of JohnVillalovos February 6, 2026 23:53 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Update type comparisons to strict equality for CustomAttributeTy…#974

fix: Update type comparisons to strict equality for CustomAttributeTy…#974
JohnVillalovos wants to merge 1 commit intodevelopfrom
jlvillal/strict_equality

JohnVillalovos commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -185,7 +185,7 @@ private function AttributeValueMatches($attribute, $value)
		return false;
		}

	if ((int)$value->Type() == CustomAttributeTypes::SINGLE_LINE_TEXTBOX \|\| (int)$value->Type() == CustomAttributeTypes::MULTI_LINE_TEXTBOX) {
	$type = (int)$value->Type();
	if ($type === CustomAttributeTypes::SINGLE_LINE_TEXTBOX \|\| $type === CustomAttributeTypes::MULTI_LINE_TEXTBOX) {

	if ((int)$this->category != CustomAttributeCategory::RESERVATION) {
	if ((int)$this->category !== CustomAttributeCategory::RESERVATION) {

Conversation

JohnVillalovos commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants