Merge pull request #220 from php-school/force-strict-types-patch

AydinHassan · web-flow · commit 2677e76882e1 · 2021-06-07T18:29:53.000+01:00
Force strict types patch
diff --git 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<Stmt> $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<Stmt> $statements
diff --git 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<CodeInsertion|Closure>
+     * @var array<CodeInsertion|Closure|Transformer>
      */
     private $modifications = [];
 
@@ -35,22 +39,23 @@ 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;
         return $new;
     }
 
     /**
-     * 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<CodeInsertion|Closure>
+     * @return array<CodeInsertion|Closure|Transformer>
      */
     public function getModifiers(): array
     {
diff --git a/src/Patch/ForceStrictTypes.php b/src/Patch/ForceStrictTypes.php
@@ -0,0 +1,39 @@
+<?php
+
+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
+{
+    /**
+     * @param array<Stmt> $statements
+     * @return array<Stmt>
+     */
+    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<Stmt> $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
@@ -0,0 +1,14 @@
+<?php
+
+namespace PhpSchool\PhpWorkshop\Patch;
+
+use PhpParser\Node\Stmt;
+
+interface Transformer
+{
+    /**
+     * @param array<Stmt> $statements
+     * @return array<Stmt>
+     */
+    public function transform(array $statements): array;
+}
diff --git 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, '    <?php $before = "here";')),
                 "<?php\n\n\$before = \"here\";\n\$original = true;"
             ],
-            'transformer' => [
+            'transformer-closure' => [
                 '<?php $original = true;',
                 (new Patch())
                     ->withTransformer(function (array $statements) {
@@ -119,6 +122,22 @@ public function codeProvider(): array
                     }),
                 "<?php\n\ntry {\n    \$before = \"here\";\n    \$original = true;\n} catch (Exception \$e) {\n}"
             ],
+            'transformer-class' => [
+                '<?php $original = true;',
+                (new Patch())
+                    ->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'), [])]
+                                )
+                            ];
+                        }
+                    }),
+                "<?php\n\ntry {\n    \$original = true;\n} catch (Exception \$e) {\n}"
+            ],
         ];
     }
 
@@ -169,4 +188,61 @@ public function testTransformerWithStrictTypes(): void
             $patcher->patch($exercise, $code)
         );
     }
+
+    public function testTransformerWhichAddsStrictTypesDoesNotResultInDoubleStrictTypesStatement(): void
+    {
+        $code = '<?php declare(strict_types=1); $original = true;';
+        $patch = (new Patch())
+            ->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(
+            "<?php\n\ndeclare (strict_types=1);",
+            $patcher->patch($exercise, $code)
+        );
+    }
+
+    public function testAddingStrictTypesDeclareDoesNotBreakBeforeInsertion(): void
+    {
+        $code = '<?php $original = true;';
+        $patch = (new Patch())
+            ->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(
+            "<?php\n\ndeclare (strict_types=1);\n\$before = \"here\";\n\$original = true;",
+            $patcher->patch($exercise, $code)
+        );
+    }
 }
diff --git a/test/Patch/ForceStrictTypesTest.php b/test/Patch/ForceStrictTypesTest.php
@@ -0,0 +1,39 @@
+<?php
+
+namespace PhpSchool\PhpWorkshopTest\Patch;
+
+use PhpParser\ParserFactory;
+use PhpParser\PrettyPrinter\Standard;
+use PhpSchool\PhpWorkshop\Patch\ForceStrictTypes;
+use PHPUnit\Framework\TestCase;
+
+class ForceStrictTypesTest extends TestCase
+{
+    public function testStrictTypesDeclareIsAppended(): void
+    {
+        $parser = (new ParserFactory())->create(ParserFactory::PREFER_PHP7);
+        $ast = $parser->parse("<?php echo 'Hello World';");
+
+        $transformer = new ForceStrictTypes();
+        $ast = $transformer->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("<?php declare (strict_types=1);\necho 'Hello World';");
+        $transformer = new ForceStrictTypes();
+
+        $ast = $transformer->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
@@ -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());
+    }
 }
