ronantakizawa/codereview-bench huggingface dataset:
Repository: laravel/framework
File: src/Illuminate/Auth/SessionGuard.php (PHP)

Code before:
    {
        $this->getCookieJar()->queue($this->createRecaller(
            $user->getAuthIdentifier().'|'.$user->getRememberToken().'|'.$this->hashPasswordForCookie($user->getAuthPassword())
        ));
    }

    /**
     * Create a "remember me" cookie for a given ID.
     *
     * @param  string  $value
     * @return \Symfony\Component\HttpFoundation\Cookie
     */
    protected function createRecaller($value)
    {
        return $this->getCookieJar()->make($this->getRecallerName(), $value, $this->getRememberDuration());
    }

    /**
     * Create a HMAC of the password hash for storage in cookies.
     *
     * @param  string  $passwordHash
     * @return string
     */
    public function hashPasswordForCookie($passwordHash)
    {
        $container = Container::getInstance();

        if ($container && $container->bound('config')) {
            $key = $container->make('config')->get('app.key');
        } else {
            $key = 'base-key-for-password-hash-mac';
        }

        return hash_hmac('sha256', $passwordHash, $key);
    }

    /**
     * Log the user out of the application.
     *
     * @return void
     */
    public function logout()
    {
        $user = $this->user();

        $this->clearUserDataFromStorage();

        if (! is_null($this->user) && ! empty($user->getRememberToken())) {
            $this->cycleRememberToken($user);
        }


Review comment (security):
This is pretty hacky. The data should be passed through the session guard, rather than it calling out to the container. The container isn't even a dependency of the auth package, so this would break people installing this package directly.

Code after:
    /**
     * Queue the recaller cookie into the cookie jar.
     *
     * @param  \Illuminate\Contracts\Auth\Authenticatable  $user
     * @return void
     */
    protected function queueRecallerCookie(AuthenticatableContract $user)
    {
        $this->getCookieJar()->queue($this->createRecaller(
            $user->getAuthIdentifier().'|'.
            $user->getRememberToken().'|'.
            $this->hashPasswordForCookie($user->getAuthPassword())
        ));
    }

    /**
     * Create a "remember me" cookie for a given ID.
     *
     * @param  string  $value
     * @return \Symfony\Component\HttpFoundation\Cookie
     */
    protected function createRecaller($value)
    {
        return $this->getCookieJar()->make($this->getRecallerName(), $value, $this->getRememberDuration());
    }

    /**
     * Create a HMAC of the password hash for storage in cookies.
     *
     * @param  string  $passwordHash
     * @return string
     */
    public function hashPasswordForCookie($passwordHash)
    {
        return hash_hmac(
            'sha256',
            $passwordHash,
            $this->hashKey ?? 'base-key-for-password-hash-mac'
        );
    }

    /**
     * Log the user out of the application.
     *
     * @return void
     */
    public function logout()
    {
        $user = $this->user();

        $this->clearUserDataFromStorage();
Repository: laravel/framework
File: src/Illuminate/Database/Connection.php (PHP)

Code before:
     * Get the current PDO connection parameter without executing any reconnect logic.
     *
     * @return \PDO|\Closure|null
     */
    public function getRawPdo()
    {
        return $this->pdo;
    }

    /**
     * Get the current PDO connection used for reading.
     *
     * @return \PDO
     */
    public function getReadPdo()
    {
        if ($this->transactions > 0) {
            return $this->getPdo();
        }

        if ($this->readOnWriteConnection ||
            ($this->recordsModified && $this->getConfig('sticky'))) {
            return $this->getPdo();
        }

        $this->retrievingPdoType('read');

        if ($this->readPdo instanceof Closure) {
            return $this->readPdo = call_user_func($this->readPdo);
        }

        return $this->readPdo ?: $this->getPdo();
    }

    /**
     * Get the current read PDO connection parameter without executing any reconnect logic.
     *
     * @return \PDO|\Closure|null
     */
    public function getRawReadPdo()
    {
        return $this->readPdo;
    }

    /**
     * Set the PDO connection.
     *
     * @param  \PDO|\Closure|null  $pdo
     * @return $this
     */
    public function setPdo($pdo)

Review comment (suggestion):
We could call this at the beginning of the method. If `$this->getPdo()` is called from this method, it would overwrite the `latestPdoTypeUsed`. Also, when `$this->readPdo` is null (i.e. read/write connections are not configured), it will call `$this->getPdo()` anyway.

Code after:
        return $this->pdo;
    }

    /**
     * Get the current PDO connection parameter without executing any reconnect logic.
     *
     * @return \PDO|\Closure|null
     */
    public function getRawPdo()
    {
        return $this->pdo;
    }

    /**
     * Get the current PDO connection used for reading.
     *
     * @return \PDO
     */
    public function getReadPdo()
    {
        if ($this->transactions > 0) {
            return $this->getPdo();
        }

        if ($this->readOnWriteConnection ||
            ($this->recordsModified && $this->getConfig('sticky'))) {
            return $this->getPdo();
        }

        $this->latestPdoTypeRetrieved = 'read';

        if ($this->readPdo instanceof Closure) {
            return $this->readPdo = call_user_func($this->readPdo);
        }

        return $this->readPdo ?: $this->getPdo();
    }

    /**
     * Get the current read PDO connection parameter without executing any reconnect logic.
     *
     * @return \PDO|\Closure|null
     */
    public function getRawReadPdo()
    {
        return $this->readPdo;
    }

    /**
     * Set the PDO connection.
     *
Repository: laravel/framework
File: src/Illuminate/Database/Connection.php (PHP)

Code before:
        $this->readWriteType = $readWriteType;

        return $this;
    }

    /**
     * Handle retrieving PDO read / write type.
     *
     * @param  'read'|'write'  $type
     * @return $this
     */
    protected function retrievingPdoType($type)
    {
        $this->latestPdoTypeUsed = $type;

        return $this;
    }

    /**
     * Retrieve the latest pdo read / write type.
     *
     * @return 'read'|'write'
     */
    protected function latestPdoTypeUsed()
    {
        return $this->readWriteType ?? $this->latestPdoTypeUsed;
    }

    /**
     * Get the table prefix for the connection.
     *
     * @return string
     */
    public function getTablePrefix()
    {
        return $this->tablePrefix;
    }

    /**
     * Set the table prefix in use by the connection.
     *
     * @param  string  $prefix
     * @return $this
     */
    public function setTablePrefix($prefix)
    {
        $this->tablePrefix = $prefix;

        return $this;
    }


Review comment (suggestion):
Should this always return `$this->latestPdoTypeUsed` if we want to know the actual PDO that was used to run the query?

`$this->readWriteType` does not tell the real read/write type that was actually used.
If you run the following query, it sets the `readWriteType` to `write`, but it still uses the read connection as it's a select query.

```php
DB::connection('pgsql::write')->table('users')->count();
```

Code after:
    }

    /**
     * Set the name of the connected database.
     *
     * @param  string  $database
     * @return $this
     */
    public function setDatabaseName($database)
    {
        $this->database = $database;

        return $this;
    }

    /**
     * Set the read / write type of the connection.
     *
     * @param  string|null  $readWriteType
     * @return $this
     */
    public function setReadWriteType($readWriteType)
    {
        $this->readWriteType = $readWriteType;

        return $this;
    }

    /**
     * Retrieve the latest read / write type used.
     *
     * @return 'read'|'write'|null
     */
    protected function latestReadWriteTypeUsed()
    {
        return $this->readWriteType ?? $this->latestPdoTypeRetrieved;
    }

    /**
     * Get the table prefix for the connection.
     *
     * @return string
     */
    public function getTablePrefix()
    {
        return $this->tablePrefix;
    }

    /**
     * Set the table prefix in use by the connection.
     *
Repository: laravel/framework
File: src/Illuminate/Session/Store.php (PHP)

Code before:

use Closure;
use Illuminate\Contracts\Session\Session;
use Illuminate\Support\Arr;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Date;
use Illuminate\Support\MessageBag;
use Illuminate\Support\Str;
use Illuminate\Support\Traits\Macroable;
use Illuminate\Support\Uri;
use Illuminate\Support\ViewErrorBag;
use RuntimeException;
use SessionHandlerInterface;
use stdClass;

class Store implements Session
{
    use Macroable;

    /**
     * The length used when generating session IDs.
     *
     * @var int
     */
    private const SESSION_ID_LENGTH = 40;

    /**
     * The session ID.
     *
     * @var string
     */
    protected $id;

    /**
     * The session name.
     *
     * @var string
     */
    protected $name;

    /**
     * The session attributes.
     *
     * @var array
     */
    protected $attributes = [];

    /**
     * The session handler implementation.
     *

Review comment (suggestion):
I'd change its visibility to `protected`, so subclasses can override this value if needed.

Also, on the `laravel/framework` codebase:

- There are no `private const`
- There is a single `protected const` on (`Illuminate\Support\Js`)
- All other class constants have no modifier, which makes them `public` by default.

Code after:

use Closure;
use Illuminate\Contracts\Session\Session;
use Illuminate\Support\Arr;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Date;
use Illuminate\Support\MessageBag;
use Illuminate\Support\Str;
use Illuminate\Support\Traits\Macroable;
use Illuminate\Support\Uri;
use Illuminate\Support\ViewErrorBag;
use RuntimeException;
use SessionHandlerInterface;
use stdClass;

class Store implements Session
{
    use Macroable;

    /**
     * The length of session ID strings.
     *
     * @var int
     */
    protected const SESSION_ID_LENGTH = 40;

    /**
     * The session ID.
     *
     * @var string
     */
    protected $id;

    /**
     * The session name.
     *
     * @var string
     */
    protected $name;

    /**
     * The session attributes.
     *
     * @var array
     */
    protected $attributes = [];

    /**
     * The session handler implementation.
     *
Repository: laravel/framework
File: src/Illuminate/Support/helpers.php (PHP)

Code before:
     *
     * @param  mixed  $value
     */
    function filled($value): bool
    {
        return ! blank($value);
    }
}

