ronantakizawa/codereview-bench huggingface dataset:
Repository: laravel/framework
File: src/Illuminate/Database/Eloquent/Model.php (PHP)

Code before:
    /**
     * Create a new Eloquent model instance.
     *
     * @param  array  $attributes
     */
    public function __construct(array $attributes = [])
    {
        $this->bootIfNotBooted();

        $this->initializeTraits();

        $this->syncOriginal();

        $this->fill($attributes);
    }

    /**
     * Check if the model needs to be booted and if so, do it.
     *
     * @return void
     */
    protected function bootIfNotBooted()
    {
        if (! isset(static::$booted[static::class])) {
            if (isset(static::$booting[static::class])) {
                throw new Exception('"'.__METHOD__.'" cannot be called on the "'.static::class.'" class while it is already being booted.');
            }

            static::$booting[static::class] = true;

            $this->fireModelEvent('booting', false);

            static::booting();
            static::boot();

            static::$booted[static::class] = true;
            unset(static::$booting[static::class]);

            static::booted();

            static::$bootedCallbacks[static::class] ??= [];

            foreach (static::$bootedCallbacks[static::class] as $callback) {
                $callback();
            }

            $this->fireModelEvent('booted', false);
        }
    }

    /**

Review comment (suggestion):
Shouldn't it be a `LogicException`?

>  Exception that represents error in the program logic. This kind of exception should lead directly to a fix in your code. 

https://www.php.net/manual/en/class.logicexception.php

Code after:
    const UPDATED_AT = 'updated_at';

    /**
     * Create a new Eloquent model instance.
     *
     * @param  array  $attributes
     */
    public function __construct(array $attributes = [])
    {
        $this->bootIfNotBooted();

        $this->initializeTraits();

        $this->syncOriginal();

        $this->fill($attributes);
    }

    /**
     * Check if the model needs to be booted and if so, do it.
     *
     * @return void
     */
    protected function bootIfNotBooted()
    {
        if (! isset(static::$booted[static::class])) {
            if (isset(static::$booting[static::class])) {
                throw new LogicException('The ['.__METHOD__.'] method may not be called on model ['.static::class.'] while it is being booted.');
            }

            static::$booting[static::class] = true;

            $this->fireModelEvent('booting', false);

            static::booting();
            static::boot();

            static::$booted[static::class] = true;
            unset(static::$booting[static::class]);

            static::booted();

            static::$bootedCallbacks[static::class] ??= [];

            foreach (static::$bootedCallbacks[static::class] as $callback) {
                $callback();
            }

            $this->fireModelEvent('booted', false);
        }
    }
Repository: laravel/framework
File: src/Illuminate/Bus/PendingBatch.php (PHP)

