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

feat: Ability to skip "meta" tokens (whitespace, comments, attributes) #7559

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 49 additions & 0 deletions src/Tokenizer/Tokens.php
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,18 @@ public function getCodeHash(): string
return $this->codeHash;
}

/**
* Get index for closest next token which is non meta (whitespace, comments, PHP8 attribute).
*
* This method is shorthand for getNonMetaSibling method.
*
* @param int $index token index
*/
public function getNextNonMeta(int $index): ?int
{
return $this->getNonMetaSibling($index, 1);
}

/**
* Get index for closest next token which is non whitespace.
*
Expand Down Expand Up @@ -572,6 +584,43 @@ public function getNonWhitespaceSibling(int $index, int $direction, ?string $whi
}
}

/**
Copy link
Member

@keradus keradus Jan 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, for any code:

I struggle to understand the need for code without that code being used. For now, it looks only as a dead code
( and I would love to have automation to prevent any deadcode being in repo, as side note ;) )

I also fail to understand how common this will get used - is it's needed only for very limited usecase (and thus encapsulated into method of class, where will be the usage) or spread across widely (thus should be exposed via "main" Tokens).

For that, I would love to see actual usage of code, eg in the PR that is actually using it.
We could also have a PR adding Tokens code (without usage), and other feature PR build on top, that shows how it will be used.
Or other way around, merge feature PR without having this logic externalised to Tokens, and then refactoring PR with code moving to Tokens and getting use in multiple rules.

* Get index for closest sibling token which is non meta (whitespace, comment, PHP8 attribute).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious - is meta some formal naming here, or only a word you picked? (happy with both, just curious)

*
* @param int $index token index
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param int $index token index

* @param -1|1 $direction
*/
public function getNonMetaSibling(int $index, int $direction): ?int
{
if (!\defined('T_ATTRIBUTE')) {
return $this->getMeaningfulTokenSibling($index, $direction);
}

do {
$index = $this->getMeaningfulTokenSibling($index, $direction);

if (1 === $direction && $this[$index]->isGivenKind(T_ATTRIBUTE)) {
$index = $this->findBlockEnd(self::BLOCK_TYPE_ATTRIBUTE, $index);
} elseif (-1 === $direction && $this[$index]->isGivenKind(CT::T_ATTRIBUTE_CLOSE)) {
$index = $this->findBlockStart(self::BLOCK_TYPE_ATTRIBUTE, $index);
}
} while ($this[$index]->isGivenKind([T_ATTRIBUTE, CT::T_ATTRIBUTE_CLOSE]));

return $index;
}

/**
* Get index for closest previous token which is non meta (whitespace, comment, PHP8 attribute).
*
* This method is shorthand for getNonMetaSibling method.
*
* @param int $index token index
*/
public function getPrevNonMeta(int $index): ?int
{
return $this->getNonMetaSibling($index, -1);
}

/**
* Get index for closest previous token which is non whitespace.
*
Expand Down
86 changes: 86 additions & 0 deletions tests/Tokenizer/TokensTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1806,6 +1806,92 @@ public function testFindingToken(): void
self::assertTrue($tokens->isTokenKindFound(T_VARIABLE));
}

/**
* @param -1|1 $direction
*
* @requires PHP 8.0
*
* @dataProvider provideGetNonMetaSiblingCases
*/
public function testGetNonMetaSibling(string $code, int $index, int $direction, int $expectedIndex): void
{
$tokens = Tokens::fromCode($code);

self::assertSame($expectedIndex, $tokens->getNonMetaSibling($index, $direction));
}

/**
* @return iterable<array{0: string, 1: int, 2: -1|1, 3: int}>
*/
public static function provideGetNonMetaSiblingCases(): iterable
{
$cases = [
'outside function signature' => [
'<?php

$a = 1;

#[Foo]
// comment
# comment
#[Bar]
/* comment */
/** comment */
/*
* comment
*/
#[
Baz,
Buzz
]
function xyz() {};
',
7,
36,
],

'inside function signature' => [
'<?php function xyz(#[Foo] #[Bar] $a) {};',
4,
13,
],

'inside class' => [
'<?php
class Xyz {
#[Foo]
private $a;
}',
5,
11,
],

'between class properties' => [
'<?php
class Xyz {
private $a; // Something

#[Foo]
#[Bar]
/** @var int */
#[
Baz(new Xyz()),
Buzz(1, 2, 3)
]
private $b;
}',
10,
49,
],
];

foreach ($cases as $name => $case) {
yield "{$name}, next sibling" => [$case[0], $case[1], 1, $case[2]];

yield "{$name}, previous sibling" => [$case[0], $case[2], -1, $case[1]];
}
}

private function getBlockEdgeCachingTestTokens(): Tokens
{
Tokens::clearCache();
Expand Down