if (! function_exists('fluent')) {
    /**
     * Create a Fluent object from the given value.
     *
     * @param  iterable|object|null  $value
     */
    function fluent($value = null): Fluent
    {
        return new Fluent($value ?? []);
    }
}

if (! function_exists('is_binary')) {
    /**
     * Checks if passed value is binary formatted.
     *
     * @param string|mixed $value
     */
    function is_binary($value): bool
    {
        if (! is_string($value) || $value === '') {
            return false;
        }

        if (str_contains($value, "\0")) {
            return true;
        }

        return ! mb_check_encoding($value, 'UTF-8');
    }
}

if (! function_exists('literal')) {
    /**
     * Return a new literal or anonymous object using named arguments.
     *
     * @return mixed
     */
    function literal(...$arguments)
    {
        if (count($arguments) === 1 && array_is_list($arguments)) {
            return $arguments[0];

Review comment (suggestion):
You could add an assertion here, which might be helpful for static analysis

```suggestion
     * @param string|mixed $value
     * @phpstan-assert-if-true =non-empty-string $value
```

Code after:
     *
     * @param  mixed  $value
     */
    function filled($value): bool
    {
        return ! blank($value);
    }
}

if (! function_exists('fluent')) {
    /**
     * Create a Fluent object from the given value.
     *
     * @param  iterable|object|null  $value
     */
    function fluent($value = null): Fluent
    {
        return new Fluent($value ?? []);
    }
}

if (! function_exists('literal')) {
    /**
     * Return a new literal or anonymous object using named arguments.
     *
     * @return mixed
     */
    function literal(...$arguments)
    {
        if (count($arguments) === 1 && array_is_list($arguments)) {
            return $arguments[0];
        }

        return (object) $arguments;
    }
}

if (! function_exists('object_get')) {
    /**
     * Get an item from an object using "dot" notation.
     *
     * @template TValue of object
     *
     * @param  TValue  $object
     * @param  string|null  $key
     * @param  mixed  $default
     * @return ($key is empty ? TValue : mixed)
     */
    function object_get($object, $key, $default = null)
    {
        if (is_null($key) || trim($key) === '') {
Repository: laravel/framework
File: src/Illuminate/Database/Connection.php (PHP)

Code before:
        // name we are connected to since it is needed when some reflective
        // type commands are run such as checking whether a table exists.
        $this->database = $database;

        $this->tablePrefix = $tablePrefix;

        $this->config = $config;

        // We need to initialize a query grammar and the query post processors
        // which are both very important parts of the database abstractions
        // so we initialize these to their default values while starting.
        $this->useDefaultQueryGrammar();

        $this->useDefaultPostProcessor();
    }

    /**
     * Prepare the instance for cloning.
     *
     * @return void
     */
    public function __clone()
    {
        // When cloning, we need to re-initialize the grammars and post processor
        // so they reference the cloned connection instead of the original.
        $this->queryGrammar = $this->getDefaultQueryGrammar();

        $this->postProcessor = $this->getDefaultPostProcessor();

        if (! is_null($this->schemaGrammar)) {
            $this->useDefaultSchemaGrammar();
        }
    }

    /**
     * Set the query grammar to the default implementation.
     *
     * @return void
     */
    public function useDefaultQueryGrammar()
    {
        $this->queryGrammar = $this->getDefaultQueryGrammar();
    }

    /**
     * Get the default query grammar instance.
     *
     * @return \Illuminate\Database\Query\Grammars\Grammar
     */
    protected function getDefaultQueryGrammar()
    {

Review comment (suggestion):
```suggestion
        $this->useDefaultQueryGrammar();
```

Code after:
        // name we are connected to since it is needed when some reflective
        // type commands are run such as checking whether a table exists.
        $this->database = $database;

        $this->tablePrefix = $tablePrefix;

        $this->config = $config;

        // We need to initialize a query grammar and the query post processors
        // which are both very important parts of the database abstractions
        // so we initialize these to their default values while starting.
        $this->useDefaultQueryGrammar();

        $this->useDefaultPostProcessor();
    }

    /**
     * Set the query grammar to the default implementation.
     *
     * @return void
     */
    public function useDefaultQueryGrammar()
    {
        $this->queryGrammar = $this->getDefaultQueryGrammar();
    }

    /**
     * Get the default query grammar instance.
     *
     * @return \Illuminate\Database\Query\Grammars\Grammar
     */
    protected function getDefaultQueryGrammar()
    {
        return new QueryGrammar($this);
    }

    /**
     * Set the schema grammar to the default implementation.
     *
     * @return void
     */
    public function useDefaultSchemaGrammar()
    {
        $this->schemaGrammar = $this->getDefaultSchemaGrammar();
    }

    /**
     * Get the default schema grammar instance.
     *
     * @return \Illuminate\Database\Schema\Grammars\Grammar|null
     */
Repository: laravel/framework
File: src/Illuminate/Database/Connection.php (PHP)

Code before:
        $this->database = $database;

        $this->tablePrefix = $tablePrefix;

        $this->config = $config;

        // We need to initialize a query grammar and the query post processors
        // which are both very important parts of the database abstractions
        // so we initialize these to their default values while starting.
        $this->useDefaultQueryGrammar();

        $this->useDefaultPostProcessor();
    }

    /**
     * Prepare the instance for cloning.
     *
     * @return void
     */
    public function __clone()
    {
        // When cloning, we need to re-initialize the grammars and post processor
        // so they reference the cloned connection instead of the original.
        $this->queryGrammar = $this->getDefaultQueryGrammar();

        $this->postProcessor = $this->getDefaultPostProcessor();

        if (! is_null($this->schemaGrammar)) {
            $this->useDefaultSchemaGrammar();
        }
    }

    /**
     * Set the query grammar to the default implementation.
     *
     * @return void
     */
    public function useDefaultQueryGrammar()
    {
        $this->queryGrammar = $this->getDefaultQueryGrammar();
    }

    /**
     * Get the default query grammar instance.
     *
     * @return \Illuminate\Database\Query\Grammars\Grammar
     */
    protected function getDefaultQueryGrammar()
    {
        return new QueryGrammar($this);
    }

Review comment (suggestion):
```suggestion
        $this->useDefaultPostProcessor();
```

Code after:
        $this->database = $database;

        $this->tablePrefix = $tablePrefix;

        $this->config = $config;

        // We need to initialize a query grammar and the query post processors
        // which are both very important parts of the database abstractions
        // so we initialize these to their default values while starting.
        $this->useDefaultQueryGrammar();

        $this->useDefaultPostProcessor();
    }

    /**
     * Set the query grammar to the default implementation.
     *
     * @return void
     */
    public function useDefaultQueryGrammar()
    {
        $this->queryGrammar = $this->getDefaultQueryGrammar();
    }

    /**
     * Get the default query grammar instance.
     *
     * @return \Illuminate\Database\Query\Grammars\Grammar
     */
    protected function getDefaultQueryGrammar()
    {
        return new QueryGrammar($this);
    }

    /**
     * Set the schema grammar to the default implementation.
     *
     * @return void
     */
    public function useDefaultSchemaGrammar()
    {
        $this->schemaGrammar = $this->getDefaultSchemaGrammar();
    }

    /**
     * Get the default schema grammar instance.
     *
     * @return \Illuminate\Database\Schema\Grammars\Grammar|null
     */
    protected function getDefaultSchemaGrammar()
    {
Repository: laravel/framework
File: src/Illuminate/Collections/Collection.php (PHP)

Code before:
     * @return static<TValue, TCombineValue>
     */
    public function combine($values)
    {
        return new static(array_combine($this->all(), $this->getArrayableItems($values)));
    }

    /**
     * Union the collection with the given items.
     *
     * @param  \Illuminate\Contracts\Support\Arrayable<TKey, TValue>|iterable<TKey, TValue>  $items
     * @return static
     */
    public function union($items)
    {
        return new static($this->items + $this->getArrayableItems($items));
    }

    /**
     * Create a new collection consisting of every n-th element.
     *
     * @param  int  $step
     * @param  int  $offset
     * @return static
     */
    public function nth($step, $offset = 0)
    {
        if ($step < 1) {
            throw new InvalidArgumentException('Step value must be at least 1.');
        }

        $new = [];

        $position = 0;

        foreach ($this->slice($offset)->items as $item) {
            if ($position % $step === 0) {
                $new[] = $item;
            }

            $position++;
        }

        return new static($new);
    }

    /**
     * Get the items with the specified keys.
     *
     * @param  \Illuminate\Support\Enumerable<array-key, TKey>|array<array-key, TKey>|string|null  $keys
     * @return static

Review comment (suggestion):
You can even narrow the return here:

```suggestion
     * @return ($step is positive-int ? static : never)
     *
     * @throws \InvalidArgumentException
```

Code after:
     * @return static<TValue, TCombineValue>
     */
    public function combine($values)
    {
        return new static(array_combine($this->all(), $this->getArrayableItems($values)));
    }

    /**
     * Union the collection with the given items.
     *
     * @param  \Illuminate\Contracts\Support\Arrayable<TKey, TValue>|iterable<TKey, TValue>  $items
     * @return static
     */
    public function union($items)
    {
        return new static($this->items + $this->getArrayableItems($items));
    }

    /**
     * Create a new collection consisting of every n-th element.
     *
     * @param  int  $step
     * @param  int  $offset
     * @return static
     *
     * @throws \InvalidArgumentException
     */
    public function nth($step, $offset = 0)
    {
        if ($step < 1) {
            throw new InvalidArgumentException('Step value must be at least 1.');
        }

        $new = [];

        $position = 0;

        foreach ($this->slice($offset)->items as $item) {
            if ($position % $step === 0) {
                $new[] = $item;
            }

            $position++;
        }

        return new static($new);
    }

    /**
     * Get the items with the specified keys.
     *