Repository navigation
Buncha updates - #75
Merged
Merged
Conversation
Co-authored-by: dcarbone <1392439+dcarbone@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Array casts convert numeric string object keys to integers across metadata, tagged addresses, arguments, and claim mappings, causing strict string setters to throw TypeErrors for previously supported inputs.
6 open issues
What changed in this PR
This PR modernizes the PHP Consul client's tooling and type handling, and adds nanosecond-preserving timestamp parsing and serialization.
Changes:
- Replaces PHP-CS-Fixer with Rector and raises PHPStan analysis to level 8.
- Upgrades
dcarbone/gotimeto v2 and adds timestamp round-trip coverage. - Refactors response checks, nullable types, collection handling, and tests.
| File | Description |
|---|---|
| tools/rector/composer.lock | Locks Rector dependencies. |
| tools/rector/composer.json | Adds isolated Rector installation. |
| tools/php-cs-fixer/composer.lock | Removes obsolete tool lockfile. |
| tools/php-cs-fixer/composer.json | Removes PHP-CS-Fixer dependencies. |
| tools/.gitignore | Updates ignored tool dependencies. |
| tests/Unit/PreparedQuery/QueryDatacenterOptionsTest.php | Reorders named arguments. |
| tests/Unit/KV/KVPairTest.php | Reorders named arguments. |
| tests/Unit/KV/KVClientTest.php | Reorders named arguments. |
| tests/Unit/ConfigTest.php | Converts environment helpers to instance methods. |
| tests/Unit/Catalog/CatalogServiceTest.php | Reorders named arguments. |
| tests/Unit/Agent/CARootTest.php | Tests nanosecond timestamp round trips. |
| tests/Unit/Agent/AgentServiceTest.php | Reorders named arguments. |
| tests/Unit/Agent/AgentServiceCheckTest.php | Reorders named arguments. |
| tests/Unit/Agent/AgentClientTest.php | Reorders named arguments. |
| tests/Unit/Agent/AgentCheckRegistrationTest.php | Reorders named arguments. |
| tests/Unit/ACL/OIDCAuthMethodConfigTest.php | Reorders named arguments. |
| tests/Integration/Session/SessionClientTest.php | Removes redundant type comment. |
| tests/Integration/Random/ConsulAdditionalClientCoverageTest.php | Removes redundant type comment. |
| tests/Integration/KV/KVClientTest.php | Reorders named arguments. |
| tests/Integration/Health/HealthClientTest.php | Reorders named arguments. |
| tests/Integration/Event/EventClientTest.php | Removes redundant type comment. |
| tests/Integration/Coordinate/CoordinateClientTest.php | Cleans comments and exception names. |
| tests/Integration/Catalog/CatalogClientTest.php | Cleans comments and exception names. |
| tests/Integration/Agent/AgentClientTest.php | Cleans comments and exception names. |
| tests/Integration/ACL/ACLClientTest.php | Converts client helper to instance method. |
| tests/Integration/AbstractIntegrationTestCase.php | Removes redundant type comments. |
| tests/funcs.php | Removes redundant docblock annotations. |
| tests/ConsulManager.php | Removes redundant docblock annotations. |
| src/Txn/TxnClient.php | Uses shared response-error check. |
| src/Txn/NodeTxnOp.php | Simplifies default node initialization. |
| src/Txn/CheckTxnOp.php | Simplifies default check initialization. |
| src/Session/SessionClient.php | Uses shared response-error checks. |
| src/RequestMeta.php | Explicitly implements Stringable. |
| src/QueryOptions.php | Removes redundant collection guard. |
| src/PreparedQuery/PreparedQueryClient.php | Uses shared response-error check. |
| src/PHPLib/Values.php | Simplifies lookups and declares Stringable. |
| src/PHPLib/TaggedAddressField.php | Simplifies initialization and casts collection input. |
| src/PHPLib/ServiceMetaField.php | Simplifies initialization and casts collection input. |
| src/PHPLib/RequestResponse.php | Adds annotated response-error predicate. |
| src/PHPLib/Request.php | Marks internal fields readonly. |
| src/PHPLib/NodeMetaField.php | Simplifies initialization and casts collection input. |
| src/PHPLib/MetaField.php | Simplifies metadata initialization. |
| src/PHPLib/MapResponse.php | Refactors collection validation and iteration. |
| src/PHPLib/funcs.php | Preserves nanoseconds when parsing timestamps. |
| src/PHPLib/Error.php | Handles missing responses and adds readonly fields. |
| src/PHPLib/DecodedBody.php | Removes redundant property default. |
| src/PHPLib/AbstractType.php | Explicitly implements Stringable. |
| src/PHPLib/AbstractClient.php | Updates error checks and bounds decode depth. |
| src/Peering/PeeringClient.php | Uses shared response-error checks. |
| src/Operator/ServerHealth.php | Adds fallback for unparseable timestamps. |
| src/Operator/OperatorRaftConfigurationResponse.php | Initializes nullable response value. |
| src/Operator/OperatorHealthReplyResponse.php | Initializes nullable response value. |
| src/Operator/OperatorClient.php | Uses shared response-error checks. |
| src/Operator/OperatorAutopilotConfigurationResponse.php | Initializes nullable response value. |
| src/Operator/AutopilotStateResponse.php | Initializes nullable response value. |
| src/Operator/AutopilotServer.php | Adds fallback for unparseable timestamps. |
| src/KV/KVPairs.php | Normalizes variadic input indexes. |
| src/KV/KVClient.php | Uses shared response-error checks. |
| src/HttpAuth.php | Explicitly implements Stringable. |
| src/Health/ServiceEntry.php | Simplifies default checks initialization. |
| src/Health/HealthChecks.php | Simplifies status aggregation control flow. |
| src/Debug/DebugClient.php | Uses shared response-error check. |
| src/Coordinate/Coordinate.php | Refactors helpers and adds analysis annotation. |
| src/Consul.php | Casts token-file contents before trimming. |
| src/ConfigEntry/TerminatingGatewayConfigEntry.php | Removes redundant variadic annotation. |
| src/ConfigEntry/SourceIntention.php | Preserves timestamp precision in serialization. |
| src/ConfigEntry/ServiceResolverConfigEntry.php | Simplifies collection initialization. |
| src/ConfigEntry/ProxyConfigEntry.php | Refactors constructor and handles nullable mode. |
| src/ConfigEntry/MeshGatewayConfig.php | Simplifies serialization return. |
| src/ConfigEntry/LoadBalancer.php | Corrects nullable getter type. |
| src/ConfigEntry/IngressGatewayConfigEntry.php | Removes redundant variadic annotation. |
| src/ConfigEntry/HTTPHeaderModifiers.php | Casts collection inputs before iteration. |
| src/ConfigEntry/EnvoyExtension.php | Refactors argument collection handling. |
| src/Config.php | Corrects nullable getters and simplifies resolution. |
| src/Catalog/CatalogService.php | Simplifies tagged-address initialization. |
| src/Catalog/CatalogRegistration.php | Corrects nullable checks getter. |
| src/Catalog/CatalogNodeServiceList.php | Removes redundant parameter annotation. |
| src/Agent/Upstream.php | Casts configuration input before iteration. |
| src/Agent/SampledValue.php | Casts label input before iteration. |
| src/Agent/LeafCert.php | Preserves certificate timestamp precision. |
| src/Agent/GaugeValue.php | Casts label input before iteration. |
| src/Agent/ConnectProxyConfig.php | Casts configuration input before iteration. |
| src/Agent/CARootList.php | Removes redundant variadic annotation. |
| src/Agent/CARoot.php | Preserves root-certificate timestamp precision. |
| src/Agent/AgentServicesResponse.php | Refactors service-map iteration. |
| src/Agent/AgentServiceRegistration.php | Simplifies tagged-address initialization. |
| src/Agent/AgentServiceConnectProxyConfig.php | Casts configuration input before iteration. |
| src/Agent/AgentServiceCheck.php | Accepts object-form constructor headers. |
| src/Agent/AgentMember.php | Refactors tag iteration and removes annotation. |
| src/Agent/AgentHealthServicesResponse.php | Removes redundant docblock annotations. |
| src/Agent/AgentClient.php | Uses shared response-error checks. |
| src/Agent/AgentChecksResponse.php | Refactors check-map iteration. |
| src/ACL/OIDCAuthMethodConfig.php | Casts claim-mapping inputs before iteration. |
| src/ACL/ACLTokenListEntry.php | Preserves token timestamp precision. |
| src/ACL/ACLTokenFields.php | Narrows hydration type and preserves timestamp precision. |
| src/ACL/ACLReplicationStatus.php | Updates timestamp fallback and serialization. |
| src/ACL/ACLAuthMethod.php | Casts configuration input before iteration. |
| rector.php | Configures refactoring rules and exclusions. |
| phpstan.neon | Raises analysis level to 8. |
| composer.lock | Locks gotime v2 upgrade. |
| composer.json | Upgrades gotime and replaces formatter script. |
| .php-cs-fixer.php | Removes obsolete formatter configuration. |
| .gitignore | Removes obsolete formatter cache exclusions. |
| .github/workflows/tests.yaml | Installs and checks Rector in CI. |
| .github/copilot-instructions.md | Documents required Rector runs. |
| .codex/config.toml | Configures local PhpStorm MCP endpoint. |
Review effort: Balanced
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unsigned index validation rejects valid high-bit event IDs, breaking blocking queries, and case-sensitive header checks can overwrite explicitly supplied content types.
2 open issues
6 resolved since last review
Review effort: Balanced
Owner
Author
|
@copilot Fix the code for all comments in this review thread. When a review comment includes a suggested change, apply the suggestion exactly. Do not make changes beyond what is described in the linked review thread. Add unit tests proving issue and efficacy of fix. |
Co-authored-by: dcarbone <1392439+dcarbone@users.noreply.github.com>
Co-authored-by: dcarbone <1392439+dcarbone@users.noreply.github.com>
Co-authored-by: dcarbone <1392439+dcarbone@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The header factory returns the wrong type and triggers a TypeError, while header-name collisions and incorrect SSL and 32-bit test expectations introduce additional failures.
4 open issues
2 resolved since last review
Review effort: Balanced
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Header normalization replaces underscores with hyphens, causing distinct names such as X_Custom and X-Custom to collide, lose values, and be emitted under the wrong name.
No open issues 🎉
4 resolved since last review
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.