Fix inconsistencies and invalid examples in specification - #155
Conversation
| class Foo extends Bar implements FooInterface | ||
| { | ||
| public function sampleFunction(int $a, int $b = null): array | ||
| public function sampleFunction(int $a, ?int $b = null): array |
There was a problem hiding this comment.
This syntax still works in PHP 8.4, but emits a deprecation notice. Using ?int avoids the notice and follows the specification’s later recommendation for explicit nullable types.
| public function isOdd(): bool | ||
| { | ||
| return $this->value() % 2; | ||
| return $this->value % 2 !== 0; |
There was a problem hiding this comment.
Backed enum values are exposed through the value property, not a value() method. The explicit comparison also makes the expression produce the declared bool under strict_types=1.
| intersections be encased with parentheses. | ||
| Compound types include intersection, union, and mixed intersection and union type declarations. PHP requires | ||
| that mixed intersection and union types be structured as an ORed (unioned) series of ANDs (intersections), | ||
| and that each intersection in such a union be encased with parentheses. |
There was a problem hiding this comment.
A standalone intersection is written A&B, without parentheses. Parentheses are required only when that intersection is one option in a union, as in A|(B&C). The previous wording incorrectly implied that standalone intersections should also be parenthesized.
There was a problem hiding this comment.
Technically the previous text is correct: A single set of ANDs is still technically ORed with the empty set. However, you're correct that the parens are only meaningful in mixed cases, so we'll go with these revisions.
| |\ReflectionProperty $reflect | ||
| ): object|null { | ||
| // ... | ||
| // ... |
There was a problem hiding this comment.
This example is presented as correctly formatted, so its body should use the mandatory four-space indentation defined in section 2.4.
| Import statements MUST never begin with a leading backslash as they | ||
| must always be fully qualified. | ||
| Import statements MUST never begin with a leading backslash, as imported names | ||
| are always resolved from the namespace root. |
There was a problem hiding this comment.
An imported name is resolved from the namespace root by PHP; calling it “fully qualified” is potentially confusing because fully qualified names are normally written with a leading backslash.
| public function __construct( | ||
| public readonly int $x, | ||
| public readonly int $y, | ||
| public readonly int $x, |
There was a problem hiding this comment.
Constructor-promoted parameters are still parameters and therefore use the standard four-space indentation level.
|
|
||
| somefunction($foo, $bar, [ | ||
| // ... | ||
| // ... |
There was a problem hiding this comment.
The array is a single multiline argument rather than a split argument list, but its contents still need the standard four-space indentation.
| ### 5.2 `switch`, `case`, `match` | ||
|
|
||
| A switch structure must follow the rules below: | ||
| A switch structure MUST follow the rules below: |
There was a problem hiding this comment.
The BCP 14 preamble gives normative meaning only to uppercase keywords. Since the detailed switch rules are requirements, this introductory MUST should be normative too.
| $muchLongerArgument, | ||
| ) { | ||
| // ... | ||
| // ... |
There was a problem hiding this comment.
These closure-body comments used three spaces while section 2.4 requires four. The same correction is applied to every example in this group.
There was a problem hiding this comment.
Pull request overview
This PR updates spec.md to correct invalid PHP examples and tighten/clarify several normative style rules so the specification is internally consistent and aligns with PHP language behavior (nullable types, backed enums, mixed union/intersection typing, import resolution, strict types formatting, and multiline layout guidance).
Changes:
- Fix invalid/incorrect PHP examples (nullable default parameter typing; backed-enum value access and odd check; multiple indentation corrections in code snippets).
- Clarify specification language for mixed union/intersection (DNF) parentheses and for how
useimports are resolved (namespace root, no leading\). - Refine normative requirements (strict types declaration formatting scoped correctly;
switchrules made normative; multiline array bracket placement generalized beyond assignments).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Declare statements MUST NOT contain any spaces and MUST be exactly `declare(strict_types=1)` | ||
| (with an optional semicolon terminator). | ||
| Declare statements MUST NOT contain any spaces inside the parentheses. For example: | ||
| `declare(strict_types=1)` (with an optional semicolon terminator). |
There was a problem hiding this comment.
The no-spaces rule applies to every declare statement. The previous wording made declare(strict_types=1) sound like the only permitted declaration, even though it is just an example of the required formatting; other directives such as declare(ticks=1) are also allowed.
9496a79 to
56573ac
Compare
56573ac to
f389c1b
Compare
| When the array declaration is split across multiple lines, the opening bracket | ||
| MUST be placed on the same line as the equals sign. The closing bracket | ||
| When an array declaration is split across multiple lines, the opening bracket | ||
| MUST NOT be placed on its own line. The closing bracket |
There was a problem hiding this comment.
The old wording mentioned an equals sign, but multiline arrays can also appear in arguments, return statements, defaults, attributes, and nested expressions. Requiring the opening bracket not to be on its own line makes the rule applicable in every context.
This fixes several inconsistencies and invalid examples in the specification.
Changes include:
declarespacing rule apply to all directives while presentingstrict_typesas an example