Skip to content

Commit

Permalink
DX: Narrow docblock types in FixerConfiguration (#6580)
Browse files Browse the repository at this point in the history
  • Loading branch information
jrmajor authored Aug 25, 2022
1 parent 1000511 commit 67e28fb
Show file tree
Hide file tree
Showing 22 changed files with 92 additions and 54 deletions.
2 changes: 1 addition & 1 deletion phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ parameters:
-
message: '#^.+no value type specified in iterable type.+\.$#'
path: *
count: 482
count: 461
tipsOfTheDay: false
tmpDir: dev-tools/phpstan/cache
18 changes: 7 additions & 11 deletions src/Console/Command/DescribeCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -196,19 +196,15 @@ private function describeRule(OutputInterface $output, string $name): void

if (null === $allowed) {
$allowed = array_map(
static function (string $type): string {
return '<comment>'.$type.'</comment>';
},
$option->getAllowedTypes()
static fn (string $type): string => '<comment>'.$type.'</comment>',
$option->getAllowedTypes(),
);
} else {
foreach ($allowed as &$value) {
if ($value instanceof AllowedValueSubset) {
$value = 'a subset of <comment>'.HelpCommand::toString($value->getAllowedValues()).'</comment>';
} else {
$value = '<comment>'.HelpCommand::toString($value).'</comment>';
}
}
$allowed = array_map(static function ($value): string {
return $value instanceof AllowedValueSubset
? 'a subset of <comment>'.HelpCommand::toString($value->getAllowedValues()).'</comment>'
: '<comment>'.HelpCommand::toString($value).'</comment>';
}, $allowed);
}

$line .= ' ('.implode(', ', $allowed).')';
Expand Down
4 changes: 3 additions & 1 deletion src/Console/Command/HelpCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,16 @@ public static function toString($value): string

/**
* Returns the allowed values of the given option that can be converted to a string.
*
* @return list<null|AllowedValueSubset|scalar>
*/
public static function getDisplayableAllowedValues(FixerOptionInterface $option): ?array
{
$allowed = $option->getAllowedValues();

if (null !== $allowed) {
$allowed = array_filter($allowed, static function ($value): bool {
return !$value instanceof \Closure;
return null === $value || \is_scalar($value) || $value instanceof AllowedValueSubset;
});

usort($allowed, static function ($valueA, $valueB): int {
Expand Down
20 changes: 9 additions & 11 deletions src/Documentation/FixerDocumentGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,19 +148,17 @@ public function generateFixerDocumentation(FixerInterface $fixer): string

if (null === $allowed) {
$allowedKind = 'Allowed types';
$allowed = array_map(static function ($value): string {
return '``'.$value.'``';
}, $option->getAllowedTypes());
$allowed = array_map(
static fn ($value): string => '``'.$value.'``',
$option->getAllowedTypes(),
);
} else {
$allowedKind = 'Allowed values';

foreach ($allowed as &$value) {
if ($value instanceof AllowedValueSubset) {
$value = 'a subset of ``'.HelpCommand::toString($value->getAllowedValues()).'``';
} else {
$value = '``'.HelpCommand::toString($value).'``';
}
}
$allowed = array_map(static function ($value): string {
return $value instanceof AllowedValueSubset
? 'a subset of ``'.HelpCommand::toString($value->getAllowedValues()).'``'
: '``'.HelpCommand::toString($value).'``';
}, $allowed);
}

$allowed = implode(', ', $allowed);
Expand Down
20 changes: 9 additions & 11 deletions src/Documentation/ListDocumentGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,19 +108,17 @@ static function (FixerInterface $fixer1, FixerInterface $fixer2): int {

if (null === $allowed) {
$allowedKind = 'Allowed types';
$allowed = array_map(static function ($value): string {
return '``'.$value.'``';
}, $option->getAllowedTypes());
$allowed = array_map(
static fn ($value): string => '``'.$value.'``',
$option->getAllowedTypes(),
);
} else {
$allowedKind = 'Allowed values';

foreach ($allowed as &$value) {
if ($value instanceof AllowedValueSubset) {
$value = 'a subset of ``'.HelpCommand::toString($value->getAllowedValues()).'``';
} else {
$value = '``'.HelpCommand::toString($value).'``';
}
}
$allowed = array_map(static function ($value): string {
return $value instanceof AllowedValueSubset
? 'a subset of ``'.HelpCommand::toString($value->getAllowedValues()).'``'
: '``'.HelpCommand::toString($value).'``';
}, $allowed);
}

$allowed = implode(', ', $allowed);
Expand Down
5 changes: 4 additions & 1 deletion src/FixerConfiguration/AliasedFixerOptionBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function setDefault($default): self
}

/**
* @param string[] $allowedTypes
* @param list<string> $allowedTypes
*/
public function setAllowedTypes(array $allowedTypes): self
{
Expand All @@ -51,6 +51,9 @@ public function setAllowedTypes(array $allowedTypes): self
return $this;
}

/**
* @param list<(callable(mixed): bool)|null|scalar> $allowedValues
*/
public function setAllowedValues(array $allowedValues): self
{
$this->optionBuilder->setAllowedValues($allowedValues);
Expand Down
9 changes: 9 additions & 0 deletions src/FixerConfiguration/AllowedValueSubset.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,14 @@
*/
final class AllowedValueSubset
{
/**
* @var list<string>
*/
private array $allowedValues;

/**
* @param list<string> $allowedValues
*/
public function __construct(array $allowedValues)
{
$this->allowedValues = $allowedValues;
Expand All @@ -46,6 +52,9 @@ public function __invoke($values): bool
return true;
}

/**
* @return list<string>
*/
public function getAllowedValues(): ?array
{
return $this->allowedValues;
Expand Down
4 changes: 2 additions & 2 deletions src/FixerConfiguration/FixerConfigurationResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
final class FixerConfigurationResolver implements FixerConfigurationResolverInterface
{
/**
* @var FixerOptionInterface[]
* @var list<FixerOptionInterface>
*/
private array $options = [];

/**
* @var string[]
* @var list<string>
*/
private array $registeredNames = [];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
interface FixerConfigurationResolverInterface
{
/**
* @return FixerOptionInterface[]
* @return list<FixerOptionInterface>
*/
public function getOptions(): array;

Expand Down
9 changes: 5 additions & 4 deletions src/FixerConfiguration/FixerOption.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ final class FixerOption implements FixerOptionInterface
private $default;

/**
* @var null|string[]
* @var null|list<string>
*/
private $allowedTypes;

/**
* @var null|array
* @var null|list<(callable(mixed): bool)|null|scalar>
*/
private $allowedValues;

Expand All @@ -43,8 +43,9 @@ final class FixerOption implements FixerOptionInterface
private $normalizer;

/**
* @param mixed $default
* @param null|string[] $allowedTypes
* @param mixed $default
* @param null|list<string> $allowedTypes
* @param null|list<(callable(mixed): bool)|null|scalar> $allowedValues
*/
public function __construct(
string $name,
Expand Down
8 changes: 5 additions & 3 deletions src/FixerConfiguration/FixerOptionBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ final class FixerOptionBuilder
private bool $isRequired = true;

/**
* @var null|string[]
* @var null|list<string>
*/
private $allowedTypes;

/**
* @var null|array
* @var null|list<(callable(mixed): bool)|null|scalar>
*/
private $allowedValues;

Expand Down Expand Up @@ -67,7 +67,7 @@ public function setDefault($default): self
}

/**
* @param string[] $allowedTypes
* @param list<string> $allowedTypes
*
* @return $this
*/
Expand All @@ -79,6 +79,8 @@ public function setAllowedTypes(array $allowedTypes): self
}

/**
* @param list<(callable(mixed): bool)|null|scalar> $allowedValues
*
* @return $this
*/
public function setAllowedValues(array $allowedValues): self
Expand Down
5 changes: 4 additions & 1 deletion src/FixerConfiguration/FixerOptionInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,13 @@ public function hasDefault(): bool;
public function getDefault();

/**
* @return null|string[]
* @return null|list<string>
*/
public function getAllowedTypes(): ?array;

/**
* @return null|list<(callable(mixed): bool)|null|scalar>
*/
public function getAllowedValues(): ?array;

public function getNormalizer(): ?\Closure;
Expand Down
6 changes: 6 additions & 0 deletions src/FixerDefinition/CodeSample.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,14 @@ final class CodeSample implements CodeSampleInterface
{
private string $code;

/**
* @var null|array<string, mixed>
*/
private ?array $configuration;

/**
* @param null|array<string, mixed> $configuration
*/
public function __construct(string $code, ?array $configuration = null)
{
$this->code = $code;
Expand Down
3 changes: 3 additions & 0 deletions src/FixerDefinition/CodeSampleInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,8 @@ interface CodeSampleInterface
{
public function getCode(): string;

/**
* @return null|array<string, mixed>
*/
public function getConfiguration(): ?array;
}
3 changes: 3 additions & 0 deletions src/FixerDefinition/FileSpecificCodeSample.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ final class FileSpecificCodeSample implements FileSpecificCodeSampleInterface

private \SplFileInfo $splFileInfo;

/**
* @param null|array<string, mixed> $configuration
*/
public function __construct(
string $code,
\SplFileInfo $splFileInfo,
Expand Down
6 changes: 3 additions & 3 deletions src/FixerDefinition/FixerDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ final class FixerDefinition implements FixerDefinitionInterface
private string $summary;

/**
* @var CodeSampleInterface[]
* @var list<CodeSampleInterface>
*/
private array $codeSamples;

Expand All @@ -31,8 +31,8 @@ final class FixerDefinition implements FixerDefinitionInterface
private ?string $riskyDescription;

/**
* @param CodeSampleInterface[] $codeSamples array of samples, where single sample is [code, configuration]
* @param null|string $riskyDescription null for non-risky fixer
* @param list<CodeSampleInterface> $codeSamples array of samples, where single sample is [code, configuration]
* @param null|string $riskyDescription null for non-risky fixer
*/
public function __construct(
string $summary,
Expand Down
2 changes: 1 addition & 1 deletion src/FixerDefinition/FixerDefinitionInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public function getRiskyDescription(): ?string;
/**
* Array of samples, where single sample is [code, configuration].
*
* @return CodeSampleInterface[]
* @return list<CodeSampleInterface>
*/
public function getCodeSamples(): array;
}
3 changes: 3 additions & 0 deletions src/FixerDefinition/VersionSpecificCodeSample.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ final class VersionSpecificCodeSample implements VersionSpecificCodeSampleInterf

private VersionSpecificationInterface $versionSpecification;

/**
* @param null|array<string, mixed> $configuration
*/
public function __construct(
string $code,
VersionSpecificationInterface $versionSpecification,
Expand Down
11 changes: 11 additions & 0 deletions src/FixerDefinition/VersionSpecification.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,20 @@
*/
final class VersionSpecification implements VersionSpecificationInterface
{
/**
* @var null|int<1, max>
*/
private ?int $minimum;

/**
* @var null|int<1, max>
*/
private ?int $maximum;

/**
* @param null|int<1, max> $minimum
* @param null|int<1, max> $maximum
*
* @throws \InvalidArgumentException
*/
public function __construct(?int $minimum = null, ?int $maximum = null)
Expand All @@ -32,11 +41,13 @@ public function __construct(?int $minimum = null, ?int $maximum = null)
throw new \InvalidArgumentException('Minimum or maximum need to be specified.');
}

// @phpstan-ignore-next-line
if (null !== $minimum && 1 > $minimum) {
throw new \InvalidArgumentException('Minimum needs to be either null or an integer greater than 0.');
}

if (null !== $maximum) {
// @phpstan-ignore-next-line
if (1 > $maximum) {
throw new \InvalidArgumentException('Maximum needs to be either null or an integer greater than 0.');
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Console/Command/HelpCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public function provideGetDisplayableAllowedValuesCases(): iterable

yield [['A', 'B', 'x', 'z'], new FixerOption('foo', 'bar', false, null, ['string'], ['z', 'x', 'B', 'A'])];

yield [[0, 3, 9], new FixerOption('foo', 'bar', false, null, ['int'], [0, 3, 9, static fn () => null])];
yield [[0, 3, 9], new FixerOption('foo', 'bar', false, null, ['int'], [0, 3, 9, static fn () => true])];

yield [null, new FixerOption('foo', 'bar')];
}
Expand Down
2 changes: 1 addition & 1 deletion tests/FixerConfiguration/AliasedFixerOptionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public function provideGetAllowedValuesCases(): array

public function testGetAllowedValuesClosure(): void
{
$option = new AliasedFixerOption(new FixerOption('foo', 'Bar.', true, null, null, [static fn () => null]), 'baz');
$option = new AliasedFixerOption(new FixerOption('foo', 'Bar.', true, null, null, [static fn () => true]), 'baz');
$allowedTypes = $option->getAllowedValues();
static::assertIsArray($allowedTypes);
static::assertCount(1, $allowedTypes);
Expand Down
2 changes: 1 addition & 1 deletion tests/FixerConfiguration/FixerOptionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public function testGetAllowedValues(): void
$option = new FixerOption('foo', 'Bar.', true, null, null, ['baz', 'qux']);
static::assertSame(['baz', 'qux'], $option->getAllowedValues());

$option = new FixerOption('foo', 'Bar.', true, null, null, [static fn () => null]);
$option = new FixerOption('foo', 'Bar.', true, null, null, [static fn () => true]);
$allowedTypes = $option->getAllowedValues();
static::assertIsArray($allowedTypes);
static::assertCount(1, $allowedTypes);
Expand Down

0 comments on commit 67e28fb

Please sign in to comment.