Skip to content

Commit

Permalink
Simplify collector-data to reduce memory consumption (#104)
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm authored Aug 13, 2024
1 parent 1ee4274 commit 27ab19c
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 57 deletions.
6 changes: 4 additions & 2 deletions src/TodoByDateRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ public function processNode(Node $node, Scope $scope): array

if (!checkdate((int) $month, (int) $day, (int) $year)) {
$errors[] = $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
'Invalid date "'. $date .'". Expected format "YYYY-MM-DD".',
self::ERROR_IDENTIFIER,
null,
Expand Down Expand Up @@ -96,7 +97,8 @@ public function processNode(Node $node, Scope $scope): array
}

$errors[] = $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
$errorMessage,
self::ERROR_IDENTIFIER,
null,
Expand Down
6 changes: 4 additions & 2 deletions src/TodoByIssueUrlRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ public function processNode(Node $node, Scope $scope): array

if (!array_key_exists($apiUrl, $fetchedStatuses) || null === $fetchedStatuses[$apiUrl]) {
$errors[] = $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
"Ticket $url doesn't exist or provided credentials do not allow for viewing it.",
self::ERROR_IDENTIFIER,
null,
Expand All @@ -90,7 +91,8 @@ public function processNode(Node $node, Scope $scope): array
}

$errors[] = $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
$errorMessage,
self::ERROR_IDENTIFIER,
null,
Expand Down
24 changes: 16 additions & 8 deletions src/TodoByPackageVersionRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ public function processNode(Node $node, Scope $scope): array
}

