diff --git a/src/CodePatcher.php b/src/CodePatcher.php index 3cd1c0d5..150c4175 100644 --- a/src/CodePatcher.php +++ b/src/CodePatcher.php @@ -6,10 +6,12 @@ use PhpParser\Error; use PhpParser\Node\Stmt; +use PhpParser\Node\Stmt\Declare_; use PhpParser\Parser; use PhpParser\PrettyPrinter\Standard; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Exercise\SubmissionPatchable; +use PhpSchool\PhpWorkshop\Patch\Transformer; /** * Service to apply patches to a student's solution. Accepts a default patch via the constructor. @@ -90,30 +92,43 @@ private function applyPatch(Patch $patch, string $code): string $statements = []; } - $declare = null; - if (isset($statements[0]) && $statements[0] instanceof \PhpParser\Node\Stmt\Declare_) { - $declare = array_shift($statements); - } - foreach ($patch->getModifiers() as $modifier) { + $declare = null; + if ($this->isFirstStatementStrictTypesDeclare($statements)) { + $declare = array_shift($statements); + } + if ($modifier instanceof CodeInsertion) { $statements = $this->applyCodeInsertion($modifier, $statements); - continue; } - if (is_callable($modifier)) { + if ($modifier instanceof \Closure) { $statements = $modifier($statements); - continue; } - } - if ($declare !== null) { - array_unshift($statements, $declare); + if ($modifier instanceof Transformer) { + $statements = $modifier->transform($statements); + } + + if ($declare !== null && !$this->isFirstStatementStrictTypesDeclare($statements)) { + array_unshift($statements, $declare); + } } return $this->printer->prettyPrintFile($statements); } + /** + * @param array $statements + */ + public function isFirstStatementStrictTypesDeclare(array $statements): bool + { + return isset($statements[0]) + && $statements[0] instanceof Declare_ + && isset($statements[0]->declares[0]) + && $statements[0]->declares[0]->key->name === 'strict_types'; + } + /** * @param CodeInsertion $codeInsertion * @param array $statements diff --git a/src/Patch.php b/src/Patch.php index aabb2697..c4c3d5f1 100644 --- a/src/Patch.php +++ b/src/Patch.php @@ -5,19 +5,23 @@ namespace PhpSchool\PhpWorkshop; use Closure; +use PhpSchool\PhpWorkshop\Patch\Transformer; /** * This class is responsible for storing the modifications that should * be made to a PHP file. That includes insertions and transformers. * A transformer is a simple closure which should receives an AST * representation of the student's solution and it should return the modified AST. + * + * Transformers can also be class implementing `Transformer`. + * * An insertion is a block of code that can be inserted at the top or bottom of * the students solution. */ final class Patch { /** - * @var array + * @var array */ private $modifications = []; @@ -35,12 +39,12 @@ public function withInsertion(CodeInsertion $insertion): self } /** - * Add a new transformer (`Closure`). `Patch` is immutable so a new instance is returned. + * Add a new transformer (`Closure` OR `Transformer`). `Patch` is immutable so a new instance is returned. * - * @param Closure $closure + * @param Closure|Transformer $closure * @return self */ - public function withTransformer(Closure $closure): self + public function withTransformer($closure): self { $new = clone $this; $new->modifications[] = $closure; @@ -48,9 +52,10 @@ public function withTransformer(Closure $closure): self } /** - * Retrieve all the modifications including insertions (`CodeInsertion`'s) & transformers (`Closure`'s) + * Retrieve all the modifications including insertions (`CodeInsertion`'s) + * & transformers (`Closure`'s OR `Transformer`'s) * - * @return array + * @return array */ public function getModifiers(): array { diff --git a/src/Patch/ForceStrictTypes.php b/src/Patch/ForceStrictTypes.php new file mode 100644 index 00000000..445996cf --- /dev/null +++ b/src/Patch/ForceStrictTypes.php @@ -0,0 +1,39 @@ + $statements + * @return array + */ + public function transform(array $statements): array + { + if ($this->isFirstStatementStrictTypesDeclare($statements)) { + return $statements; + } + + $declare = new \PhpParser\Node\Stmt\Declare_([ + new DeclareDeclare( + new \PhpParser\Node\Identifier('strict_types'), + new LNumber(1) + ) + ]); + + return array_merge([$declare], $statements); + } + + /** + * @param array $statements + */ + public function isFirstStatementStrictTypesDeclare(array $statements): bool + { + return isset($statements[0]) && $statements[0] instanceof Declare_; + } +} diff --git a/src/Patch/Transformer.php b/src/Patch/Transformer.php new file mode 100644 index 00000000..0c37e687 --- /dev/null +++ b/src/Patch/Transformer.php @@ -0,0 +1,14 @@ + $statements + * @return array + */ + public function transform(array $statements): array; +} diff --git a/test/CodePatcherTest.php b/test/CodePatcherTest.php index 3f8df187..1093ca71 100644 --- a/test/CodePatcherTest.php +++ b/test/CodePatcherTest.php @@ -4,10 +4,13 @@ use PhpParser\Node\Expr\Variable; use PhpParser\Node\Name; +use PhpParser\Node\Scalar\LNumber; use PhpParser\Node\Stmt\Catch_; +use PhpParser\Node\Stmt\DeclareDeclare; use PhpParser\Node\Stmt\TryCatch; use PhpParser\ParserFactory; use PhpParser\PrettyPrinter\Standard; +use PhpParser\PrettyPrinterAbstract; use PhpSchool\PhpWorkshop\CodePatcher; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Patch; @@ -92,7 +95,7 @@ public function codeProvider(): array (new Patch())->withInsertion(new Insertion(Insertion::TYPE_BEFORE, ' [ + 'transformer-closure' => [ 'withTransformer(function (array $statements) { @@ -119,6 +122,22 @@ public function codeProvider(): array }), " [ + 'withTransformer(new class implements Patch\Transformer { + public function transform(array $statements): array + { + return [ + new TryCatch( + $statements, + [new Catch_([new Name(\Exception::class)], new Variable('e'), [])] + ) + ]; + } + }), + "patch($exercise, $code) ); } + + public function testTransformerWhichAddsStrictTypesDoesNotResultInDoubleStrictTypesStatement(): void + { + $code = 'withTransformer(function (array $statements) { + return [new \PhpParser\Node\Stmt\Declare_([ + new DeclareDeclare( + new \PhpParser\Node\Identifier('strict_types'), + new LNumber(1) + ) + ])]; + }); + + $patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard()); + + $exercise = $this->createMock(PatchableExercise::class); + + $exercise + ->expects($this->once()) + ->method('getPatch') + ->willReturn($patch); + + $this->assertEquals( + "patch($exercise, $code) + ); + } + + public function testAddingStrictTypesDeclareDoesNotBreakBeforeInsertion(): void + { + $code = 'withTransformer(function (array $statements) { + return array_merge([new \PhpParser\Node\Stmt\Declare_([ + new DeclareDeclare( + new \PhpParser\Node\Identifier('strict_types'), + new LNumber(1) + ) + ])], $statements); + }) + ->withInsertion(new Insertion(Insertion::TYPE_BEFORE, '$before = "here";')); + + $patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard()); + + $exercise = $this->createMock(PatchableExercise::class); + + $exercise + ->expects($this->once()) + ->method('getPatch') + ->willReturn($patch); + + $this->assertEquals( + "patch($exercise, $code) + ); + } } diff --git a/test/Patch/ForceStrictTypesTest.php b/test/Patch/ForceStrictTypesTest.php new file mode 100644 index 00000000..001ee9ac --- /dev/null +++ b/test/Patch/ForceStrictTypesTest.php @@ -0,0 +1,39 @@ +create(ParserFactory::PREFER_PHP7); + $ast = $parser->parse("transform($ast); + + self::assertSame( + "declare (strict_types=1);\necho 'Hello World';", + (new Standard())->prettyPrint($ast) + ); + } + + public function testStrictTypesDeclareIsNotAppendedIfItAlreadyExists(): void + { + $parser = (new ParserFactory())->create(ParserFactory::PREFER_PHP7); + $ast = $parser->parse("transform($ast); + + self::assertSame( + "declare (strict_types=1);\necho 'Hello World';", + (new Standard())->prettyPrint($ast) + ); + } +} diff --git a/test/PatchTest.php b/test/PatchTest.php index 29886afd..d85e6d87 100644 --- a/test/PatchTest.php +++ b/test/PatchTest.php @@ -19,7 +19,7 @@ public function testWithInsertion(): void $this->assertEquals([$insertion], $new->getModifiers()); } - public function testWithTransformer(): void + public function testWithTransformerWithClosure(): void { $patch = new Patch(); $transformer = function (array $statements) { @@ -30,4 +30,42 @@ public function testWithTransformer(): void $this->assertEmpty($patch->getModifiers()); $this->assertEquals([$transformer], $new->getModifiers()); } + + public function testWithTransformerWithTransformer(): void + { + $patch = new Patch(); + $transformer = new class implements Patch\Transformer { + public function transform(array $ast): array + { + return $ast; + } + }; + + $new = $patch->withTransformer($transformer); + $this->assertNotSame($patch, $new); + $this->assertEmpty($patch->getModifiers()); + $this->assertEquals([$transformer], $new->getModifiers()); + } + + public function testWithTransformerMultiple(): void + { + $transformer1 = new class implements Patch\Transformer { + public function transform(array $ast): array + { + return $ast; + } + }; + $transformer2 = function (array $statements) { + return $statements; + }; + + $patch = new Patch(); + $new = $patch + ->withTransformer($transformer1) + ->withTransformer($transformer2); + + $this->assertNotSame($patch, $new); + $this->assertEmpty($patch->getModifiers()); + $this->assertEquals([$transformer1, $transformer2], $new->getModifiers()); + } }