overtrue / phplint

:bug: A tool that can speed up linting of php files by running several lint processes at once.
MIT License
984 stars 118 forks source link

Loosing of settings from config file #169

Closed ElisDN closed 1 year ago

ElisDN commented 1 year ago

Right now with ConfigResolver we loose configuration options like #163 because of array_replace_recursive:

final class ConfigResolver
{
    public function resolve(): array
    {
        // ...

        // It loads $conf values from configuration file:

        $conf = $this->loadConfiguration($this->options[self::OPTION_CONFIG_FILE]);

        // But here default $this->options values replace $conf values:

        $conf = array_replace_recursive($conf, $this->options);

        // ...
    }
}
ElisDN commented 1 year ago

As a decision we can split default options and passed options:

final class ConfigResolver
{
    private const DEFAULT_OPTIONS = [
        // ...
        self::OPTION_CACHE_FILE => self::DEFAULT_CACHE_FILE,
        self::OPTION_NO_CACHE => false,
        // ...
    ];

    private array $options = [];

    public function resolve(): array
    {
        // ...

        $conf = array_replace_recursive(self::DEFAULT_OPTIONS, $conf, $this->options);

        // ...
    }
}
llaville commented 1 year ago

Could you provide real example of usage and what values you lost ?

ElisDN commented 1 year ago

@llaville, for example it is cache option:

path: ./
cache: .custom-cache
extensions:
    - php
hotrush commented 1 year ago

Can confirm that cache option in .phplint.yml is getting lost and doesn't have any effect.

llaville commented 1 year ago

I'll give a full answer tomorrow.

llaville commented 1 year ago

My answer is available by #171

llaville commented 1 year ago

Situation is came back to normal with new releases 3.4.0, 4.5.0, 5.5.0 or 6.1.0 depending of your PHP version / platform.

This issue will be fixed for future version 7.0

llaville commented 1 year ago

As previous releases came back to a stable situation, I'll close this issue that is no more valuable. Code for future version 7.0 that will use the ConfigResolver component is not yet available. Will be soon on main branch !