Skip to content

Comments

Simplify AutoCaster#1920

Closed
stloyd wants to merge 1 commit intoflow-php:1.xfrom
stloyd:simplify-auto-caster
Closed

Simplify AutoCaster#1920
stloyd wants to merge 1 commit intoflow-php:1.xfrom
stloyd:simplify-auto-caster

Conversation

@stloyd
Copy link
Member

@stloyd stloyd commented Oct 28, 2025

While working on #1917, I have noticed that AutoCaster always gets pre-converted data (array instead of string json, \DOMDocument instead of string XML), so validating if the given value is scalar with logic type inside is not needed. This also removes the check for the array key, as it was never used outside of the loop.

Change Log


Added

Fixed

Changed

  • Simplify `AutoCaster`

Removed

Deprecated

Security

@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.30%. Comparing base (4cce83f) to head (1dbcc60).
⚠️ Report is 5 commits behind head on 1.x.

Additional details and impacted files
@@            Coverage Diff             @@
##              1.x    #1920      +/-   ##
==========================================
- Coverage   77.31%   77.30%   -0.01%     
==========================================
  Files         828      828              
  Lines       24569    24559      -10     
==========================================
- Hits        18995    18986       -9     
+ Misses       5574     5573       -1     
Components Coverage Δ
etl 88.80% <ø> (ø)
cli 85.96% <ø> (ø)
lib-array-dot 95.00% <ø> (ø)
lib-azure-sdk 60.39% <ø> (ø)
lib-doctrine-dbal-bulk 95.14% <ø> (ø)
lib-filesystem 80.08% <ø> (ø)
lib-types 53.32% <100.00%> (-0.18%) ⬇️
lib-parquet 68.34% <ø> (ø)
lib-parquet-viewer 83.04% <ø> (ø)
lib-snappy 89.71% <ø> (-0.47%) ⬇️
bridge-filesystem-async-aws 90.16% <ø> (ø)
bridge-filesystem-azure 89.47% <ø> (ø)
bridge-monolog-http 96.91% <ø> (ø)
bridge-openapi-specification 94.50% <ø> (ø)
symfony-http-foundation 73.17% <ø> (ø)
adapter-chartjs 86.36% <ø> (ø)
adapter-csv 88.46% <ø> (ø)
adapter-doctrine 90.97% <ø> (ø)
adapter-elasticsearch 97.17% <ø> (ø)
adapter-google-sheet 91.40% <ø> (ø)
adapter-http 60.56% <ø> (ø)
adapter-json 87.74% <ø> (ø)
adapter-logger 83.33% <ø> (ø)
adapter-meilisearch 97.87% <ø> (ø)
adapter-parquet 78.15% <ø> (ø)
adapter-text 82.92% <ø> (ø)
adapter-xml 82.50% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@stloyd stloyd marked this pull request as ready for review October 28, 2025 07:17
@stloyd stloyd requested a review from norberttech as a code owner October 28, 2025 07:17

if ($typeChecker->isUuid()) {
return type_uuid()->cast($value);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think those two can be removed, it seems that we are missing tests for them but by removing those lines following code will fail, breaking backward compatibility of caster:

<?php

declare(strict_types=1);

use Flow\Types\Type\AutoCaster;
use Symfony\Component\Uid\Uuid;

require_once __DIR__ . '/vendor/autoload.php';

$output = (new AutoCaster())->cast(Uuid::v7()->toString());

dd($output); // this should be Uuid instance, not string 

@@ -36,24 +34,15 @@ public function cast(mixed $value) : mixed
*/
private function castArray(array $value) : array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm I think I messed up a bit this castArray.

The only thing it does now is that it narrows array types.

For example when we have array [1, 2, 3.0] in which we have int's and float, it will narrow the type to float as all int can be represented by float however I don't think this should be responsibility of a class called AutoCaster 😅
Probably I was trying to fix some bug and I did it here without properly thinking this through 🤦‍♂️

I think this is much bigger issue that will need to be solved in separated PR (probably will require some more explicit array types narrowing mechanism) but for now removing key types makes sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stloyd stloyd closed this Oct 28, 2025
@stloyd stloyd deleted the simplify-auto-caster branch October 28, 2025 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants