Skip to content

Conversation

@N1ebieski
Copy link
Contributor

Before:

obraz obraz obraz

After:

obraz obraz obraz

This feature needs more testing, especially in large projects with many scope methods using union/intersect type parameters.

@N1ebieski N1ebieski marked this pull request as ready for review November 11, 2025 13:28
@TitasGailius
Copy link
Collaborator

I pushed a few changes to this PR.

The main remaining bit I'm not 100% sure about is how we should generate docblocks for parameters with new … default values, e.g.:

#[Scope]
public function withObject(Builder $query, Stringable $stringable = new Stringable(''))
{
    //
}

Ideally, we'd emit something like:

/**
 * withObject(\Illuminate\Support\Stringable $stringable = new \Illuminate\Support\Stringable(''))
 */

…but doing that accurately would require parsing the source text which I'd rather avoid for now.

So the current implementation generates:

/**
 * withObject(\Illuminate\Support\Stringable $stringable = new \Illuminate\Support\Stringable(...))
 */

Alternatively, we could drop the (...) entirely:

/**
 * withObject(\Illuminate\Support\Stringable $stringable = new \Illuminate\Support\Stringable)
 */

Aside from that, I think this is good to go.

@N1ebieski
Copy link
Contributor Author

@TitasGailius I suggest the option without (...) because this:

obraz

@TitasGailius
Copy link
Collaborator

@N1ebieski both options show the same thing in VS Code, because I think docblocks like this aren't properly supported yet. That said, I agree that omitting (...) makes more sense since that's valid PHP syntax, whereas the other option would be invalid in PHP.

@N1ebieski
Copy link
Contributor Author

N1ebieski commented Dec 23, 2025

@TitasGailius Without new looks decent...

obraz

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