Skip to content

Commit

Permalink
[dx] Add HTMLAverseRectorInterface to teach rules to skip HTML + PHP …
Browse files Browse the repository at this point in the history
…mix that would go wrong (#6445)

* skip html + php mix

* [dx] Add HTMLAverseRectorInterface
  • Loading branch information
TomasVotruba authored Nov 16, 2024
1 parent c7dfbe7 commit e562253
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

namespace Rector\Tests\CodingStyle\Rector\Stmt\NewlineAfterStatementRector\Fixture;

if (mt_rand(0, 1)) { ?>
<div>
<h1>Hi</h1>
</div>

<?php } else { ?>
<div>
<h1>Bye</h1>
</div>

<?php } ?>

End of file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\Function_;
use PhpParser\Node\Stmt\Nop;
use Rector\Contract\Rector\HTMLAverseRectorInterface;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Rector\Tests\CodingStyle\Rector\ClassMethod\NewlineBeforeNewAssignSetRector\NewlineBeforeNewAssignSetRectorTest
*/
final class NewlineBeforeNewAssignSetRector extends AbstractRector
final class NewlineBeforeNewAssignSetRector extends AbstractRector implements HTMLAverseRectorInterface
{
private ?string $previousStmtVariableName = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use PhpParser\Node\Stmt\TryCatch;
use PhpParser\Node\Stmt\While_;
use Rector\Contract\PhpParser\Node\StmtsAwareInterface;
use Rector\Contract\Rector\HTMLAverseRectorInterface;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
Expand All @@ -32,7 +33,7 @@
/**
* @see \Rector\Tests\CodingStyle\Rector\Stmt\NewlineAfterStatementRector\NewlineAfterStatementRectorTest
*/
final class NewlineAfterStatementRector extends AbstractRector
final class NewlineAfterStatementRector extends AbstractRector implements HTMLAverseRectorInterface
{
/**
* @var array<class-string<Node>>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,21 @@
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Declare_;
use PhpParser\Node\Stmt\DeclareDeclare;
use PhpParser\Node\Stmt\InlineHTML;
use PhpParser\Node\Stmt\Nop;
use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
use Rector\Contract\PhpParser\Node\StmtsAwareInterface;
use Rector\Contract\Rector\HTMLAverseRectorInterface;
use Rector\PhpParser\Node\CustomNode\FileWithoutNamespace;
use Rector\Rector\AbstractRector;
use Rector\TypeDeclaration\NodeAnalyzer\DeclareStrictTypeFinder;
use Rector\ValueObject\Application\File;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Rector\Tests\TypeDeclaration\Rector\StmtsAwareInterface\DeclareStrictTypesRector\DeclareStrictTypesRectorTest
*/
final class DeclareStrictTypesRector extends AbstractRector
final class DeclareStrictTypesRector extends AbstractRector implements HTMLAverseRectorInterface
{
public function __construct(
private readonly DeclareStrictTypeFinder $declareStrictTypeFinder
Expand Down Expand Up @@ -65,6 +66,10 @@ public function beforeTraverse(array $nodes): ?array
return null;
}

if ($this->startWithShebang($this->file)) {
return null;
}

if ($nodes === []) {
return null;
}
Expand All @@ -79,10 +84,6 @@ public function beforeTraverse(array $nodes): ?array
return null;
}

if ($currentStmt instanceof InlineHTML) {
return null;
}

$nodes = $rootStmt->stmts;
$stmt = $currentStmt;
}
Expand Down Expand Up @@ -124,4 +125,9 @@ public function refactor(Node $node): ?Node
// workaroudn, as Rector now only hooks to specific nodes, not arrays
return null;
}

private function startWithShebang(File $file): bool
{
return str_starts_with($file->getFileContent(), '#!');
}
}
13 changes: 13 additions & 0 deletions src/Contract/Rector/HTMLAverseRectorInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

declare(strict_types=1);

namespace Rector\Contract\Rector;

/**
* Rector rule with this marker interface will skip all files
* with any HTML node. This is practical to avoid malformed PHP+HTML files
*/
interface HTMLAverseRectorInterface
{
}
6 changes: 6 additions & 0 deletions src/Rector/AbstractRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use Rector\Application\Provider\CurrentFileProvider;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
use Rector\Contract\Rector\HTMLAverseRectorInterface;
use Rector\Contract\Rector\RectorInterface;
use Rector\Exception\ShouldNotHappenException;
use Rector\NodeDecorator\CreatedByRuleDecorator;
Expand Down Expand Up @@ -125,6 +126,10 @@ final public function enterNode(Node $node): int|Node|null
return null;
}

if (is_a($this, HTMLAverseRectorInterface::class, true) && $this->file->containsHTML()) {
return null;
}

$filePath = $this->file->getFilePath();
if ($this->skipper->shouldSkipCurrentNode($this, $filePath, static::class, $node)) {
return null;
Expand All @@ -134,6 +139,7 @@ final public function enterNode(Node $node): int|Node|null

// ensure origNode pulled before refactor to avoid changed during refactor, ref https://3v4l.org/YMEGN
$originalNode = $node->getAttribute(AttributeKey::ORIGINAL_NODE) ?? $node;

$refactoredNode = $this->refactor($node);

// @see NodeTraverser::* codes, e.g. removal of node of stopping the traversing
Expand Down
19 changes: 19 additions & 0 deletions src/ValueObject/Application/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

use PhpParser\Node;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\InlineHTML;
use PhpParser\NodeFinder;
use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
use Rector\Exception\ShouldNotHappenException;
use Rector\ValueObject\Reporting\FileDiff;
Expand Down Expand Up @@ -38,6 +40,11 @@ final class File
*/
private array $rectorWithLineChanges = [];

/**
* Cached result per file
*/
private ?bool $containsHtml = null;

public function __construct(
private readonly string $filePath,
private string $fileContent
Expand Down Expand Up @@ -150,4 +157,16 @@ public function getRectorWithLineChanges(): array
{
return $this->rectorWithLineChanges;
}

public function containsHTML(): bool
{
if ($this->containsHtml !== null) {
return $this->containsHtml;
}

$nodeFinder = new NodeFinder();

$this->containsHtml = (bool) $nodeFinder->findFirstInstanceOf($this->oldStmts, InlineHTML::class);
return $this->containsHtml;
}
}

0 comments on commit e562253

Please sign in to comment.