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

Code before:
     *
     * @return void
     */
    protected function mergeAttributeFromCachedCasts(string $key)
    {
        $this->mergeAttributeFromClassCasts($key);
        $this->mergeAttributeFromAttributeCasts($key);
    }

    /**
     * Merge the cast class attributes back into the model.
     *
     * @return void
     */
    protected function mergeAttributesFromClassCasts()
    {
        foreach ($this->classCastCache as $key => $value) {
            $this->mergeAttributeFromClassCasts($key);
        }
    }

    private function mergeAttributeFromClassCasts(string $key): void
    {
        if (! isset($this->classCastCache[$key])) {
            return;
        }

        $value = $this->classCastCache[$key];

        $caster = $this->resolveCasterClass($key);

        $this->attributes = array_merge(
            $this->attributes,
            $caster instanceof CastsInboundAttributes
                ? [$key => $value]
                : $this->normalizeCastClassResponse($key, $caster->set($this, $key, $value, $this->attributes))
        );
    }

    /**
     * Merge the cast class attributes back into the model.
     *
     * @return void
     */
    protected function mergeAttributesFromAttributeCasts()
    {
        foreach ($this->attributeCastCache as $key => $value) {
            $this->mergeAttributeFromAttributeCasts($key);
        }
    }


Review comment (refactor):
I extracted the code that was inside this loop to a separate method, so I can use it here and also independently for getAttribute

Code after:
    protected function mergeAttributesFromCachedCasts()
    {
        $this->mergeAttributesFromClassCasts();
        $this->mergeAttributesFromAttributeCasts();
    }

    /**
     * Merge the a cast class and attribute cast attribute back into the model.
     *
     * @return void
     */
    protected function mergeAttributeFromCachedCasts(string $key)
    {
        $this->mergeAttributeFromClassCasts($key);
        $this->mergeAttributeFromAttributeCasts($key);
    }

    /**
     * Merge the cast class attributes back into the model.
     *
     * @return void
     */
    protected function mergeAttributesFromClassCasts()
    {
        foreach ($this->classCastCache as $key => $value) {
            $this->mergeAttributeFromClassCasts($key);
        }
    }

    /**
     * Merge the cast class attribute back into the model.
     *
     * @return void
     */
    protected function mergeAttributeFromClassCasts(string $key): void
    {
        if (! isset($this->classCastCache[$key])) {
            return;
        }

        $value = $this->classCastCache[$key];

        $caster = $this->resolveCasterClass($key);

        $this->attributes = array_merge(
            $this->attributes,
            $caster instanceof CastsInboundAttributes
                ? [$key => $value]
                : $this->normalizeCastClassResponse($key, $caster->set($this, $key, $value, $this->attributes))
        );
    }
Repository: laravel/framework
File: src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php (PHP)

