From 98d37d9dcfbcf1a70bb7a36ecea7c6ca7430c347 Mon Sep 17 00:00:00 2001 From: Emil Masiakowski Date: Wed, 27 Dec 2023 16:47:46 +0100 Subject: [PATCH] Add TodoByTicketRule (issue tracker support) (#26) Co-authored-by: Markus Staab Co-authored-by: Emil Masiakowski Co-authored-by: Markus Staab --- README.md | 95 ++++++++++++++++++ composer.json | 2 + extension.neon | 53 +++++++++++ src/TodoByTicketRule.php | 106 +++++++++++++++++++++ src/utils/TicketStatusFetcher.php | 9 ++ src/utils/jira/JiraAuthorization.php | 36 +++++++ src/utils/jira/JiraTicketStatusFetcher.php | 99 +++++++++++++++++++ tests/TodoByTicketRuleTest.php | 43 +++++++++ tests/data/ticket.php | 13 +++ tests/jira/JiraAuthorizationTest.php | 49 ++++++++++ tests/jira/data/jira-credentials.txt | 1 + tests/utils/StaticTicketStatusFetcher.php | 28 ++++++ 12 files changed, 534 insertions(+) create mode 100644 src/TodoByTicketRule.php create mode 100644 src/utils/TicketStatusFetcher.php create mode 100644 src/utils/jira/JiraAuthorization.php create mode 100644 src/utils/jira/JiraTicketStatusFetcher.php create mode 100644 tests/TodoByTicketRuleTest.php create mode 100644 tests/data/ticket.php create mode 100644 tests/jira/JiraAuthorizationTest.php create mode 100644 tests/jira/data/jira-credentials.txt create mode 100644 tests/utils/StaticTicketStatusFetcher.php diff --git a/README.md b/README.md index b5e8898..ee9b234 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,11 @@ function doFooBar() { // TODO: php:8 drop this polyfill when php 8.x is required +// TODO: APP-2137 This has to be fixed +function doBaz() { + +} + ``` @@ -71,6 +76,8 @@ see examples of different comment variants which are supported: // TODO: phpunit/phpunit:<5 This has to be fixed before updating to phpunit 5.x // TODO@markus: phpunit/phpunit:5.3 This has to be fixed when updating phpunit to 5.3.x or higher + +// TODO: APP-123 fix it ``` ## Configuration @@ -158,6 +165,94 @@ In case you are using git submodules, or the analyzed codebase consists of multi set the `singleGitRepo` option to `false` which resolves the reference version for each directory beeing analyzed. +### Issue tracker key support + +Optionally you can configure this extension to analyze your comments with issue tracker ticket keys. +The extension fetches issue tracker API for issue status. If the remote issue is resolved, the comment will be reported. + +Currently only Jira is supported. + +This feature is disabled by default. To enable it, you must set `ticket.enabled` parameter to `true`. +You also need to set these parameters: + +```yaml +parameters: + todo_by: + ticket: + enabled: true + + # a case-sensitive list of status names. + # only tickets having any of these statuses are considered resolved. + resolvedStatuses: + - Done + - Resolved + - Declined + + # if your ticket key is FOO-12345, then this value should be ["FOO"]. + # multiple key prefixes are allowed, e.g. ["FOO", "APP"]. + # only comments with keys containing this prefix will be analyzed. + keyPrefixes: + - FOO + + jira: + # e.g. https://your-company.atlassian.net + server: https://acme.atlassian.net + + # see below for possible formats. + # if this value is empty, credentials file will be used instead. + credentials: %env.JIRA_TOKEN% + + # path to a file containing Jira credentials. + # see below for possible formats. + # if credentials parameter is not empty, it will be used instead of this file. + # this file must not be commited into the repository! + credentialsFilePath: .secrets/jira-credentials.txt +``` + +#### Jira Credentials + +This extension uses Jira's REST API to fetch ticket's status. In order for it to work, you need to configure valid credentials. +These authentication methods are supported: +- [OAuth 2.0 Access Tokens](https://confluence.atlassian.com/adminjiraserver/jira-oauth-2-0-provider-api-1115659070.html) +- [Personal Access Tokens](https://confluence.atlassian.com/enterprise/using-personal-access-tokens-1026032365.html) +- [Basic Authentication](https://developer.atlassian.com/server/jira/platform/basic-authentication/) + +We recommend you use OAuth over basic authentication, especially if you use phpstan in CI. +There are multiple ways to pass your credentials to this extension. +You should choose one of them - if you define both parameters, only `credentials` parameter is considered and the file is ignored. + +##### Pass credentials in environment variable + +Configure `credentials` parameter to [read value from environment variable](https://phpstan.org/config-reference#environment-variables): +```yaml +parameters: + todo_by: + ticket: + jira: + credentials: %env.JIRA_TOKEN% +``` + +Depending on chosen authentication method its content should be: +* Access Token for token based authentication, e.g. `JIRA_TOKEN=ATATT3xFfGF0Gv_pLFSsunmigz8wq5Y0wkogoTMgxDFHyR...` +* `:` for basic authentication, e.g. `JIRA_TOKEN=john.doe@example.com:p@ssword` + +##### Pass credentials in text file + +Create text file in your project's directory (or anywhere else on your computer) and put its path into configuration: + +```yaml +parameters: + todo_by: + ticket: + jira: + credentialsFilePath: path/to/file +``` + +**Remember not to commit this file to repository!** +Depending on chosen authentication method its value should be: +* Access Token for token based authentication, e.g. `JATATT3xFfGF0Gv_pLFSsunmigz8wq5Y0wkogoTMgxDFHyR...` +* `:` for basic authentication, e.g. `john.doe@example.com:p@ssword` + ## Installation diff --git a/composer.json b/composer.json index 2448786..981e328 100644 --- a/composer.json +++ b/composer.json @@ -12,6 +12,8 @@ ], "require": { "php": "^7.4 || ^8.0", + "ext-curl": "*", + "ext-json": "*", "composer-runtime-api": "^2", "composer/semver": "^3.4", "nikolaposa/version": "^4.1" diff --git a/extension.neon b/extension.neon index 2099e18..73e5b19 100644 --- a/extension.neon +++ b/extension.neon @@ -4,6 +4,16 @@ parametersSchema: referenceTime: string() referenceVersion: string() singleGitRepo: bool() + ticket: structure([ + enabled: bool() + keyPrefixes: listOf(string()) + resolvedStatuses: listOf(string()) + jira: structure([ + server: string() + credentials: schema(string(), nullable()) + credentialsFilePath: schema(string(), nullable()) + ]) + ]) ]) # default parameters @@ -21,12 +31,48 @@ parameters: # If set to false, the git tags are fetched for each directory individually (slower) singleGitRepo: true + ticket: + # whether to analyze comments by issue tracker ticket key + enabled: false + + # a case-sensitive list of status names. + # only tickets having any of these statuses are considered resolved. + resolvedStatuses: [] + + # if your ticket key is FOO-12345, then this value should be ["FOO"]. + # multiple key prefixes are allowed, e.g. ["FOO", "APP"]. + # only comments with keys containing this prefix will be analyzed. + keyPrefixes: [] + + jira: + # e.g. https://your-company.atlassian.net + server: https://jira.atlassian.com + + # see README for possible formats. + # if this value is empty, credentials file will be used instead. + credentials: null + + # path to a file containing Jira credentials. + # see README for possible formats. + # if credentials parameter is not empty, it will be used instead of this file. + # this file must not be commited into the repository! + credentialsFilePath: null + +conditionalTags: + staabm\PHPStanTodoBy\TodoByTicketRule: + phpstan.rules.rule: %todo_by.ticket.enabled% + services: - class: staabm\PHPStanTodoBy\TodoByDateRule tags: [phpstan.rules.rule] arguments: - %todo_by.referenceTime% + - + class: staabm\PHPStanTodoBy\TodoByTicketRule + arguments: + - %todo_by.ticket.resolvedStatuses% + - %todo_by.ticket.keyPrefixes% - class: staabm\PHPStanTodoBy\TodoByVersionRule @@ -52,3 +98,10 @@ services: class: staabm\PHPStanTodoBy\utils\ExpiredCommentErrorBuilder arguments: - %todo_by.nonIgnorable% + + - + class: staabm\PHPStanTodoBy\utils\jira\JiraTicketStatusFetcher + arguments: + - %todo_by.ticket.jira.server% + - %todo_by.ticket.jira.credentials% + - %todo_by.ticket.jira.credentialsFilePath% diff --git a/src/TodoByTicketRule.php b/src/TodoByTicketRule.php new file mode 100644 index 0000000..574747a --- /dev/null +++ b/src/TodoByTicketRule.php @@ -0,0 +1,106 @@ + + */ +final class TodoByTicketRule implements Rule +{ + private const PATTERN = <<<'REGEXP' + { + @?TODO # possible @ prefix + @?[a-zA-Z0-9_-]* # optional username + \s*[:-]?\s* # optional colon or hyphen + \s+ # keyword/ticket separator + (?P[A-Z0-9]+-\d+) # ticket key consisting of ABC-123 or F01-12345 format + \s*[:-]?\s* # optional colon or hyphen + (?P.*) # rest of line as comment text + }ix + REGEXP; + + /** @var list */ + private array $resolvedStatuses; + /** @var list */ + private array $keyPrefixes; + private TicketStatusFetcher $fetcher; + private ExpiredCommentErrorBuilder $errorBuilder; + + /** + * @param list $resolvedStatuses + * @param list $keyPrefixes + */ + public function __construct(array $resolvedStatuses, array $keyPrefixes, TicketStatusFetcher $fetcher, ExpiredCommentErrorBuilder $errorBuilder) + { + $this->resolvedStatuses = $resolvedStatuses; + $this->keyPrefixes = $keyPrefixes; + $this->fetcher = $fetcher; + $this->errorBuilder = $errorBuilder; + } + + 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> $matches */ + foreach ($matches as $match) { + $ticketKey = $match['ticketKey'][0]; + $todoText = trim($match['comment'][0]); + + if (!$this->hasPrefix($ticketKey)) { + continue; + } + + $ticketStatus = $this->fetcher->fetchTicketStatus($ticketKey); + + if (null === $ticketStatus || !in_array($ticketStatus, $this->resolvedStatuses, true)) { + continue; + } + + if ('' !== $todoText) { + $errorMessage = "Should have been resolved in {$ticketKey}: ". rtrim($todoText, '.') .'.'; + } else { + $errorMessage = "Comment should have been resolved in {$ticketKey}."; + } + + $errors[] = $this->errorBuilder->buildError( + $comment, + $errorMessage, + null, + $match[0][1] + ); + } + } + + return $errors; + } + + private function hasPrefix(string $ticketKey): bool + { + foreach ($this->keyPrefixes as $prefix) { + if (substr($ticketKey, 0, strlen($prefix)) === $prefix) { + return true; + } + } + + return false; + } +} diff --git a/src/utils/TicketStatusFetcher.php b/src/utils/TicketStatusFetcher.php new file mode 100644 index 0000000..d57dce0 --- /dev/null +++ b/src/utils/TicketStatusFetcher.php @@ -0,0 +1,9 @@ + + */ + private array $cache; + + public function __construct(string $host, ?string $credentials, ?string $credentialsFilePath) + { + $credentials = JiraAuthorization::getCredentials($credentials, $credentialsFilePath); + + $this->host = $host; + $this->authorizationHeader = JiraAuthorization::createAuthorizationHeader($credentials); + + $this->cache = []; + } + + public function fetchTicketStatus(string $ticketKey): ?string + { + if (array_key_exists($ticketKey, $this->cache)) { + return $this->cache[$ticketKey]; + } + + $apiVersion = self::API_VERSION; + + $curl = curl_init("{$this->host}/rest/api/$apiVersion/issue/$ticketKey?expand=status"); + if (!$curl) { + throw new RuntimeException('Could not initialize cURL connection'); + } + + curl_setopt($curl, CURLOPT_FOLLOWLOCATION, true); + curl_setopt($curl, CURLOPT_RETURNTRANSFER, true); + curl_setopt($curl, CURLOPT_HTTPHEADER, [ + "Authorization: $this->authorizationHeader", + ]); + + $response = curl_exec($curl); + if (!is_string($response) || curl_getinfo($curl, CURLINFO_RESPONSE_CODE) !== 200) { + throw new RuntimeException("Could not fetch ticket's status from Jira"); + } + + curl_close($curl); + + $data = self::decodeAndValidateResponse($response); + + return $this->cache[$ticketKey] = $data['fields']['status']['name']; + } + + /** @return array{fields: array{status: array{name: string}}} */ + private static function decodeAndValidateResponse(string $body): array + { + $data = json_decode($body, true); + + if (!is_array($data) || !array_key_exists('fields', $data)) { + self::throwInvalidResponse(); + } + + $fields = $data['fields']; + + if (!is_array($fields) || !array_key_exists('status', $fields)) { + self::throwInvalidResponse(); + } + + $status = $fields['status']; + if (!is_array($status) || !array_key_exists('name', $status)) { + self::throwInvalidResponse(); + } + + $name = $status['name']; + + if (!is_string($name) || '' === trim($name)) { + self::throwInvalidResponse(); + } + + return $data; + } + + /** @return never */ + private static function throwInvalidResponse(): void + { + throw new RuntimeException('Jira returned invalid response body'); + } +} diff --git a/tests/TodoByTicketRuleTest.php b/tests/TodoByTicketRuleTest.php new file mode 100644 index 0000000..c2c1ab2 --- /dev/null +++ b/tests/TodoByTicketRuleTest.php @@ -0,0 +1,43 @@ + + * @internal + */ +final class TodoByTicketRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + $fetcher = new StaticTicketStatusFetcher([ + 'APP-123' => 'Done', + 'FOO-0001' => 'Done', + 'F01-12345' => 'Done', + 'APP-4444' => 'Resolved', + 'APP-5000' => 'To Do', + ]); + + return new TodoByTicketRule( + ['Done', 'Resolved'], + ['APP', 'FOO', 'F01'], + $fetcher, + new ExpiredCommentErrorBuilder(true), + ); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/ticket.php'], [ + ['Should have been resolved in APP-123: rename this to doBar().', 5], + ['Comment should have been resolved in APP-4444.', 11], + ['Comment should have been resolved in FOO-0001.', 12], + ['Should have been resolved in F01-12345: please change me.', 13], + ]); + } +} diff --git a/tests/data/ticket.php b/tests/data/ticket.php new file mode 100644 index 0000000..ef6a92c --- /dev/null +++ b/tests/data/ticket.php @@ -0,0 +1,13 @@ +expectException(RuntimeException::class); + $this->expectExceptionMessage('Either credentials or credentialsFilePath parameter must be configured'); + + JiraAuthorization::getCredentials(null, null); + } + + public function testCredentialsStringIsPreferredOverFilePath(): void + { + $credentials = JiraAuthorization::getCredentials('secret_token', __DIR__ . '/data/jira-credentials.txt'); + + static::assertSame('secret_token', $credentials); + } + + public function testReadsCredentialsFromFile(): void + { + $credentials = JiraAuthorization::getCredentials(null, __DIR__ . '/data/jira-credentials.txt'); + + static::assertSame('john.doe@example.com:th!s#isSecret_token', $credentials); + } + + public function testCreatesBasicAuthorizationHeader(): void + { + $authorization = JiraAuthorization::createAuthorizationHeader('john.doe@example.com:th!s#isSecret_token'); + + static::assertSame('Basic am9obi5kb2VAZXhhbXBsZS5jb206dGghcyNpc1NlY3JldF90b2tlbg==', $authorization); + } + + public function testCreatesBearerAuthorizationHeader(): void + { + $authorization = JiraAuthorization::createAuthorizationHeader('th!s#isSecret_token'); + + static::assertSame('Bearer th!s#isSecret_token', $authorization); + } +} diff --git a/tests/jira/data/jira-credentials.txt b/tests/jira/data/jira-credentials.txt new file mode 100644 index 0000000..2bcc159 --- /dev/null +++ b/tests/jira/data/jira-credentials.txt @@ -0,0 +1 @@ +john.doe@example.com:th!s#isSecret_token diff --git a/tests/utils/StaticTicketStatusFetcher.php b/tests/utils/StaticTicketStatusFetcher.php new file mode 100644 index 0000000..496fb15 --- /dev/null +++ b/tests/utils/StaticTicketStatusFetcher.php @@ -0,0 +1,28 @@ + */ + private array $statuses; + + /** @param array $statuses */ + public function __construct(array $statuses) + { + $this->statuses = $statuses; + } + + public function fetchTicketStatus(string $ticketKey): ?string + { + if (!array_key_exists($ticketKey, $this->statuses)) { + return null; + } + + return $this->statuses[$ticketKey]; + } +}