Skip to content

fix(php): stop re-checking scalar types PHP already enforces#3013

Merged
schani merged 2 commits into
masterfrom
agent/fix-issue-2770
Jul 21, 2026
Merged

fix(php): stop re-checking scalar types PHP already enforces#3013
schani merged 2 commits into
masterfrom
agent/fix-issue-2770

Conversation

@schani

@schani schani commented Jul 20, 2026

Copy link
Copy Markdown
Member

Bug

quicktype's PHP output generates a validate<Property>() static method for
every property. For primitive-typed properties (bool, int, float,
string), the generated body contains runtime is_bool/is_integer/
is_float/is_int/is_string guards that can never fire, because PHP
itself already enforces the parameter's type hint (e.g. ?float $value)
before the method body ever runs — PHP throws a TypeError first, or, in
non-strict-types mode, coerces the value to match the type. Example (before
the fix):

public static function validateLatitude(?float $value): bool {
    if (!is_null($value)) {
        if (!is_float($value) && !is_int($value)) {
            throw new Exception("Attribute Error:Coordinate::latitude");
        }
    }
    return true;
}

Root cause

emitValidateMethod in packages/quicktype-core/src/language/Php/PhpRenderer.ts
calls phpValidate() unconditionally for the property's declared type. For
scalar (bool/int/float/string) branches, phpValidate emits an
is_* check on the parameter — but the parameter to validate<Property>()
already carries that exact scalar type hint, so the check is dead code that
can never throw.

Fix

phpValidate now accepts a skipPrimitiveTypeCheck flag. The
validate<Property>() call site passes true, so the top-level
bool/int/float/string checks are skipped there (PHP's own type hint already
guarantees them), while validation for nested/recursive shapes — array
elements, union members, transformed strings (e.g. UUID format) — is left
untouched, since those still need runtime checks that PHP's type system
cannot express.

Test coverage

  • test/unit/php-validation.test.ts: generates PHP for a schema with
    boolean/integer/number/nullable-number/string/array/union properties and
    asserts the scalar validate<Property>() bodies no longer contain
    is_bool/is_integer/is_float/is_string checks, while confirming
    checks are still emitted for array elements and union members.
  • test/inputs/json/priority/php-validation.json was added to the PHP
    fixture's JSON inputs (test/languages.ts) so the generated code is also
    exercised end-to-end through the PHP fixture's round-trip test.

Verification

  • npm run build passes.
  • npx vitest run test/unit/php-validation.test.ts passes.
  • Re-ran the original repro (node dist/index.js --lang php --src-lang schema coord.schema.json)
    and confirmed validateLatitude/validateLongitude now generate as:
    public static function validateLatitude(float $value): bool {
        return true;
    }
  • The PHP fixture test (FIXTURE=php script/test) requires a local PHP/
    composer toolchain that isn't available in this environment, so it was not
    run locally; CI will exercise it.

Fixes #2770

🤖 Generated with Claude Code

schani and others added 2 commits July 20, 2026 17:36
Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
Reformat the scalarUnion schema literal to Biome's canonical layout and
move the "method found" assertion out of the validationMethod helper
(noMisplacedAssertion) by throwing instead, so `npm run lint` passes.

Co-Authored-By: Claude <noreply@anthropic.com>
@schani
schani merged commit 37db752 into master Jul 21, 2026
28 checks passed
@schani
schani deleted the agent/fix-issue-2770 branch July 21, 2026 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: php validation code never executes

1 participant