Code before:
                : $this->normalizeCastClassResponse($key, $caster->set($this, $key, $value, $this->attributes))
        );
    }

    /**
     * Merge the cast class attributes back into the model.
     *
     * @return void
     */
    protected function mergeAttributesFromAttributeCasts()
    {
        foreach ($this->attributeCastCache as $key => $value) {
            $this->mergeAttributeFromAttributeCasts($key);
        }
    }

    private function mergeAttributeFromAttributeCasts(string $key): void
    {
        if (! isset($this->attributeCastCache[$key])) {
            return;
        }

        $value = $this->attributeCastCache[$key];

        $attribute = $this->{Str::camel($key)}();

        if ($attribute->get && ! $attribute->set) {
            return;
        }

        $callback = $attribute->set ?: function ($value) use ($key) {
            $this->attributes[$key] = $value;
        };

        $this->attributes = array_merge(
            $this->attributes,
            $this->normalizeCastClassResponse(
                $key, $callback($value, $this->attributes)
            )
        );
    }

    /**
     * Normalize the response from a custom class caster.
     *
     * @param  string  $key
     * @param  mixed  $value
     * @return array
     */
    protected function normalizeCastClassResponse($key, $value)
    {

Review comment (refactor):
I extracted the code that was inside this loop to a separate method, so I can use it here and also independently for getAttribute

Code after:
    {
        if (! isset($this->classCastCache[$key])) {
            return;
        }

        $value = $this->classCastCache[$key];

        $caster = $this->resolveCasterClass($key);

        $this->attributes = array_merge(
            $this->attributes,
            $caster instanceof CastsInboundAttributes
                ? [$key => $value]
                : $this->normalizeCastClassResponse($key, $caster->set($this, $key, $value, $this->attributes))
        );
    }

    /**
     * Merge the cast class attributes back into the model.
     *
     * @return void
     */
    protected function mergeAttributesFromAttributeCasts()
    {
        foreach ($this->attributeCastCache as $key => $value) {
            $this->mergeAttributeFromAttributeCasts($key);
        }
    }

    /**
     * Merge the cast class attribute back into the model.
     *
     * @return void
     */
    protected function mergeAttributeFromAttributeCasts(string $key): void
    {
        if (! isset($this->attributeCastCache[$key])) {
            return;
        }

        $value = $this->attributeCastCache[$key];

        $attribute = $this->{Str::camel($key)}();

        if ($attribute->get && ! $attribute->set) {
            return;
        }

        $callback = $attribute->set ?: function ($value) use ($key) {
            $this->attributes[$key] = $value;
        };
Repository: laravel/framework
File: src/Illuminate/Foundation/Testing/WithCachedConfig.php (PHP)

Code before:
use Illuminate\Foundation\Support\Providers\RouteServiceProvider;

trait WithCachedConfig
{
    protected function setUpWithCachedConfig(): void
    {
        if ((CachedState::$cachedConfig ?? null) === null) {
            CachedState::$cachedConfig = $this->app->make('config')->all();
        }

        $this->markConfigCached($this->app);
    }

    protected function tearDownWithCachedConfig(): void
    {
        LoadConfiguration::setAlwaysUseConfig(null);
    }

    /**
     * Inform the container to treat configuration as cached.
     */
    protected function markConfigCached(Application $app): void
    {
        $app->instance('config_loaded_from_cache', true); // I'm not sure this is actually needed

        LoadConfiguration::setAlwaysUseConfig(static fn () => CachedState::$cachedConfig);
    }
}


Review comment (performance):
Is there any concern here about someone storing an object in cache that can be modified between tests? We could serialize/unserialize it on each test. This would cost some CPU cycles for each test, but could insulate us from that problem. Not sure how often people are storing mutable objects in their configurations 🤔 

Code after:

trait WithCachedConfig
{
    /**
     * After resolving the configuration once, we can cache it for the remaining tests.
     */
    protected function setUpWithCachedConfig(): void
    {
        if ((CachedState::$cachedConfig ?? null) === null) {
            CachedState::$cachedConfig = $this->app->make('config')->all();
        }

        $this->markConfigCached($this->app);
    }

    /**
     * Reset the cached configuration.
     *
     * This is helpful if some of the tests in the suite apply this trait while others do not.
     */
    protected function tearDownWithCachedConfig(): void
    {
        LoadConfiguration::setAlwaysUseConfig(null);
    }

    /**
     * Inform the container that the configuration is cached.
     */
    protected function markConfigCached(Application $app): void
    {
        $app->instance('config_loaded_from_cache', true); // I'm not sure this is actually needed

        LoadConfiguration::alwaysUse(static fn () => CachedState::$cachedConfig);
    }
}
Repository: laravel/framework
File: src/Illuminate/Database/Console/ShowModelCommand.php (PHP)

Code before:
                    sprintf('%s', $event['event']),
                    sprintf('%s', $event['class']),
                );
            }
        }

        $this->newLine();

        $this->components->twoColumnDetail('<fg=green;options=bold>Observers</>');

        if ($observers->count()) {
            foreach ($observers as $observer) {
                $this->components->twoColumnDetail(
                    sprintf('%s', $observer['event']),
                    implode(', ', $observer['observer'])
                );
            }
        }

        $this->newLine();
    }

    /**
     * Prompt for missing input arguments using the returned questions.
     *
     * @return array
     */
    protected function promptForMissingArgumentsUsing(): array
    {
        return [
            'model' => fn() => suggest('Which model would you like to show?', $this->possibleModels()),
        ];
    }
}


Review comment (refactor):
You could narrow this further down

```suggestion
     * @return array<string, \Closure(): string>
```

See https://github.com/laravel/prompts/blob/3a84de9ee4fab2fab188ff66cfb8c80b5a6feec1/src/helpers.php#L140-L159

Code after:
                    sprintf('%s', $event['event']),
                    sprintf('%s', $event['class']),
                );
            }
        }

        $this->newLine();

        $this->components->twoColumnDetail('<fg=green;options=bold>Observers</>');

        if ($observers->count()) {
            foreach ($observers as $observer) {
                $this->components->twoColumnDetail(
                    sprintf('%s', $observer['event']),
                    implode(', ', $observer['observer'])
                );
            }
        }

        $this->newLine();
    }

    /**
     * Prompt for missing input arguments using the returned questions.
     *
     * @return array<string, \Closure(): string>
     */
    protected function promptForMissingArgumentsUsing(): array
    {
        return [
            'model' => fn (): string => suggest('Which model would you like to show?', $this->findAvailableModels()),
        ];
    }
}
Repository: laravel/framework
File: src/Illuminate/Queue/QueueManager.php (PHP)

Code before:

    /**
     * Set the application instance used by the manager.
     *
     * @param  \Illuminate\Contracts\Foundation\Application  $app
     * @return $this
     */
    public function setApplication($app)
    {
        $this->app = $app;

        foreach ($this->connections as $connection) {
            $connection->setContainer($app);
        }

        return $this;
    }

    /**
     * Pause a queue by name.
     *
     * @param  string  $queue
     * @param  int  $ttl
     * @return void
     */
    public function pause($queue, $ttl = 86400)
    {
        $cache = $this->app['cache']->store();

        $cache->put("queue_paused:{$queue}", true, $ttl);

        // Add to the list of paused queues
        $pausedQueues = $this->getPausedQueues();

        if (! in_array($queue, $pausedQueues)) {
            $pausedQueues[] = $queue;
            $this->storePausedQueuesList($pausedQueues);
        }
    }

    /**
     * Resume a paused queue by name.
     *
     * @param  string  $queue
     * @return void
     */
    public function resume($queue)
    {
        $cache = $this->app['cache']->store();

        $cache->forget("queue_paused:{$queue}");

Review comment (suggestion):
Very strange solution to pause just for one day. Why not week, year, or 30 seconds? It's better to pass null as default, meaning by default pausing queue until it's resumed manualy.

For other cases developers still will have a way to pass any ttl they want in certain case

Code after:

    /**
     * Get the name of the default queue connection.
     *
     * @return string
     */
    public function getDefaultDriver()
    {
        return $this->app['config']['queue.default'];
    }

    /**
     * Set the name of the default queue connection.
     *
     * @param  string  $name
     * @return void
     */
    public function setDefaultDriver($name)
    {
        $this->app['config']['queue.default'] = $name;
    }

    /**
     * Get the full name for the given connection.
     *
     * @param  string|null  $connection
     * @return string
     */
    public function getName($connection = null)
    {
        return $connection ?: $this->getDefaultDriver();
    }

    /**
     * Get the application instance used by the manager.
     *
     * @return \Illuminate\Contracts\Foundation\Application
     */
    public function getApplication()
    {
        return $this->app;
    }

    /**
     * Set the application instance used by the manager.
     *
     * @param  \Illuminate\Contracts\Foundation\Application  $app
     * @return $this
     */
    public function setApplication($app)
    {
Repository: laravel/framework
File: src/Illuminate/Http/Request.php (PHP)

Code before:
    /**
     * Get an input element from the request.
     *
     * @param  string  $key
     * @return mixed
     */
    public function __get($key)
    {
        return Arr::get($this->all(), $key, fn () => $this->route($key));
    }

    /**
     * {@inheritdoc}
     *
     * Override to clear cache when Accept header changes.
     */
    #[\Override]
    public function getAcceptableContentTypes(): array
    {
        $currentAcceptHeader = $this->headers->get('Accept');

        if ($this->cachedAcceptHeader !== $currentAcceptHeader) {
            $this->acceptableContentTypes = null;
        }

        $this->cachedAcceptHeader = $currentAcceptHeader;

        return parent::getAcceptableContentTypes();
    }
}


Review comment (suggestion):
I think this statement can be moved into the `if` block so the assignment only happens when needed.

Code after:
     */
    public function offsetUnset($offset): void
    {
        $this->getInputSource()->remove($offset);
    }

    /**
     * Check if an input element is set on the request.
     *
     * @param  string  $key
     * @return bool
     */
    public function __isset($key)
    {
        return ! is_null($this->__get($key));
    }

    /**
     * Get an input element from the request.
     *
     * @param  string  $key
     * @return mixed
     */
    public function __get($key)
    {
        return Arr::get($this->all(), $key, fn () => $this->route($key));
    }
}
Repository: laravel/framework
File: src/Illuminate/Collections/Collection.php (PHP)