Code before:
     *
     * @return array
     */
    public function finallyCallbacks()
    {
        return $this->options['finally'] ?? [];
    }

    /**
     * Indicate that the batch should not be cancelled when a job within the batch fails;
     * optionally add callbacks to be executed upon each job failure (this may be useful for
     * running other jobs when a job fails).
     */
    public function allowFailures(bool|callable|Closure|array $param = true): self
    {
        if (! is_bool($param)) {
            $param = Arr::wrap($param);

            foreach ($param as $callback) {
                $this->options['failure'][] = $callback instanceof Closure
                    ? new SerializableClosure($callback)
                    : $callback;
            }
        }

        $this->options['allowFailures'] = (bool) $param;

        return $this;
    }

    /**
     * Determine if the pending batch allows jobs to fail without cancelling the batch.
     *
     * @return bool
     */
    public function allowsFailures()
    {
        return Arr::get($this->options, 'allowFailures', false) === true;
    }

    /**
     * Get the "failure" callbacks that have been registered with the pending batch.
     */
    public function failureCallbacks(): array
    {
        return $this->options['failure'] ?? [];
    }

    /**
     * Set the name for the batch.
     *

Review comment (suggestion):
So, if `$param` isn't a `bool`, we cast it to `array` and cast that to `bool`? Doesn't sound all that logical to me

Code after:
     *
     * Optionally, add callbacks to be executed upon each job failure.
     *
     * @template TParam of Closure(\Illuminate\Bus\Batch, \Throwable|null): mixed)|(callable(\Illuminate\Bus\Batch, \Throwable|null): mixed)
     *
     * @param  bool|TParam|array<array-key, TParam>  $param
     * @return $this
     */
    public function allowFailures(Closure|callable|array|bool $param = true)
    {
        if (! is_bool($param)) {
            $param = Arr::wrap($param);

            foreach ($param as $callback) {
                if (is_callable($callback)) {
                    $this->registerCallback('failure', $callback);
                }
            }
        }

        $this->options['allowFailures'] = ! ($param === false);

        return $this;
    }

    /**
     * Determine if the pending batch allows jobs to fail without cancelling the batch.
     *
     * @return bool
     */
    public function allowsFailures()
    {
        return Arr::get($this->options, 'allowFailures', false) === true;
    }

    /**
     * Get the "failure" callbacks that have been registered with the pending batch.
     *
     * @return array<array-key, Closure|callable>
     */
    public function failureCallbacks(): array
    {
        return $this->options['failure'] ?? [];
    }

    /**
     * Register a callback with proper serialization.
     */
    private function registerCallback(string $type, Closure|callable $callback): void
    {
        $this->options[$type][] = $callback instanceof Closure
Repository: laravel/framework
File: src/Illuminate/Bus/PendingBatch.php (PHP)

Code before:
        if (! is_bool($param)) {
            $param = Arr::wrap($param);

            foreach ($param as $callback) {
                $this->options['failure'][] = $callback instanceof Closure
                    ? new SerializableClosure($callback)
                    : $callback;
            }
        }

        $this->options['allowFailures'] = (bool) $param;

        return $this;
    }

    /**
     * Determine if the pending batch allows jobs to fail without cancelling the batch.
     *
     * @return bool
     */
    public function allowsFailures()
    {
        return Arr::get($this->options, 'allowFailures', false) === true;
    }

    /**
     * Get the "failure" callbacks that have been registered with the pending batch.
     */
    public function failureCallbacks(): array
    {
        return $this->options['failure'] ?? [];
    }

    /**
     * Set the name for the batch.
     *
     * @param  string  $name
     * @return $this
     */
    public function name(string $name)
    {
        $this->name = $name;

        return $this;
    }

    /**
     * Specify the queue connection that the batched jobs should run on.
     *
     * @param  string  $connection
     * @return $this

Review comment (bug):
You can further type this array
```suggestion
     *
     * @return bool|callable|Closure|array<array-key, callable|Closure>
     */
    public function failureCallbacks(): array
