From 9f52bcbcf6493298281ddc4cf88eb3eac0cc4871 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Sat, 15 Oct 2022 22:53:53 +0100 Subject: [PATCH 1/2] [4.3] Fixed static analysis runs (#543) --- composer.json | 4 ++++ vendor-bin/phpstan/composer.json | 5 ++++- vendor-bin/psalm/composer.json | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 2a0dc7a..ef42729 100644 --- a/composer.json +++ b/composer.json @@ -47,6 +47,10 @@ "preferred-install": "dist" }, "extra": { + "bamarni-bin": { + "bin-links": true, + "forward-command": true + }, "branch-alias": { "dev-master": "4.2-dev" } diff --git a/vendor-bin/phpstan/composer.json b/vendor-bin/phpstan/composer.json index 125548d..b358769 100644 --- a/vendor-bin/phpstan/composer.json +++ b/vendor-bin/phpstan/composer.json @@ -7,6 +7,9 @@ "thecodingmachine/phpstan-strict-rules": "1.0.0" }, "config": { - "preferred-install": "dist" + "preferred-install": "dist", + "allow-plugins": { + "phpstan/extension-installer": true + } } } diff --git a/vendor-bin/psalm/composer.json b/vendor-bin/psalm/composer.json index 72bd6bb..47ba3de 100644 --- a/vendor-bin/psalm/composer.json +++ b/vendor-bin/psalm/composer.json @@ -1,6 +1,6 @@ { "require": { - "psalm/phar": "4.15.0" + "psalm/phar": "4.19.0" }, "config": { "preferred-install": "dist" From 3b56df14b76e8f3dcec6376e5e4ac5cb0386b2c6 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Sat, 15 Oct 2022 22:56:13 +0100 Subject: [PATCH 2/2] [4.3] Made repository checks a little stricter (#541) --- src/Repository/AbstractRepository.php | 22 ++++----- src/Repository/Adapter/ApacheAdapter.php | 14 +++--- src/Repository/Adapter/ArrayAdapter.php | 16 +++---- src/Repository/Adapter/EnvConstAdapter.php | 24 +++++++--- src/Repository/Adapter/PutenvAdapter.php | 8 ++-- src/Repository/Adapter/ReaderInterface.php | 2 +- src/Repository/Adapter/ServerConstAdapter.php | 24 +++++++--- src/Repository/Adapter/WriterInterface.php | 6 +-- src/Repository/AdapterRepository.php | 8 ++-- tests/Dotenv/RepositoryTest.php | 45 ++++++++++++++++--- 10 files changed, 114 insertions(+), 55 deletions(-) diff --git a/src/Repository/AbstractRepository.php b/src/Repository/AbstractRepository.php index 57c7b43..04994e0 100644 --- a/src/Repository/AbstractRepository.php +++ b/src/Repository/AbstractRepository.php @@ -46,8 +46,8 @@ public function __construct($immutable) */ public function get($name) { - if (!is_string($name)) { - throw new InvalidArgumentException('Expected name to be a string.'); + if (!is_string($name) || '' === $name) { + throw new InvalidArgumentException('Expected name to be a non-empty string.'); } return $this->getInternal($name); @@ -56,7 +56,7 @@ public function get($name) /** * Get an environment variable. * - * @param string $name + * @param non-empty-string $name * * @return string|null */ @@ -74,8 +74,8 @@ abstract protected function getInternal($name); */ public function set($name, $value = null) { - if (!is_string($name)) { - throw new InvalidArgumentException('Expected name to be a string.'); + if (!is_string($name) || '' === $name) { + throw new InvalidArgumentException('Expected name to be a non-empty string.'); } // Don't overwrite existing environment variables if we're immutable @@ -91,8 +91,8 @@ public function set($name, $value = null) /** * Set an environment variable. * - * @param string $name - * @param string|null $value + * @param non-empty-string $name + * @param string|null $value * * @return void */ @@ -109,8 +109,8 @@ abstract protected function setInternal($name, $value = null); */ public function clear($name) { - if (!is_string($name)) { - throw new InvalidArgumentException('Expected name to be a string.'); + if (!is_string($name) || '' === $name) { + throw new InvalidArgumentException('Expected name to be a non-empty string.'); } // Don't clear anything if we're immutable. @@ -124,7 +124,7 @@ public function clear($name) /** * Clear an environment variable. * - * @param string $name + * @param non-empty-string $name * * @return void */ @@ -139,7 +139,7 @@ abstract protected function clearInternal($name); */ public function has($name) { - return is_string($name) && $this->get($name) !== null; + return is_string($name) && $name !== '' && $this->get($name) !== null; } /** diff --git a/src/Repository/Adapter/ApacheAdapter.php b/src/Repository/Adapter/ApacheAdapter.php index 73ce87d..4c95d31 100644 --- a/src/Repository/Adapter/ApacheAdapter.php +++ b/src/Repository/Adapter/ApacheAdapter.php @@ -24,7 +24,7 @@ public function isSupported() * This is intentionally not implemented, since this adapter exists only as * a means to overwrite existing apache environment variables. * - * @param string $name + * @param non-empty-string $name * * @return \PhpOption\Option */ @@ -38,22 +38,24 @@ public function get($name) * * Only if an existing apache variable exists do we overwrite it. * - * @param string $name - * @param string|null $value + * @param non-empty-string $name + * @param string|null $value * * @return void */ public function set($name, $value = null) { - if (apache_getenv($name) !== false) { - apache_setenv($name, (string) $value); + if (apache_getenv($name) === false) { + return; } + + apache_setenv($name, (string) $value); } /** * Clear an environment variable. * - * @param string $name + * @param non-empty-string $name * * @return void */ diff --git a/src/Repository/Adapter/ArrayAdapter.php b/src/Repository/Adapter/ArrayAdapter.php index dc234aa..58e4958 100644 --- a/src/Repository/Adapter/ArrayAdapter.php +++ b/src/Repository/Adapter/ArrayAdapter.php @@ -10,7 +10,7 @@ class ArrayAdapter implements AvailabilityInterface, ReaderInterface, WriterInte /** * The variables and their values. * - * @var array + * @var array */ private $variables = []; @@ -27,24 +27,24 @@ public function isSupported() /** * Get an environment variable, if it exists. * - * @param string $name + * @param non-empty-string $name * * @return \PhpOption\Option */ public function get($name) { - if (array_key_exists($name, $this->variables)) { - return Some::create($this->variables[$name]); + if (!array_key_exists($name, $this->variables)) { + return None::create(); } - return None::create(); + return Some::create($this->variables[$name]); } /** * Set an environment variable. * - * @param string $name - * @param string|null $value + * @param non-empty-string $name + * @param string|null $value * * @return void */ @@ -56,7 +56,7 @@ public function set($name, $value = null) /** * Clear an environment variable. * - * @param string $name + * @param non-empty-string $name * * @return void */ diff --git a/src/Repository/Adapter/EnvConstAdapter.php b/src/Repository/Adapter/EnvConstAdapter.php index 41bfc67..858e5e2 100644 --- a/src/Repository/Adapter/EnvConstAdapter.php +++ b/src/Repository/Adapter/EnvConstAdapter.php @@ -20,14 +20,26 @@ public function isSupported() /** * Get an environment variable, if it exists. * - * @param string $name + * @param non-empty-string $name * * @return \PhpOption\Option */ public function get($name) { - if (array_key_exists($name, $_ENV)) { - return Some::create($_ENV[$name]); + if (!array_key_exists($name, $_ENV)) { + return None::create(); + } + + $value = $_ENV[$name]; + + if (is_scalar($value)) { + /** @var \PhpOption\Option */ + return Some::create((string) $value); + } + + if (null === $value) { + /** @var \PhpOption\Option */ + return Some::create(null); } return None::create(); @@ -36,8 +48,8 @@ public function get($name) /** * Set an environment variable. * - * @param string $name - * @param string|null $value + * @param non-empty-string $name + * @param string|null $value * * @return void */ @@ -49,7 +61,7 @@ public function set($name, $value = null) /** * Clear an environment variable. * - * @param string $name + * @param non-empty-string $name * * @return void */ diff --git a/src/Repository/Adapter/PutenvAdapter.php b/src/Repository/Adapter/PutenvAdapter.php index 766afd3..a824a50 100644 --- a/src/Repository/Adapter/PutenvAdapter.php +++ b/src/Repository/Adapter/PutenvAdapter.php @@ -19,7 +19,7 @@ public function isSupported() /** * Get an environment variable, if it exists. * - * @param string $name + * @param non-empty-string $name * * @return \PhpOption\Option */ @@ -32,8 +32,8 @@ public function get($name) /** * Set an environment variable. * - * @param string $name - * @param string|null $value + * @param non-empty-string $name + * @param string|null $value * * @return void */ @@ -45,7 +45,7 @@ public function set($name, $value = null) /** * Clear an environment variable. * - * @param string $name + * @param non-empty-string $name * * @return void */ diff --git a/src/Repository/Adapter/ReaderInterface.php b/src/Repository/Adapter/ReaderInterface.php index 90a1fab..008eed1 100644 --- a/src/Repository/Adapter/ReaderInterface.php +++ b/src/Repository/Adapter/ReaderInterface.php @@ -7,7 +7,7 @@ interface ReaderInterface extends AvailabilityInterface /** * Get an environment variable, if it exists. * - * @param string $name + * @param non-empty-string $name * * @return \PhpOption\Option */ diff --git a/src/Repository/Adapter/ServerConstAdapter.php b/src/Repository/Adapter/ServerConstAdapter.php index 529df96..3189f3b 100644 --- a/src/Repository/Adapter/ServerConstAdapter.php +++ b/src/Repository/Adapter/ServerConstAdapter.php @@ -20,14 +20,26 @@ public function isSupported() /** * Get an environment variable, if it exists. * - * @param string $name + * @param non-empty-string $name * * @return \PhpOption\Option */ public function get($name) { - if (array_key_exists($name, $_SERVER)) { - return Some::create($_SERVER[$name]); + if (!array_key_exists($name, $_SERVER)) { + return None::create(); + } + + $value = $_SERVER[$name]; + + if (is_scalar($value)) { + /** @var \PhpOption\Option */ + return Some::create((string) $value); + } + + if (null === $value) { + /** @var \PhpOption\Option */ + return Some::create(null); } return None::create(); @@ -36,8 +48,8 @@ public function get($name) /** * Set an environment variable. * - * @param string $name - * @param string|null $value + * @param non-empty-string $name + * @param string|null $value * * @return void */ @@ -49,7 +61,7 @@ public function set($name, $value = null) /** * Clear an environment variable. * - * @param string $name + * @param non-empty-string $name * * @return void */ diff --git a/src/Repository/Adapter/WriterInterface.php b/src/Repository/Adapter/WriterInterface.php index 6670b72..968aeea 100644 --- a/src/Repository/Adapter/WriterInterface.php +++ b/src/Repository/Adapter/WriterInterface.php @@ -7,8 +7,8 @@ interface WriterInterface extends AvailabilityInterface /** * Set an environment variable. * - * @param string $name - * @param string|null $value + * @param non-empty-string $name + * @param string|null $value * * @return void */ @@ -17,7 +17,7 @@ public function set($name, $value = null); /** * Clear an environment variable. * - * @param string $name + * @param non-empty-string $name * * @return void */ diff --git a/src/Repository/AdapterRepository.php b/src/Repository/AdapterRepository.php index e1b6bcc..d997c9a 100644 --- a/src/Repository/AdapterRepository.php +++ b/src/Repository/AdapterRepository.php @@ -39,7 +39,7 @@ public function __construct(array $readers, array $writers, $immutable) * * We do this by querying our readers sequentially. * - * @param string $name + * @param non-empty-string $name * * @return string|null */ @@ -58,8 +58,8 @@ protected function getInternal($name) /** * Set an environment variable. * - * @param string $name - * @param string|null $value + * @param non-empty-string $name + * @param string|null $value * * @return void */ @@ -73,7 +73,7 @@ protected function setInternal($name, $value = null) /** * Clear an environment variable. * - * @param string $name + * @param non-empty-string $name * * @return void */ diff --git a/tests/Dotenv/RepositoryTest.php b/tests/Dotenv/RepositoryTest.php index ffef3db..1f2fe29 100644 --- a/tests/Dotenv/RepositoryTest.php +++ b/tests/Dotenv/RepositoryTest.php @@ -141,15 +141,26 @@ public function testGettingVariableByName() /** * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Expected name to be a string. + * @expectedExceptionMessage Expected name to be a non-empty string. */ - public function testGettingBadVariable() + public function testGettingNullVariable() { $repo = RepositoryBuilder::create()->make(); $repo->get(null); } + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Expected name to be a non-empty string. + */ + public function testGettingEmptyVariable() + { + $repo = RepositoryBuilder::create()->make(); + + $repo->get(''); + } + public function testSettingVariable() { $this->load(); @@ -163,15 +174,26 @@ public function testSettingVariable() /** * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Expected name to be a string. + * @expectedExceptionMessage Expected name to be a non-empty string. */ - public function testSettingBadVariable() + public function testSettingNullVariable() { $repo = RepositoryBuilder::create()->make(); $repo->set(null, 'foo'); } + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Expected name to be a non-empty string. + */ + public function testSettingEmptyVariable() + { + $repo = RepositoryBuilder::create()->make(); + + $repo->set('', 'foo'); + } + public function testClearingVariable() { $this->load(); @@ -197,15 +219,26 @@ public function testClearingVariableWithArrayAdapter() /** * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Expected name to be a string. + * @expectedExceptionMessage Expected name to be a non-empty string. */ - public function testClearingBadVariable() + public function testClearingNullVariable() { $repo = RepositoryBuilder::create()->make(); $repo->clear(null); } + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Expected name to be a non-empty string. + */ + public function testClearingEmptyVariable() + { + $repo = RepositoryBuilder::create()->make(); + + $repo->clear(''); + } + public function testCannotSetVariableOnImmutableInstance() { $this->load();