declare strict_types in all files

[Art4]

closes #464.

Summary by CodeRabbit

• Chores
  • Enabled strict type checking across the codebase to improve type safety and runtime consistency.
  • Updated legacy listing calls to consistently pass boolean flags.
• Bug Fixes
  • Improved XML serialization to consistently treat values and attributes as strings, reducing parsing edge cases.
• Tests
  • Test files updated to opt into strict typing to match production behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: de730c3d-a2ff-400b-8d05-95ebf475a329

📥 Commits

Reviewing files that changed from the base of the PR and between 47748bd and 98f41f3.

📒 Files selected for processing (3)

• src/Redmine/Api/Group.php
• src/Redmine/Api/Search.php
• tests/Unit/Api/Search/SearchTest.php

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

• tests/Unit/Api/Search/SearchTest.php
• src/Redmine/Api/Group.php

---

📝 Walkthrough

Walkthrough

Adds PHP strict typing across the repository by enabling declare(strict_types=1); in many source and test files; also updates a few deprecated wrappers to cast arguments and narrows exception catching plus explicit casts in the XML serializer.

Changes

Cohort / File(s)	Summary
PHP-CS-Fixer Configuration \.php-cs-fixer.dist.php	Added the declare_strict_types rule to enforce declare(strict_types=1);.
API root & helpers src/Redmine/Api.php, src/Redmine/Api/.../*.php	Inserted declare(strict_types=1); across API files; multiple deprecated listing(...) wrappers now cast $forceUpdate to (bool) before delegating.
Client & Traits src/Redmine/Client/Client.php, src/Redmine/Client/ClientApiTrait.php	Added declare(strict_types=1);.
Exceptions src/Redmine/Exception.php, src/Redmine/Exception/*	Added declare(strict_types=1); to exception classes.
Serializers (JSON, Path, XML) src/Redmine/Serializer/JsonSerializer.php, src/Redmine/Serializer/PathSerializer.php, src/Redmine/Serializer/XmlSerializer.php	Added declare(strict_types=1);. XmlSerializer: narrowed catch Throwable → catch Exception (added use Exception), added explicit string casts for root element, ids and custom field values.
Other src files src/... (client/exception/other listed files)	Added file-level declare(strict_types=1); and small explicit strval/(string) coercions where concatenating or passing to typed methods.
Tests tests/Unit/... (many files across Api/Client/Exception/...)	Added declare(strict_types=1); to unit tests. Several tests replaced repeated listing(true) calls with listing(1) and added @phpstan-ignore annotations to suppress int→bool static-analysis warnings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Housekeeping for v2.10.0-dev #473 — overlaps in adding declare_strict_types and edits to API files; strongly related.
• Issue #464 — request to "Declare strict types in all files"; this PR implements that objective.

"I hop through code with tiny feet,
I add strict types to make it neat.
I nudge a cast, avoid a slip,
And leave the repo with a carrot-tip. 🥕"

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: adding declare(strict_types=1); declarations across all files.
Linked Issues check	✅ Passed	The pull request successfully implements issue #464 by adding declare(strict_types=1); to all source and test files, with explicit type casting added where needed under strict typing rules.
Out of Scope Changes check	✅ Passed	All changes are in-scope: adding strict types declarations and necessary type casts to enforce strict typing semantics across the codebase.
Docstring Coverage	✅ Passed	Docstring coverage is 88.46% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 464-strict-types

---

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 and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.72%. Comparing base (9e16d0e) to head (98f41f3).
⚠️ Report is 4 commits behind head on v2.x.

Additional details and impacted files

@@            Coverage Diff            @@
##               v2.x     #474   +/-   ##
=========================================
  Coverage     98.72%   98.72%           
  Complexity      783      783           
=========================================
  Files            29       29           
  Lines          2275     2275           
=========================================
  Hits           2246     2246           
  Misses           29       29           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Redmine/Api/CustomField.php`:
- Line 3: The listing() method currently forwards an untyped $forceUpdate to
doListing(bool $forceUpdate, ...) under declare(strict_types=1), causing a
TypeError when callers pass non-bool values; update listing() (in
CustomField.php) to coerce or validate $forceUpdate to a bool before calling
doListing — e.g., explicitly cast (bool)$forceUpdate or perform is_bool check
and convert — so the forwarded argument matches doListing(bool $forceUpdate,
...) and avoids strict-type failures.

In `@src/Redmine/Api/Search.php`:
- Line 3: The search() method currently forwards an untyped $query to
listByQuery(string $query, ...) under strict_types, causing a TypeError; fix
this by explicitly casting $query to string when calling listByQuery (e.g., pass
(string)$query) inside the search() implementation so the call matches
listByQuery's signature and avoids uncaught Error types; update any related call
sites in the same method (references: search(), listByQuery()) accordingly.

In `@src/Redmine/Serializer/XmlSerializer.php`:
- Line 80: The catch blocks in XmlSerializer (the catch clauses currently
written as "catch (Exception $e)") are too narrow and will let TypeError/other
Error subclasses escape under strict_types; update both catch clauses in the
XmlSerializer class (the blocks that currently catch Exception and wrap/rethrow
as SerializerException) to catch Throwable instead, preserving the existing
handling logic (wrap/throw SerializerException with the same message/context) so
TypeError and other Errors are captured and converted to SerializerException as
callers expect.

🪄 Autofix (Beta)

✅ Autofix completed

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 34594283-87b3-4479-a24f-4084a0dace48

📥 Commits

Reviewing files that changed from the base of the PR and between 9e16d0e and ec15698.

📒 Files selected for processing (144)

• .php-cs-fixer.dist.php
• src/Redmine/Api.php
• src/Redmine/Api/AbstractApi.php
• src/Redmine/Api/Attachment.php
• src/Redmine/Api/CustomField.php
• src/Redmine/Api/Group.php
• src/Redmine/Api/Issue.php
• src/Redmine/Api/IssueCategory.php
• src/Redmine/Api/IssuePriority.php
• src/Redmine/Api/IssueRelation.php
• src/Redmine/Api/IssueStatus.php
• src/Redmine/Api/Membership.php
• src/Redmine/Api/News.php
• src/Redmine/Api/Project.php
• src/Redmine/Api/Query.php
• src/Redmine/Api/Role.php
• src/Redmine/Api/Search.php
• src/Redmine/Api/TimeEntry.php
• src/Redmine/Api/TimeEntryActivity.php
• src/Redmine/Api/Tracker.php
• src/Redmine/Api/User.php
• src/Redmine/Api/Version.php
• src/Redmine/Api/Wiki.php
• src/Redmine/Client/Client.php
• src/Redmine/Client/ClientApiTrait.php
• src/Redmine/Exception.php
• src/Redmine/Exception/ClientException.php
• src/Redmine/Exception/InvalidApiNameException.php
• src/Redmine/Exception/InvalidParameterException.php
• src/Redmine/Exception/MissingParameterException.php
• src/Redmine/Exception/SerializerException.php
• src/Redmine/Serializer/JsonSerializer.php
• src/Redmine/Serializer/PathSerializer.php
• src/Redmine/Serializer/XmlSerializer.php
• tests/Unit/Api/AbstractApiTest.php
• tests/Unit/Api/Attachment/DownloadTest.php
• tests/Unit/Api/Attachment/FromHttpClientTest.php
• tests/Unit/Api/Attachment/ShowTest.php
• tests/Unit/Api/Attachment/UpdateTest.php
• tests/Unit/Api/Attachment/UploadTest.php
• tests/Unit/Api/AttachmentTest.php
• tests/Unit/Api/CustomField/FromHttpClientTest.php
• tests/Unit/Api/CustomField/ListTest.php
• tests/Unit/Api/CustomFieldTest.php
• tests/Unit/Api/Group/AddUserTest.php
• tests/Unit/Api/Group/FromHttpClientTest.php
• tests/Unit/Api/Group/ListTest.php
• tests/Unit/Api/Group/ShowTest.php
• tests/Unit/Api/Issue/AddWatcherTest.php
• tests/Unit/Api/Issue/AttachManyTest.php
• tests/Unit/Api/Issue/AttachTest.php
• tests/Unit/Api/Issue/CreateTest.php
• tests/Unit/Api/Issue/FromHttpClientTest.php
• tests/Unit/Api/Issue/ListTest.php
• tests/Unit/Api/Issue/RemoveTest.php
• tests/Unit/Api/Issue/RemoveWatcherTest.php
• tests/Unit/Api/Issue/SetIssueStatusTest.php
• tests/Unit/Api/Issue/ShowTest.php
• tests/Unit/Api/IssueCategory/CreateTest.php
• tests/Unit/Api/IssueCategory/FromHttpClientTest.php
• tests/Unit/Api/IssueCategory/ListByProjectTest.php
• tests/Unit/Api/IssueCategory/RemoveTest.php
• tests/Unit/Api/IssueCategory/ShowTest.php
• tests/Unit/Api/IssueCategoryTest.php
• tests/Unit/Api/IssuePriority/FromHttpClientTest.php
• tests/Unit/Api/IssuePriority/ListTest.php
• tests/Unit/Api/IssuePriorityTest.php
• tests/Unit/Api/IssueRelation/CreateTest.php
• tests/Unit/Api/IssueRelation/FromHttpClientTest.php
• tests/Unit/Api/IssueRelation/ListByIssueIdTest.php
• tests/Unit/Api/IssueRelation/RemoveTest.php
• tests/Unit/Api/IssueRelation/ShowTest.php
• tests/Unit/Api/IssueRelationTest.php
• tests/Unit/Api/IssueStatus/FromHttpClientTest.php
• tests/Unit/Api/IssueStatus/ListTest.php
• tests/Unit/Api/IssueStatusTest.php
• tests/Unit/Api/IssueTest.php
• tests/Unit/Api/Membership/CreateTest.php
• tests/Unit/Api/Membership/FromHttpClientTest.php
• tests/Unit/Api/Membership/ListByProjectTest.php
• tests/Unit/Api/Membership/RemoveMemberTest.php
• tests/Unit/Api/Membership/RemoveTest.php
• tests/Unit/Api/Membership/UpdateTest.php
• tests/Unit/Api/MembershipTest.php
• tests/Unit/Api/News/FromHttpClientTest.php
• tests/Unit/Api/News/ListByProjectTest.php
• tests/Unit/Api/News/ListTest.php
• tests/Unit/Api/NewsTest.php
• tests/Unit/Api/Project/CloseTest.php
• tests/Unit/Api/Project/CreateTest.php
• tests/Unit/Api/Project/FromHttpClientTest.php
• tests/Unit/Api/Project/ListTest.php
• tests/Unit/Api/Project/RemoveTest.php
• tests/Unit/Api/Project/ReopenTest.php
• tests/Unit/Api/Project/ShowTest.php
• tests/Unit/Api/Project/UnarchiveTest.php
• tests/Unit/Api/ProjectTest.php
• tests/Unit/Api/Query/FromHttpClientTest.php
• tests/Unit/Api/Query/ListTest.php
• tests/Unit/Api/QueryTest.php
• tests/Unit/Api/Role/FromHttpClientTest.php
• tests/Unit/Api/Role/ListTest.php
• tests/Unit/Api/Role/ShowTest.php
• tests/Unit/Api/RoleTest.php
• tests/Unit/Api/Search/FromHttpClientTest.php
• tests/Unit/Api/Search/ListByQueryTest.php
• tests/Unit/Api/Search/SearchTest.php
• tests/Unit/Api/SearchTest.php
• tests/Unit/Api/TimeEntry/CreateTest.php
• tests/Unit/Api/TimeEntry/FromHttpClientTest.php
• tests/Unit/Api/TimeEntry/ListTest.php
• tests/Unit/Api/TimeEntry/RemoveTest.php
• tests/Unit/Api/TimeEntry/ShowTest.php
• tests/Unit/Api/TimeEntryActivity/FromHttpClientTest.php
• tests/Unit/Api/TimeEntryActivity/ListTest.php
• tests/Unit/Api/TimeEntryActivityTest.php
• tests/Unit/Api/TimeEntryTest.php
• tests/Unit/Api/Tracker/FromHttpClientTest.php
• tests/Unit/Api/Tracker/ListTest.php
• tests/Unit/Api/TrackerTest.php
• tests/Unit/Api/User/CreateTest.php
• tests/Unit/Api/User/FromHttpClientTest.php
• tests/Unit/Api/User/ListTest.php
• tests/Unit/Api/User/RemoveTest.php
• tests/Unit/Api/User/ShowTest.php
• tests/Unit/Api/UserTest.php
• tests/Unit/Api/Version/CreateTest.php
• tests/Unit/Api/Version/FromHttpClientTest.php
• tests/Unit/Api/Version/ListByProjectTest.php
• tests/Unit/Api/Version/RemoveTest.php
• tests/Unit/Api/Version/ShowTest.php
• tests/Unit/Api/Version/UpdateTest.php
• tests/Unit/Api/VersionTest.php
• tests/Unit/Api/Wiki/FromHttpClientTest.php
• tests/Unit/Api/Wiki/ListByProjectTest.php
• tests/Unit/Api/Wiki/ShowTest.php
• tests/Unit/Api/WikiTest.php
• tests/Unit/Client/NativeCurlClient/RequestTest.php
• tests/Unit/Client/Psr18Client/RequestTest.php
• tests/Unit/Client/Psr18ClientTest.php
• tests/Unit/Exception/ClientExceptionTest.php
• tests/Unit/Exception/InvalidApiNameExceptionTest.php
• tests/Unit/Exception/InvalidParameterExceptionTest.php
• tests/Unit/Exception/MissingParameterExceptionTest.php

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Narrowing catch from Throwable to Exception may let TypeError escape uncaught.

With declare(strict_types=1), type mismatches now throw TypeError which extends Error, not Exception. This catch block will no longer capture:

• TypeError from strict type violations during XML processing
• Other Error subclasses that were previously caught

Call sites like AbstractApi::getResponseAsArray() and various API create()/update() methods expect SerializerException but will now receive raw TypeError if caller-provided data has incorrect types.

Proposed fix: Keep catching Throwable

-        } catch (Exception $e) {
+        } catch (\Throwable $e) {

Apply to both catch blocks at lines 80 and 129.

Also applies to: 129-129

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Redmine/Serializer/XmlSerializer.php` at line 80, The catch blocks in
XmlSerializer (the catch clauses currently written as "catch (Exception $e)")
are too narrow and will let TypeError/other Error subclasses escape under
strict_types; update both catch clauses in the XmlSerializer class (the blocks
that currently catch Exception and wrap/rethrow as SerializerException) to catch
Throwable instead, preserving the existing handling logic (wrap/throw
SerializerException with the same message/context) so TypeError and other Errors
are captured and converted to SerializerException as callers expect.

[Art4]

I want to catch only Exceptions thrown by SimpleXMLElement, not TypeError

[coderabbitai]

@Art4, understood — that's a valid and intentional design choice. Since deserialize() is typed as string, any TypeError from a wrong argument type would be thrown at the call boundary (before entering the try block), not by SimpleXMLElement itself. The constructor only throws Exception-derived errors for malformed XML content, so catching Exception is both correct and sufficient here. I'll dismiss my concern.

---

✏️ Learnings added

Learnt from: Art4
Repo: kbsali/php-redmine-api PR: 474
File: src/Redmine/Serializer/XmlSerializer.php:80-80
Timestamp: 2026-04-10T22:04:24.727Z
Learning: In `src/Redmine/Serializer/XmlSerializer.php` (php-redmine-api), the catch blocks around `new SimpleXMLElement(...)` intentionally catch `Exception` (not `Throwable`). `TypeError` is expected to surface at the call boundary due to strict type hints, not inside the `SimpleXMLElement` constructor. The maintainer does not want `TypeError` swallowed and wrapped in `SerializerException`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[coderabbitai]

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 3 file(s) based on 3 unresolved review comments.

A stacked PR containing fixes has been created.

• Stacked PR: #475
• Files modified:

• src/Redmine/Api/CustomField.php
• src/Redmine/Api/Search.php
• src/Redmine/Serializer/XmlSerializer.php

Time taken: 2m 12s

[coderabbitai]

🧹 Nitpick comments (1)

tests/Unit/Api/RoleTest.php (1)

258-259: Consider splitting this into a separate test case for clarity.

The test now verifies two different scenarios: listing(true) and listing(1). While this validates that the $forceUpdate parameter correctly handles truthy int values (important given the untyped parameter in Role::listing()), mixing these cases in one test method slightly obscures the intent.

If this is intentionally testing backward compatibility for non-bool inputs, a dedicated test method like testListingAcceptsIntAsForceUpdate() would make the purpose clearer.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/Unit/Api/RoleTest.php` around lines 258 - 259, The current test mixes
two scenarios (calling Role::listing() with a boolean and with an int) which
reduces clarity; extract the assertion that calls $api->listing(1) into its own
test method (e.g. testListingAcceptsIntAsForceUpdate) that sets up the same
expected return and invokes listing(1) (retaining the // `@phpstan-ignore` comment
if needed), and leave the original test to only assert listing(true); ensure
both tests use the same mock/setup for $api and $expectedReturn so behavior is
preserved.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/Unit/Api/RoleTest.php`:
- Around line 258-259: The current test mixes two scenarios (calling
Role::listing() with a boolean and with an int) which reduces clarity; extract
the assertion that calls $api->listing(1) into its own test method (e.g.
testListingAcceptsIntAsForceUpdate) that sets up the same expected return and
invokes listing(1) (retaining the // `@phpstan-ignore` comment if needed), and
leave the original test to only assert listing(true); ensure both tests use the
same mock/setup for $api and $expectedReturn so behavior is preserved.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 004e6551-2925-4c29-953e-2c055c2024d7

📥 Commits

Reviewing files that changed from the base of the PR and between ec15698 and 47748bd.

📒 Files selected for processing (17)

• src/Redmine/Api/CustomField.php
• src/Redmine/Api/IssueCategory.php
• src/Redmine/Api/IssueStatus.php
• src/Redmine/Api/Project.php
• src/Redmine/Api/Tracker.php
• src/Redmine/Api/User.php
• src/Redmine/Api/Version.php
• tests/Unit/Api/CustomFieldTest.php
• tests/Unit/Api/GroupTest.php
• tests/Unit/Api/IssueCategoryTest.php
• tests/Unit/Api/IssueStatusTest.php
• tests/Unit/Api/ProjectTest.php
• tests/Unit/Api/RoleTest.php
• tests/Unit/Api/TimeEntryActivityTest.php
• tests/Unit/Api/TrackerTest.php
• tests/Unit/Api/UserTest.php
• tests/Unit/Api/VersionTest.php

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

• tests/Unit/Api/IssueStatusTest.php
• tests/Unit/Api/TimeEntryActivityTest.php

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

• tests/Unit/Api/ProjectTest.php
• src/Redmine/Api/Project.php
• tests/Unit/Api/CustomFieldTest.php
• tests/Unit/Api/VersionTest.php
• src/Redmine/Api/CustomField.php
• tests/Unit/Api/IssueCategoryTest.php
• tests/Unit/Api/TrackerTest.php

[kbsali]

👍