Skip to content

Commit

Permalink
Support full github issue urls (#95)
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm authored May 24, 2024
1 parent af9da67 commit bdb8af2
Show file tree
Hide file tree
Showing 6 changed files with 203 additions and 2 deletions.
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,11 @@ When a text is given after the date, this text will be picked up for the PHPStan
- the comment might be mixed with `:` or `-` characters
- multi line `/* */` and `/** */` comments are supported

The comment can expire by different constraints, examples are:
Out of the box comments can expire by different constraints:
- by date with format of `YYYY-MM-DD` matched against the [reference-time](https://github.com/staabm/phpstan-todo-by#reference-time)
- by a full github issue url

There are more builtin constraints, but these require additional configuration:
- by a semantic version constraint matched against the projects [reference-version](https://github.com/staabm/phpstan-todo-by#reference-version)
- by a semantic version constraint matched against a Composer dependency (via `composer.lock` or [`virtualPackages`](https://github.com/staabm/phpstan-todo-by#virtual-packages) config)
- by ticket reference, matched against the status of a ticket (e.g. in github.com or JIRA)
Expand All @@ -57,6 +60,8 @@ see examples of different comment variants which are supported:
// TODO@staabm 2023-12-14 - fix it
// TODO@markus: 2023-12-14 - fix it

// TODO https://github.com/staabm/phpstan-todo-by/issues/91 fix me when this GitHub issue is closed

/*
* other text
*
Expand Down
5 changes: 5 additions & 0 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ services:
tags: [phpstan.rules.rule]
arguments:
- %todo_by.referenceTime%

-
class: staabm\PHPStanTodoBy\TodoByTicketRule

Expand All @@ -141,6 +142,10 @@ services:
workingDirectory: %currentWorkingDirectory%
virtualPackages: %todo_by.virtualPackages%

-
class: staabm\PHPStanTodoBy\TodoByIssueUrlRule
tags: [phpstan.rules.rule]

-
class: staabm\PHPStanTodoBy\utils\GitTagFetcher

Expand Down
104 changes: 104 additions & 0 deletions src/TodoByIssueUrlRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
<?php

namespace staabm\PHPStanTodoBy;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use staabm\PHPStanTodoBy\utils\CommentMatcher;
use staabm\PHPStanTodoBy\utils\ExpiredCommentErrorBuilder;
use staabm\PHPStanTodoBy\utils\ticket\GitHubTicketStatusFetcher;

use function array_key_exists;
use function in_array;
use function trim;

/**
* @implements Rule<Node>
*/
final class TodoByIssueUrlRule implements Rule
{
private const ERROR_IDENTIFIER = 'url';

private const PATTERN = <<<'REGEXP'
{
@?(?:TODO|FIXME|XXX) # possible @ prefix
@?[a-zA-Z0-9_-]* # optional username
\s*[:-]?\s* # optional colon or hyphen
\s+ # keyword/version separator
(?P<url>https://github.com/(?P<owner>[\S]{2,})/(?P<repo>[\S]+)/issues/(?P<issueNumber>\d+)) # url
\s*[:-]?\s* # optional colon or hyphen
(?P<comment>(?:(?!\*+/).)*) # rest of line as comment text, excluding block end
}ix
REGEXP;

private ExpiredCommentErrorBuilder $errorBuilder;
private GitHubTicketStatusFetcher $fetcher;

public function __construct(
ExpiredCommentErrorBuilder $errorBuilder,
GitHubTicketStatusFetcher $fetcher
) {
$this->errorBuilder = $errorBuilder;
$this->fetcher = $fetcher;
}

public function getNodeType(): string
{
return Node::class;
}

public function processNode(Node $node, Scope $scope): array
{
$it = CommentMatcher::matchComments($node, self::PATTERN);

$errors = [];
foreach ($it as $comment => $matches) {
/** @var array<int, array<array{0: string, 1: int}>> $matches */
foreach ($matches as $match) {
$url = $match['url'][0];
$owner = $match['owner'][0];
$repo = $match['repo'][0];
$issueNumber = $match['issueNumber'][0];
$todoText = trim($match['comment'][0]);
$wholeMatchStartOffset = $match[0][1];

$apiUrl = $this->fetcher->buildUrl($owner, $repo, $issueNumber);
$fetchedStatuses = $this->fetcher->fetchTicketStatusByUrls([$apiUrl => $apiUrl]);

if (!array_key_exists($apiUrl, $fetchedStatuses) || null === $fetchedStatuses[$apiUrl]) {
$errors[] = $this->errorBuilder->buildError(
$comment,
"Ticket $url doesn't exist or provided credentials do not allow for viewing it.",
self::ERROR_IDENTIFIER,
null,
$wholeMatchStartOffset
);

continue;
}

$ticketStatus = $fetchedStatuses[$apiUrl];
if (!in_array($ticketStatus, GitHubTicketStatusFetcher::RESOLVED_STATUSES, true)) {
continue;
}

if ('' !== $todoText) {
$errorMessage = "Should have been resolved in {$url}: ". rtrim($todoText, '.') .'.';
} else {
$errorMessage = "Comment should have been resolved with {$url}.";
}

$errors[] = $this->errorBuilder->buildError(
$comment,
$errorMessage,
self::ERROR_IDENTIFIER,
null,
$wholeMatchStartOffset
);
}
}

return $errors;
}
}
21 changes: 20 additions & 1 deletion src/utils/ticket/GitHubTicketStatusFetcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@

final class GitHubTicketStatusFetcher implements TicketStatusFetcher
{
public const RESOLVED_STATUSES = ['closed'];

private const API_VERSION = '2022-11-28';

private const KEY_REGEX = '
((?P<githubOwner>[\w\-\.]+)\/)? # optional owner with slash separator
(?P<githubRepo>[\w\-\.]+)? # optional repo
Expand Down Expand Up @@ -41,9 +44,25 @@ public function fetchTicketStatus(array $ticketKeys): array
foreach ($ticketKeys as $ticketKey) {
[$owner, $repo, $number] = $this->processKey($ticketKey);

$ticketUrls[$ticketKey] = "https://api.github.com/repos/$owner/$repo/issues/$number";
$ticketUrls[$ticketKey] = $this->buildUrl($owner, $repo, $number);
}

return $this->fetchTicketStatusByUrls($ticketUrls);
}

/** @return non-empty-string */
public function buildUrl(string $owner, string $repo, string $number): string
{
return "https://api.github.com/repos/$owner/$repo/issues/$number";
}

/**
* @param non-empty-array<non-empty-string, non-empty-string> $ticketUrls
*
* @return non-empty-array<non-empty-string, string|null>
*/
public function fetchTicketStatusByUrls(array $ticketUrls): array
{
$apiVersion = self::API_VERSION;

$headers = [
Expand Down
59 changes: 59 additions & 0 deletions tests/TodoByIssueUrlRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php

namespace staabm\PHPStanTodoBy\Tests;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use staabm\PHPStanTodoBy\TodoByIssueUrlRule;
use staabm\PHPStanTodoBy\utils\ExpiredCommentErrorBuilder;
use staabm\PHPStanTodoBy\utils\ticket\GitHubTicketStatusFetcher;

/**
* @extends RuleTestCase<TodoByIssueUrlRule>
* @internal
*/
final class TodoByIssueUrlRuleTest extends RuleTestCase
{
protected function getRule(): Rule
{
return new TodoByIssueUrlRule(
new ExpiredCommentErrorBuilder(true),
self::getContainer()->getByType(GitHubTicketStatusFetcher::class)
);
}

/**
* @param list<array{0: string, 1: int, 2?: string|null}> $errors
* @dataProvider provideErrors
*/
public function testRule(array $errors): void
{
$this->analyse([__DIR__ . '/data/issue-urls.php'], $errors);
}

/**
* @return iterable<array{list<array{0: string, 1: int, 2?: string|null}>}>
*/
public static function provideErrors(): iterable
{
yield [
[
[
'Should have been resolved in https://github.com/staabm/phpstan-todo-by/issues/47: we need todo something when this issue is resolved.',
5,
],
[
'Comment should have been resolved with https://github.com/staabm/phpstan-todo-by/issues/47.',
6,
],
],
];
}

public static function getAdditionalConfigFiles(): array
{
return [
__DIR__ . '/../extension.neon',
];
}
}
9 changes: 9 additions & 0 deletions tests/data/issue-urls.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

namespace IssueUrls;

// TODO: https://github.com/staabm/phpstan-todo-by/issues/47 we need todo something when this issue is resolved
// TODO: https://github.com/staabm/phpstan-todo-by/issues/47

// FIXME: https://github.com/staabm/xhprof.io/issues/3 refencing a open issue should not trigger a error

0 comments on commit bdb8af2

Please sign in to comment.