deprecated-packages / symplify

[DISCONTINUED] Check split packages in their own repositories :)
MIT License
619 stars 188 forks source link

Something is wrong #4010

Closed ghost closed 2 years ago

ghost commented 2 years ago

I have installed the latest ECS release 10.1.4 which uses php-cs-fixer version 3.7.0 (PHP_CS_FIXER_IGNORE_ENV=-1 php vendor/symplify/easy-coding-standard/vendor/bin/php-cs-fixer -v). Then I open file vendor/symplify/easy-coding-standard/vendor/friendsofphp/php-cs-fixer/src/AbstractPhpdocTypesFixer.php and it's content

Expand ```php * Dariusz Rumiński * * This source file is subject to the MIT license that is bundled * with this source code in the file LICENSE. */ namespace PhpCsFixer; use PhpCsFixer\DocBlock\Annotation; use PhpCsFixer\DocBlock\DocBlock; use PhpCsFixer\Tokenizer\Token; use PhpCsFixer\Tokenizer\Tokens; /** * This abstract fixer provides a base for fixers to fix types in PHPDoc. * * @author Graham Campbell * * @internal */ abstract class AbstractPhpdocTypesFixer extends \PhpCsFixer\AbstractFixer { /** * The annotation tags search inside. * * @var string[] */ protected $tags; /** * {@inheritdoc} */ public function __construct() { parent::__construct(); $this->tags = \PhpCsFixer\DocBlock\Annotation::getTagsWithTypes(); } /** * {@inheritdoc} */ public function isCandidate(\PhpCsFixer\Tokenizer\Tokens $tokens) : bool { return $tokens->isTokenKindFound(\T_DOC_COMMENT); } /** * {@inheritdoc} */ protected function applyFix(\SplFileInfo $file, \PhpCsFixer\Tokenizer\Tokens $tokens) : void { foreach ($tokens as $index => $token) { if (!$token->isGivenKind(\T_DOC_COMMENT)) { continue; } $doc = new \PhpCsFixer\DocBlock\DocBlock($token->getContent()); $annotations = $doc->getAnnotationsOfType($this->tags); if (0 === \count($annotations)) { continue; } foreach ($annotations as $annotation) { $this->fixTypes($annotation); } $tokens[$index] = new \PhpCsFixer\Tokenizer\Token([\T_DOC_COMMENT, $doc->getContent()]); } } /** * Actually normalize the given type. */ protected abstract function normalize(string $type) : string; /** * Fix the types at the given line. * * We must be super careful not to modify parts of words. * * This will be nicely handled behind the scenes for us by the annotation class. */ private function fixTypes(\PhpCsFixer\DocBlock\Annotation $annotation) : void { $types = $annotation->getTypes(); $new = $this->normalizeTypes($types); if ($types !== $new) { $annotation->setTypes($new); } } /** * @param string[] $types * * @return string[] */ private function normalizeTypes(array $types) : array { foreach ($types as $index => $type) { $types[$index] = $this->normalizeType($type); } return $types; } /** * Prepare the type and normalize it. */ private function normalizeType(string $type) : string { return \substr_compare($type, '[]', -\strlen('[]')) === 0 ? $this->normalizeType(\substr($type, 0, -2)) . '[]' : $this->normalize($type); } } ```

is not the same as here AbstractPhpdocTypesFixer.php

What is going on?

My PHP version: 7.2

ghost commented 2 years ago

Hmmm, does ECS somehow compile php-cs-fixer files and dumps it's own? Does it replace polyfill functions? I can see that the original AbstractRepositoryFixer::normalizeType has different code than the ECS's one.

Does ECS 10.1.4 use the latest symfony/polyfill-* versions when compiling?

TomasVotruba commented 2 years ago

ECS is downgraded on the release to PHP 7.1 syntax.

ghost commented 2 years ago

Ok. Then the question that remains is whether ECS downgrade process uses the latest polyfill versions. Because if not then we do not have the most recent fixes included in the dumped code that is finally used when running fixers.

TomasVotruba commented 2 years ago

That depends on the choice of composer. Here is the job run if that helps: https://github.com/symplify/symplify/runs/5572273146?check_suite_focus=true#step:7:76

TomasVotruba commented 2 years ago

The ECS itself does not use any polyfills as developed on PHP 8.0: https://github.com/symplify/symplify/blob/171595216c64f839eefccf5e1384296d093ebcfe/packages/easy-coding-standard/composer.json#L8-L28

Polyfills might work propperly in a project. But downgrade and scoping requires clear settings of composer autoload, where polyfills hack in. Mostly due to aliasing and way the scoper creates prefixed functions. Thus they're avoided and native PHP downgraded code is used.

ghost commented 2 years ago

I would be grateful if the issues stay open until the author is satisfied with an answer. Closing issue asap is not the way to go.

ghost commented 2 years ago

I can see drawbacks with this approach for downgrading polyfilled code. Look at this: if php-cs-fixer uses symfony/polyfill-* then it's code works properly with this specific polyfill code. If ECS downgrades polyfilled functions in another way then php-cs-fixer may not work properly. Good example is https://github.com/symfony/polyfill-php80/blob/f4386d7f6f66346254ed4e6bfa4354d2d16b83f8/Php80.php#L101 while ECS's code downgraded str_ends_with function the other way.

Do you say that I should use prefixed ECS? If so then how do I install it in my project so I have the exact code as if no downgrade would happend?

TomasVotruba commented 2 years ago

I answered in the 1st comment: https://github.com/symplify/symplify/issues/4010#issuecomment-1069668487

If you find a better way that works on downgraded and prefixed code, we'd love to merge such PR :+1:

ghost commented 2 years ago

I try to upgrade ECS in my project but when running I receive tons of warnings PHP Warning: substr_compare(): The start position cannot exceed initial string length in vendor/symplify/easy-coding-standard/vendor/friendsofphp/php-cs-fixer/src/AbstractPhpdocTypesFixer.php on line 105 These warnings do not show up in the standaone php-cs-fixer 3.7.0.

What can we do about this?

ghost commented 2 years ago

This warning starts showing up on 10.0.22. At 10.0.21 it works fine.

ghost commented 2 years ago

I've got this! I had invalid annotations like @param $paramName without type. When fixed everything is fine. Thank you and have a good day.