Skip to content

Commit c6a89a3

Browse files
authored
feature: variadic arguments ADR (#2241)
1 parent 369b458 commit c6a89a3

2 files changed

Lines changed: 117 additions & 7 deletions

File tree

documentation/adrs.md

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,23 @@
33
[TOC]
44

55
# What are ADRs?
6-
Architecture Decision Records (ADRs) are structured documents that capture significant architectural decisions made during the development of this project.
6+
7+
Architecture Decision Records (ADRs) are structured documents that capture significant architectural decisions made
8+
during the development of this project.
79

810
Each ADR explains:
9-
- **The context**: Why the decision was necessary.
10-
- **The decision**: What was decided and why.
11-
- **The consequences**: The impact of the decision, both positive and negative.
1211

13-
ADRs act as a single source of truth for important design and architectural choices, ensuring that everyone involved in the project has a shared understanding of why things are the way they are.
12+
- **The context**: Why the decision was necessary.
13+
- **The decision**: What was decided and why.
14+
- **The consequences**: The impact of the decision, both positive and negative.
15+
16+
ADRs act as a single source of truth for important design and architectural choices, ensuring that everyone involved in
17+
the project has a shared understanding of why things are the way they are.
1418

1519
## How to Use ADRs in This Project
16-
**Follow Existing ADRs**: ADRs are mandatory to follow unless explicitly overridden by a new ADR. This ensures consistency in decision-making across the project.
20+
21+
**Follow Existing ADRs**: ADRs are mandatory to follow unless explicitly overridden by a new ADR. This ensures
22+
consistency in decision-making across the project.
1723

1824
> Whenever ADR is merged into the main branch, it is considered accepted and must be followed by all contributors.
1925
@@ -22,13 +28,16 @@ ADRs act as a single source of truth for important design and architectural choi
2228
- If you encounter a situation that requires a significant architectural or design change, you must create a new ADR.
2329
- Use the provided [ADR Template](/documentation/adrs/template.md) to create your proposal.
2430
- Submit the ADR via a pull request and engage in a discussion with maintainers and contributors.
25-
- **Document Decisions Transparently**: All significant decisions must be documented. This includes not only what was decided but also the alternatives that were considered and why they were rejected.
31+
- **Document Decisions Transparently**: All significant decisions must be documented. This includes not only what was
32+
decided but also the alternatives that were considered and why they were rejected.
2633

2734
## Index of ADRs
2835

2936
### [Accepted AD](https://github.com/flow-php/flow/pulls?q=is%3Apr+is%3Aclosed+is%3Amerged+label%3AAD+)
37+
3038
- [2025-01-07: Static Analysis Baseline](/documentation/adrs/static-analysis-baseline.md)
3139
- [2025-01-09: Extension Points](/documentation/adrs/extension-points.md)
40+
- [2026-03-02: Variadic Arguments Pattern for Required Parameters](/documentation/adrs/variadic-arguments-pattern.md)
3241

3342
### [Proposed AD](https://github.com/flow-php/flow/pulls?q=is%3Apr+is%3Aopen+label%3AAD+)
3443

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
# Variadic Arguments Pattern for Required Parameters
2+
3+
[TOC]
4+
5+
Proposed by: @norberttech
6+
Date: 2026-03-02
7+
8+
## Context
9+
---
10+
11+
Several Flow PHP methods accept multiple arguments where at least one is required. Examples include:
12+
- `DataFrame::partitionBy()`
13+
- `Rows::partitionBy()`
14+
- `Window::partitionBy()`
15+
- Type comparison methods
16+
17+
A proposal was made to simplify the API signatures from:
18+
19+
```php
20+
public function partitionBy(string|Reference $entry, string|Reference ...$entries) : self
21+
```
22+
23+
To:
24+
25+
```php
26+
public function partitionBy(string|Reference ...$entries) : self
27+
```
28+
29+
The rationale for the proposal was to reduce array operations like `array_unshift()` used internally to merge the first required argument with the variadic rest.
30+
31+
## Decision
32+
---
33+
34+
**Keep the explicit first argument pattern**
35+
36+
Methods that require at least one argument should use the signature pattern:
37+
38+
```php
39+
public function method(Type $first, Type ...$rest) : ReturnType
40+
```
41+
42+
Rather than:
43+
44+
```php
45+
public function method(Type ...$args) : ReturnType
46+
```
47+
48+
## Pros & Cons
49+
---
50+
51+
**Advantages of the chosen pattern:**
52+
53+
- **Compile-time enforcement**: PHP enforces at least one argument at the language level
54+
- **Static analysis support**: PHPStan, Psalm, and IDEs immediately understand the constraint
55+
- **Better developer experience**: The method signature clearly communicates the requirement without needing to read documentation or encounter runtime exceptions
56+
- **Fail-fast principle**: Errors are caught before execution, not during
57+
58+
**Disadvantages:**
59+
60+
- Internal implementation requires merging arguments: `array_unshift($rest, $first)`
61+
- Slightly more verbose method signatures
62+
- Minor performance overhead from array operations (negligible in practice)
63+
64+
## Alternatives Considered
65+
---
66+
67+
### 1. Pure variadic with runtime validation
68+
69+
```php
70+
public function partitionBy(string|Reference ...$entries) : self
71+
{
72+
if ([] === $entries) {
73+
throw new InvalidArgumentException('At least one entry is required.');
74+
}
75+
// ...
76+
}
77+
```
78+
79+
**Rejected because:** Developers only discover the constraint at runtime, leading to worse developer experience.
80+
81+
### 2. Validation in the References class
82+
83+
```php
84+
final class References
85+
{
86+
public function __construct(string|Reference ...$references)
87+
{
88+
if ([] === $references) {
89+
throw new InvalidArgumentException('References cannot be empty.');
90+
}
91+
// ...
92+
}
93+
}
94+
```
95+
96+
**Rejected because:** While this centralizes validation, it still defers the error to runtime. The explicit first argument pattern catches errors earlier in the development cycle.
97+
98+
## Links and References
99+
---
100+
101+
- [GitHub Issue #2236](https://github.com/flow-php/flow/issues/2236) - Original proposal discussion

0 commit comments

Comments
 (0)