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

[Cleaning] Get rid of nullable parameters #2977

Merged
merged 14 commits into from
Feb 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

should the branch name be escaped for use in a url?

Copy link
Member Author

Choose a reason for hiding this comment

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

It might be, but Changelog Linker is now deprecated, see: #3027

$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