Code before:
        $results = [];

        $collectionCount = $this->count();

        foreach (range(1, min($count, $collectionCount)) as $item) {
            $results[] = array_shift($this->items);
        }

        return new static($results);
    }

    /**
     * Shuffle the items in the collection.
     *
     * @return static
     */
    public function shuffle()
    {
        return new static(Arr::shuffle($this->items));
    }

    /**
     * Create chunks representing a "sliding window" view of the items in the collection.
     *
     * @param  int  $size
     * @param  int  $step
     * @return static<int, static>
     *
     * @throws \InvalidArgumentException
     */
    public function sliding($size = 2, $step = 1)
    {
        if ($size < 1) {
            throw new InvalidArgumentException('Size must be at least 1.');
        }

        if ($step < 1) {
            throw new InvalidArgumentException('Step must be at least 1.');
        }

        $chunks = floor(($this->count() - $size) / $step) + 1;

        return static::times($chunks, fn ($number) => $this->slice(($number - 1) * $step, $size));
    }

    /**
     * Skip the first {$count} items.
     *
     * @param  int  $count
     * @return static
     */

Review comment (suggestion):
You can narrow this further down then:

```suggestion
    * @param  positive-int  $size
    * @param  positive-int  $step
```

Code after:
        $results = [];

        $collectionCount = $this->count();

        foreach (range(1, min($count, $collectionCount)) as $item) {
            $results[] = array_shift($this->items);
        }

        return new static($results);
    }

    /**
     * Shuffle the items in the collection.
     *
     * @return static
     */
    public function shuffle()
    {
        return new static(Arr::shuffle($this->items));
    }

    /**
     * Create chunks representing a "sliding window" view of the items in the collection.
     *
     * @param  positive-int  $size
     * @param  positive-int  $step
     * @return static<int, static>
     *
     * @throws \InvalidArgumentException
     */
    public function sliding($size = 2, $step = 1)
    {
        if ($size < 1) {
            throw new InvalidArgumentException('Size value must be at least 1.');
        } elseif ($step < 1) {
            throw new InvalidArgumentException('Step value must be at least 1.');
        }

        $chunks = floor(($this->count() - $size) / $step) + 1;

        return static::times($chunks, fn ($number) => $this->slice(($number - 1) * $step, $size));
    }

    /**
     * Skip the first {$count} items.
     *
     * @param  int  $count
     * @return static
     */
    public function skip($count)
    {
Repository: laravel/framework
File: src/Illuminate/Collections/Collection.php (PHP)

Code before:

        $collectionCount = $this->count();

        foreach (range(1, min($count, $collectionCount)) as $item) {
            $results[] = array_shift($this->items);
        }

        return new static($results);
    }

    /**
     * Shuffle the items in the collection.
     *
     * @return static
     */
    public function shuffle()
    {
        return new static(Arr::shuffle($this->items));
    }

    /**
     * Create chunks representing a "sliding window" view of the items in the collection.
     *
     * @param  int  $size
     * @param  int  $step
     * @return static<int, static>
     *
     * @throws \InvalidArgumentException
     */
    public function sliding($size = 2, $step = 1)
    {
        if ($size < 1) {
            throw new InvalidArgumentException('Size must be at least 1.');
        }

        if ($step < 1) {
            throw new InvalidArgumentException('Step must be at least 1.');
        }

        $chunks = floor(($this->count() - $size) / $step) + 1;

        return static::times($chunks, fn ($number) => $this->slice(($number - 1) * $step, $size));
    }

    /**
     * Skip the first {$count} items.
     *
     * @param  int  $count
     * @return static
     */
    public function skip($count)

