PHPCS v4 compatibility bug

[dereuromark]

Bug Report: AnnotationHelper::fixAnnotation Changes Indentation from Spaces to Tabs

When trying to get a sniffer package to v4 compatibility.

Summary

When using AnnotationHelper::fixAnnotation() to modify a PHPDoc annotation in a union type containing an array type hint, the helper changes the indentation of the entire doc comment from spaces to tabs, even when the original file uses spaces.

Environment

• SlevomatCodingStandard version: 8.23 AND dev-phpcs4 (commit fe765b3)
• PHP_CodeSniffer version: ^4.0.0
• PHP version: 8.4.12

Steps to Reproduce

1. Create a PHP file with space-indented PHPDoc comment:

<?php
class Test {
    /**
     * @return \ArrayObject<string>|int[]
     */
    protected function getArrayObjectOfStringsOrArrayOfInts()
    {
        return $this->foo();
    }
}

Note: The file uses 4 spaces for indentation (not tabs).

2. Use AnnotationHelper::fixAnnotation() to replace int[] with array<int> in the union type.

3. Apply the fix using FixerHelper::change().

Expected Behavior

The fixed doc comment should preserve the original indentation style (spaces):

    /**
     * @return \ArrayObject<string>|array<int>
     */

Actual Behavior

The fixed doc comment is changed to use tabs:

    /**
	 * @return \ArrayObject<string>|array<int>
	 */

Note the second and third lines now use tabs (shown as ^I in cat -A output).

Code Path

The issue occurs when:

1. AnnotationHelper::fixAnnotation() is called to fix a union type annotation
2. The fixed content is applied using FixerHelper::change()

Example code from our sniff:

$fixedDocComment = AnnotationHelper::fixAnnotation(
    $parsedDocComment,
    $annotation,
    $unionTypeNode,
    $genericTypeNode,
);

// Later...
$phpcsFile->fixer->beginChangeset();
FixerHelper::change(
    $phpcsFile,
    $parsedDocComment->getOpenPointer(),
    $parsedDocComment->getClosePointer(),
    $fixedDocComment,
);
$phpcsFile->fixer->endChangeset();

Impact

This causes issues in codebases that enforce space-only indentation, as the fixer changes the indentation style of doc comments it modifies.

Possible Solution

AnnotationHelper::fixAnnotation() should detect and preserve the original indentation style of the doc comment, or provide an option to specify the desired indentation style.

Test Case

Input file (with spaces):

    /**
     * @return \ArrayObject<string>|int[]
     */

After running through AnnotationHelper::fixAnnotation + FixerHelper::change:

    /**
	 * @return \ArrayObject<string>|array<int>
	 */

The indentation characters have changed from spaces to tabs.

[dereuromark]

A possible fix:

diff --git a/SlevomatCodingStandard/Helpers/AnnotationHelper.php b/SlevomatCodingStandard/Helpers/AnnotationHelper.php
index ebce8e72..14e15ba0 100644
--- a/SlevomatCodingStandard/Helpers/AnnotationHelper.php
+++ b/SlevomatCodingStandard/Helpers/AnnotationHelper.php
@@ -25,7 +25,12 @@ use PHPStan\PhpDocParser\Ast\Type\ObjectShapeItemNode;
 use PHPStan\PhpDocParser\Ast\Type\ObjectShapeNode;
 use PHPStan\PhpDocParser\Ast\Type\UnionTypeNode;
 use function in_array;
+use function preg_match;
+use function preg_replace_callback;
 use function sprintf;