```

Code after:
                    $this->registerCallback('failure', $callback);
                }
            }
        }

        $this->options['allowFailures'] = ! ($param === false);

        return $this;
    }

    /**
     * Determine if the pending batch allows jobs to fail without cancelling the batch.
     *
     * @return bool
     */
    public function allowsFailures()
    {
        return Arr::get($this->options, 'allowFailures', false) === true;
    }

    /**
     * Get the "failure" callbacks that have been registered with the pending batch.
     *
     * @return array<array-key, Closure|callable>
     */
    public function failureCallbacks(): array
    {
        return $this->options['failure'] ?? [];
    }

    /**
     * Register a callback with proper serialization.
     */
    private function registerCallback(string $type, Closure|callable $callback): void
    {
        $this->options[$type][] = $callback instanceof Closure
            ? new SerializableClosure($callback)
            : $callback;
    }

    /**
     * Set the name for the batch.
     *
     * @param  string  $name
     * @return $this
     */
    public function name(string $name)
    {
        $this->name = $name;

        return $this;
Repository: laravel/framework
File: src/Illuminate/Validation/Rules/RequiredIf.php (PHP)

Code before:
<?php

namespace Illuminate\Validation\Rules;

use Closure;
use InvalidArgumentException;
use Stringable;

class RequiredIf implements Stringable
{
    /**
     * The condition that validates the attribute.
     *
     * @var (\Closure(): bool)|bool
     */
    public $condition;

    /**
     * Create a new required validation rule based on a condition.
     *
     * @param  (\Closure(): bool)|bool  $condition
     */
    public function __construct($condition)
    {
        if ($condition instanceof Closure || is_bool($condition)) {
            $this->condition = $condition;
        } else {
            throw new InvalidArgumentException('The provided condition must be a callable or boolean.');
        }
    }

    /**
     * Convert the rule to a validation string.
     *
     * @return string
     */
    public function __toString()
    {
        if (is_callable($this->condition)) {
            return call_user_func($this->condition) ? 'required' : '';
        }

        return $this->condition ? 'required' : '';
    }
}


Review comment (suggestion):
I would love to learn why is_null = false, and instead of is_empty = false.

Code after:
<?php

namespace Illuminate\Validation\Rules;

use Closure;
use InvalidArgumentException;
use Stringable;

class RequiredIf implements Stringable
{
    /**
     * The condition that validates the attribute.
     *
     * @var (\Closure(): bool)|bool
     */
    public $condition;

    /**
     * Create a new required validation rule based on a condition.
     *
     * @param  (\Closure(): bool)|bool|null  $condition
     */
    public function __construct($condition)
    {
        if (is_null($condition)) {
            $condition = false;
        }

        if ($condition instanceof Closure || is_bool($condition)) {
            $this->condition = $condition;
        } else {
            throw new InvalidArgumentException('The provided condition must be a callable or boolean.');
        }
    }

    /**
     * Convert the rule to a validation string.
     *
     * @return string
     */
    public function __toString()
    {
        if (is_callable($this->condition)) {
            return call_user_func($this->condition) ? 'required' : '';
        }

        return $this->condition ? 'required' : '';
    }
}
Repository: laravel/framework
File: src/Illuminate/Foundation/Application.php (PHP)

Code before:
use Illuminate\Support\Collection;
use Illuminate\Support\Env;
use Illuminate\Support\ServiceProvider;
use Illuminate\Support\Str;
use Illuminate\Support\Traits\Macroable;
use RuntimeException;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\ConsoleOutput;
use Symfony\Component\HttpFoundation\Request as SymfonyRequest;
use Symfony\Component\HttpFoundation\Response as SymfonyResponse;
use Symfony\Component\HttpKernel\Exception\HttpException;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\HttpKernel\HttpKernelInterface;

use function Illuminate\Filesystem\join_paths;

class Application extends Container implements ApplicationContract, CachesConfiguration, CachesRoutes, HttpKernelInterface
{
    use Macroable;

    /**
     * The Laravel framework version.
     *
     * @var string
     */
    const VERSION = '12.23.1';

    /**
     * The base path for the Laravel installation.
     *
     * @var string
     */
    protected $basePath;

    /**
     * The array of registered callbacks.
     *
     * @var callable[]
     */
    protected $registeredCallbacks = [];

    /**
     * Indicates if the application has been bootstrapped before.
     *
     * @var bool
     */
    protected $hasBeenBootstrapped = false;

    /**
     * Indicates if the application has "booted".
     *

Review comment (suggestion):
This just to make sure Testbench logic works correctly:

<img width="1221" height="679" alt="image" src="https://github.com/user-attachments/assets/d18a2382-65c3-4d19-9759-dedbc5f6286f" />


Code after:
use Illuminate\Support\Collection;
use Illuminate\Support\Env;
use Illuminate\Support\ServiceProvider;
use Illuminate\Support\Str;
use Illuminate\Support\Traits\Macroable;
use RuntimeException;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\ConsoleOutput;
use Symfony\Component\HttpFoundation\Request as SymfonyRequest;
use Symfony\Component\HttpFoundation\Response as SymfonyResponse;
use Symfony\Component\HttpKernel\Exception\HttpException;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\HttpKernel\HttpKernelInterface;

use function Illuminate\Filesystem\join_paths;

class Application extends Container implements ApplicationContract, CachesConfiguration, CachesRoutes, HttpKernelInterface
{
    use Macroable;

    /**
     * The Laravel framework version.
     *
     * @var string
     */
    const VERSION = '12.23.2';

    /**
     * The base path for the Laravel installation.
     *
     * @var string
     */
    protected $basePath;

    /**
     * The array of registered callbacks.
     *
     * @var callable[]
     */
    protected $registeredCallbacks = [];

    /**
     * Indicates if the application has been bootstrapped before.
     *
     * @var bool
     */
    protected $hasBeenBootstrapped = false;

    /**
     * Indicates if the application has "booted".
     *
Repository: laravel/framework
File: src/Illuminate/Database/SQLiteConnection.php (PHP)

Code before:
     * @param  \Illuminate\Filesystem\Filesystem|null  $files
     * @param  callable|null  $processFactory
     *
     * @throws \RuntimeException
     */
    public function getSchemaState(?Filesystem $files = null, ?callable $processFactory = null)
    {
        return new SqliteSchemaState($this, $files, $processFactory);
    }

    /**
     * Get the default post processor instance.
     *
     * @return \Illuminate\Database\Query\Processors\SQLiteProcessor
     */
    protected function getDefaultPostProcessor()
    {
        return new SQLiteProcessor;
    }

    public function executeBeginTransactionStatement()
    {
        if (version_compare(PHP_VERSION, '8.4.0') >= 0) {
            $mode = $this->getConfig('transaction_mode') ?? 'DEFERRED';

            $this->getPdo()->exec("BEGIN $mode TRANSACTION");

            return;
        }

        $this->getPdo()->beginTransaction();
    }
}


Review comment (suggestion):
Just for some context, as far as I can tell, there is no issue whatsoever with using a manual `exec("BEGIN ...")` instead of `beginTransaction()` with `Pdo\Sqlite`. They are equivalent.

PDO has some extra logic for tracking whether a connection is in a transaction, but it looks like this:
```c
static bool pdo_is_in_transaction(pdo_dbh_t *dbh) {
	if (dbh->methods->in_transaction) {
		return dbh->methods->in_transaction(dbh);
	}
	return dbh->in_txn;
}
```
Bypassing `beginTransaction()` bypasses setting `in_txn = true`. However, the SQLite driver does implement `in_transaction`, so that serves as the source of truth for whether the connection is in a transaction.

Simple test:
```
$ cat /tmp/intxn.php
<?php

