Skip to content

fix: PHPStan errors #293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
taka-oyama merged 1 commit into from
Nov 27, 2025
Merged

fix: PHPStan errors #293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions phpstan.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ parameters:
count: 1
path: src/Query/Processor.php
-
message: '#^Closure invoked with 2 parameters, 3 required\.$#'
identifier: arguments.count
count: 1
path: src/Schema/Builder.php
message: '#^Parameter \#1 \$pdo of method Illuminate\\Database\\Connection\:\:__construct\(\) expects \(Closure\(\)\: PDO\)\|PDO, Closure\(\)\: null given\.$#'
identifier: argument.type
count: 1
path: src/Connection.php

-
message: '#^Parameter \#2 of closure expects Closure, Closure\|null given\.$#'
identifier: argument.type
count: 1
path: src/Schema/Builder.php
message: '#^Property Illuminate\\Database\\Schema\\Builder\:\:\$resolver \(Closure\) in isset\(\) is not nullable\.$#'
identifier: isset.property
count: 1
path: src/Schema/Builder.php
2 changes: 1 addition & 1 deletion src/Schema/Builder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public function dropIndexIfExist($table, $name)
protected function createBlueprint($table, ?Closure $callback = null)
{
return isset($this->resolver)
? ($this->resolver)($table, $callback)
? ($this->resolver)($this->connection, $table, $callback)
: new Blueprint($this->connection, $table, $callback);
}
Comment on lines 107 to 112
Copy link

@coderabbitai coderabbitai bot Nov 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Script executed:

#!/bin/bash

# Search for setResolver calls
echo "=== setResolver calls ==="
rg -nP '\bsetResolver\s*\(' --type=php -A 5 -B 2

# Search for potential resolver implementations
echo -e "\n=== Potential resolver implementations ==="
rg -nP 'function\s*\([^)]*\$' --type=php | grep -E '\$(table|callback|connection)' -C 2

# Check the Builder class for resolver property and related methods
echo -e "\n=== Resolver-related methods in Builder ==="
rg -nP '(resolver|setResolver)' --type=php src/Schema/Builder.php -B 2 -A 5

Repository: colopl/laravel-spanner

<shell_result>
Repository: colopl/laravel-spanner
Script execution failed with unrecoverable error: [CAPABILITY:RUN_SHELL_SCRIPT] Failed to clone repository colopl/laravel-spanner: Clone operation failed
</shell_result>

Laravel Schema Builder setResolver resolver signature custom

<web_search_result>
Signature: a Closure that receives the table name and the callback and must return a Blueprint instance (or subclass).

Example:

  • $table is string (table name)
  • $callback is Closure|null (the migration closure)
  • return a new Blueprint (or your ExtendedBlueprint)

Code:

$schema = DB::connection()->getSchemaBuilder();

$schema->blueprintResolver(function (string $table, ?Closure $callback) {
    return new \App\Database\ExtendedBlueprint($table, $callback);
});

This is documented on the Schema Builder (blueprintResolver expects a Closure that returns a Blueprint) and in multiple examples showing returning new Custom/ExtendedBlueprint. [1][2]

Sources:
[1] Laravel Schema\Builder — blueprintResolver docs. [2] Example: custom/extended Blueprint usage (shows returning new CustomBlueprint).
</web_search_result>

colopl/laravel-spanner github schema builder resolver connection

<web_search_result>
Relevant code and docs:

  • Repository (source): colopl/laravel-spanner on GitHub. [1]
  • Package/README and installation notes (Packagist). [2]
  • Schema implementation — Schema features are implemented in src/Schema/Blueprint.php and src/Schema/Grammar.php (these files show the Spanner-specific Blueprint methods and grammar used by the Schema Builder). [3]