$errors[] = $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
$errorMessage,
self::ERROR_IDENTIFIER,
null,
Expand Down Expand Up @@ -154,7 +155,8 @@ private function satisfiesPhpPlatformPackage(string $package, string $version, C
$constraint = $versionParser->parseConstraints($version);
} catch (UnexpectedValueException $e) {
return $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
'Invalid version constraint "' . $version . '" for package "' . $package . '".',
self::ERROR_IDENTIFIER,
null,
Expand All @@ -177,7 +179,8 @@ private function satisfiesVirtualPackage(string $package, string $version, Comme
);
} catch (UnexpectedValueException $e) {
return $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
'Invalid virtual-package "' . $package . '": "' . $this->virtualPackages[$package] . '" provided via PHPStan config file.',
self::ERROR_IDENTIFIER,
null,
Expand All @@ -189,7 +192,8 @@ private function satisfiesVirtualPackage(string $package, string $version, Comme
$constraint = $versionParser->parseConstraints($version);
} catch (UnexpectedValueException $e) {
return $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
'Invalid version constraint "' . $version . '" for virtual-package "' . $package . '".',
self::ERROR_IDENTIFIER,
null,
Expand Down Expand Up @@ -220,7 +224,8 @@ private function readPhpPlatformVersion(Comment $comment, int $wholeMatchStartOf

if (null === $config) {
return $this->phpPlatformVersion = $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
'Unable to find composer.json in '. $this->workingDirectory,
self::ERROR_IDENTIFIER,
null,
Expand All @@ -235,7 +240,8 @@ private function readPhpPlatformVersion(Comment $comment, int $wholeMatchStartOf
|| !is_string($config['require']['php'])
) {
return $this->phpPlatformVersion = $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
'Missing php platform requirement in '. $this->workingDirectory .'/composer.json',
self::ERROR_IDENTIFIER,
null,
Expand All @@ -256,7 +262,8 @@ private function satisfiesInstalledPackage(string $package, string $version, Com
// see https://getcomposer.org/doc/07-runtime.md#installed-versions
if (!InstalledVersions::isInstalled($package)) {
return $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
'Unknown package "' . $package . '". It is neither installed via composer.json nor declared as virtual package via PHPStan config.',
self::ERROR_IDENTIFIER,
null,
Expand All @@ -268,7 +275,8 @@ private function satisfiesInstalledPackage(string $package, string $version, Com
return InstalledVersions::satisfies($versionParser, $package, $version);
} catch (UnexpectedValueException $e) {
return $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
'Invalid version constraint "' . $version . '" for package "' . $package . '".',
self::ERROR_IDENTIFIER,
null,
Expand Down
14 changes: 6 additions & 8 deletions src/TodoByTicketCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
use function trim;

/**
* @implements Collector<Node, list<array{string, string, string, int, int}>>
* @implements Collector<Node, list<array{string, int, string, string, int, int}>>
*/
final class TodoByTicketCollector implements Collector
{
Expand All @@ -37,19 +37,17 @@ public function processNode(Node $node, Scope $scope)
// use deprecated method for nikic/php-parser 4.x compat
$line = $comment->getLine();

$text = $comment->getText();
$startLine =$comment->getStartLine();

/** @var array<int, array<array{0: string, 1: int}>> $matches */
foreach ($matches as $match) {
$ticketKey = $match['ticketKey'][0];
$todoText = trim($match['comment'][0]);

// collectors do not support serializing objects, pass a string instead.
$json = json_encode($comment);
if (false === $json) {
throw new RuntimeException('Failed to encode comment as JSON: ' . json_last_error_msg());
}

$tickets[] = [
$json,
$text,
$startLine,
$ticketKey,
$todoText,
$match[0][1], // wholeMatchStartOffset
Expand Down
36 changes: 6 additions & 30 deletions src/TodoByTicketRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public function processNode(Node $node, Scope $scope): array
$ticketKeys = [];
foreach ($collectorData as $collected) {
foreach ($collected as $tickets) {
foreach ($tickets as [$json, $ticketKey, $todoText, $wholeMatchStartOffset, $line]) {
foreach ($tickets as [$comment, $startLine, $ticketKey, $todoText, $wholeMatchStartOffset, $line]) {
if ([] !== $this->configuration->getKeyPrefixes() && !$this->hasPrefix($ticketKey)) {
continue;
}
Expand All @@ -67,7 +67,7 @@ public function processNode(Node $node, Scope $scope): array
$errors = [];
foreach ($collectorData as $file => $collected) {
foreach ($collected as $tickets) {
foreach ($tickets as [$json, $ticketKey, $todoText, $wholeMatchStartOffset, $line]) {
foreach ($tickets as [$comment, $startLine, $ticketKey, $todoText, $wholeMatchStartOffset, $line]) {
if ([] !== $this->configuration->getKeyPrefixes() && !$this->hasPrefix($ticketKey)) {
continue;
}
Expand All @@ -79,7 +79,8 @@ public function processNode(Node $node, Scope $scope): array

if (null === $ticketStatus) {
$errors[] = $this->errorBuilder->buildFileError(
$this->commentFromJson($json),
$comment,
$startLine,
"Ticket $ticketKey doesn't exist or provided credentials do not allow for viewing it.",
self::ERROR_IDENTIFIER,
null,
Expand All @@ -102,7 +103,8 @@ public function processNode(Node $node, Scope $scope): array
}

$errors[] = $this->errorBuilder->buildFileError(
$this->commentFromJson($json),
$comment,
$startLine,
$errorMessage,
self::ERROR_IDENTIFIER,
"See {$this->configuration->getFetcher()->resolveTicketUrl($ticketKey)}",
Expand All @@ -128,30 +130,4 @@ private function hasPrefix(string $ticketKey): bool
return false;
}

private function commentFromJson(string $json): Comment
{
$arr = json_decode($json, true);

if ('Comment_Doc' === $arr['nodeType']) {
return new Comment\Doc(
$arr['text'],
$arr['line'],
$arr['filePos'],
$arr['tokenPos'],
$arr['endLine'],
$arr['endFilePos'],
$arr['endTokenPos']
);
}

return new Comment(
$arr['text'],
$arr['line'],
$arr['filePos'],
$arr['tokenPos'],
$arr['endLine'],
$arr['endFilePos'],
$arr['endTokenPos']
);
}
}
6 changes: 4 additions & 2 deletions src/TodoByVersionRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ public function processNode(Node $node, Scope $scope): array
$constraint = $versionParser->parseConstraints($version);
} catch (UnexpectedValueException $e) {
$errors[] = $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
'Invalid version constraint "' . $version . '".',
self::ERROR_IDENTIFIER,
null,
Expand All @@ -122,7 +123,8 @@ public function processNode(Node $node, Scope $scope): array
}

$errors[] = $this->errorBuilder->buildError(
$comment,
$comment->getText(),
$comment->getStartLine(),
$errorMessage,
self::ERROR_IDENTIFIER,
"Calculated reference version is '". $referenceVersion ."'.\n\n See also:\n https://github.com/staabm/phpstan-todo-by#reference-version",
Expand Down
15 changes: 10 additions & 5 deletions src/utils/ExpiredCommentErrorBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,16 @@ public function __construct(bool $nonIgnorable)
}

public function buildError(
Comment $comment,
string $comment,
int $startLine,
string $errorMessage,
string $errorIdentifier,
?string $tip,
int $wholeMatchStartOffset
): \PHPStan\Rules\RuleError {
return $this->build(
$comment,
$startLine,
$errorMessage,
$errorIdentifier,
$tip,
Expand All @@ -35,7 +37,8 @@ public function buildError(
}

public function buildFileError(
Comment $comment,
string $comment,
int $startLine,
string $errorMessage,
string $errorIdentifier,
?string $tip,
Expand All @@ -45,6 +48,7 @@ public function buildFileError(
): \PHPStan\Rules\RuleError {
return $this->build(
$comment,
$startLine,
$errorMessage,
$errorIdentifier,
$tip,
Expand All @@ -55,7 +59,8 @@ public function buildFileError(
}

private function build(
Comment $comment,
string $comment,
int $startLine,
string $errorMessage,
string $errorIdentifier,
?string $tip,
Expand All @@ -64,10 +69,10 @@ private function build(
?int $line
): \PHPStan\Rules\RuleError {
// Count the number of newlines between the start of the whole comment, and the start of the match.
$newLines = substr_count($comment->getText(), "\n", 0, $wholeMatchStartOffset);
$newLines = substr_count($comment, "\n", 0, $wholeMatchStartOffset);

// Set the message line to match the line the comment actually starts on.
$messageLine = $comment->getStartLine() + $newLines;
$messageLine = $startLine + $newLines;

$errBuilder = RuleErrorBuilder::message($errorMessage)
->line($messageLine)
Expand Down

0 comments on commit 27ab19c

Please sign in to comment.