Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify collector-data to reduce memory consumption #104

Merged
merged 1 commit into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading