Skip to content

Preserve empty-string env() defaults and support nested function-calls - #41

Merged
JohnathonKoster merged 1 commit into
Stillat:masterfrom
edalzell:fix/function-writer-scalar-args
Jul 19, 2026
Merged

Preserve empty-string env() defaults and support nested function-calls#41
JohnathonKoster merged 1 commit into
Stillat:masterfrom
edalzell:fix/function-writer-scalar-args

Conversation

@edalzell

@edalzell edalzell commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Problem

FunctionWriter::convertToArgs() silently drops any argument that's an empty string, treating it the same as an omitted one:

$f->env('SOME_PATH', '');
// renders as: env('SOME_PATH')   — the '' default is gone

That's indistinguishable from actually wanting env('SOME_PATH'), so there was no way to write a genuinely-empty default via the fluent API.

Separately, every argument was coerced through gettype()-based scalar conversion, which breaks the moment an argument is itself a built expression rather than a plain scalar:

$f->env('CONTENT_PATH', $f->basePath('content'));
// throws / mis-renders — there's no way to nest one function-call builder result inside another

That meant you couldn't build something like env('CONTENT_PATH', base_path('content')) through the fluent API at all — you had to drop into raw PhpParser node construction by hand.

Solution

  • convertToArgs() now only treats an explicit null as "omit this argument" — empty strings pass through as real values.
  • FunctionWriter::arg() now checks $arg instanceof Expr first and passes it straight through, instead of trying to scalar-convert it. This is what makes nesting work: $f->basePath('content') already returns a FuncCall (an Expr), so it now flows straight into the outer env() call's args.
  • The single-arg path helpers (resourcePath, storagePath, basePath, etc.) now default to null instead of requiring an argument. That preserves the "call with no arguments" result (f()->resourcePath()resource_path()) — previously the only way to get that was to pass '', which is exactly the behavior being fixed.

Testing

Three new cases in LaravelFunctionCallTest: an empty-string default is preserved, a path helper called with no arguments renders as a bare call, and a basePath() call nested as an env() default renders correctly. All pre-existing tests still pass (2 unrelated, pre-existing closure-formatting failures aside, confirmed present before this change too).

… args

FunctionWriter::convertToArgs() dropped any argument that was an empty
string (mb_strlen($arg) === 0), treating it the same as an omitted
argument. That meant f()->env('KEY', '') silently rendered as
env('KEY') instead of env('KEY', ''), losing an intentionally-empty
default.

It also unconditionally coerced every argument through gettype()-based
scalar conversion, which broke for arguments that are themselves an
Expr (e.g. building env('X', base_path('content')) via
f()->env('X', f()->basePath('content'))) - there was no way to nest
one function-call builder result inside another.

Fixes:
- Only treat an explicit null as "omit this argument"; empty strings
  are now passed through as real Arg values.
- FunctionWriter::arg() now passes an Expr argument straight through
  instead of trying to scalar-convert it, enabling nested calls.
- The single-arg path helpers (resourcePath, storagePath, etc.) now
  default to null so `f()->resourcePath()` still produces a bare
  resource_path() call - the same result the old empty-string
  behavior gave you, but as an explicit "no argument" rather than a
  special-cased empty string.
@edalzell edalzell changed the title Preserve empty-string env() defaults and support nested function-call… Preserve empty-string env() defaults and support nested function-calls Jul 14, 2026
@JohnathonKoster
JohnathonKoster merged commit 4cf2162 into Stillat:master Jul 19, 2026
13 checks passed
@edalzell
edalzell deleted the fix/function-writer-scalar-args branch July 19, 2026 19:29
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.

2 participants