Skip to content

Commit 95e3abb

Browse files
committed
fix(flow-php/types): array type detection never returns a type that rejects its own input
- scalar values mixed with arrays no longer count as homogeneous lists/maps; they detect as array<mixed> - valueType() widens to array<mixed> instead of dropping types by order ([] still keeps a list/map element type)
1 parent d2222fd commit 95e3abb

73 files changed

Lines changed: 990 additions & 654 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

documentation/adrs/extension-points.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ not provide any help or support for those cases.
6060
- more predictable behavior
6161
- reduced cost of maintaining backward compatibility
6262
- easier to find an actual extension points
63-
- impossible to mock classes that arent marked as `final` in tests (which is a good thing, users shouldnt mock Flow classes in their test suites)
63+
- impossible to mock classes that aren't marked as `final` in tests (which is a good thing, users shouldn't mock Flow classes in their test suites)
6464

6565
## Alternatives Considered (optional)
6666
---

documentation/adrs/schema-immutability.md

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Date: 2026-07-27
88
## Context
99
---
1010

11-
`Schema` was mutable `add()`, `remove()`, `rename()`, `merge()` and every other mutator rewrote
11+
`Schema` was mutable - `add()`, `remove()`, `rename()`, `merge()` and every other mutator rewrote
1212
`$this->definitions` and returned `$this`. `Definition::addMetadata()` and `Definition::setMetadata()` did the same
1313
with `$this->metadata`.
1414

