Skip to content

Commit

Permalink
Skip files with short <?= PHP tag as leads to invalid changes (#6068)
Browse files Browse the repository at this point in the history
* Skip files with echo PHP tags as leads to invalid changes

* exclude with starting position only

* skip testWithFollowingBrokenSymlinks() on windows as finder does not handle broken symlinks and notContains()
  • Loading branch information
TomasVotruba authored Jun 28, 2024
1 parent 1865d51 commit a77b00d
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 13 deletions.
5 changes: 5 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -300,3 +300,8 @@ parameters:
-
message: '#Parameters should have "PhpParser\\Node\\Stmt\\ClassMethod" types as the only types passed to this method#'
path: src/VendorLocker/ParentClassMethodTypeOverrideGuard.php

# on purpose to allow checking broken symlinks on windows
-
message: '#"@file_get_contents\(\$file\)" is forbidden to use#'
path: src/FileSystem/FilesFinder.php
2 changes: 1 addition & 1 deletion src/Caching/UnchangedFilesFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public function __construct(
* @param string[] $filePaths
* @return string[]
*/
public function filterFileInfos(array $filePaths): array
public function filterFilePaths(array $filePaths): array
{
$changedFileInfos = [];

Expand Down
22 changes: 21 additions & 1 deletion src/FileSystem/FilesFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
*/
final readonly class FilesFinder
{
/**
* @var string
* @see https://regex101.com/r/3NwDLo/1
*/
private const OPEN_SHORTTAG_REGEX = '#^\<\?=#';

public function __construct(
private FilesystemTweaker $filesystemTweaker,
private UnchangedFilesFilter $unchangedFilesFilter,
Expand All @@ -31,6 +37,18 @@ public function findInDirectoriesAndFiles(array $source, array $suffixes = [], b
$filesAndDirectories = $this->filesystemTweaker->resolveWithFnmatch($source);

$files = $this->fileAndDirectoryFilter->filterFiles($filesAndDirectories);

// exclude short "<?=" tags as lead to invalid changes
$files = array_filter($files, static function (string $file): bool {
// @ on purpose to deal with broken symlinks
$fileContents = @file_get_contents($file);
if (! is_string($fileContents)) {
return true;
}

return ! str_starts_with($fileContents, '<?=');
});

$filteredFilePaths = array_filter(
$files,
fn (string $filePath): bool => ! $this->pathSkipper->shouldSkip($filePath)
Expand All @@ -48,7 +66,7 @@ public function findInDirectoriesAndFiles(array $source, array $suffixes = [], b
$filteredFilePathsInDirectories = $this->findInDirectories($directories, $suffixes, $sortByName);
$filePaths = [...$filteredFilePaths, ...$filteredFilePathsInDirectories];

return $this->unchangedFilesFilter->filterFileInfos($filePaths);
return $this->unchangedFilesFilter->filterFilePaths($filePaths);
}

/**
Expand All @@ -66,6 +84,8 @@ private function findInDirectories(array $directories, array $suffixes, bool $so
->files()
// skip empty files
->size('> 0')
// changes in PHP files with short echo tag will mostly create invalid code, "<?php" tags are required
->notContains(self::OPEN_SHORTTAG_REGEX)
->in($directories);

if ($sortByName) {
Expand Down
10 changes: 1 addition & 9 deletions src/PostRector/Rector/NameImportingPostRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,7 @@ private function shouldSkipFileWithoutNamespace(File $file): bool
}

$currentStmt = current($firstStmt->stmts);

if ($currentStmt instanceof InlineHTML || $currentStmt === false) {
return true;
}

$oldTokens = $file->getOldTokens();
$tokenStartPos = $currentStmt->getStartTokenPos();

return isset($oldTokens[$tokenStartPos][1]) && $oldTokens[$tokenStartPos][1] === '<?=';
return $currentStmt instanceof InlineHTML || $currentStmt === false;
}

private function processNodeName(FullyQualified $fullyQualified, File $file): ?Node
Expand Down
12 changes: 11 additions & 1 deletion tests/FileSystem/FilesFinder/FilesFinderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,24 @@ protected function setUp(): void
$this->filesFinder = $this->make(FilesFinder::class);
}

public function testDefault(): void
public function test(): void
{
$foundFiles = $this->filesFinder->findInDirectoriesAndFiles([__DIR__ . '/SourceWithSymlinks'], ['txt']);
$this->assertCount(1, $foundFiles);

$foundFiles = $this->filesFinder->findInDirectoriesAndFiles([__DIR__ . '/SourceWithShortEchoes'], ['php']);
$this->assertCount(0, $foundFiles);
}

public function testWithFollowingBrokenSymlinks(): void
{
// detect Windows
if (DIRECTORY_SEPARATOR === '\\') {
$this->markTestSkipped(
'This test fails on Windows, due to possible bug in symfony/finder. When Finder is applied on broken symlinks and uses ->notContains(). Fix needed.'
);
}

SimpleParameterProvider::setParameter(Option::SKIP, [__DIR__ . '/../SourceWithBrokenSymlinks/folder1']);

$foundFiles = $this->filesFinder->findInDirectoriesAndFiles([__DIR__ . '/SourceWithBrokenSymlinks']);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<?= $world ?>
hello

This file was deleted.

0 comments on commit a77b00d

Please sign in to comment.