From ab94531a4d3904000275adf0984c8cd05fc97032 Mon Sep 17 00:00:00 2001 From: Oskar Stark Date: Mon, 16 Sep 2024 18:02:48 +0200 Subject: [PATCH 1/2] Enhancement: Executed rector PHP 8.2 set --- .github/workflows/ci.yml | 2 +- config/packages/doctrine.yaml | 2 +- public/config.php | 7 +- public/index.php | 4 +- src/Api/Issue/GithubIssueApi.php | 20 ++--- src/Api/Label/GithubLabelApi.php | 36 ++------- src/Api/Milestone/GithubMilestoneApi.php | 24 ++---- src/Api/Milestone/MilestoneApi.php | 2 +- src/Api/Milestone/NullMilestoneApi.php | 2 +- src/Api/PullRequest/GithubPullRequestApi.php | 13 ++- src/Api/PullRequest/NullPullRequestApi.php | 2 +- src/Api/PullRequest/PullRequestApi.php | 2 +- src/Api/Status/GitHubStatusApi.php | 25 ++---- src/Api/Status/NullStatusApi.php | 4 +- src/Api/Status/Status.php | 11 +-- src/Api/Status/StatusApi.php | 4 +- src/Api/Workflow/GithubWorkflowApi.php | 11 +-- src/Command/ListTaskCommand.php | 11 ++- src/Command/PingStaleIssuesCommand.php | 32 +++----- src/Command/RunTaskCommand.php | 19 ++--- src/Controller/DefaultController.php | 5 +- src/Controller/WebhookController.php | 4 +- src/Entity/Task.php | 79 +++++-------------- src/Event/EventDispatcher.php | 4 +- src/Event/GitHubEvent.php | 24 +++--- src/Model/Repository.php | 26 ++---- src/Service/GitHubRequestHandler.php | 22 +++--- src/Service/LabelNameExtractor.php | 31 +++----- src/Service/RepositoryProvider.php | 6 +- src/Service/StaleIssueCommentGenerator.php | 14 ++-- src/Service/SymfonyVersionProvider.php | 20 ++--- src/Service/TaskHandler/CloseDraftHandler.php | 17 ++-- .../TaskHandler/CloseStaleIssuesHandler.php | 17 ++-- .../InformAboutClosingStaleIssuesHandler.php | 22 ++---- src/Service/TaskRunner.php | 18 ++--- src/Service/TaskScheduler.php | 8 +- .../AbstractStatusChangeSubscriber.php | 16 ++-- .../AllowEditFromMaintainerSubscriber.php | 15 ++-- .../ApproveCiForNonContributors.php | 15 ++-- .../AutoLabelFromContentSubscriber.php | 27 ++++--- .../AutoUpdateTitleWithLabelSubscriber.php | 21 +++-- src/Subscriber/BugLabelNewIssueSubscriber.php | 18 ++--- src/Subscriber/CloseDraftPRSubscriber.php | 18 ++--- .../MilestoneMergedPRSubscriber.php | 15 ++-- src/Subscriber/MilestoneNewPRSubscriber.php | 24 +++--- .../MismatchBranchDescriptionSubscriber.php | 14 ++-- src/Subscriber/NeedsReviewNewPRSubscriber.php | 15 ++-- .../RemoveStalledLabelOnCommentSubscriber.php | 18 ++--- .../RewriteUnwantedPhrasesSubscriber.php | 18 ++--- .../StatusChangeByCommentSubscriber.php | 18 +++-- .../StatusChangeByReviewSubscriber.php | 37 ++++----- .../StatusChangeOnPushSubscriber.php | 15 ++-- .../UnsupportedBranchSubscriber.php | 21 +++-- ...eWhenLabeledWaitingCodeMergeSubscriber.php | 15 ++-- .../WelcomeFirstTimeContributorSubscriber.php | 18 ++--- tests/Api/Label/GithubLabelApiTest.php | 4 +- tests/Controller/WebhookControllerTest.php | 4 +- ...AutoUpdateTitleWithLabelSubscriberTest.php | 32 ++++---- 58 files changed, 382 insertions(+), 566 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dd32b6d6..efe79d18 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,7 +75,7 @@ jobs: with: php-version: 8.3 coverage: none - tools: php-cs-fixer:3.42, cs2pr + tools: php-cs-fixer:3.64, cs2pr - name: Checkout code uses: actions/checkout@v4 diff --git a/config/packages/doctrine.yaml b/config/packages/doctrine.yaml index 5e80e77b..893350ee 100644 --- a/config/packages/doctrine.yaml +++ b/config/packages/doctrine.yaml @@ -12,7 +12,7 @@ doctrine: mappings: App: is_bundle: false - type: annotation + type: attribute dir: '%kernel.project_dir%/src/Entity' prefix: 'App\Entity' alias: App diff --git a/public/config.php b/public/config.php index 1b4ff9f3..a31b89c6 100644 --- a/public/config.php +++ b/public/config.php @@ -4,15 +4,12 @@ exit("This script cannot be run from the CLI. Run it from a browser.\n"); } -if (!in_array(@$_SERVER['REMOTE_ADDR'], array( - '127.0.0.1', - '::1', -))) { +if (!in_array(@$_SERVER['REMOTE_ADDR'], ['127.0.0.1', '::1'])) { header('HTTP/1.0 403 Forbidden'); exit('This script is only accessible from localhost.'); } -require_once dirname(__FILE__).'/../app/SymfonyRequirements.php'; +require_once __DIR__.'/../app/SymfonyRequirements.php'; $symfonyRequirements = new SymfonyRequirements(); diff --git a/public/index.php b/public/index.php index 9982c218..cf18ce02 100644 --- a/public/index.php +++ b/public/index.php @@ -4,6 +4,4 @@ require_once dirname(__DIR__).'/vendor/autoload_runtime.php'; -return function (array $context) { - return new Kernel($context['APP_ENV'], (bool) $context['APP_DEBUG']); -}; +return fn(array $context) => new Kernel($context['APP_ENV'], (bool) $context['APP_DEBUG']); diff --git a/src/Api/Issue/GithubIssueApi.php b/src/Api/Issue/GithubIssueApi.php index 7b6ba46d..b52be75a 100644 --- a/src/Api/Issue/GithubIssueApi.php +++ b/src/Api/Issue/GithubIssueApi.php @@ -10,19 +10,13 @@ class GithubIssueApi implements IssueApi { - private $resultPager; - private $issueCommentApi; - private $issueApi; - private $searchApi; - private $botUsername; - - public function __construct(ResultPager $resultPager, Comments $issueCommentApi, Issue $issueApi, Search $searchApi, string $botUsername) - { - $this->resultPager = $resultPager; - $this->issueCommentApi = $issueCommentApi; - $this->issueApi = $issueApi; - $this->searchApi = $searchApi; - $this->botUsername = $botUsername; + public function __construct( + private readonly ResultPager $resultPager, + private readonly Comments $issueCommentApi, + private readonly Issue $issueApi, + private readonly Search $searchApi, + private readonly string $botUsername, + ) { } public function open(Repository $repository, string $title, string $body, array $labels) diff --git a/src/Api/Label/GithubLabelApi.php b/src/Api/Label/GithubLabelApi.php index 350bc9b7..e72b1959 100644 --- a/src/Api/Label/GithubLabelApi.php +++ b/src/Api/Label/GithubLabelApi.php @@ -20,34 +20,14 @@ class GithubLabelApi implements LabelApi * * @var array> */ - private $labelCache = []; - - /** - * @var Labels - */ - private $labelsApi; - - /** - * @var ResultPager - */ - private $resultPager; - - /** - * @var CacheInterface - */ - private $cache; - - /** - * @var LoggerInterface - */ - private $logger; - - public function __construct(Labels $labelsApi, ResultPager $resultPager, CacheInterface $cache, LoggerInterface $logger) - { - $this->labelsApi = $labelsApi; - $this->resultPager = $resultPager; - $this->cache = $cache; - $this->logger = $logger; + private array $labelCache = []; + + public function __construct( + private readonly Labels $labelsApi, + private readonly ResultPager $resultPager, + private readonly CacheInterface $cache, + private readonly LoggerInterface $logger, + ) { } public function getIssueLabels($issueNumber, Repository $repository): array diff --git a/src/Api/Milestone/GithubMilestoneApi.php b/src/Api/Milestone/GithubMilestoneApi.php index bfdcf9bf..efad194f 100644 --- a/src/Api/Milestone/GithubMilestoneApi.php +++ b/src/Api/Milestone/GithubMilestoneApi.php @@ -11,25 +11,15 @@ */ class GithubMilestoneApi implements MilestoneApi { - /** - * @var Milestones - */ - private $milestonesApi; - - /** - * @var Issue - */ - private $issuesApi; - /** * @var string[][] */ - private $cache = []; + private array $cache = []; - public function __construct(Milestones $milestonesApi, Issue $issuesApi) - { - $this->milestonesApi = $milestonesApi; - $this->issuesApi = $issuesApi; + public function __construct( + private readonly Milestones $milestonesApi, + private readonly Issue $issuesApi, + ) { } private function getMilestones(Repository $repository): array @@ -48,7 +38,7 @@ private function getMilestones(Repository $repository): array return $this->cache[$key]; } - public function updateMilestone(Repository $repository, int $issueNumber, string $milestoneName) + public function updateMilestone(Repository $repository, int $issueNumber, string $milestoneName): void { $milestoneNumber = null; foreach ($this->getMilestones($repository) as $milestone) { @@ -77,7 +67,7 @@ public function exists(Repository $repository, string $milestoneName): bool return false; } - private function getCacheKey(Repository $repository) + private function getCacheKey(Repository $repository): string { return sprintf('%s_%s', $repository->getVendor(), $repository->getName()); } diff --git a/src/Api/Milestone/MilestoneApi.php b/src/Api/Milestone/MilestoneApi.php index 9417dea5..4c7d3299 100644 --- a/src/Api/Milestone/MilestoneApi.php +++ b/src/Api/Milestone/MilestoneApi.php @@ -9,7 +9,7 @@ */ interface MilestoneApi { - public function updateMilestone(Repository $repository, int $issueNumber, string $milestoneName); + public function updateMilestone(Repository $repository, int $issueNumber, string $milestoneName): void; public function exists(Repository $repository, string $milestoneName): bool; } diff --git a/src/Api/Milestone/NullMilestoneApi.php b/src/Api/Milestone/NullMilestoneApi.php index 719ee023..2d58e7aa 100644 --- a/src/Api/Milestone/NullMilestoneApi.php +++ b/src/Api/Milestone/NullMilestoneApi.php @@ -8,7 +8,7 @@ class NullMilestoneApi implements MilestoneApi { - public function updateMilestone(Repository $repository, int $issueNumber, string $milestoneName) + public function updateMilestone(Repository $repository, int $issueNumber, string $milestoneName): void { } diff --git a/src/Api/PullRequest/GithubPullRequestApi.php b/src/Api/PullRequest/GithubPullRequestApi.php index 3c3e4e3f..65789590 100644 --- a/src/Api/PullRequest/GithubPullRequestApi.php +++ b/src/Api/PullRequest/GithubPullRequestApi.php @@ -11,13 +11,10 @@ */ class GithubPullRequestApi implements PullRequestApi { - private $pullRequest; - private $search; - - public function __construct(PullRequest $pullRequest, Search $search) - { - $this->pullRequest = $pullRequest; - $this->search = $search; + public function __construct( + private readonly PullRequest $pullRequest, + private readonly Search $search, + ) { } public function show(Repository $repository, $number): array @@ -25,7 +22,7 @@ public function show(Repository $repository, $number): array return (array) $this->pullRequest->show($repository->getVendor(), $repository->getName(), $number); } - public function updateTitle(Repository $repository, $number, string $title, string $body = null): void + public function updateTitle(Repository $repository, $number, string $title, ?string $body = null): void { $params = ['title' => $title]; diff --git a/src/Api/PullRequest/NullPullRequestApi.php b/src/Api/PullRequest/NullPullRequestApi.php index f8e459ea..31960242 100644 --- a/src/Api/PullRequest/NullPullRequestApi.php +++ b/src/Api/PullRequest/NullPullRequestApi.php @@ -16,7 +16,7 @@ public function show(Repository $repository, $number): array return []; } - public function updateTitle(Repository $repository, $number, string $title, string $body = null): void + public function updateTitle(Repository $repository, $number, string $title, ?string $body = null): void { } diff --git a/src/Api/PullRequest/PullRequestApi.php b/src/Api/PullRequest/PullRequestApi.php index 67c2fb31..8b566d74 100644 --- a/src/Api/PullRequest/PullRequestApi.php +++ b/src/Api/PullRequest/PullRequestApi.php @@ -16,7 +16,7 @@ interface PullRequestApi { public function show(Repository $repository, $number): array; - public function updateTitle(Repository $repository, $number, string $title, string $body = null): void; + public function updateTitle(Repository $repository, $number, string $title, ?string $body = null): void; public function getAuthorCount(Repository $repository, string $author): int; } diff --git a/src/Api/Status/GitHubStatusApi.php b/src/Api/Status/GitHubStatusApi.php index c0e7cd02..ac147031 100644 --- a/src/Api/Status/GitHubStatusApi.php +++ b/src/Api/Status/GitHubStatusApi.php @@ -8,36 +8,27 @@ class GitHubStatusApi implements StatusApi { - private static $statusToLabel = [ + private static array $statusToLabel = [ Status::NEEDS_REVIEW => 'Status: Needs Review', Status::NEEDS_WORK => 'Status: Needs Work', Status::WORKS_FOR_ME => 'Status: Works for me', Status::REVIEWED => 'Status: Reviewed', ]; - private $labelToStatus = []; + private array $labelToStatus; - /** - * @var LabelApi - */ - private $labelsApi; - /** - * @var LoggerInterface - */ - private $logger; - - public function __construct(LabelApi $labelsApi, LoggerInterface $logger) - { - $this->labelsApi = $labelsApi; + public function __construct( + private readonly LabelApi $labelsApi, + private readonly LoggerInterface $logger, + ) { $this->labelToStatus = array_flip(self::$statusToLabel); - $this->logger = $logger; } /** * @param int $issueNumber The GitHub issue number * @param string|null $newStatus A Status::* constant */ - public function setIssueStatus($issueNumber, ?string $newStatus, Repository $repository) + public function setIssueStatus(int $issueNumber, ?string $newStatus, Repository $repository): void { if (null !== $newStatus && !isset(self::$statusToLabel[$newStatus])) { throw new \InvalidArgumentException(sprintf('Invalid status "%s"', $newStatus)); @@ -92,7 +83,7 @@ public function getIssueStatus($issueNumber, Repository $repository): ?string return null; } - public static function getNeedsReviewLabel() + public static function getNeedsReviewLabel(): string { return self::$statusToLabel[Status::NEEDS_REVIEW]; } diff --git a/src/Api/Status/NullStatusApi.php b/src/Api/Status/NullStatusApi.php index 11daea93..77ad4f34 100644 --- a/src/Api/Status/NullStatusApi.php +++ b/src/Api/Status/NullStatusApi.php @@ -9,12 +9,12 @@ */ class NullStatusApi implements StatusApi { - public function getIssueStatus($issueNumber, Repository $repository): ?string + public function getIssueStatus(int $issueNumber, Repository $repository): ?string { return null; } - public function setIssueStatus($issueNumber, ?string $newStatus, Repository $repository) + public function setIssueStatus(int $issueNumber, ?string $newStatus, Repository $repository): void { } } diff --git a/src/Api/Status/Status.php b/src/Api/Status/Status.php index 895692b1..88cb3b2d 100644 --- a/src/Api/Status/Status.php +++ b/src/Api/Status/Status.php @@ -9,11 +9,8 @@ */ final class Status { - public const NEEDS_REVIEW = 'needs_review'; - - public const NEEDS_WORK = 'needs_work'; - - public const WORKS_FOR_ME = 'works_for_me'; - - public const REVIEWED = 'reviewed'; + public const string NEEDS_REVIEW = 'needs_review'; + public const string NEEDS_WORK = 'needs_work'; + public const string WORKS_FOR_ME = 'works_for_me'; + public const string REVIEWED = 'reviewed'; } diff --git a/src/Api/Status/StatusApi.php b/src/Api/Status/StatusApi.php index 0b79201a..8b987ec2 100644 --- a/src/Api/Status/StatusApi.php +++ b/src/Api/Status/StatusApi.php @@ -11,7 +11,7 @@ */ interface StatusApi { - public function getIssueStatus($issueNumber, Repository $repository): ?string; + public function getIssueStatus(int $issueNumber, Repository $repository): ?string; - public function setIssueStatus($issueNumber, ?string $newStatus, Repository $repository); + public function setIssueStatus(int $issueNumber, ?string $newStatus, Repository $repository): void; } diff --git a/src/Api/Workflow/GithubWorkflowApi.php b/src/Api/Workflow/GithubWorkflowApi.php index 66d7ed65..b8a59004 100644 --- a/src/Api/Workflow/GithubWorkflowApi.php +++ b/src/Api/Workflow/GithubWorkflowApi.php @@ -11,13 +11,10 @@ */ class GithubWorkflowApi implements WorkflowApi { - private ResultPager $resultPager; - private WorkflowRuns $workflowApi; - - public function __construct(ResultPager $resultPager, WorkflowRuns $workflowApi) - { - $this->resultPager = $resultPager; - $this->workflowApi = $workflowApi; + public function __construct( + private readonly ResultPager $resultPager, + private readonly WorkflowRuns $workflowApi, + ) { } public function approveWorkflowsForPullRequest(Repository $repository, string $headRepository, string $headBranch): void diff --git a/src/Command/ListTaskCommand.php b/src/Command/ListTaskCommand.php index 09db1c3f..007e2b5b 100644 --- a/src/Command/ListTaskCommand.php +++ b/src/Command/ListTaskCommand.php @@ -18,12 +18,11 @@ class ListTaskCommand extends Command { protected static $defaultName = 'app:task:list'; - private $repository; - public function __construct(TaskRepository $repository) - { + public function __construct( + private readonly TaskRepository $repository, + ) { parent::__construct(); - $this->repository = $repository; } protected function configure() @@ -31,7 +30,7 @@ protected function configure() $this->addOption('number', null, InputOption::VALUE_REQUIRED, 'The issue number we are interested in', null); } - protected function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): int { /** @var mixed|null $number */ $number = $input->getOption('number'); @@ -61,7 +60,7 @@ protected function execute(InputInterface $input, OutputInterface $output) } $io->newLine(); - return 0; + return Command::SUCCESS; } private function actionToString(int $action): string diff --git a/src/Command/PingStaleIssuesCommand.php b/src/Command/PingStaleIssuesCommand.php index 6bbf41e4..f09b0f44 100644 --- a/src/Command/PingStaleIssuesCommand.php +++ b/src/Command/PingStaleIssuesCommand.php @@ -27,30 +27,24 @@ class PingStaleIssuesCommand extends Command protected static $defaultName = 'app:issue:ping-stale'; - private $repositoryProvider; - private $issueApi; - private $scheduler; - private $commentGenerator; - private $labelApi; - - public function __construct(RepositoryProvider $repositoryProvider, IssueApi $issueApi, TaskScheduler $scheduler, StaleIssueCommentGenerator $commentGenerator, LabelApi $labelApi) - { + public function __construct( + private readonly RepositoryProvider $repositoryProvider, + private readonly IssueApi $issueApi, + private readonly TaskScheduler $scheduler, + private readonly StaleIssueCommentGenerator $commentGenerator, + private readonly LabelApi $labelApi, + ) { parent::__construct(); - $this->repositoryProvider = $repositoryProvider; - $this->issueApi = $issueApi; - $this->scheduler = $scheduler; - $this->commentGenerator = $commentGenerator; - $this->labelApi = $labelApi; } - protected function configure() + protected function configure(): void { $this->addArgument('repository', InputArgument::REQUIRED, 'The full name to the repository, eg symfony/symfony-docs'); $this->addOption('not-updated-for', null, InputOption::VALUE_REQUIRED, 'A string representing a time period to for how long the issue has been stalled.', '12months'); $this->addOption('dry-run', null, InputOption::VALUE_NONE, 'Do a test search without making any comments or changes'); } - protected function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): int { /** @var string $repositoryName */ $repositoryName = $input->getArgument('repository'); @@ -58,7 +52,7 @@ protected function execute(InputInterface $input, OutputInterface $output) if (null === $repository) { $output->writeln('Repository not configured'); - return 1; + return Command::FAILURE; } /** @var string $timeString */ @@ -71,7 +65,7 @@ protected function execute(InputInterface $input, OutputInterface $output) $output->writeln(sprintf('Marking issue #%s as "Stalled". Link https://github.com/%s/issues/%s', $issue['number'], $repository->getFullName(), $issue['number'])); } - return 0; + return Command::SUCCESS; } foreach ($issues as $issue) { @@ -83,14 +77,14 @@ protected function execute(InputInterface $input, OutputInterface $output) $this->scheduler->runLater($repository, (int) $issue['number'], Task::ACTION_INFORM_CLOSE_STALE, new \DateTimeImmutable(self::MESSAGE_TWO_AFTER)); } - return 0; + return Command::SUCCESS; } /** * Extract type from issue array. Make sure we prioritize labels if there are * more than one type defined. */ - private function extractType(array $issue) + private function extractType(array $issue): string { $types = [ IssueType::FEATURE => false, diff --git a/src/Command/RunTaskCommand.php b/src/Command/RunTaskCommand.php index 75f7294d..399562cf 100644 --- a/src/Command/RunTaskCommand.php +++ b/src/Command/RunTaskCommand.php @@ -15,24 +15,21 @@ class RunTaskCommand extends Command { protected static $defaultName = 'app:task:run'; - private $repository; - private $taskRunner; - private $logger; - public function __construct(TaskRepository $repository, TaskRunner $taskRunner, LoggerInterface $logger) - { + public function __construct( + private readonly TaskRepository $repository, + private readonly TaskRunner $taskRunner, + private readonly LoggerInterface $logger, + ) { parent::__construct(); - $this->repository = $repository; - $this->taskRunner = $taskRunner; - $this->logger = $logger; } - protected function configure() + protected function configure(): void { $this->addOption('limit', 'l', InputOption::VALUE_REQUIRED, 'Limit the number of tasks to run', '10'); } - protected function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): int { $limit = (int) $input->getOption('limit'); foreach ($this->repository->getTasksToVerify($limit) as $task) { @@ -44,6 +41,6 @@ protected function execute(InputInterface $input, OutputInterface $output) } } - return 0; + return Command::SUCCESS; } } diff --git a/src/Controller/DefaultController.php b/src/Controller/DefaultController.php index 1f1021e6..e34bea5c 100644 --- a/src/Controller/DefaultController.php +++ b/src/Controller/DefaultController.php @@ -4,12 +4,13 @@ use App\Service\RepositoryProvider; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Attribute\Route; -class DefaultController extends AbstractController +final class DefaultController extends AbstractController { #[Route('/')] - public function index(RepositoryProvider $repositoryProvider) + public function index(RepositoryProvider $repositoryProvider): Response { $repositories = $repositoryProvider->getAllRepositories(); diff --git a/src/Controller/WebhookController.php b/src/Controller/WebhookController.php index 221e95bc..9bcaa89b 100644 --- a/src/Controller/WebhookController.php +++ b/src/Controller/WebhookController.php @@ -8,10 +8,10 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Attribute\Route; -class WebhookController extends AbstractController +final class WebhookController extends AbstractController { #[Route('/webhooks/github', name: 'webhooks_github', methods: ['POST'])] - public function github(Request $request, GitHubRequestHandler $requestHandler) + public function github(Request $request, GitHubRequestHandler $requestHandler): JsonResponse { $responseData = $requestHandler->handle($request); diff --git a/src/Entity/Task.php b/src/Entity/Task.php index 7e5942bc..800aa973 100644 --- a/src/Entity/Task.php +++ b/src/Entity/Task.php @@ -2,19 +2,17 @@ namespace App\Entity; +use App\Repository\TaskRepository; use Doctrine\ORM\Mapping as ORM; /** * A "Task" is a scheduled job. * - * @ORM\Table - * - * @ORM\Entity(repositoryClass="App\Repository\TaskRepository") - * - * @ORM\HasLifecycleCallbacks() - * * @author Tobias Nyholm */ +#[ORM\Entity(repositoryClass: TaskRepository::class)] +#[ORM\HasLifecycleCallbacks] +#[ORM\Table] class Task { public const ACTION_CLOSE_STALE = 1; @@ -23,63 +21,31 @@ class Task /** * @var int - * - * @ORM\Column(name="id", type="integer") - * - * @ORM\Id - * - * @ORM\GeneratedValue(strategy="AUTO") */ + #[ORM\Column(name: 'id', type: 'integer')] + #[ORM\Id] + #[ORM\GeneratedValue(strategy: 'AUTO')] private $id; - /** - * @var string - * - * @ORM\Column(type="string") - */ - private $repositoryFullName; - - /** - * @var int - * - * @ORM\Column(type="integer") - */ - private $number; - - /** - * @var int - * - * @ORM\Column(type="integer") - */ - private $action; - - /** - * @var \DateTimeImmutable - * - * @ORM\Column(type="datetime_immutable") - */ - private $createdAt; + #[ORM\Column(type: 'datetime_immutable')] + private \DateTimeImmutable $createdAt; /** * @var \DateTimeImmutable - * - * @ORM\Column(type="datetime_immutable") */ + #[ORM\Column(type: 'datetime_immutable')] private $updatedAt; - /** - * @var \DateTimeImmutable - * - * @ORM\Column(type="datetime_immutable") - */ - private $verifyAfter; - - public function __construct(string $repositoryFullName, int $number, int $action, \DateTimeImmutable $verifyAfter) + public function __construct( + #[ORM\Column(type: 'string')] + private readonly string $repositoryFullName, + #[ORM\Column(type: 'integer')] + private readonly int $number, + #[ORM\Column(type: 'integer')] + private readonly int $action, + #[ORM\Column(type: 'datetime_immutable')] + private \DateTimeImmutable $verifyAfter) { - $this->repositoryFullName = $repositoryFullName; - $this->number = $number; - $this->action = $action; - $this->verifyAfter = $verifyAfter; $this->createdAt = new \DateTimeImmutable(); } @@ -113,11 +79,8 @@ public function setVerifyAfter(\DateTimeImmutable $verifyAfter): void $this->verifyAfter = $verifyAfter; } - /** - * @ORM\PrePersist - * - * @ORM\PreUpdate - */ + #[ORM\PrePersist] + #[ORM\PreUpdate] public function updateUpdatedAt() { $this->updatedAt = new \DateTimeImmutable(); diff --git a/src/Event/EventDispatcher.php b/src/Event/EventDispatcher.php index a8c98f90..cebbfc8c 100644 --- a/src/Event/EventDispatcher.php +++ b/src/Event/EventDispatcher.php @@ -7,9 +7,9 @@ class EventDispatcher { /** - * @var EventDispatcherInterface[] + * @var array */ - protected $dispatchers = []; + protected array $dispatchers = []; public function addDispatcher(string $repositoryName, EventDispatcherInterface $dispatcher): void { diff --git a/src/Event/GitHubEvent.php b/src/Event/GitHubEvent.php index 43e32801..95d32523 100644 --- a/src/Event/GitHubEvent.php +++ b/src/Event/GitHubEvent.php @@ -10,36 +10,30 @@ */ class GitHubEvent extends Event { - protected $responseData = []; + protected array $responseData = []; - private $data; - private $repository; - - public function __construct(array $data, Repository $repository) - { - $this->data = $data; - $this->repository = $repository; + public function __construct( + private readonly array $data, + private readonly Repository $repository, + ) { } - public function getData() + public function getData(): array { return $this->data; } - /** - * @return Repository - */ - public function getRepository() + public function getRepository(): Repository { return $this->repository; } - public function getResponseData() + public function getResponseData(): array { return $this->responseData; } - public function setResponseData(array $responseData) + public function setResponseData(array $responseData): void { foreach ($responseData as $k => $v) { $this->responseData[$k] = $v; diff --git a/src/Model/Repository.php b/src/Model/Repository.php index 56d1d275..db7d2a04 100644 --- a/src/Model/Repository.php +++ b/src/Model/Repository.php @@ -10,27 +10,13 @@ class Repository { /** - * @var string + * @param string|null $secret the webhook secret used by GitHub */ - private $vendor; - - /** - * @var string - */ - private $name; - - /** - * The webhook secret used by GitHub. - * - * @var string|null - */ - private $secret; - - public function __construct(string $vendor, string $name, string $secret = null) - { - $this->vendor = $vendor; - $this->name = $name; - $this->secret = $secret; + public function __construct( + private readonly string $vendor, + private readonly string $name, + private readonly ?string $secret = null, + ) { } public function getVendor(): string diff --git a/src/Service/GitHubRequestHandler.php b/src/Service/GitHubRequestHandler.php index 40adde74..ff5aaed1 100644 --- a/src/Service/GitHubRequestHandler.php +++ b/src/Service/GitHubRequestHandler.php @@ -17,28 +17,24 @@ */ class GitHubRequestHandler { - private $dispatcher; - private $repositoryProvider; - private $logger; - - public function __construct(EventDispatcher $dispatcher, RepositoryProvider $repositoryProvider, LoggerInterface $logger) - { - $this->dispatcher = $dispatcher; - $this->repositoryProvider = $repositoryProvider; - $this->logger = $logger; + public function __construct( + private readonly EventDispatcher $dispatcher, + private readonly RepositoryProvider $repositoryProvider, + private readonly LoggerInterface $logger, + ) { } /** * @return array The response data */ - public function handle(Request $request) + public function handle(Request $request): array { - $data = json_decode((string) $request->getContent(), true); + $data = json_decode($request->getContent(), true); if (null === $data) { throw new BadRequestHttpException('Invalid JSON body!'); } - $repositoryFullName = isset($data['repository']['full_name']) ? $data['repository']['full_name'] : null; + $repositoryFullName = $data['repository']['full_name'] ?? null; if (empty($repositoryFullName)) { throw new BadRequestHttpException('No repository name!'); } @@ -88,7 +84,7 @@ public function handle(Request $request) return $responseData; } - private function authenticate($hash, $key, $data) + private function authenticate(string $hash, string $key, string $data): bool { if (!extension_loaded('hash')) { throw new \RuntimeException('"hash" extension is needed to check request signature.'); diff --git a/src/Service/LabelNameExtractor.php b/src/Service/LabelNameExtractor.php index 56426e5e..d1c0ed31 100644 --- a/src/Service/LabelNameExtractor.php +++ b/src/Service/LabelNameExtractor.php @@ -13,10 +13,7 @@ */ class LabelNameExtractor { - private $labelsApi; - private $logger; - - private static $labelAliases = [ + private static array $labelAliases = [ 'bridge\doctrine' => 'DoctrineBridge', 'bridge/doctrine' => 'DoctrineBridge', 'bridge\monolog' => 'MonologBridge', @@ -35,17 +32,17 @@ class LabelNameExtractor 'wdt' => 'WebProfilerBundle', ]; - public function __construct(LabelApi $labelsApi, LoggerInterface $logger) - { - $this->labelsApi = $labelsApi; - $this->logger = $logger; + public function __construct( + private readonly LabelApi $labelsApi, + private readonly LoggerInterface $logger, + ) { } /** * Get labels from title string. * Example title: "[PropertyAccess] [RFC] [WIP] Allow custom methods on property accesses". */ - public function extractLabels($title, Repository $repository) + public function extractLabels(string $title, Repository $repository): array { $labels = []; if (preg_match_all('/\[(?P.+)\]/U', $title, $matches)) { @@ -77,14 +74,12 @@ public function getAliasesForLabel($label) /** * Creates a key=>val array, but the key is lowercased. * - * @return array + * @return array */ - private function getLabels(Repository $repository) + private function getLabels(Repository $repository): array { $allLabels = $this->labelsApi->getAllLabelsForRepository($repository); - $closure = function ($s) { - return strtolower($s); - }; + $closure = fn ($s) => strtolower($s); return array_combine(array_map($closure, $allLabels), $allLabels); } @@ -93,14 +88,10 @@ private function getLabels(Repository $repository) * It fixes common misspellings and aliases commonly used for label names * (e.g. DI -> DependencyInjection). */ - private function fixLabelName($label) + private function fixLabelName(string $label): string { $labelAliases = self::$labelAliases; - if (isset($labelAliases[strtolower($label)])) { - return $labelAliases[strtolower($label)]; - } - - return $label; + return $labelAliases[strtolower($label)] ?? $label; } } diff --git a/src/Service/RepositoryProvider.php b/src/Service/RepositoryProvider.php index 578d1c7f..36bb2a89 100644 --- a/src/Service/RepositoryProvider.php +++ b/src/Service/RepositoryProvider.php @@ -12,7 +12,7 @@ class RepositoryProvider /** * @var Repository[] */ - private $repositories = []; + private array $repositories = []; public function __construct(array $repositories) { @@ -21,7 +21,7 @@ public function __construct(array $repositories) throw new \InvalidArgumentException(sprintf('The repository name %s is invalid: it must be the form "username/repo_name"', $repositoryFullName)); } - list($vendorName, $repositoryName) = explode('/', $repositoryFullName); + [$vendorName, $repositoryName] = explode('/', $repositoryFullName); $this->addRepository(new Repository( $vendorName, @@ -46,7 +46,7 @@ public function getAllRepositories(): array return array_values($this->repositories); } - private function addRepository(Repository $repository) + private function addRepository(Repository $repository): void { $this->repositories[strtolower($repository->getVendor().'/'.$repository->getName())] = $repository; } diff --git a/src/Service/StaleIssueCommentGenerator.php b/src/Service/StaleIssueCommentGenerator.php index 32021dde..9e64fb5b 100644 --- a/src/Service/StaleIssueCommentGenerator.php +++ b/src/Service/StaleIssueCommentGenerator.php @@ -47,15 +47,11 @@ public function getClosingComment(): string */ public function getComment(string $type): string { - switch ($type) { - case IssueType::BUG: - return $this->bug(); - case IssueType::FEATURE: - case IssueType::RFC: - return $this->feature(); - default: - return $this->unknown(); - } + return match ($type) { + IssueType::BUG => $this->bug(), + IssueType::FEATURE, IssueType::RFC => $this->feature(), + default => $this->unknown(), + }; } private function bug(): string diff --git a/src/Service/SymfonyVersionProvider.php b/src/Service/SymfonyVersionProvider.php index 3fcd334b..15b8ffdb 100644 --- a/src/Service/SymfonyVersionProvider.php +++ b/src/Service/SymfonyVersionProvider.php @@ -10,20 +10,10 @@ class SymfonyVersionProvider { - /** - * @var HttpClientInterface - */ - private $httpClient; - - /** - * @var CacheInterface - */ - private $cache; - - public function __construct(HttpClientInterface $httpClient, CacheInterface $cache) - { - $this->httpClient = $httpClient; - $this->cache = $cache; + public function __construct( + private HttpClientInterface $httpClient, + private readonly CacheInterface $cache, + ) { } public function getCurrentVersion(): string @@ -36,7 +26,7 @@ public function getCurrentVersion(): string $response = $httpClient->request('GET', 'https://symfony.com/releases.json'); $data = $response->toArray(true); $version = $data['latest_stable_version'] ?? $defaultValue; - } catch (\Throwable $e) { + } catch (\Throwable) { $version = $defaultValue; } diff --git a/src/Service/TaskHandler/CloseDraftHandler.php b/src/Service/TaskHandler/CloseDraftHandler.php index f8eb5132..ec6c2646 100644 --- a/src/Service/TaskHandler/CloseDraftHandler.php +++ b/src/Service/TaskHandler/CloseDraftHandler.php @@ -15,17 +15,12 @@ */ class CloseDraftHandler implements TaskHandlerInterface { - private $issueApi; - private $repositoryProvider; - private $pullRequestApi; - private $logger; - - public function __construct(PullRequestApi $pullRequestApi, IssueApi $issueApi, RepositoryProvider $repositoryProvider, LoggerInterface $logger) - { - $this->issueApi = $issueApi; - $this->repositoryProvider = $repositoryProvider; - $this->pullRequestApi = $pullRequestApi; - $this->logger = $logger; + public function __construct( + private readonly PullRequestApi $pullRequestApi, + private readonly IssueApi $issueApi, + private readonly RepositoryProvider $repositoryProvider, + private readonly LoggerInterface $logger, + ) { } public function handle(Task $task): void diff --git a/src/Service/TaskHandler/CloseStaleIssuesHandler.php b/src/Service/TaskHandler/CloseStaleIssuesHandler.php index 42ee6e8c..e95d9e8f 100644 --- a/src/Service/TaskHandler/CloseStaleIssuesHandler.php +++ b/src/Service/TaskHandler/CloseStaleIssuesHandler.php @@ -15,17 +15,12 @@ */ class CloseStaleIssuesHandler implements TaskHandlerInterface { - private $issueApi; - private $repositoryProvider; - private $labelApi; - private $commentGenerator; - - public function __construct(LabelApi $labelApi, IssueApi $issueApi, RepositoryProvider $repositoryProvider, StaleIssueCommentGenerator $commentGenerator) - { - $this->issueApi = $issueApi; - $this->repositoryProvider = $repositoryProvider; - $this->labelApi = $labelApi; - $this->commentGenerator = $commentGenerator; + public function __construct( + private readonly LabelApi $labelApi, + private readonly IssueApi $issueApi, + private readonly RepositoryProvider $repositoryProvider, + private readonly StaleIssueCommentGenerator $commentGenerator, + ) { } /** diff --git a/src/Service/TaskHandler/InformAboutClosingStaleIssuesHandler.php b/src/Service/TaskHandler/InformAboutClosingStaleIssuesHandler.php index 26412756..63a09a9d 100644 --- a/src/Service/TaskHandler/InformAboutClosingStaleIssuesHandler.php +++ b/src/Service/TaskHandler/InformAboutClosingStaleIssuesHandler.php @@ -17,23 +17,17 @@ */ class InformAboutClosingStaleIssuesHandler implements TaskHandlerInterface { - private $issueApi; - private $repositoryProvider; - private $labelApi; - private $commentGenerator; - private $scheduler; - - public function __construct(LabelApi $labelApi, IssueApi $issueApi, RepositoryProvider $repositoryProvider, StaleIssueCommentGenerator $commentGenerator, TaskScheduler $scheduler) - { - $this->issueApi = $issueApi; - $this->repositoryProvider = $repositoryProvider; - $this->labelApi = $labelApi; - $this->commentGenerator = $commentGenerator; - $this->scheduler = $scheduler; + public function __construct( + private readonly LabelApi $labelApi, + private readonly IssueApi $issueApi, + private readonly RepositoryProvider $repositoryProvider, + private readonly StaleIssueCommentGenerator $commentGenerator, + private readonly TaskScheduler $scheduler, + ) { } /** - * Close the issue if the last comment was made by the bot and if "Keep open" label does not exist. + * Close the issue if the last comment was made by the bot and "Keep open" label does not exist. */ public function handle(Task $task): void { diff --git a/src/Service/TaskRunner.php b/src/Service/TaskRunner.php index db8e4058..956d1f5c 100644 --- a/src/Service/TaskRunner.php +++ b/src/Service/TaskRunner.php @@ -10,20 +10,16 @@ /** * @author Tobias Nyholm */ -class TaskRunner +readonly class TaskRunner { /** - * @var iterable + * @param iterable $handlers */ - private $handlers; - private $repository; - private $logger; - - public function __construct(TaskRepository $repository, iterable $handlers, LoggerInterface $logger) - { - $this->handlers = $handlers; - $this->repository = $repository; - $this->logger = $logger; + public function __construct( + private TaskRepository $repository, + private iterable $handlers, + private LoggerInterface $logger, + ) { } public function run(Task $task) diff --git a/src/Service/TaskScheduler.php b/src/Service/TaskScheduler.php index 9c46014e..5bb94f0f 100644 --- a/src/Service/TaskScheduler.php +++ b/src/Service/TaskScheduler.php @@ -15,11 +15,9 @@ */ class TaskScheduler { - private $taskRepo; - - public function __construct(TaskRepository $taskRepo) - { - $this->taskRepo = $taskRepo; + public function __construct( + private readonly TaskRepository $taskRepo, + ) { } public function runLater(Repository $repository, int $number, int $action, \DateTimeImmutable $checkAt) diff --git a/src/Subscriber/AbstractStatusChangeSubscriber.php b/src/Subscriber/AbstractStatusChangeSubscriber.php index db486c11..11d9a90e 100644 --- a/src/Subscriber/AbstractStatusChangeSubscriber.php +++ b/src/Subscriber/AbstractStatusChangeSubscriber.php @@ -8,29 +8,23 @@ abstract class AbstractStatusChangeSubscriber implements EventSubscriberInterface { - protected static $triggerWordToStatus = [ + protected static array $triggerWordToStatus = [ 'needs review' => Status::NEEDS_REVIEW, 'needs work' => Status::NEEDS_WORK, 'works for me' => Status::WORKS_FOR_ME, 'reviewed' => Status::REVIEWED, ]; - protected $statusApi; - - public function __construct(StatusApi $statusApi) - { - $this->statusApi = $statusApi; + public function __construct( + protected StatusApi $statusApi, + ) { } /** * Parses the text and looks for keywords to see if this should cause any * status change. - * - * @param string $body - * - * @return string|null */ - protected function parseStatusFromText($body) + protected function parseStatusFromText(string $body): ?string { $triggerWord = implode('|', array_keys(static::$triggerWordToStatus)); $formatting = '[\\s\\*]*'; diff --git a/src/Subscriber/AllowEditFromMaintainerSubscriber.php b/src/Subscriber/AllowEditFromMaintainerSubscriber.php index 3b63a39b..ddcec6ee 100644 --- a/src/Subscriber/AllowEditFromMaintainerSubscriber.php +++ b/src/Subscriber/AllowEditFromMaintainerSubscriber.php @@ -12,14 +12,12 @@ */ class AllowEditFromMaintainerSubscriber implements EventSubscriberInterface { - private $commentsApi; - - public function __construct(IssueApi $commentsApi) - { - $this->commentsApi = $commentsApi; + public function __construct( + private readonly IssueApi $commentsApi, + ) { } - public function onPullRequest(GitHubEvent $event) + public function onPullRequest(GitHubEvent $event): void { $data = $event->getData(); if (!in_array($data['action'], ['opened', 'ready_for_review']) || ($data['pull_request']['draft'] ?? false)) { @@ -51,7 +49,10 @@ public function onPullRequest(GitHubEvent $event) ]); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onPullRequest', diff --git a/src/Subscriber/ApproveCiForNonContributors.php b/src/Subscriber/ApproveCiForNonContributors.php index 90ae8442..5427f094 100644 --- a/src/Subscriber/ApproveCiForNonContributors.php +++ b/src/Subscriber/ApproveCiForNonContributors.php @@ -14,14 +14,12 @@ */ class ApproveCiForNonContributors implements EventSubscriberInterface { - private WorkflowApi $workflowApi; - - public function __construct(WorkflowApi $workflowApi) - { - $this->workflowApi = $workflowApi; + public function __construct( + private readonly WorkflowApi $workflowApi, + ) { } - public function onPullRequest(GitHubEvent $event) + public function onPullRequest(GitHubEvent $event): void { $data = $event->getData(); if (!in_array($data['action'], ['opened', 'reopened', 'synchronize'])) { @@ -36,7 +34,10 @@ public function onPullRequest(GitHubEvent $event) $event->setResponseData(['approved_run' => true]); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onPullRequest', diff --git a/src/Subscriber/AutoLabelFromContentSubscriber.php b/src/Subscriber/AutoLabelFromContentSubscriber.php index 96f144ac..b5cf4a1b 100644 --- a/src/Subscriber/AutoLabelFromContentSubscriber.php +++ b/src/Subscriber/AutoLabelFromContentSubscriber.php @@ -13,20 +13,18 @@ */ class AutoLabelFromContentSubscriber implements EventSubscriberInterface { - private $labelsApi; - - private $labelExtractor; - - public function __construct(LabelApi $labelsApi, LabelNameExtractor $labelExtractor) - { - $this->labelsApi = $labelsApi; - $this->labelExtractor = $labelExtractor; + public function __construct( + private readonly LabelApi $labelsApi, + private readonly LabelNameExtractor $labelExtractor, + ) { } - public function onPullRequest(GitHubEvent $event) + public function onPullRequest(GitHubEvent $event): void { $data = $event->getData(); - if (!in_array($data['action'], ['opened', 'ready_for_review']) || ($data['pull_request']['draft'] ?? false)) { + if (!in_array($data['action'], ['opened', 'ready_for_review']) + || ($data['pull_request']['draft'] ?? false) + ) { return; } @@ -64,10 +62,10 @@ public function onPullRequest(GitHubEvent $event) ]); } - public function onIssue(GitHubEvent $event) + public function onIssue(GitHubEvent $event): void { $data = $event->getData(); - if ('opened' !== $action = $data['action']) { + if ('opened' !== $data['action']) { return; } $repository = $event->getRepository(); @@ -89,7 +87,10 @@ public function onIssue(GitHubEvent $event) ]); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onPullRequest', diff --git a/src/Subscriber/AutoUpdateTitleWithLabelSubscriber.php b/src/Subscriber/AutoUpdateTitleWithLabelSubscriber.php index 0e6626b5..0d352360 100644 --- a/src/Subscriber/AutoUpdateTitleWithLabelSubscriber.php +++ b/src/Subscriber/AutoUpdateTitleWithLabelSubscriber.php @@ -16,18 +16,14 @@ */ class AutoUpdateTitleWithLabelSubscriber implements EventSubscriberInterface { - private $labelExtractor; - private $pullRequestApi; - private $lockFactory; - - public function __construct(LabelNameExtractor $labelExtractor, PullRequestApi $pullRequestApi, LockFactory $lockFactory) - { - $this->labelExtractor = $labelExtractor; - $this->pullRequestApi = $pullRequestApi; - $this->lockFactory = $lockFactory; + public function __construct( + private readonly LabelNameExtractor $labelExtractor, + private readonly PullRequestApi $pullRequestApi, + private readonly LockFactory $lockFactory, + ) { } - public function onPullRequest(GitHubEvent $event) + public function onPullRequest(GitHubEvent $event): void { $data = $event->getData(); $action = $data['action']; @@ -92,7 +88,10 @@ public function onPullRequest(GitHubEvent $event) ]); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onPullRequest', diff --git a/src/Subscriber/BugLabelNewIssueSubscriber.php b/src/Subscriber/BugLabelNewIssueSubscriber.php index 75a4b4b1..e1af2f15 100644 --- a/src/Subscriber/BugLabelNewIssueSubscriber.php +++ b/src/Subscriber/BugLabelNewIssueSubscriber.php @@ -10,20 +10,15 @@ class BugLabelNewIssueSubscriber implements EventSubscriberInterface { - /** - * @var StatusApi - */ - private $statusApi; - - public function __construct(StatusApi $statusApi) - { - $this->statusApi = $statusApi; + public function __construct( + private readonly StatusApi $statusApi, + ) { } /** * Changes "Bug" issues to "Needs Review". */ - public function onIssues(GitHubEvent $event) + public function onIssues(GitHubEvent $event): void { $data = $event->getData(); $repository = $event->getRepository(); @@ -48,7 +43,10 @@ public function onIssues(GitHubEvent $event) $event->setResponseData($responseData); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::ISSUES => 'onIssues', diff --git a/src/Subscriber/CloseDraftPRSubscriber.php b/src/Subscriber/CloseDraftPRSubscriber.php index c5250c9b..36d06f5c 100644 --- a/src/Subscriber/CloseDraftPRSubscriber.php +++ b/src/Subscriber/CloseDraftPRSubscriber.php @@ -14,16 +14,13 @@ */ class CloseDraftPRSubscriber implements EventSubscriberInterface { - private $issueApi; - private $scheduler; - - public function __construct(IssueApi $issueApi, TaskScheduler $scheduler) - { - $this->issueApi = $issueApi; - $this->scheduler = $scheduler; + public function __construct( + private readonly IssueApi $issueApi, + private readonly TaskScheduler $scheduler, + ) { } - public function onPullRequest(GitHubEvent $event) + public function onPullRequest(GitHubEvent $event): void { $data = $event->getData(); $repository = $event->getRepository(); @@ -54,7 +51,10 @@ public function onPullRequest(GitHubEvent $event) ]); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onPullRequest', diff --git a/src/Subscriber/MilestoneMergedPRSubscriber.php b/src/Subscriber/MilestoneMergedPRSubscriber.php index 3a6dc6bb..add37a32 100644 --- a/src/Subscriber/MilestoneMergedPRSubscriber.php +++ b/src/Subscriber/MilestoneMergedPRSubscriber.php @@ -12,17 +12,15 @@ */ class MilestoneMergedPRSubscriber implements EventSubscriberInterface { - private $milestonesApi; - - public function __construct(MilestoneApi $milestonesApi) - { - $this->milestonesApi = $milestonesApi; + public function __construct( + private readonly MilestoneApi $milestonesApi, + ) { } /** * Sets milestone on merged PRs. */ - public function onPullRequest(GitHubEvent $event) + public function onPullRequest(GitHubEvent $event): void { $data = $event->getData(); $repository = $event->getRepository(); @@ -49,7 +47,10 @@ public function onPullRequest(GitHubEvent $event) ]); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onPullRequest', diff --git a/src/Subscriber/MilestoneNewPRSubscriber.php b/src/Subscriber/MilestoneNewPRSubscriber.php index 99ba7184..032b731f 100644 --- a/src/Subscriber/MilestoneNewPRSubscriber.php +++ b/src/Subscriber/MilestoneNewPRSubscriber.php @@ -13,23 +13,18 @@ */ class MilestoneNewPRSubscriber implements EventSubscriberInterface { - private $milestonesApi; - private $symfonyVersionProvider; - private $ignoreCurrentVersion; - private $ignoreDefaultBranch; - - public function __construct(MilestoneApi $milestonesApi, SymfonyVersionProvider $symfonyVersionProvider, $ignoreCurrentVersion = false, $ignoreDefaultBranch = false) - { - $this->milestonesApi = $milestonesApi; - $this->symfonyVersionProvider = $symfonyVersionProvider; - $this->ignoreCurrentVersion = $ignoreCurrentVersion; - $this->ignoreDefaultBranch = $ignoreDefaultBranch; + public function __construct( + private readonly MilestoneApi $milestonesApi, + private readonly SymfonyVersionProvider $symfonyVersionProvider, + private $ignoreCurrentVersion = false, + private $ignoreDefaultBranch = false, + ) { } /** * Sets milestone on PRs. */ - public function onPullRequest(GitHubEvent $event) + public function onPullRequest(GitHubEvent $event): void { $data = $event->getData(); $repository = $event->getRepository(); @@ -59,7 +54,10 @@ public function onPullRequest(GitHubEvent $event) ]); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onPullRequest', diff --git a/src/Subscriber/MismatchBranchDescriptionSubscriber.php b/src/Subscriber/MismatchBranchDescriptionSubscriber.php index c47785de..43161a0c 100644 --- a/src/Subscriber/MismatchBranchDescriptionSubscriber.php +++ b/src/Subscriber/MismatchBranchDescriptionSubscriber.php @@ -15,13 +15,10 @@ */ class MismatchBranchDescriptionSubscriber implements EventSubscriberInterface { - private IssueApi $issueApi; - private LoggerInterface $logger; - - public function __construct(IssueApi $issueApi, LoggerInterface $logger) - { - $this->issueApi = $issueApi; - $this->logger = $logger; + public function __construct( + private readonly IssueApi $issueApi, + private readonly LoggerInterface $logger, + ) { } public function onPullRequest(GitHubEvent $event): void @@ -62,6 +59,9 @@ public function onPullRequest(GitHubEvent $event): void ]); } + /** + * @return array + */ public static function getSubscribedEvents(): array { return [ diff --git a/src/Subscriber/NeedsReviewNewPRSubscriber.php b/src/Subscriber/NeedsReviewNewPRSubscriber.php index 11ec513c..c14a24fc 100644 --- a/src/Subscriber/NeedsReviewNewPRSubscriber.php +++ b/src/Subscriber/NeedsReviewNewPRSubscriber.php @@ -11,17 +11,15 @@ class NeedsReviewNewPRSubscriber implements EventSubscriberInterface { - private $statusApi; - - public function __construct(StatusApi $statusApi) - { - $this->statusApi = $statusApi; + public function __construct( + private readonly StatusApi $statusApi, + ) { } /** * Adds a "Needs Review" label to new PRs. */ - public function onPullRequest(GitHubEvent $event) + public function onPullRequest(GitHubEvent $event): void { $data = $event->getData(); $repository = $event->getRepository(); @@ -43,7 +41,10 @@ public function onPullRequest(GitHubEvent $event) ]); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onPullRequest', diff --git a/src/Subscriber/RemoveStalledLabelOnCommentSubscriber.php b/src/Subscriber/RemoveStalledLabelOnCommentSubscriber.php index 9318f7da..a2125698 100644 --- a/src/Subscriber/RemoveStalledLabelOnCommentSubscriber.php +++ b/src/Subscriber/RemoveStalledLabelOnCommentSubscriber.php @@ -15,16 +15,13 @@ */ class RemoveStalledLabelOnCommentSubscriber implements EventSubscriberInterface { - private $labelApi; - private $botUsername; - - public function __construct(LabelApi $labelApi, string $botUsername) - { - $this->labelApi = $labelApi; - $this->botUsername = $botUsername; + public function __construct( + private readonly LabelApi $labelApi, + private readonly string $botUsername, + ) { } - public function onIssueComment(GitHubEvent $event) + public function onIssueComment(GitHubEvent $event): void { $data = $event->getData(); $repository = $event->getRepository(); @@ -56,7 +53,10 @@ public function onIssueComment(GitHubEvent $event) } } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::ISSUE_COMMENT => 'onIssueComment', diff --git a/src/Subscriber/RewriteUnwantedPhrasesSubscriber.php b/src/Subscriber/RewriteUnwantedPhrasesSubscriber.php index 3eb5e949..9a66e3e1 100644 --- a/src/Subscriber/RewriteUnwantedPhrasesSubscriber.php +++ b/src/Subscriber/RewriteUnwantedPhrasesSubscriber.php @@ -16,16 +16,13 @@ */ class RewriteUnwantedPhrasesSubscriber implements EventSubscriberInterface { - private $pullRequestApi; - private $lockFactory; - - public function __construct(PullRequestApi $pullRequestApi, LockFactory $lockFactory) - { - $this->pullRequestApi = $pullRequestApi; - $this->lockFactory = $lockFactory; + public function __construct( + private readonly PullRequestApi $pullRequestApi, + private readonly LockFactory $lockFactory, + ) { } - public function onPullRequest(GitHubEvent $event) + public function onPullRequest(GitHubEvent $event): void { $data = $event->getData(); $action = $data['action']; @@ -71,7 +68,10 @@ private function replaceUnwantedPhrases(string $text, &$count) return str_ireplace(array_keys($replace), array_values($replace), $text, $count); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onPullRequest', diff --git a/src/Subscriber/StatusChangeByCommentSubscriber.php b/src/Subscriber/StatusChangeByCommentSubscriber.php index 909a3284..bbf1bc3b 100644 --- a/src/Subscriber/StatusChangeByCommentSubscriber.php +++ b/src/Subscriber/StatusChangeByCommentSubscriber.php @@ -10,19 +10,18 @@ class StatusChangeByCommentSubscriber extends AbstractStatusChangeSubscriber { - private $logger; - - public function __construct(StatusApi $statusApi, LoggerInterface $logger) - { + public function __construct( + StatusApi $statusApi, + private readonly LoggerInterface $logger, + ) { parent::__construct($statusApi); - $this->logger = $logger; } /** * Parses the text of the comment and looks for keywords to see * if this should cause any status change. */ - public function onIssueComment(GitHubEvent $event) + public function onIssueComment(GitHubEvent $event): void { $data = $event->getData(); $repository = $event->getRepository(); @@ -46,14 +45,17 @@ public function onIssueComment(GitHubEvent $event) $this->statusApi->setIssueStatus($issueNumber, $newStatus, $repository); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::ISSUE_COMMENT => 'onIssueComment', ]; } - private function isUserAllowedToReview(array $data) + private function isUserAllowedToReview(array $data): bool { return $data['issue']['user']['login'] !== $data['comment']['user']['login']; } diff --git a/src/Subscriber/StatusChangeByReviewSubscriber.php b/src/Subscriber/StatusChangeByReviewSubscriber.php index 34c4434a..67691500 100644 --- a/src/Subscriber/StatusChangeByReviewSubscriber.php +++ b/src/Subscriber/StatusChangeByReviewSubscriber.php @@ -15,19 +15,18 @@ */ class StatusChangeByReviewSubscriber extends AbstractStatusChangeSubscriber { - private $logger; - - public function __construct(StatusApi $statusApi, LoggerInterface $logger) - { + public function __construct( + StatusApi $statusApi, + private readonly LoggerInterface $logger, + ) { parent::__construct($statusApi); - $this->logger = $logger; } /** * Sets the status based on the review state (approved/changes requested) * or the review body (using the Status: keyword). */ - public function onReview(GitHubEvent $event) + public function onReview(GitHubEvent $event): void { $data = $event->getData(); if ('submitted' !== $data['action']) { @@ -39,18 +38,11 @@ public function onReview(GitHubEvent $event) $newStatus = null; // Set status based on review state - switch (strtolower($data['review']['state'])) { - case 'approved': - $newStatus = Status::REVIEWED; - - break; - case 'changes_requested': - $newStatus = Status::NEEDS_WORK; - - break; - default: - $newStatus = $this->parseStatusFromText($data['review']['body']); - } + $newStatus = match (strtolower($data['review']['state'])) { + 'approved' => Status::REVIEWED, + 'changes_requested' => Status::NEEDS_WORK, + default => $this->parseStatusFromText($data['review']['body']), + }; if (Status::REVIEWED === $newStatus && false === $this->isUserAllowedToReview($data)) { $newStatus = null; @@ -72,7 +64,7 @@ public function onReview(GitHubEvent $event) /** * Sets the status to needs review when a review is requested. */ - public function onReviewRequested(GithubEvent $event) + public function onReviewRequested(GitHubEvent $event): void { $data = $event->getData(); if ('review_requested' !== $data['action']) { @@ -92,7 +84,10 @@ public function onReviewRequested(GithubEvent $event) ]); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST_REVIEW => 'onReview', @@ -100,7 +95,7 @@ public static function getSubscribedEvents() ]; } - private function isUserAllowedToReview(array $data) + private function isUserAllowedToReview(array $data): bool { return $data['pull_request']['user']['login'] !== $data['review']['user']['login']; } diff --git a/src/Subscriber/StatusChangeOnPushSubscriber.php b/src/Subscriber/StatusChangeOnPushSubscriber.php index d38e03bf..20716dcc 100644 --- a/src/Subscriber/StatusChangeOnPushSubscriber.php +++ b/src/Subscriber/StatusChangeOnPushSubscriber.php @@ -17,14 +17,12 @@ */ class StatusChangeOnPushSubscriber implements EventSubscriberInterface { - private $statusApi; - - public function __construct(StatusApi $statusApi) - { - $this->statusApi = $statusApi; + public function __construct( + private readonly StatusApi $statusApi, + ) { } - public function onPullRequest(GitHubEvent $event) + public function onPullRequest(GitHubEvent $event): void { $data = $event->getData(); if ('synchronize' !== $data['action']) { @@ -52,7 +50,10 @@ public function onPullRequest(GitHubEvent $event) $event->setResponseData($responseData); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onPullRequest', diff --git a/src/Subscriber/UnsupportedBranchSubscriber.php b/src/Subscriber/UnsupportedBranchSubscriber.php index fb8c8714..1a5639ea 100644 --- a/src/Subscriber/UnsupportedBranchSubscriber.php +++ b/src/Subscriber/UnsupportedBranchSubscriber.php @@ -14,21 +14,17 @@ */ class UnsupportedBranchSubscriber implements EventSubscriberInterface { - private $symfonyVersionProvider; - private $issueApi; - private $logger; - - public function __construct(SymfonyVersionProvider $symfonyVersionProvider, IssueApi $issueApi, LoggerInterface $logger) - { - $this->symfonyVersionProvider = $symfonyVersionProvider; - $this->issueApi = $issueApi; - $this->logger = $logger; + public function __construct( + private readonly SymfonyVersionProvider $symfonyVersionProvider, + private readonly IssueApi $issueApi, + private readonly LoggerInterface $logger, + ) { } /** * Sets milestone on PRs that target non-default branch. */ - public function onPullRequest(GitHubEvent $event) + public function onPullRequest(GitHubEvent $event): void { $data = $event->getData(); if (!in_array($data['action'], ['opened', 'ready_for_review']) || ($data['pull_request']['draft'] ?? false)) { @@ -72,7 +68,10 @@ public function onPullRequest(GitHubEvent $event) ]); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onPullRequest', diff --git a/src/Subscriber/UpdateMilestoneWhenLabeledWaitingCodeMergeSubscriber.php b/src/Subscriber/UpdateMilestoneWhenLabeledWaitingCodeMergeSubscriber.php index 8fb66c11..f656bbd4 100644 --- a/src/Subscriber/UpdateMilestoneWhenLabeledWaitingCodeMergeSubscriber.php +++ b/src/Subscriber/UpdateMilestoneWhenLabeledWaitingCodeMergeSubscriber.php @@ -14,14 +14,12 @@ */ class UpdateMilestoneWhenLabeledWaitingCodeMergeSubscriber implements EventSubscriberInterface { - private MilestoneApi $milestoneApi; - - public function __construct(MilestoneApi $milestoneApi) - { - $this->milestoneApi = $milestoneApi; + public function __construct( + private readonly MilestoneApi $milestoneApi, + ) { } - public function onLabel(GitHubEvent $event) + public function onLabel(GitHubEvent $event): void { $data = $event->getData(); $action = $data['action']; @@ -45,7 +43,10 @@ public function onLabel(GitHubEvent $event) } } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onLabel', diff --git a/src/Subscriber/WelcomeFirstTimeContributorSubscriber.php b/src/Subscriber/WelcomeFirstTimeContributorSubscriber.php index 69b4a399..1d9b69f3 100644 --- a/src/Subscriber/WelcomeFirstTimeContributorSubscriber.php +++ b/src/Subscriber/WelcomeFirstTimeContributorSubscriber.php @@ -13,16 +13,13 @@ */ class WelcomeFirstTimeContributorSubscriber implements EventSubscriberInterface { - private $commentsApi; - private $pullRequestApi; - - public function __construct(IssueApi $commentsApi, PullRequestApi $pullRequestApi) - { - $this->commentsApi = $commentsApi; - $this->pullRequestApi = $pullRequestApi; + public function __construct( + private readonly IssueApi $commentsApi, + private readonly PullRequestApi $pullRequestApi, + ) { } - public function onPullRequest(GitHubEvent $event) + public function onPullRequest(GitHubEvent $event): void { $data = $event->getData(); if (!in_array($data['action'], ['opened', 'ready_for_review']) || ($data['pull_request']['draft'] ?? false)) { @@ -74,7 +71,10 @@ public function onPullRequest(GitHubEvent $event) ]); } - public static function getSubscribedEvents() + /** + * @return array + */ + public static function getSubscribedEvents(): array { return [ GitHubEvents::PULL_REQUEST => 'onPullRequest', diff --git a/tests/Api/Label/GithubLabelApiTest.php b/tests/Api/Label/GithubLabelApiTest.php index e62816d0..aed40018 100644 --- a/tests/Api/Label/GithubLabelApiTest.php +++ b/tests/Api/Label/GithubLabelApiTest.php @@ -42,9 +42,7 @@ protected function setUp(): void ->disableOriginalConstructor() ->setMethods(['fetchAll']) ->getMock(); - $resultPager->method('fetchAll')->willReturnCallback(function () { - return $this->backendApi->all('x', 'y'); - }); + $resultPager->method('fetchAll')->willReturnCallback(fn () => $this->backendApi->all('x', 'y')); $this->api = new GithubLabelApi($this->backendApi, $resultPager, new NullAdapter(), new NullLogger()); $this->repository = new Repository( diff --git a/tests/Controller/WebhookControllerTest.php b/tests/Controller/WebhookControllerTest.php index e62c9a49..e93d2238 100644 --- a/tests/Controller/WebhookControllerTest.php +++ b/tests/Controller/WebhookControllerTest.php @@ -50,13 +50,13 @@ public function testIssueComment($eventHeader, $payloadFilename, $expectedRespon $response = $client->getResponse(); $responseData = json_decode($response->getContent(), true); - $this->assertResponseIsSuccessful(isset($responseData['error']) ? $responseData['error'] : 'An error occurred.'); + $this->assertResponseIsSuccessful($responseData['error'] ?? 'An error occurred.'); // a weak sanity check that we went down "the right path" in the controller $this->assertSame($expectedResponse, $responseData); } - public function getTests() + public function getTests(): array { return [ 'On issue commented' => [ diff --git a/tests/Subscriber/AutoUpdateTitleWithLabelSubscriberTest.php b/tests/Subscriber/AutoUpdateTitleWithLabelSubscriberTest.php index b59f7df4..6a935f42 100644 --- a/tests/Subscriber/AutoUpdateTitleWithLabelSubscriberTest.php +++ b/tests/Subscriber/AutoUpdateTitleWithLabelSubscriberTest.php @@ -84,12 +84,12 @@ public function testOnPullRequestLabeledWithExisting() { $event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository); $this->pullRequestApi->method('show')->willReturn([ - 'title' => '[Messenger] Fix JSON', - 'labels' => [ - ['name' => 'Status: Needs Review', 'color' => 'abcabc'], - ['name' => 'Messenger', 'color' => 'dddddd'], - ], - ]); + 'title' => '[Messenger] Fix JSON', + 'labels' => [ + ['name' => 'Status: Needs Review', 'color' => 'abcabc'], + ['name' => 'Messenger', 'color' => 'dddddd'], + ], + ]); $this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST); $responseData = $event->getResponseData(); @@ -100,11 +100,11 @@ public function testRemoveLabel() { $event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository); $this->pullRequestApi->method('show')->willReturn([ - 'title' => '[Console][FrameworkBundle] [Random] Foo normal title', - 'labels' => [ - ['name' => 'Console', 'color' => 'dddddd'], - ], - ]); + 'title' => '[Console][FrameworkBundle] [Random] Foo normal title', + 'labels' => [ + ['name' => 'Console', 'color' => 'dddddd'], + ], + ]); $this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST); $responseData = $event->getResponseData(); @@ -117,11 +117,11 @@ public function testExtraBlankSpace() { $event = new GitHubEvent(['action' => 'labeled', 'number' => 57753, 'pull_request' => []], $this->repository); $this->pullRequestApi->method('show')->willReturn([ - 'title' => '[ErrorHandler] restrict the maximum length of the X-Debug-Exception header', - 'labels' => [ - ['name' => 'ErrorHandler', 'color' => 'dddddd'], - ], - ]); + 'title' => '[ErrorHandler] restrict the maximum length of the X-Debug-Exception header', + 'labels' => [ + ['name' => 'ErrorHandler', 'color' => 'dddddd'], + ], + ]); $this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST); $responseData = $event->getResponseData(); From c22bce66f6198327361b6bfb31062f989343e31e Mon Sep 17 00:00:00 2001 From: Oskar Stark Date: Tue, 17 Sep 2024 10:09:18 +0200 Subject: [PATCH 2/2] Fix: Psalm --- src/Service/GitHubRequestHandler.php | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Service/GitHubRequestHandler.php b/src/Service/GitHubRequestHandler.php index ff5aaed1..cfcecd71 100644 --- a/src/Service/GitHubRequestHandler.php +++ b/src/Service/GitHubRequestHandler.php @@ -46,11 +46,23 @@ public function handle(Request $request): array throw new PreconditionFailedHttpException(sprintf('Unsupported repository "%s".', $repositoryFullName)); } - if (!empty($repository->getSecret())) { + $secret = $repository->getSecret(); + if (is_string($secret) && '' !== trim($secret)) { if (!$request->headers->has('X-Hub-Signature')) { throw new AccessDeniedHttpException('The request is not secured.'); } - if (!$this->authenticate($request->headers->get('X-Hub-Signature'), $repository->getSecret(), $request->getContent())) { + + $content = $request->getContent(); + if (!is_string($content)) { + throw new BadRequestHttpException('Empty request body!'); + } + + $signature = $request->headers->get('X-Hub-Signature'); + if (!is_string($signature)) { + throw new BadRequestHttpException('Invalid signature!'); + } + + if (!$this->authenticate($signature, $secret, $content)) { throw new AccessDeniedHttpException('Invalid signature.'); } }