Skip to content
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

Update phpstan-codeigniter and fix errors on Modules #8036

Merged
merged 5 commits into from
Oct 25, 2023
Merged

Update phpstan-codeigniter and fix errors on Modules #8036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2969640
Update phpstan-codeigniter to v1.4.1
paulbalandan Oct 21, 2023
94130de
Fix Modules does not extend BaseConfig
paulbalandan Oct 21, 2023
89f1cc6
Inject Modules into constructor
paulbalandan Oct 22, 2023
3142108
Use static setter
paulbalandan Oct 24, 2023
61bbbca
Simplify test
paulbalandan Oct 25, 2023
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
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
},
"require-dev": {
"codeigniter/coding-standard": "^1.5",
"codeigniter/phpstan-codeigniter": "^v1.1",
"codeigniter/phpstan-codeigniter": "^1.4",
"ergebnis/composer-normalize": "^2.28",
"fakerphp/faker": "^1.9",
"kint-php/kint": "^5.0.4",
Expand Down
10 changes: 0 additions & 10 deletions phpstan-baseline.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,16 +871,6 @@
'count' => 4,
'path' => __DIR__ . '/system/Config/BaseConfig.php',
];
$ignoreErrors[] = [
'message' => '#^Argument \\#1 \\$name \\(\'Config\\\\\\\\Modules\'\\) passed to function config does not extend CodeIgniter\\\\\\\\Config\\\\\\\\BaseConfig\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Config/BaseConfig.php',
];
$ignoreErrors[] = [
'message' => '#^Argument \\#1 \\$name \\(\'Config\\\\\\\\Modules\'\\) passed to function config does not extend CodeIgniter\\\\\\\\Config\\\\\\\\BaseConfig\\.$#',
'count' => 2,
'path' => __DIR__ . '/system/Config/BaseService.php',
];
$ignoreErrors[] = [
'message' => '#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#',
'count' => 3,
Expand Down
3 changes: 0 additions & 3 deletions phpstan.neon.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,3 @@ parameters:
booleansInConditions: true
disallowedConstructs: true
matchingInheritedMethodNames: true
codeigniter:
additionalConfigNamespaces:
- CodeIgniter\Config\
13 changes: 11 additions & 2 deletions system/Config/BaseConfig.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class BaseConfig
/**
* The modules configuration.
*
* @var Modules
* @var Modules|null
*/
protected static $moduleConfig;

Expand All @@ -74,6 +74,15 @@ public static function __set_state(array $array)
return $obj;
}

/**
* @internal For testing purposes only.
* @testTag
*/
public static function setModules(Modules $modules): void
{
static::$moduleConfig = $modules;
}

