Skip to content
This repository has been archived by the owner on Dec 3, 2023. It is now read-only.

Commit

Permalink
[Cleaning] Get rid of nullable parameters (#2977)
Browse files Browse the repository at this point in the history
Co-authored-by: kaizen-ci <info@kaizen-ci.org>
  • Loading branch information
TomasVotruba and kaizen-ci authored Feb 21, 2021
1 parent 1738443 commit f5d8133
Show file tree
Hide file tree
Showing 61 changed files with 631 additions and 377 deletions.
3 changes: 3 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ packages/*/docs/ export-ignore

# highlight neon files as yaml
*.neon linguist-language=YAML

# for git-wrapper
packages/git-wrapper/tests/build/*
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"nette/forms": "^3.1",
"ondram/ci-detector": "^3.5",
"psr/log": "^1.1",
"rector/rector": "^0.9.26",
"rector/rector": "^0.9.30",
"symfony/doctrine-bridge": "^4.4|^5.2",
"symfony/framework-bundle": "^4.4|^5.2",
"symfony/security-bundle": "^4.4|^5.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ protected function setUp(): void
/**
* @dataProvider provideData()
*/
public function test(string $docBlock, string $parameterName, ?string $expectedType): void
public function test(string $docBlock, string $parameterName, string $expectedType): void
{
$resolvedType = $this->paramTypeDocBlockResolver->resolve($docBlock, $parameterName);
$this->assertSame($expectedType, $resolvedType);
Expand All @@ -32,8 +32,21 @@ public function test(string $docBlock, string $parameterName, ?string $expectedT
public function provideData(): Iterator
{
yield ['/** @param Type[] $name */', 'name', 'Type'];
yield ['/** @param Type[] $name */', '___not', null];
yield ['/** @param array<Type> $name */', 'name', 'Type'];
yield ['/** @param iterable<Type> $name */', 'name', 'Type'];
}

/**
* @dataProvider provideDataMissmatchName()
*/
public function testMissmatchName(string $docBlock, string $parameterName): void
{
$resolvedType = $this->paramTypeDocBlockResolver->resolve($docBlock, $parameterName);
$this->assertNull($resolvedType);
}

public function provideDataMissmatchName(): Iterator
{
yield ['/** @param Type[] $name */', '___not'];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public function __construct(

public function createContentFromPullRequestsBySortPriority(
array $pullRequests,
?string $sortPriority,
string $sortPriority,
bool $inCategories,
bool $inPackages
): string {
Expand Down
6 changes: 4 additions & 2 deletions packages/changelog-linker/src/ChangeTree/ChangeFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
use Symplify\ChangelogLinker\Git\GitCommitDateTagResolver;
use Symplify\ChangelogLinker\ValueObject\ChangeTree\Change;
use Symplify\ChangelogLinker\ValueObject\Option;
use Symplify\ChangelogLinker\ValueObject\PackageName;
use Symplify\MonorepoBuilder\ValueObject\Package;
use Symplify\PackageBuilder\Parameter\ParameterProvider;

/**
Expand Down Expand Up @@ -91,9 +93,9 @@ private function escapeMarkdown(string $content): string
return Strings::replace($content, self::ASTERISK_REGEX, '\\\$1');
}

private function resolveMessageWithoutPackage(string $message, ?string $package): string
private function resolveMessageWithoutPackage(string $message, string $package): string
{
if ($package === null) {
if ($package === PackageName::UNKNOWN) {
return $message;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public function __construct(ChangeFactory $changeFactory, ChangeSorter $changeSo
*/
public function resolveSortedChangesFromPullRequestsWithSortPriority(
array $pullRequests,
?string $sortPriority
string $sortPriority
): array {
$changes = [];

Expand Down
15 changes: 3 additions & 12 deletions packages/changelog-linker/src/ChangeTree/ChangeSorter.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,13 @@
namespace Symplify\ChangelogLinker\ChangeTree;

use Symplify\ChangelogLinker\ValueObject\ChangeTree\Change;
use Symplify\ChangelogLinker\ValueObject\PackageCategoryPriority;

/**
* @see \Symplify\ChangelogLinker\Tests\ChangeTree\ChangeSorterTest
*/
final class ChangeSorter
{
/**
* @var string
*/
public const PRIORITY_PACKAGES = 'packages';

/**
* @var string
*/
public const PRIORITY_CATEGORIES = 'categories';

/**
* Inspiration: https://stackoverflow.com/a/8598241/1348344
*
Expand All @@ -29,7 +20,7 @@ final class ChangeSorter
* @param Change[] $changes
* @return Change[]
*/
public function sort(array $changes, ?string $priority): array
public function sort(array $changes, string $priority): array
{
// pur newer versions to the top, and "Unreleased" first
usort($changes, function (Change $firstChange, Change $secondChange) use ($priority): int {
Expand All @@ -38,7 +29,7 @@ public function sort(array $changes, ?string $priority): array
return $comparisonStatus;
}

if ($priority === self::PRIORITY_PACKAGES) {
if ($priority === PackageCategoryPriority::PACKAGES) {
return $this->comparePackagesOverCategories($firstChange, $secondChange);
}

Expand Down
16 changes: 8 additions & 8 deletions packages/changelog-linker/src/ChangelogDumper.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

namespace Symplify\ChangelogLinker;

use Symplify\ChangelogLinker\ChangeTree\ChangeSorter;
use Symplify\ChangelogLinker\Git\GitCommitDateTagResolver;
use Symplify\ChangelogLinker\ValueObject\ChangeTree\Change;
use Symplify\ChangelogLinker\ValueObject\PackageCategoryPriority;

/**
* @see \Symplify\ChangelogLinker\Tests\ChangelogDumper\ChangelogDumperTest
Expand Down Expand Up @@ -58,7 +58,7 @@ public function reportChangesWithHeadlines(
array $changes,
bool $withCategories,
bool $withPackages,
?string $priority
string $priority
): string {
$this->content .= PHP_EOL;

Expand All @@ -77,12 +77,12 @@ public function reportChangesWithHeadlines(
private function displayHeadlines(
bool $withCategories,
bool $withPackages,
?string $priority,
string $priority,
Change $change
): void {
$this->displayTag($change);

if ($priority === ChangeSorter::PRIORITY_PACKAGES) {
if ($priority === PackageCategoryPriority::PACKAGES) {
$this->displayPackageIfDesired($change, $withPackages, $priority);
$this->displayCategoryIfDesired($change, $withCategories, $priority);
} else {
Expand All @@ -101,28 +101,28 @@ private function displayTag(Change $change): void
$this->previousTag = $change->getTag();
}

private function displayPackageIfDesired(Change $change, bool $withPackages, ?string $priority): void
private function displayPackageIfDesired(Change $change, bool $withPackages, string $priority): void
{
if (! $withPackages) {
return;
}
if ($this->previousPackage === $change->getPackage()) {
return;
}
$headlineLevel = $priority === ChangeSorter::PRIORITY_CATEGORIES ? 4 : 3;
$headlineLevel = $priority === PackageCategoryPriority::CATEGORIES ? 4 : 3;
$this->content .= str_repeat('#', $headlineLevel) . ' ' . $change->getPackage() . PHP_EOL;
$this->previousPackage = $change->getPackage();
}

private function displayCategoryIfDesired(Change $change, bool $withCategories, ?string $priority): void
private function displayCategoryIfDesired(Change $change, bool $withCategories, string $priority): void
{
if (! $withCategories) {
return;
}
if ($this->previousCategory === $change->getCategory()) {
return;
}
$headlineLevel = $priority === ChangeSorter::PRIORITY_PACKAGES ? 4 : 3;
$headlineLevel = $priority === PackageCategoryPriority::PACKAGES ? 4 : 3;
$this->content .= str_repeat('#', $headlineLevel) . ' ' . $change->getCategory() . PHP_EOL;
$this->previousCategory = $change->getCategory();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$sinceId = $this->highestMergedIdResolver->resolveFromInputAndChangelogContent($input, $content);

/** @var string $baseBranch */
$baseBranch = $input->getOption(Option::BASE_BRANCH);
$baseBranch = (string) $input->getOption(Option::BASE_BRANCH);

$pullRequests = $this->githubApi->getMergedPullRequestsSinceId($sinceId, $baseBranch);
if ($pullRequests === []) {
Expand Down
11 changes: 6 additions & 5 deletions packages/changelog-linker/src/Console/Input/PriorityResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Symfony\Component\Console\Input\InputInterface;
use Symplify\ChangelogLinker\ValueObject\Option;
use Symplify\ChangelogLinker\ValueObject\PackageCategoryPriority;
use Symplify\PackageBuilder\Reflection\PrivatesAccessor;

final class PriorityResolver
Expand All @@ -24,7 +25,7 @@ public function __construct()
* Detects the order in which "--in-packages" and "--in-categories" are both called.
* The first has a priority.
*/
public function resolveFromInput(InputInterface $input): ?string
public function resolveFromInput(InputInterface $input): string
{
$rawOptions = $this->privatesAccessor->getPrivateProperty($input, 'options');

Expand All @@ -34,17 +35,17 @@ public function resolveFromInput(InputInterface $input): ?string
$usedOptions = array_intersect($requiredOptions, $optionNames);

if (count($usedOptions) !== count($requiredOptions)) {
return null;
return PackageCategoryPriority::NONE;
}

foreach ($optionNames as $optionName) {
if ($optionName === Option::IN_PACKAGES) {
return 'packages';
return PackageCategoryPriority::PACKAGES;
}

return 'categories';
return PackageCategoryPriority::CATEGORIES;
}

return null;
return PackageCategoryPriority::NONE;
}
}
8 changes: 3 additions & 5 deletions packages/changelog-linker/src/Github/GithubApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public function __construct(
/**
* @return mixed[]
*/
public function getMergedPullRequestsSinceId(int $id, ?string $baseBranch = null): array
public function getMergedPullRequestsSinceId(int $id, string $baseBranch): array
{
$pullRequests = $this->getPullRequestsSinceId($id, $baseBranch);

Expand Down Expand Up @@ -113,15 +113,13 @@ public function isPullRequestMergedToBaseBranch(int $pullRequestId, string $base
/**
* @return mixed[]
*/
private function getPullRequestsSinceId(int $id, ?string $baseBranch = null): array
private function getPullRequestsSinceId(int $id, string $baseBranch): array
{
$pullRequests = [];

for ($i = 1; $i <= self::MAX_PAGE; ++$i) {
$url = sprintf(self::URL_CLOSED_PULL_REQUESTS, $this->repositoryName) . '&page=' . $i;
if ($baseBranch !== null) {
$url .= '&base=' . $baseBranch;
}
$url .= '&base=' . $baseBranch;
$response = $this->getResponseToUrl($url);

// already no more pages → stop
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

namespace Symplify\ChangelogLinker\ValueObject;

final class PackageCategoryPriority
{
/**
* @var string
*/
public const CATEGORIES = 'categories';

/**
* @var string
*/
public const NONE = 'none';

/**
* @var string
*/
public const PACKAGES = 'packages';
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use PHPUnit\Framework\TestCase;
use Symplify\ChangelogLinker\ChangeTree\ChangeSorter;
use Symplify\ChangelogLinker\ValueObject\ChangeTree\Change;
use Symplify\ChangelogLinker\ValueObject\PackageCategoryPriority;

final class ChangeSorterTest extends TestCase
{
Expand All @@ -24,7 +25,7 @@ public function testSortWithCategoryPriority(): void
{
$changes = $this->createChanges();

$sortedChanges = $this->changeSorter->sort($changes, ChangeSorter::PRIORITY_CATEGORIES);
$sortedChanges = $this->changeSorter->sort($changes, PackageCategoryPriority::CATEGORIES);
$this->assertNotSame($changes, $sortedChanges);

/** @var Change $firstChange */
Expand All @@ -42,7 +43,7 @@ public function testSortWithPackagePriority(): void
{
$changes = $this->createChanges();

$sortedChanges = $this->changeSorter->sort($changes, ChangeSorter::PRIORITY_PACKAGES);
$sortedChanges = $this->changeSorter->sort($changes, PackageCategoryPriority::PACKAGES);
$this->assertNotSame($changes, $sortedChanges);

/** @var Change $firstChange */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Iterator;
use PHPUnit\Framework\TestCase;
use Symplify\ChangelogLinker\ChangeTree\ChangeSorter;
use Symplify\ChangelogLinker\ValueObject\PackageCategoryPriority;

final class ChangeSorterTogetherTest extends TestCase
{
Expand All @@ -30,7 +31,7 @@ protected function setUp(): void
* Tags should keep the same order for whatever priority
* @dataProvider provideDataForTags()
*/
public function testTags(?string $priority): void
public function testTags(string $priority): void
{
$changes = $this->dummyChangesFactory->create();
$sortedChanges = $this->changeSorter->sort($changes, $priority);
Expand All @@ -53,16 +54,16 @@ public function testTags(?string $priority): void

public function provideDataForTags(): Iterator
{
yield [ChangeSorter::PRIORITY_CATEGORIES];
yield [ChangeSorter::PRIORITY_PACKAGES];
yield [null];
yield [PackageCategoryPriority::CATEGORIES];
yield [PackageCategoryPriority::PACKAGES];
yield [PackageCategoryPriority::NONE];
}

public function testSortWithCategoryPriority(): void
{
$changes = $this->dummyChangesFactory->create();

$sortedChanges = $this->changeSorter->sort($changes, ChangeSorter::PRIORITY_CATEGORIES);
$sortedChanges = $this->changeSorter->sort($changes, PackageCategoryPriority::CATEGORIES);

$categoriesByTags = [];
foreach ($sortedChanges as $sortedChange) {
Expand Down Expand Up @@ -117,7 +118,7 @@ public function testSortWithCategoryPriority(): void
public function testSortWithPackagePriority(): void
{
$changes = $this->dummyChangesFactory->create();
$sortedChanges = $this->changeSorter->sort($changes, ChangeSorter::PRIORITY_PACKAGES);
$sortedChanges = $this->changeSorter->sort($changes, PackageCategoryPriority::PACKAGES);

$packagesByTags = [];
foreach ($sortedChanges as $sortedChange) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Symplify\ChangelogLinker\ChangelogDumper;
use Symplify\ChangelogLinker\HttpKernel\ChangelogLinkerKernel;
use Symplify\ChangelogLinker\ValueObject\ChangeTree\Change;
use Symplify\ChangelogLinker\ValueObject\PackageCategoryPriority;
use Symplify\PackageBuilder\Testing\AbstractKernelTestCase;

final class WithTagsTest extends AbstractKernelTestCase
Expand Down Expand Up @@ -46,7 +47,7 @@ public function testReportChanges(): void
public function testReportBothWithCategoriesPriority(
bool $withCategories,
bool $withPackages,
?string $priority,
string $priority,
string $expectedOutputFile
): void {
$this->markTestSkipped('Random false positives on Github Actions');
Expand All @@ -63,7 +64,7 @@ public function testReportBothWithCategoriesPriority(

public function provideDataForReportChangesWithHeadlines(): Iterator
{
yield [true, false, null, __DIR__ . '/WithTagsSource/expected2.md'];
yield [false, true, null, __DIR__ . '/WithTagsSource/expected3.md'];
yield [true, false, PackageCategoryPriority::NONE, __DIR__ . '/WithTagsSource/expected2.md'];
yield [false, true, PackageCategoryPriority::NONE, __DIR__ . '/WithTagsSource/expected3.md'];
}
}
Loading

0 comments on commit f5d8133

Please sign in to comment.