@@ -18,16 +18,16 @@ undeclared row keys, so a column that should materialize in rows must be present
1818
storing the caller's `Schema` therefore wrote every internal extension into the caller's object, producing three
1919
observable defects (#2536 regression):
2020

21-
1. **Caller schema pollution** a `Schema` the user holds for other purposes gains non-nullable columns it never
21+
1. **Caller schema pollution** - a `Schema` the user holds for other purposes gains non-nullable columns it never
2222
declared.
23-
2. **Extractor-lifetime pollution** columns added during one `extract()` run persist into subsequent runs.
24-
3. **Cross-stream pollution** partition columns of one stream leak into the next, and
23+
2. **Extractor-lifetime pollution** - columns added during one `extract()` run persist into subsequent runs.
24+
3. **Cross-stream pollution** - partition columns of one stream leak into the next, and
2525
`Hydrator::cast(fillMissing: true)` injects `null` into a non-nullable definition.
2626

2727
The first fix cloned: `withSchema()` stored `clone $schema`, and extractors cloned again per run and per stream.
2828
That fix was incomplete. `clone` is shallow and `Schema`'s only state is `array<string, Definition>`, so a cloned
2929
`Schema` **shares its `Definition` instances**. `Schema::addMetadata()` / `setMetadata()` reached into a shared
30-
`Definition` and mutated it in place, so metadata writes aliased through every copy including the caller's.
30+
`Definition` and mutated it in place, so metadata writes aliased through every copy - including the caller's.
3131

3232
The same mutable contract left latent aliasing traps elsewhere: `Rows::schema()` seeded its merge loop with row 0's
3333
*memoized* `Schema` and corrupted it, `FloeStreamWriter` retained a caller-owned `Schema` for the lifetime of a
@@ -39,23 +39,23 @@ write session, and `merge()`'s fast paths returned `$this` or the argument.
3939
**`Schema` and its whole state chain are immutable. Every mutator returns a new instance; nothing is ever written
4040
in place.**
4141

42-
- `Schema` a `final readonly class`. All 19 mutators return `new self(...)`; `setDefinitions()` is the
42+
- `Schema` - a `final readonly class`. All 19 mutators return `new self(...)`; `setDefinitions()` is the
4343
constructor's validation helper and is called from the constructor only.
44-
- `Definition` (19 implementations) each a `final readonly class`. `addMetadata()` and `setMetadata()` return a
44+
- `Definition` (19 implementations) - each a `final readonly class`. `addMetadata()` and `setMetadata()` return a
4545
per-class `new self(...)`, matching the idiom `makeNullable()` and `rename()` already used.
46-
- `Metadata` already a `final readonly class`.
46+
- `Metadata` - already a `final readonly class`.
4747

4848
Immutability is declared at the class level, not per property: a `readonly class` cannot gain a writable property
4949
later, so the guarantee survives future edits instead of depending on whoever adds property number 20 remembering
5050
the rule. It is compiler-enforced, not convention. Extractors and the DSL hold caller-provided `Schema` instances
51-
directly: there is nothing to clone because there is nothing to mutate. Sharing an instance `merge()`'s fast
52-
paths, a retained base `Definition` in the hydrator, `Rows`' memoized schema is safe by construction.
51+
directly: there is nothing to clone because there is nothing to mutate. Sharing an instance - `merge()`'s fast
52+
paths, a retained base `Definition` in the hydrator, `Rows`' memoized schema - is safe by construction.
5353

5454
DSL `from_*()` functions stay pure delegation.
5555

5656
### Out of scope
5757

58-
`EntryReference` remains mutable `as()`, `asc()` and `desc()` write `$alias` / `$sort` on `$this`. It is shared
58+
`EntryReference` remains mutable - `as()`, `asc()` and `desc()` write `$alias` / `$sort` on `$this`. It is shared
5959
with the entire expression DSL, so making it immutable is a separate project and is not attempted here.
6060

6161
### Breaking change
@@ -73,7 +73,7 @@ $schema = $schema->add(str_schema('x')); // correct
7373

7474
**Advantages:**
7575

76-
- **The bug class is gone**, not patched aliasing is impossible because there is no writable state to alias.
76+
- **The bug class is gone**, not patched - aliasing is impossible because there is no writable state to alias.
7777
- **Compiler-enforced**: a `readonly` violation is a fatal error, not a convention a new extractor can forget.
7878
- **Sharing becomes free**: no defensive clones in extractors, `PhpRowHydrator`, or the native hydrator.
7979
- Fixes the `Rows::schema()` and `FloeStreamWriter` aliasing traps without touching either.
@@ -92,8 +92,8 @@ $schema = $schema->add(str_schema('x')); // correct
9292
Every `withSchema()` stores `clone $schema`; extractors clone again per run and per stream.
9393

9494
**Rejected because:** the clone is shallow, so `Definition` instances stay shared and metadata mutations alias
95-
through every copy anyway. It is also convention rather than a compiler-enforced rule a new extractor can forget
96-
to clone and it leaves unobservable dead clones in extractors that never extend the schema.
95+
through every copy anyway. It is also convention rather than a compiler-enforced rule - a new extractor can forget
96+
to clone - and it leaves unobservable dead clones in extractors that never extend the schema.
9797

9898
### 2. Deep `Schema::__clone()` + `Definition::__clone()`
9999

@@ -105,7 +105,7 @@ also makes every clone more expensive without removing the need to remember to c
105105

106106
### 3. Materialize auto-added columns post-hydration via `Row::add()` (Floe style)
107107

108-
**Rejected because:** the extended schema *is* the hydrator's instruction set `Hydrator::cast()` drops undeclared
108+
**Rejected because:** the extended schema *is* the hydrator's instruction set - `Hydrator::cast()` drops undeclared
109109
row keys, and the `findDefinition()` guard lets a user-declared partition column keep its user-defined type.
110110
Post-hydration adds would bypass both.
111111

documentation/components/adapters/avro.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
[DOC_LINK:/documentation/introduction.md]
44

5-
- [➡️ Installation](/documentation/installation/packages/etl-adapter-avro.md)
5+
- [Installation](/documentation/installation/packages/etl-adapter-avro.md)
66

77
Avro integration was temporarily abandoned due to the lack of availability of good libraries for PHP.
88
If you are interested in this integration, please let us know by creating an issue in the repository.

documentation/components/adapters/http.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,16 +97,16 @@ data_frame()
9797
->run();
9898
```
9999

100-
This walks `?per_page=100&page=1,2,3` until the `items` path of a response comes back empty.
100+
This walks `?per_page=100&page=1,2,3...` until the `items` path of a response comes back empty.
101101

102102
### Supported strategies
103103

104104
```php
105-
// page number - ?per_page=100&page=1,2,3 default stop: empty page at records_path
105+
// page number - ?per_page=100&page=1,2,3... default stop: empty page at records_path
106106
http_pagination_page_number(http_request_option_query('page'),
107107
page_size: 100, size_option: http_request_option_query('per_page'), records_path: 'items');
108108

109-
// offset / limit - ?limit=100&offset=0,100,200 default stop: total_path reached, else empty page
109+
// offset / limit - ?limit=100&offset=0,100,200... default stop: total_path reached, else empty page
110110
http_pagination_offset(http_request_option_query('offset'), http_request_option_query('limit'),
111111
limit: 100, total_path: 'meta.total');
112112

documentation/components/adapters/postgresql.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -438,8 +438,8 @@ Two helpers convert between a Flow `Schema` and a PostgreSQL table definition:
438438
- `pgsql_table_to_flow_schema()` turns a `Flow\PostgreSql\Schema\Table` back into a Flow `Schema`.
439439

440440
Column types are resolved through the shared `EntryTypesMap` (Flow type → PostgreSQL column type), and per-column
441-
details primary keys, unique constraints, indexes, length, precision/scale, defaults, identity, generated columns,
442-
and explicit type overrides are driven by `PostgreSqlMetadata` entries attached to each schema definition.
441+
details - primary keys, unique constraints, indexes, length, precision/scale, defaults, identity, generated columns,
442+
and explicit type overrides - are driven by `PostgreSqlMetadata` entries attached to each schema definition.
443443

444444
### Creating a Table from a Flow Schema
445445

@@ -512,7 +512,7 @@ expressed by attaching the same name to several definitions.
512512

513513
For composite indexes and unique constraints, column order matters. By default columns are ordered the way they appear
514514
in the schema. Pass an explicit `$position` (ascending, lower first) to `index()` / `indexUnique()` to control the
515-
order independently of schema field order useful, for example, for a keyset-pagination index where the leading
515+
order independently of schema field order - useful, for example, for a keyset-pagination index where the leading
516516
column must serve `ORDER BY`:
517517

518518
```php
@@ -533,7 +533,7 @@ $table = to_pgsql_schema_table(
533533
// => CREATE INDEX orders_created_at_id_idx ON public.orders (created_at, id)
534534
```
535535

536-
Columns without an explicit position default to the end (`PHP_INT_MAX`), and schema order breaks ties so leaving
536+
Columns without an explicit position default to the end (`PHP_INT_MAX`), and schema order breaks ties - so leaving
537537
positions off keeps the schema-order behavior.
538538

539539
#### A Column in Multiple Indexes
@@ -549,7 +549,7 @@ $schema = schema(
549549
);
550550
```
551551

552-
> Index and unique-constraint names must not contain a colon (`:`) it is reserved internally as the name/position
552+
> Index and unique-constraint names must not contain a colon (`:`) - it is reserved internally as the name/position
553553
> separator and passing one throws an `InvalidArgumentException`.
554554
555555
### Reading a Flow Schema back from a Table

documentation/components/bridges/filesystem-async-aws-bridge.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ $aws = aws_s3_filesystem(
2828
$fstab = fstab($aws);
2929
```
3030

31-
The **mount protocol** the URI scheme under which the filesystem is registered in the
32-
`FilesystemTable` defaults to `'aws-s3'`. Override by passing a fourth argument (e.g.
31+
The **mount protocol** - the URI scheme under which the filesystem is registered in the
32+
`FilesystemTable` - defaults to `'aws-s3'`. Override by passing a fourth argument (e.g.
3333
`aws_s3_filesystem($bucket, $client, options: new Options(), protocol: 'warehouse')`) when you
3434
need to mount the same bucket twice under distinct names or pick a scheme more meaningful to
3535
your application.
@@ -59,5 +59,5 @@ data_frame($config)
5959
```
6060

6161
`FileStatus` values returned from `list()` and `status()` carry `size` (from S3 `Size` / `ContentLength`)
62-
and `lastModifiedAt` (from `LastModified`) populated directly from the S3 response no extra HEAD call
62+
and `lastModifiedAt` (from `LastModified`) populated directly from the S3 response - no extra HEAD call
6363
is issued when the CLI `flow:filesystem:ls --long` or `flow:filesystem:stat` prints them.

documentation/components/bridges/filesystem-azure-bridge.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ $sdk = azure_blob_service(
5151
To use the Azure Blob filesystem with Flow, you need to mount the filesystem to the configuration.
5252
This operation will mount the Azure Blob filesystem to the fstab instance available in the DataFrame runtime.
5353

54-
The **mount protocol** the URI scheme under which the filesystem is registered in the
55-
`FilesystemTable` defaults to `'azure-blob'`. Override via the third argument
54+
The **mount protocol** - the URI scheme under which the filesystem is registered in the
55+
`FilesystemTable` - defaults to `'azure-blob'`. Override via the third argument
5656
(`azure_filesystem($blobService, $options, protocol: 'warehouse')`) when you need to mount the same
5757
container under a different scheme.
5858

@@ -81,5 +81,5 @@ data_frame($config)
8181

8282
`FileStatus` values returned from `list()` and `status()` carry `size` (from the `Content-Length`
8383
header / listing property) and `lastModifiedAt` (from the `Last-Modified` header, parsed as RFC 7231)
84-
populated directly from the Azure response no extra stream is opened when the CLI
84+
populated directly from the Azure response - no extra stream is opened when the CLI
8585
`flow:filesystem:ls --long` or `flow:filesystem:stat` prints them.

documentation/components/bridges/phpstan-types-bridge.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,4 @@ includes:
5959
- vendor/flow-php/phpstan-types-bridge/extension.neon
6060
```
6161

62-
That's it `type_structure()` calls are now narrowed by PHPStan across your codebase.
62+
That's it - `type_structure()` calls are now narrowed by PHPStan across your codebase.

0 commit comments

Comments
 (0)