Review comment (suggestion):
You might even reflect that in the return type:

```suggestion
     * @return $size is positive-int ? $step is positive-int ? static<int, static> : never : never
```

Code after:

        $collectionCount = $this->count();

        foreach (range(1, min($count, $collectionCount)) as $item) {
            $results[] = array_shift($this->items);
        }

        return new static($results);
    }

    /**
     * Shuffle the items in the collection.
     *
     * @return static
     */
    public function shuffle()
    {
        return new static(Arr::shuffle($this->items));
    }

    /**
     * Create chunks representing a "sliding window" view of the items in the collection.
     *
     * @param  positive-int  $size
     * @param  positive-int  $step
     * @return static<int, static>
     *
     * @throws \InvalidArgumentException
     */
    public function sliding($size = 2, $step = 1)
    {
        if ($size < 1) {
            throw new InvalidArgumentException('Size value must be at least 1.');
        } elseif ($step < 1) {
            throw new InvalidArgumentException('Step value must be at least 1.');
        }

        $chunks = floor(($this->count() - $size) / $step) + 1;

        return static::times($chunks, fn ($number) => $this->slice(($number - 1) * $step, $size));
    }

    /**
     * Skip the first {$count} items.
     *
     * @param  int  $count
     * @return static
     */
    public function skip($count)
    {
        return $this->slice($count);
Repository: laravel/framework
File: src/Illuminate/Queue/QueueManager.php (PHP)

Code before:
            throw new InvalidArgumentException("No connector for [$driver].");
        }

        return call_user_func($this->connectors[$driver]);
    }

    /**
     * Pause a queue by its connection and name.
     *
     * @param  string  $connection
     * @param  string  $queue
     * @return void
     */
    public function pause($connection, $queue)
    {
        $this->app['cache']
            ->store()
            ->forever("illuminate:queue:paused:{$connection}:{$queue}", true);
    }

    /**
     * Pause a queue by its connection and name for a given number of seconds.
     *
     * @param  string  $connection
     * @param  string  $queue
     * @param  int  $seconds
     * @return void
     */
    public function pauseFor($connection, $queue, $seconds)
    {
        $this->app['cache']
            ->store()
            ->put("illuminate:queue:paused:{$connection}:{$queue}", true, $seconds);
    }

    /**
     * Resume a paused queue by its connection and name.
     *
     * @param  string  $connection
     * @param  string  $queue
     * @return void
     */
    public function resume($connection, $queue)
    {
        $this->app['cache']
            ->store()
            ->forget("illuminate:queue:paused:{$connection}:{$queue}");
    }

    /**
     * Determine if a queue is paused.

Review comment (suggestion):
```suggestion
     * @param  int|null  $seconds
```

As demonstrated in the `testPauseQueueIndefinitely()` test $seconds can accept null.

Code after:
            throw new InvalidArgumentException("No connector for [$driver].");
        }

        return call_user_func($this->connectors[$driver]);
    }

    /**
     * Pause a queue by its connection and name.
     *
     * @param  string  $connection
     * @param  string  $queue
     * @return void
     */
    public function pause($connection, $queue)
    {
        $this->app['cache']
            ->store()
            ->forever("illuminate:queue:paused:{$connection}:{$queue}", true);
    }

    /**
     * Pause a queue by its connection and name for a given amount of time.
     *
     * @param  string  $connection
     * @param  string  $queue
     * @param  \DateTimeInterface|\DateInterval|int  $ttl
     * @return void
     */
    public function pauseFor($connection, $queue, $ttl)
    {
        $this->app['cache']
            ->store()
            ->put("illuminate:queue:paused:{$connection}:{$queue}", true, $ttl);
    }

    /**
     * Resume a paused queue by its connection and name.
     *
     * @param  string  $connection
     * @param  string  $queue
     * @return void
     */
    public function resume($connection, $queue)
    {
        $this->app['cache']
            ->store()
            ->forget("illuminate:queue:paused:{$connection}:{$queue}");
    }

    /**
     * Determine if a queue is paused.