/**
* Will attempt to get environment variables with names
* that match the properties of the child class.
Expand All @@ -82,7 +91,7 @@ public static function __set_state(array $array)
*/
public function __construct()
{
static::$moduleConfig = config(Modules::class);
static::$moduleConfig ??= new Modules();

if (! static::$override) {
return;
Expand Down
8 changes: 2 additions & 6 deletions system/Config/BaseService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,9 +330,7 @@ public static function injectMock(string $name, $mock)
protected static function discoverServices(string $name, array $arguments)
{
if (! static::$discovered) {
$config = config(Modules::class);

if ($config->shouldDiscover('services')) {
if ((new Modules())->shouldDiscover('services')) {
$locator = static::locator();
$files = $locator->search('Config/Services');

Expand Down Expand Up @@ -372,9 +370,7 @@ protected static function discoverServices(string $name, array $arguments)
protected static function buildServicesCache(): void
{
if (! static::$discovered) {
$config = config(Modules::class);

if ($config->shouldDiscover('services')) {
if ((new Modules())->shouldDiscover('services')) {
$locator = static::locator();
$files = $locator->search('Config/Services');

Expand Down
2 changes: 0 additions & 2 deletions tests/system/Cache/FactoriesCacheFileVarExportHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use CodeIgniter\Config\Factories;
use CodeIgniter\Test\CIUnitTestCase;
use Config\App;
use Config\Modules;

/**
* @internal
Expand Down Expand Up @@ -58,7 +57,6 @@ public function testSave()

$this->assertArrayHasKey('aliases', $cachedData);
$this->assertArrayHasKey('instances', $cachedData);
$this->assertArrayHasKey(Modules::class, $cachedData['aliases']);
$this->assertArrayHasKey('App', $cachedData['aliases']);
}

Expand Down
38 changes: 20 additions & 18 deletions tests/system/Config/BaseConfigTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@

namespace CodeIgniter\Config;

use CodeIgniter\Autoloader\FileLocator;
use CodeIgniter\Test\CIUnitTestCase;
use Config\Modules;
use Encryption;
use PHPUnit\Framework\MockObject\MockObject;
use RegistrarConfig;
use RuntimeException;
use SimpleConfig;
Expand Down Expand Up @@ -45,6 +48,9 @@ protected function setUp(): void
if (! class_exists('Encryption', false)) {
require $this->fixturesFolder . '/Encryption.php';
}

BaseConfig::$registrars = [];
BaseConfig::setModules(new Modules()); // reset to clean copy of Modules
}

public function testBasicValues(): void
Expand Down Expand Up @@ -265,32 +271,28 @@ public function testBadRegistrar(): void
$this->assertSame('bar', $config->foo);
}

public function testNotEnabled(): void
public function testDiscoveryNotEnabledWillNotPopulateRegistrarsArray(): void
{
$modulesConfig = config('Modules');
$modulesConfig->enabled = false;

$config = new RegistrarConfig();
$config::$registrars = [];
$expected = $config::$registrars;
/** @var MockObject&Modules $modules */
$modules = $this->createMock(Modules::class);
$modules->method('shouldDiscover')->with('registrars')->willReturn(false);

$method = $this->getPrivateMethodInvoker($config, 'registerProperties');
$method();
RegistrarConfig::setModules($modules);
$config = new RegistrarConfig();

$this->assertSame($expected, $config::$registrars);
$this->assertSame([], $config::$registrars);
}

public function testDidDiscovery(): void
public function testRedoingDiscoveryWillStillSetDidDiscoveryPropertyToTrue(): void
{
$modulesConfig = config('Modules');
$modulesConfig->enabled = true;
/** @var FileLocator&MockObject $locator */
$locator = $this->createMock(FileLocator::class);
$locator->method('search')->with('Config/Registrar.php')->willReturn([]);
Services::injectMock('locator', $locator);

$config = new RegistrarConfig();
$config::$registrars = [];
$this->setPrivateProperty($config, 'didDiscovery', false);
$this->setPrivateProperty(RegistrarConfig::class, 'didDiscovery', false);

$method = $this->getPrivateMethodInvoker($config, 'registerProperties');
$method();
$config = new RegistrarConfig();

$this->assertTrue($this->getPrivateProperty($config, 'didDiscovery'));
Copy link
Member

Choose a reason for hiding this comment

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

The following is easier to understand.

--- a/tests/system/Config/BaseConfigTest.php
+++ b/tests/system/Config/BaseConfigTest.php
@@ -290,9 +290,9 @@ final class BaseConfigTest extends CIUnitTestCase
         $locator->method('search')->with('Config/Registrar.php')->willReturn([]);
         Services::injectMock('locator', $locator);
 
+        $this->setPrivateProperty(RegistrarConfig::class, 'didDiscovery', false);
+
         $config = new RegistrarConfig();
-        $this->setPrivateProperty($config, 'didDiscovery', false);
-        ($this->getPrivateMethodInvoker($config, 'registerProperties'))();
 
         $this->assertTrue($this->getPrivateProperty($config, 'didDiscovery'));
     }

}
Expand Down
Loading