$pdo = PDO::connect('sqlite::memory:');
$pdo->beginTransaction();
var_dump($pdo->inTransaction());
$pdo->rollBack();
var_dump($pdo->inTransaction());
$pdo->exec('begin transaction');
var_dump($pdo->inTransaction());
$ php /tmp/intxn.php
bool(true)
bool(false)
bool(true)
```

Code after:
        }

        return new SQLiteBuilder($this);
    }

    /**
     * Get the default schema grammar instance.
     *
     * @return \Illuminate\Database\Schema\Grammars\SQLiteGrammar
     */
    protected function getDefaultSchemaGrammar()
    {
        return new SchemaGrammar($this);
    }

    /**
     * Get the schema state for the connection.
     *
     * @param  \Illuminate\Filesystem\Filesystem|null  $files
     * @param  callable|null  $processFactory
     *
     * @throws \RuntimeException
     */
    public function getSchemaState(?Filesystem $files = null, ?callable $processFactory = null)
    {
        return new SqliteSchemaState($this, $files, $processFactory);
    }

    /**
     * Get the default post processor instance.
     *
     * @return \Illuminate\Database\Query\Processors\SQLiteProcessor
     */
    protected function getDefaultPostProcessor()
    {
        return new SQLiteProcessor;
    }
}
Repository: laravel/framework
File: src/Illuminate/Collections/helpers.php (PHP)

Code before:
                data_forget($target[$segment], $segments);
            } else {
                Arr::forget($target, $segment);
            }
        } elseif (is_object($target)) {
            if ($segments && isset($target->{$segment})) {
                data_forget($target->{$segment}, $segments);
            } elseif (isset($target->{$segment})) {
                unset($target->{$segment});
            }
        }

        return $target;
    }
}

if (! function_exists('head')) {
    /**
     * Get the first element of an array. Useful for method chaining.
     *
     * @param  array  $array
     * @return mixed
     */
    function head($array)
    {
        return array_first($array);
    }
}

if (! function_exists('last')) {
    /**
     * Get the last element from an array.
     *
     * @param  array  $array
     * @return mixed
     */
    function last($array)
    {
        return array_last($array);
    }
}

if (! function_exists('value')) {
    /**
     * Return the default value of the given value.
     *
     * @template TValue
     * @template TArgs
     *
     * @param  TValue|\Closure(TArgs): TValue  $value
     * @param  TArgs  ...$args

Review comment (suggestion):
This is not exactly the same: [`reset()`](https://www.php.net/manual/en/function.reset.php) updates the internal array pointer to the first element, while [`array_first()`](https://wiki.php.net/rfc/array_first_last) does not update internal pointer. Additionally, `reset()` returns `false` if the array is empty, while `array_first()` returns `null`.

Same goes to the `end()` and `array_last()`.

Code after:
                data_forget($target[$segment], $segments);
            } else {
                Arr::forget($target, $segment);
            }
        } elseif (is_object($target)) {
            if ($segments && isset($target->{$segment})) {
                data_forget($target->{$segment}, $segments);
            } elseif (isset($target->{$segment})) {
                unset($target->{$segment});
            }
        }

        return $target;
    }
}

if (! function_exists('head')) {
    /**
     * Get the first element of an array. Useful for method chaining.
     *
     * @param  array  $array
     * @return mixed
     */
    function head($array)
    {
        return empty($array) ? false : array_first($array);
    }
}

if (! function_exists('last')) {
    /**
     * Get the last element from an array.
     *
     * @param  array  $array
     * @return mixed
     */
    function last($array)
    {
        return empty($array) ? false : array_last($array);
    }
}

if (! function_exists('value')) {
    /**
     * Return the default value of the given value.
     *
     * @template TValue
     * @template TArgs
     *
     * @param  TValue|\Closure(TArgs): TValue  $value
     * @param  TArgs  ...$args
Repository: laravel/framework
File: src/Illuminate/Queue/SqsQueue.php (PHP)

Code before:
    /**
     * Get the underlying SQS instance.
     *
     * @return \Aws\Sqs\SqsClient
     */
    public function getSqs()
    {
        return $this->sqs;
    }

    /**
     * Get queueable options from the job.
     *
     * @param  mixed  $job
     * @param  string|null  $queue
     * @return array{MessageGroupId?: string, MessageDeduplicationId?: string}
     */
    protected function getQueueableOptions($job, $queue): array
    {
        if (! (is_object($job) && str_ends_with($queue, '.fifo'))) {
            return [];
        }

        $messageGroupId = transform($job->messageGroup ?? null, fn ($messageGroup) => strval($messageGroup));

        $messageDeduplicationId = $job instanceof ShouldBeUnique && method_exists($job, 'uniqueId')
            ? transform($job->uniqueId(), fn ($deduplication) => strval($deduplication))
            : Str::orderedUuid()->toString();

        return array_filter([
            'MessageGroupId' => $messageGroupId,
            'MessageDeduplicationId' => $messageDeduplicationId,
        ]);
    }
}


Review comment (suggestion):
I'm a bit worried about this one... people may not expect ShouldBeUnique to have the effect this would cause. 

https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/using-messagededuplicationid-property.html

AWS will not accept another job with the same de-dup ID for 5 minutes, even if there are no other jobs on the queue with that ID.

Code after:
     * @param  string|null  $queue
     * @return string
     */
    public function getQueue($queue)
    {
        $queue = $queue ?: $this->default;

        return filter_var($queue, FILTER_VALIDATE_URL) === false
            ? $this->suffixQueue($queue, $this->suffix)
            : $queue;
    }

    /**
     * Add the given suffix to the given queue name.
     *
     * @param  string  $queue
     * @param  string  $suffix
     * @return string
     */
    protected function suffixQueue($queue, $suffix = '')
    {
        if (str_ends_with($queue, '.fifo')) {
            $queue = Str::beforeLast($queue, '.fifo');

            return rtrim($this->prefix, '/').'/'.Str::finish($queue, $suffix).'.fifo';
        }

        return rtrim($this->prefix, '/').'/'.Str::finish($queue, $this->suffix);
    }

    /**
     * Get the underlying SQS instance.
     *
     * @return \Aws\Sqs\SqsClient
     */
    public function getSqs()
    {
        return $this->sqs;
    }
}