Php-cs-fixer: $token->isChanged question

Created on 19 Jul 2017  ·  17Comments  ·  Source: FriendsOfPHP/PHP-CS-Fixer

PHP 7.1.6

I detected changed line to report it to user.

2.3.2

This works:

private function detectChangedLineFromTokens(Tokens $tokens): int
{
    $line = 0;
    foreach ($tokens as $token) {
        $line += substr_count($token->getContent(), PHP_EOL);
        if ($token->isChanged()) {
            return ++$line;
        }
    }
    return $line;
}

See Travis Line

2.3.3

No token is $token->isChanged() true anymore. Not sure why.

See Travis line

Somewhere between 2.3.2 and 2.3.3 a Token.php behaviors was changed.

I spent 30 minutes looking for it but I'm lost and not sure where it came from.

kinquestion

All 17 comments

$token->isChanged() broke between 2.3.2 and 2.3.3

$token->isChanged() was not broken. it works like it should.
this method is providing information if given token instance was somehow changed.
it's detectChangedLineFromTokens is failing in some cases. if one is not changing a token, but replacing a token (with $tokens[$index] = new Token(...)), then your method will fail to perform detection, as token instance under $index was never modified, but whole collection was.

in 2.3.2->2.3.3 release we changed $tokens[$index]->override... to $tokens[$index] = new Token... in few places. but later form was used before 2.3.3 in some places as well, thus your method was not working all the time properly in lower version as well

Should we deprecate the isChanged() method?

it wasn't originally deprecated as for ->setContent we do have future-ready interface (tokens[index] = ...)
for isChanged itself, it gonna just be removed. but yeah, probably we could.

I understand changes. What is the solution to this then?

How do I found the number of line tokens were affected?

How do I found the number of line tokens were affected?

That is a question which is hard to answer as a lot of things have to be taken into account.
Typically you are not interested in which token is changed, but more how the output differs from the input.
If you're not than it gets even more complex.

The current solution in development will be change tracking on line based level. For this we are working for an unified diff format that is compat with other unified diff tooling.
Step one is to get sebastian/diff v2 released, after that we can utilize on that.

Please keep in mind that the "old" way of looking at the token line numbers was never reliable because of:

  • transformers did not (always (correctly)) keep/update the line number of the token they transformed
  • tokens might have been moved around to other line numbers, the line numbers of the moved changed tokens were not (always (correctly)) updated

Using tokens to keep track on changes is not only very hard, it typically doesn't provide a result different from one that is easier to get by diff'ing the input and output on string/text level.

Thanks for the answer. Could you provide me some link with example code to determine before/after line for specific fixer? Without that, I'm unable to write use informative output like:

Line 25 | "Your brace should be indented 1 line space from the left." | BracesFixer

I don't need to old solution to work, just any would do :)

I don't need to old solution to work

that's the thing, it wasn't, it was fuzzy (lines marked as changed were changed indeed, lines not marked could be changed anyway)

Could you provide me some link

https://github.com/FriendsOfPHP/PHP-CS-Fixer
New pull request button

@keradus Not helpful at all

ATM I know of no solution to get messages @ the LOD your sample is.
This is because I wonder on a concept level how such a system could be build.

To give a high level example I struggle with;
lets say I have a file with the following tokens:

A
B
C
D

fixer 1 transforms to:

D
C
B
A

fixer 2 transforms to:

D
C'
A
B

hinting like;
Token A @ line 1 should change to D
doesn't make sense, only in the context of all tokens that must be changed to match the new result will it help. Which would mean something like;
Token A @ line 1 should change to D because of the following fixers; A,B,C,D, [..] etc.
for each line.
These type of transformations we do not track. The only way I see how that could be done in the future is to allow listeners to be added to the system that will be called after each fixer is done fixing a collection of tokens so the diff can be tracked on a combination of file and fixer level.
(other one would be to fix a file with one fixer at the time starting the fix process as a whole again, but performance would be bad)
Please note that my "idea" for the listeners is very premature and not discussed or thought true completely. It might not be feasible at all because of architecture, performance etc.

Since we've no such system and we've a need for better change reporting we now aim for a unified diff;

@@ -1,4 +1,4 @@
-A
-B
-C
-D
+D
+C'
+A
+B

To get back to the original question; please consider if the udiff solution would solve your case. If not; than mostly likely changes would be needed and have to be thought out and discussed, or additional research should be done on the current system cause maybe there is a way to do this but I just didn't see it yet.

@TomasVotruba, that was my point, instead of demand you could help/contribute as well ;)

@SpacePossum , even outside of transformers his tracking solution was buggy - if fixer adds new token (eg missing braces or space), it could be detected only on top-collection level, not bottom-item level

@SpacePossum Thanks, I was actually asking for a code that does that in my previous comments.

@@ -1,4 +1,4 @@

This would be fine. Line 4.

I am for fair enough and pragmatic solution to fix current broken use case. Not an universal one.

Could you send me a link to particular line in PHP-CS-Fixer which does that?
That's all I really need whole this time :) I don't need more discussions, just the link :+1:

@keradus I was asking for a code reference. Instead of random link, you could actually answer me ;)

The line numbers will become available through https://github.com/sebastianbergmann/diff v2 which I try to complete. So for the linenumbers that is step one and a far bigger step than I anticipated.

After that the differ can be used in PHP-CS-Fixer just like it is done now (when we reach PHP7 or when I've backported diff package to PHP5.something).

@SpacePossum Thanks. And where exactly is this used in PHP-CS-Fixer?

@SpacePossum Great! I found this: https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/0d9b1dc237489a8598aa8a55bcfcc9efc12a44e2/src/Runner/Runner.php#L253

Thanks for the clues. I'll try to implement it next week after :notes: festival is over :+1:

I've resolved it in #273 , thank you @SpacePossum

Was this page helpful?
0 / 5 - 0 ratings