+use function str_repeat;
+use function strlen;
+use function strpos;
 use function strtolower;
 
 /**
@@ -171,11 +176,33 @@ class AnnotationHelper
 			}
 		}
 
-		return PhpDocParserHelper::getPrinter()->printFormatPreserving(
+		$result = PhpDocParserHelper::getPrinter()->printFormatPreserving(
 			$newPhpDocNode,
 			$parsedDocComment->getNode(),
 			$parsedDocComment->getTokens(),
 		);
+
+		// Detect original indentation style from the tokens
+		$originalContent = $parsedDocComment->getTokens()->getContentBetween(0, $parsedDocComment->getTokens()->getTokenCount());
+		$usesSpaces = false;
+		$indentChar = "\t";
+
+		// Check if original uses spaces for indentation by looking at lines after /**
+		if (preg_match('/\n( +)\*/', $originalContent, $matches)) {
+			$usesSpaces = true;
+			$indentChar = ' ';
+		}
+
+		// If original used spaces but result has tabs, convert back to spaces
+		if ($usesSpaces && strpos($result, "\n\t") !== false) {
+			// Replace tabs with spaces, preserving the same visual indentation
+			// Typically 1 tab = 4 spaces in PHPDoc context
+			$result = preg_replace_callback('/\n(\t+)/', function ($matches) {
+				return "\n" . str_repeat(' ', strlen($matches[1]) * 4);
+			}, $result);
+		}
+
+		return $result;
 	}
 
 	/**

[dereuromark]

I dont quite get how this wasnt an issue with this library and phpcs v3
Since the dependencies otherwise didnt change.

[kukulich]

I dont quite get how this wasnt an issue with this library and phpcs v3

So do I undestand it corretly that PHPCS 3 and SlevomatCS 8.23 is ok?

[dereuromark]

Dont quite get it either
But the tests in php-collective/code-sniffer#34 fail, while the same in master are green (and using the same library - yours - there).

[kukulich]

@jrfnl Please, do you have any idea how the upgrade of PHPCS affects this?

[jrfnl]

@kukulich @dereuromark The only change related to docblocks in PHPCS 4.0 is this:

All T_DOC_COMMENT_* tokens will now have the comment_opener and comment_closer indexes set.
This should allow for more flexibility for sniffs examining aspects of docblocks.

Ref: https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/Version-4.0-Developer-Upgrade-Guide#other-tokenizer-changes

Also see: PHPCSStandards/PHP_CodeSniffer#484

Just to ease my mind I've just tested this with the Slevomat standard itself and I cannot reproduce the behaviour.

Using this test file - mind the first code sample has space indentation, the second code sample has tab indentation:

<?php
class TestA {
    /**
     * @return \ArrayObject<string>|int[]
     */
    protected function getArrayObjectOfStringsOrArrayOfInts()
    {
        return $this->foo();
    }
}

class TestB {
	/**
	 * @return \ArrayObject<string>|int[]
	 */
	protected function getArrayObjectOfStringsOrArrayOfInts()
	{
		return $this->foo();
	}
}

and then running the following command:

phpcs -ps ./slevomat-1790.php --standard=slevomatcodingstandard --sniffs=SlevomatCodingStandard.TypeHints.DisallowArrayTypeHintSyntax

I see the following output - both for PHPCS 3.x with Slevomat 8.22.1, as well as for PHPCS 4.x with Slevomat 8.23.0:

FILE: slevomat-1790.php
------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------------------------------------------------------
  4 | ERROR | [x] Usage of array type hint syntax in "int[]" is disallowed, use generic type hint syntax instead.
    |       |     (SlevomatCodingStandard.TypeHints.DisallowArrayTypeHintSyntax.DisallowedArrayTypeHintSyntax)
 14 | ERROR | [x] Usage of array type hint syntax in "int[]" is disallowed, use generic type hint syntax instead.
    |       |     (SlevomatCodingStandard.TypeHints.DisallowArrayTypeHintSyntax.DisallowedArrayTypeHintSyntax)
------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------------------------------

I've also ran the same command with phpcbf with --suffix=.fixed3 for the PHPCS 3.x with Slevomat 8.22.1 combination and --suffix=.fixed4 for the PHPCS 4.x with Slevomat 8.23.0 combination.

The indentation type in both cases is unchanged and both fixed* files are 100% the same:

<?php
class TestA {
    /**
     * @return \ArrayObject<string>|array<int>
     */
    protected function getArrayObjectOfStringsOrArrayOfInts()
    {
        return $this->foo();
    }
}

class TestB {
	/**
	 * @return \ArrayObject<string>|array<int>
	 */
	protected function getArrayObjectOfStringsOrArrayOfInts()
	{
		return $this->foo();
	}
}

In other words, using the Slevomat native sniffs I cannot even reproduce the behaviour, so I can only suspect it may have something to do with how the PHPCollective sniffs call the Slevomat Helpers ?

[dereuromark]

Must be. Especially if u look at the diff of what i changed and pass/fail.

[jrfnl]

@dereuromark Yeah, I saw that. Could it be that somewhere in the sniff (or any of the Helpers) there is a presumption that if a comment token has the comment_opener index, it must be the comment closer ? Or visa-versa, that if a comment token has the comment_closer index, it must be the comment opener ?

That would be were I would start investigating. (Then again, I've never looked in too much detail at the PHPCollective sniffs, so what do I know?)

[jrfnl]

@dereuromark Just thinking - you might want to add yourself to the community cc-list to get a heads-up when proposals are posted which affect external standards (Tokenizer) or integrations...?

[dereuromark]

Are you fine with the diff I posted? It should resolve the indentation issue.

[kukulich]

@dereuromark

The issue is not reproduced here so no change is planed.

[dereuromark]

Fairly certain this is a bug in the helper and how v4 handles it.
php-collective/code-sniffer@fc992f7 fixes it for me though for now, so I am not interested in pursuing this further.
Feel free to close.