From 7673ce7814c6657919b16027eaf11cf2b682a7d7 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Sun, 6 Jun 2021 13:33:41 +0100 Subject: [PATCH 1/9] Add new interface for transformers + add force strict types transformer --- src/Patch/ForceStrictTypes.php | 31 +++++++++++++++++++++++ src/Patch/Transformer.php | 8 ++++++ test/Patch/ForceStrictTypesTest.php | 39 +++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) create mode 100644 src/Patch/ForceStrictTypes.php create mode 100644 src/Patch/Transformer.php create mode 100644 test/Patch/ForceStrictTypesTest.php diff --git a/src/Patch/ForceStrictTypes.php b/src/Patch/ForceStrictTypes.php new file mode 100644 index 00000000..aff1048a --- /dev/null +++ b/src/Patch/ForceStrictTypes.php @@ -0,0 +1,31 @@ +isFirstStatementStrictTypesDeclare($ast)) { + return $ast; + } + + $declare = new \PhpParser\Node\Stmt\Declare_([ + new DeclareDeclare( + new \PhpParser\Node\Identifier('strict_types'), + new LNumber(1) + ) + ]); + + return array_merge([$declare], $ast); + } + + public function isFirstStatementStrictTypesDeclare(array $statements): bool + { + return isset($statements[0]) && $statements[0] instanceof Declare_; + } +} \ No newline at end of file diff --git a/src/Patch/Transformer.php b/src/Patch/Transformer.php new file mode 100644 index 00000000..07290d09 --- /dev/null +++ b/src/Patch/Transformer.php @@ -0,0 +1,8 @@ +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) + ); + } +} From 13a032535970ebd4833416ac63aa6c244730acc8 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Sun, 6 Jun 2021 13:43:59 +0100 Subject: [PATCH 2/9] Allow Transformer instances in patches --- src/Patch.php | 17 +++++++++++------ test/PatchTest.php | 40 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 7 deletions(-) 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/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()); + } } From 0c06899a37f979f76e1db776f195dba00057aa1c Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Sun, 6 Jun 2021 13:48:03 +0100 Subject: [PATCH 3/9] Only add strict_types back in if it wasn't added by transformer --- src/CodePatcher.php | 10 ++++++++-- test/CodePatcherTest.php | 31 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/CodePatcher.php b/src/CodePatcher.php index 3cd1c0d5..7fadb7dd 100644 --- a/src/CodePatcher.php +++ b/src/CodePatcher.php @@ -6,6 +6,7 @@ use PhpParser\Error; use PhpParser\Node\Stmt; +use PhpParser\Node\Stmt\Declare_; use PhpParser\Parser; use PhpParser\PrettyPrinter\Standard; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; @@ -91,7 +92,7 @@ private function applyPatch(Patch $patch, string $code): string } $declare = null; - if (isset($statements[0]) && $statements[0] instanceof \PhpParser\Node\Stmt\Declare_) { + if ($this->isFirstStatementStrictTypesDeclare($statements)) { $declare = array_shift($statements); } @@ -107,13 +108,18 @@ private function applyPatch(Patch $patch, string $code): string } } - if ($declare !== null) { + if ($declare !== null && !$this->isFirstStatementStrictTypesDeclare($statements)) { array_unshift($statements, $declare); } return $this->printer->prettyPrintFile($statements); } + public function isFirstStatementStrictTypesDeclare(array $statements): bool + { + return isset($statements[0]) && $statements[0] instanceof Declare_; + } + /** * @param CodeInsertion $codeInsertion * @param array $statements diff --git a/test/CodePatcherTest.php b/test/CodePatcherTest.php index 3f8df187..5a70382c 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; @@ -169,4 +172,32 @@ public function testTransformerWithStrictTypes(): void $patcher->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) + ); + } } From a178a711c05518516d1e871ff6d979d4e385a388 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Sun, 6 Jun 2021 13:54:59 +0100 Subject: [PATCH 4/9] Add support for Transformer instances --- src/CodePatcher.php | 8 +++++++- test/CodePatcherTest.php | 18 +++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/CodePatcher.php b/src/CodePatcher.php index 7fadb7dd..2aa3dbe1 100644 --- a/src/CodePatcher.php +++ b/src/CodePatcher.php @@ -11,6 +11,7 @@ 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. @@ -102,10 +103,15 @@ private function applyPatch(Patch $patch, string $code): string continue; } - if (is_callable($modifier)) { + if ($modifier instanceof \Closure) { $statements = $modifier($statements); continue; } + + if ($modifier instanceof Transformer) { + $statements = $modifier->transform($statements); + continue; + } } if ($declare !== null && !$this->isFirstStatementStrictTypesDeclare($statements)) { diff --git a/test/CodePatcherTest.php b/test/CodePatcherTest.php index 5a70382c..342ea444 100644 --- a/test/CodePatcherTest.php +++ b/test/CodePatcherTest.php @@ -95,7 +95,7 @@ public function codeProvider(): array (new Patch())->withInsertion(new Insertion(Insertion::TYPE_BEFORE, ' [ + 'transformer-closure' => [ 'withTransformer(function (array $statements) { @@ -122,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'), [])] + ) + ]; + } + }), + " Date: Sun, 6 Jun 2021 14:00:04 +0100 Subject: [PATCH 5/9] Improve strict_types check --- src/CodePatcher.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/CodePatcher.php b/src/CodePatcher.php index 2aa3dbe1..fe9b5ba2 100644 --- a/src/CodePatcher.php +++ b/src/CodePatcher.php @@ -123,7 +123,10 @@ private function applyPatch(Patch $patch, string $code): string public function isFirstStatementStrictTypesDeclare(array $statements): bool { - return isset($statements[0]) && $statements[0] instanceof Declare_; + return isset($statements[0]) + && $statements[0] instanceof Declare_ + && isset($statements[0]->declares[0]) + && $statements[0]->declares[0]->key->name === 'strict_types'; } /** From 39f490bc0528f9d3c9caba57f4ddb19da18dd0a2 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Sun, 6 Jun 2021 14:02:28 +0100 Subject: [PATCH 6/9] Improve types --- src/CodePatcher.php | 3 +++ src/Patch/ForceStrictTypes.php | 18 +++++++++++++----- src/Patch/Transformer.php | 8 +++++++- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/CodePatcher.php b/src/CodePatcher.php index fe9b5ba2..92d792b6 100644 --- a/src/CodePatcher.php +++ b/src/CodePatcher.php @@ -121,6 +121,9 @@ private function applyPatch(Patch $patch, string $code): string return $this->printer->prettyPrintFile($statements); } + /** + * @param array $statements + */ public function isFirstStatementStrictTypesDeclare(array $statements): bool { return isset($statements[0]) diff --git a/src/Patch/ForceStrictTypes.php b/src/Patch/ForceStrictTypes.php index aff1048a..445996cf 100644 --- a/src/Patch/ForceStrictTypes.php +++ b/src/Patch/ForceStrictTypes.php @@ -3,15 +3,20 @@ namespace PhpSchool\PhpWorkshop\Patch; use PhpParser\Node\Scalar\LNumber; +use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Declare_; use PhpParser\Node\Stmt\DeclareDeclare; class ForceStrictTypes implements Transformer { - public function transform(array $ast): array + /** + * @param array $statements + * @return array + */ + public function transform(array $statements): array { - if ($this->isFirstStatementStrictTypesDeclare($ast)) { - return $ast; + if ($this->isFirstStatementStrictTypesDeclare($statements)) { + return $statements; } $declare = new \PhpParser\Node\Stmt\Declare_([ @@ -21,11 +26,14 @@ public function transform(array $ast): array ) ]); - return array_merge([$declare], $ast); + return array_merge([$declare], $statements); } + /** + * @param array $statements + */ public function isFirstStatementStrictTypesDeclare(array $statements): bool { return isset($statements[0]) && $statements[0] instanceof Declare_; } -} \ No newline at end of file +} diff --git a/src/Patch/Transformer.php b/src/Patch/Transformer.php index 07290d09..0c37e687 100644 --- a/src/Patch/Transformer.php +++ b/src/Patch/Transformer.php @@ -2,7 +2,13 @@ namespace PhpSchool\PhpWorkshop\Patch; +use PhpParser\Node\Stmt; + interface Transformer { - public function transform(array $ast): array; + /** + * @param array $statements + * @return array + */ + public function transform(array $statements): array; } From 0661f8fbb5ef9b612600f12e84fd651d1951a074 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Sun, 6 Jun 2021 14:15:45 +0100 Subject: [PATCH 7/9] Remove useless continue --- src/CodePatcher.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/CodePatcher.php b/src/CodePatcher.php index 92d792b6..c0bef17a 100644 --- a/src/CodePatcher.php +++ b/src/CodePatcher.php @@ -100,17 +100,14 @@ private function applyPatch(Patch $patch, string $code): string foreach ($patch->getModifiers() as $modifier) { if ($modifier instanceof CodeInsertion) { $statements = $this->applyCodeInsertion($modifier, $statements); - continue; } if ($modifier instanceof \Closure) { $statements = $modifier($statements); - continue; } if ($modifier instanceof Transformer) { $statements = $modifier->transform($statements); - continue; } } From 62ab5b514a59e60ee7ebe68aaf3ffa9d55e92579 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Sun, 6 Jun 2021 14:18:20 +0100 Subject: [PATCH 8/9] Move strict types remove/adding logic in to loop --- src/CodePatcher.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/CodePatcher.php b/src/CodePatcher.php index c0bef17a..150c4175 100644 --- a/src/CodePatcher.php +++ b/src/CodePatcher.php @@ -92,12 +92,12 @@ private function applyPatch(Patch $patch, string $code): string $statements = []; } - $declare = null; - if ($this->isFirstStatementStrictTypesDeclare($statements)) { - $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); } @@ -109,10 +109,10 @@ private function applyPatch(Patch $patch, string $code): string if ($modifier instanceof Transformer) { $statements = $modifier->transform($statements); } - } - if ($declare !== null && !$this->isFirstStatementStrictTypesDeclare($statements)) { - array_unshift($statements, $declare); + if ($declare !== null && !$this->isFirstStatementStrictTypesDeclare($statements)) { + array_unshift($statements, $declare); + } } return $this->printer->prettyPrintFile($statements); From 140c5e4c8cd3d295cf4f4b8b96b7228615398d99 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Sun, 6 Jun 2021 14:26:19 +0100 Subject: [PATCH 9/9] Add test for strict types with before insertion --- test/CodePatcherTest.php | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/CodePatcherTest.php b/test/CodePatcherTest.php index 342ea444..1093ca71 100644 --- a/test/CodePatcherTest.php +++ b/test/CodePatcherTest.php @@ -216,4 +216,33 @@ public function testTransformerWhichAddsStrictTypesDoesNotResultInDoubleStrictTy $patcher->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) + ); + } }