Skip to content

Commit

Permalink
refactor: Throw more friendly exceptions when loading a configuration (…
Browse files Browse the repository at this point in the history 
…#1045)

Make the configuration and its factory throw more comprehensible exceptions. This also makes the switch from `InvalidArgumentException` to `UnexpectedValueException` which is more correct IMO as it is values provided by the user.
  • Loading branch information
@theofidry
theofidry authored Jun 15, 2024
1 parent 3224161 commit 36b530c
Show file tree
Hide file tree
Showing 12 changed files with 323 additions and 131 deletions.
21 changes: 7 additions & 14 deletions src/Configuration/Configuration.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@

namespace Humbug\PhpScoper\Configuration;

use Humbug\PhpScoper\Configuration\Throwable\InvalidConfigurationValue;
use Humbug\PhpScoper\Patcher\Patcher;
use InvalidArgumentException;
use function Safe\preg_match;
use function sprintf;

final class Configuration
{
Expand All @@ -38,6 +37,8 @@ final class Configuration
* @param array<string, array{string, string}> $excludedFilesWithContents Array of tuple
* with the first argument being the file path and
* the second its contents
*
* @throws InvalidConfigurationValue
*/
public function __construct(
private ?string $path,
Expand Down Expand Up @@ -71,6 +72,8 @@ public function getOutputDir(): ?string

/**
* @param non-empty-string $prefix
*
* @throws InvalidConfigurationValue
*/
public function withPrefix(string $prefix): self
{
Expand Down Expand Up @@ -151,21 +154,11 @@ public function getSymbolsConfiguration(): SymbolsConfiguration
private static function validatePrefix(string $prefix): void
{
if (1 !== preg_match(self::PREFIX_PATTERN, $prefix)) {
throw new InvalidArgumentException(
sprintf(
'The prefix needs to be composed solely of letters, digits and backslashes (as namespace separators). Got "%s"',
$prefix,
),
);
throw InvalidConfigurationValue::forInvalidPrefixPattern($prefix);
}

if (preg_match('/\\\{2,}/', $prefix)) {
throw new InvalidArgumentException(
sprintf(
'Invalid namespace separator sequence. Got "%s"',
$prefix,
),
);
throw InvalidConfigurationValue::forInvalidNamespaceSeparator($prefix);
}
}
}
144 changes: 36 additions & 108 deletions src/Configuration/ConfigurationFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@

namespace Humbug\PhpScoper\Configuration;

use Humbug\PhpScoper\Configuration\Throwable\InvalidConfiguration;
use Humbug\PhpScoper\Configuration\Throwable\InvalidConfigurationFile;
use Humbug\PhpScoper\Configuration\Throwable\InvalidConfigurationValue;
use Humbug\PhpScoper\Patcher\ComposerPatcher;
use Humbug\PhpScoper\Patcher\Patcher;
use Humbug\PhpScoper\Patcher\PatcherChain;
use Humbug\PhpScoper\Patcher\SymfonyParentTraitPatcher;
use Humbug\PhpScoper\Patcher\SymfonyPatcher;
use InvalidArgumentException;
use RuntimeException;
use SplFileInfo;
use Symfony\Component\Filesystem\Filesystem;
use Symfony\Component\Finder\Finder;
Expand All @@ -33,9 +34,7 @@
use function bin2hex;
use function dirname;
use function file_exists;
use function gettype;
use function Humbug\PhpScoper\chain;
use function in_array;
use function is_array;
use function is_callable;
use function is_dir;
Expand All @@ -47,7 +46,6 @@
use function readlink as native_readlink;
use function realpath;
use function Safe\file_get_contents;
use function sprintf;
use function trim;
use const DIRECTORY_SEPARATOR;

Expand All @@ -64,14 +62,12 @@ public function __construct(
/**
* @param non-empty-string|null $path Absolute canonical path to the configuration file.
* @param list<non-empty-string> $paths List of absolute canonical paths to append besides the one configured
*
* @throws InvalidConfiguration
*/
public function create(?string $path = null, array $paths = []): Configuration
{
if (null === $path) {
$config = [];
} else {
$config = $this->loadConfigFile($path);
}
$config = null === $path ? [] : $this->loadConfigFile($path);

self::validateConfigKeys($config);

Expand Down Expand Up @@ -134,48 +130,31 @@ public function createWithPrefix(Configuration $config, string $prefix): Configu
return $config->withPrefix($prefix);
}

/**
* @throws InvalidConfigurationValue
*/
private function loadConfigFile(string $path): array
{
if (!$this->fileSystem->isAbsolutePath($path)) {
throw new InvalidArgumentException(
sprintf(
'Expected the path of the configuration file to load to be an absolute path, got "%s" instead',
$path,
),
);
throw InvalidConfigurationFile::forNonAbsolutePath($path);
}

if (!file_exists($path)) {
throw new InvalidArgumentException(
sprintf(
'Expected the path of the configuration file to exists but the file "%s" could not be found',
$path,
),
);
throw InvalidConfigurationFile::forFileNotFound($path);
}

$isADirectoryLink = is_link($path)
&& false !== native_readlink($path)
&& is_file(native_readlink($path));

if (!$isADirectoryLink && !is_file($path)) {
throw new InvalidArgumentException(
sprintf(
'Expected the path of the configuration file to be a file but "%s" appears to be a directory.',
$path,
),
);
throw InvalidConfigurationFile::forNotAFile($path);
}

$config = include $path;

if (!is_array($config)) {
throw new InvalidArgumentException(
sprintf(
'Expected configuration to be an array, found "%s" instead.',
gettype($config),
),
);
throw InvalidConfigurationFile::forInvalidValue($path);
}

return $config;
Expand All @@ -184,25 +163,11 @@ private function loadConfigFile(string $path): array
private static function validateConfigKeys(array $config): void
{
array_map(
static fn (string $key) => self::validateConfigKey($key),
ConfigurationKeys::assertIsValidKey(...),
array_keys($config),
);
}

private static function validateConfigKey(string $key): void
{
if (in_array($key, ConfigurationKeys::KEYWORDS, true)) {
return;
}

throw new InvalidArgumentException(
sprintf(
'Invalid configuration key value "%s" found.',
$key,
),
);
}

/**
* @return non-empty-string
*/
Expand All @@ -224,6 +189,8 @@ private static function retrieveOutputDir(array $config): ?string
}

/**
* @throws InvalidConfigurationValue
*
* @return array<(callable(string,string,string): string)|Patcher>
*/
private static function retrievePatchers(array $config): array
Expand All @@ -235,31 +202,21 @@ private static function retrievePatchers(array $config): array
$patchers = $config[ConfigurationKeys::PATCHERS_KEYWORD];

if (!is_array($patchers)) {
throw new InvalidArgumentException(
sprintf(
'Expected patchers to be an array of callables, found "%s" instead.',
gettype($patchers),
),
);
throw InvalidConfigurationValue::forInvalidPatchersType($patchers);
}

foreach ($patchers as $index => $patcher) {
if (is_callable($patcher)) {
continue;
if (!is_callable($patcher)) {
throw InvalidConfigurationValue::forInvalidPatcherType($index, $patcher);
}

throw new InvalidArgumentException(
sprintf(
'Expected patchers to be an array of callables, the "%d" element is not.',
$index,
),
);
}

return $patchers;
}

/**
* @throws InvalidConfigurationValue
*
* @return string[] Absolute paths
*/
private function retrieveExcludedFiles(string $dirPath, array $config): array
Expand All @@ -271,22 +228,12 @@ private function retrieveExcludedFiles(string $dirPath, array $config): array
$excludedFiles = $config[ConfigurationKeys::EXCLUDED_FILES_KEYWORD];

if (!is_array($excludedFiles)) {
throw new InvalidArgumentException(
sprintf(
'Expected excluded files to be an array of strings, found "%s" instead.',
gettype($excludedFiles),
),
);
throw InvalidConfigurationValue::forInvalidExcludedFilesTypes($excludedFiles);
}

foreach ($excludedFiles as $index => $file) {
if (!is_string($file)) {
throw new InvalidArgumentException(
sprintf(
'Expected excluded files to be an array of string, the "%d" element is not.',
$index,
),
);
throw InvalidConfigurationValue::forInvalidExcludedFilePath($index, $excludedFiles);
}

if (!$this->fileSystem->isAbsolutePath($file)) {
Expand All @@ -296,10 +243,14 @@ private function retrieveExcludedFiles(string $dirPath, array $config): array
$excludedFiles[$index] = realpath($file);
}

// We ignore files not found excluded file as we do not want to bail out just because a file we do not want to
// include does not exist.
return array_filter($excludedFiles);
}

/**
* @throws InvalidConfigurationValue
*
* @return Finder[]
*/
private static function retrieveFinders(array $config): array
Expand All @@ -311,27 +262,15 @@ private static function retrieveFinders(array $config): array
$finders = $config[ConfigurationKeys::FINDER_KEYWORD];

if (!is_array($finders)) {
throw new InvalidArgumentException(
sprintf(
'Expected finders to be an array of "%s", found "%s" instead.',
Finder::class,
gettype($finders),
),
);
throw InvalidConfigurationValue::forInvalidFinderTypes($finders);
}

foreach ($finders as $index => $finder) {
if ($finder instanceof Finder) {
continue;
}

throw new InvalidArgumentException(
sprintf(
'Expected finders to be an array of "%s", the "%d" element is not.',
Finder::class,
$index,
),
);
throw InvalidConfigurationValue::forInvalidFinderType($index, $finder);
}

return $finders;
Expand All @@ -340,6 +279,8 @@ private static function retrieveFinders(array $config): array
/**
* @param string[] $paths
*
* @throws InvalidConfigurationValue
*
* @return iterable<SplFileInfo>
*/
private static function retrieveFilesFromPaths(array $paths): iterable
Expand All @@ -353,12 +294,7 @@ private static function retrieveFilesFromPaths(array $paths): iterable

foreach ($paths as $path) {
if (!file_exists($path)) {
throw new RuntimeException(
sprintf(
'Could not find the file "%s".',
$path,
),
);
throw InvalidConfigurationValue::forFileNotFound($path);
}

if (is_dir($path)) {
Expand All @@ -384,6 +320,8 @@ private static function retrieveFilesFromPaths(array $paths): iterable
/**
* @param iterable<SplFileInfo|string> $files
*
* @throws InvalidConfigurationValue
*
* @return array<string, array{string, string}> Array of tuple with the first argument being the file path and the second its contents
*/
private static function retrieveFilesWithContents(iterable $files): array
Expand All @@ -396,21 +334,11 @@ private static function retrieveFilesWithContents(iterable $files): array
: realpath($filePathOrFileInfo);

if (!$filePath) {
throw new RuntimeException(
sprintf(
'Could not find the file "%s".',
(string) $filePathOrFileInfo,
),
);
throw InvalidConfigurationValue::forFileNotFound((string) $filePathOrFileInfo);
}

if (!is_readable($filePath)) {
throw new RuntimeException(
sprintf(
'Could not read the file "%s".',
$filePath,
),
);
throw InvalidConfigurationValue::forUnreadableFile($filePath);
}

$filesWithContents[$filePath] = [$filePath, file_get_contents($filePath)];
Expand Down
16 changes: 16 additions & 0 deletions src/Configuration/ConfigurationKeys.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

namespace Humbug\PhpScoper\Configuration;

use Humbug\PhpScoper\Configuration\Throwable\UnknownConfigurationKey;
use Humbug\PhpScoper\NotInstantiable;

final class ConfigurationKeys
Expand Down Expand Up @@ -58,4 +59,19 @@ final class ConfigurationKeys
self::FUNCTIONS_INTERNAL_SYMBOLS_KEYWORD,
self::CONSTANTS_INTERNAL_SYMBOLS_KEYWORD,
];

/**
* @throws UnknownConfigurationKey
*/
public static function assertIsValidKey(string $key): void
{
if (!self::isValidateKey($key)) {
throw UnknownConfigurationKey::forKey($key);
}
}

public static function isValidateKey(string $key): bool
{
return in_array($key, self::KEYWORDS, true);
}
}
Loading

0 comments on commit 36b530c

Please sign in to comment.