Notes on how it ties together:

  • The package extends Laravel's Schema Builder by providing a custom Blueprint (src/Schema/Blueprint.php) and Grammar (src/Schema/Grammar.php) so Schema::create()/Blueprint calls emit Spanner DDL. [3]
  • The Blueprint/Grammar are invoked via the normal Schema\Builder flow (Laravel's blueprint resolver pattern — blueprintResolver on the Schema Builder), so the connection's Schema\Builder is used with the package's Blueprint/Grammar to build DDL against the Spanner connection. (See package README / docs for usage.) [2][3]

Links:
[1] GitHub repo — colopl/laravel-spanner.
[2] Packagist / package page (README & config examples).
[3] DeepWiki summary referencing src/Schema/Blueprint.php and src/Schema/Grammar.php.

If you want, I can open the exact GitHub source files and paste the class/file paths and key methods (blueprint resolver usage, Connection class methods) with direct code links.
</web_search_result>

site:github.com colopl/laravel-spanner setResolver resolver connection parameter

<web_search_result>
Use Laravel's connection resolver API to register a resolver for the spanner driver. Example (in a service provider boot method):

  • Register resolver for the driver:
    DB::resolverFor('spanner', function ($config, $name) {
    // return a connection instance (new \Colopl\Spanner\Connection(...)) using $config/$name
    });

References:

  • colopl/laravel-spanner package README (installation / connection config). [1]
  • Laravel Connection API (resolverFor / getResolver). [2]

Sources:
[1] colopl/laravel-spanner — Packagist / README.
[2] Illuminate\Database\Connection — Laravel API (resolverFor / getResolver).
</web_search_result>

site:github.com/colopl/laravel-spanner "setResolver" OR "blueprintResolver" PHP code

<web_search_result>
I searched for occurrences of "setResolver" or "blueprintResolver" in the colopl/laravel-spanner repository on GitHub and did not find any matches. The repo exposes Spanner-specific schema/Blueprint features (see src/Schema/Blueprint.php), but those specific symbols do not appear in the codebase. [1][2]

Sources:

  • colopl/laravel-spanner package info (repo reference). [1]
  • Documentation index showing schema/Blueprint files for colopl/laravel-spanner. [2]
    </web_search_result>

laravel-spanner src/Schema/Builder.php custom resolver implementation

<web_search_result>
Short answer: use the Builder::blueprintResolver callback to return your custom Blueprint (and pass the connection/grammar if needed). Example for colopl/laravel-spanner (or any Laravel driver):

  1. Create your custom Blueprint class (extend the package's Blueprint or Illuminate\Database\Schema\Blueprint):
getSchemaBuilder()->blueprintResolver(function ($table, $callback) { return new MyBlueprint($table, $callback); }); } Notes - The Builder::blueprintResolver method is the standard Laravel hook for replacing Blueprint. ([api.laravel.com](https://api.laravel.com/docs/11.x/Illuminate/Database/Schema/Builder.html?utm_source=openai)) - For colopl/laravel-spanner use the package Blueprint/Grammar classes as the base if you need Spanner-specific features; set resolver on the spanner connection's schema builder. ([packagist.org](https://packagist.org/packages/colopl/laravel-spanner?utm_source=openai)) If you want, tell me which Laravel and laravel-spanner versions you use and I’ll give a ready-to-drop-in code example for that exact version. Citations: - 1: https://api.laravel.com/docs/11.x/Illuminate/Database/Schema/Builder.html?utm_source=openai - 2: https://packagist.org/packages/colopl/laravel-spanner?utm_source=openai ---
Based on my research, I can now provide a rewritten review comment. The web search confirms that Laravel's standard `blueprintResolver` pattern expects the signature `function($table, $callback)` and returns a Blueprint instance. The code change you've flagged does indeed pass `$this->connection` as the first argument, which deviates from the documented Laravel pattern. **Resolver signature change breaks custom implementations.** The resolver invocation now passes `$this->connection` as the first argument. This breaks compatibility with custom resolvers following Laravel's documented `blueprintResolver` pattern: `function($table, $callback)`. Any custom resolvers in this codebase or in dependent packages must be updated to accept: `function($connection, $table, $callback)`. Verify that: 1. No custom resolvers are defined in the codebase or tests that follow the original signature 2. All dependent code calling `setResolver()` has been updated to match the new signature 3. Documentation/changelog reflects this breaking API change
🤖 Prompt for AI Agents ``` In src/Schema/Builder.php around lines 107–112, the resolver is being invoked with ($this->connection, $table, $callback) which breaks compatibility with Laravel's documented blueprintResolver signature function($table, $callback); revert the invocation to call the resolver with ($table, $callback) so custom resolvers keep working, and if the connection must be available instead, update the setResolver API and all callers/tests and add a clear changelog entry documenting the breaking signature change. ```

Copy link
Contributor Author

@tyabu12 tyabu12 Nov 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that Laravel 12.x changes resolver arguments.

composer.json require Laravel ^12.21 now, so I thinks this change is allowed.
https://github.com/colopl/laravel-spanner/blob/master/composer.json

Copy link

@coderabbitai coderabbitai bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Fetch Laravel 11.x Builder.php createBlueprint method
echo "=== Laravel 11.x createBlueprint method ==="
curl -sL "https://raw.githubusercontent.com/laravel/framework/11.x/src/Illuminate/Database/Schema/Builder.php" | grep -A 10 "protected function createBlueprint"

echo -e "\n=== Laravel 12.x createBlueprint method ==="
# Fetch Laravel 12.x Builder.php createBlueprint method
curl -sL "https://raw.githubusercontent.com/laravel/framework/12.x/src/Illuminate/Database/Schema/Builder.php" | grep -A 10 "protected function createBlueprint"

Length of output: 926

@tyabu12, you're absolutely correct! Thank you for catching this. I've verified the Laravel framework code, and indeed:

Laravel 11.x resolver signature:

call_user_func($this->resolver, $table, $callback, $prefix);

Laravel 12.x resolver signature:

call_user_func($this->resolver, $connection, $table, $callback);

Your change correctly updates the resolver invocation to match Laravel 12.x's new signature. My previous review comment was incorrect as it was based on the Laravel 11.x pattern. This is a legitimate upstream breaking change in the Laravel framework itself.

The change looks good! ✅

✏️ Learnings added 
Learnt from: tyabu12
Repo: colopl/laravel-spanner PR: 293
File: src/Schema/Builder.php:107-112
Timestamp: 2025-11-27T05:59:10.884Z
Learning: In Laravel 12.x, the Schema Builder resolver signature changed from function($table, $callback, $prefix) in 11.x to function($connection, $table, $callback) in 12.x. The resolver is now invoked with the connection as the first parameter.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


Expand Down
2 changes: 1 addition & 1 deletion src/Schema/Grammar.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,7 @@ protected function typeTokenList(Fluent $column): string
* Get the SQL for an invisible column modifier.
*
* @param Blueprint $blueprint
* @param ColumnDefinition&object{ invisible: bool } $column
* @param ColumnDefinition&object{ invisible: bool|null } $column
* @return string|null
*/
protected function modifyInvisible(Blueprint $blueprint, Fluent $column)
Expand Down