From 865b8f8f0f8a5b6b8126bbb1eea695265a205d47 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Fri, 11 Jun 2021 17:42:45 +0100 Subject: [PATCH] Log code parse errors from patches --- app/config.php | 4 +-- src/CodePatcher.php | 15 ++++++-- test/CodePatcherTest.php | 77 +++++++++++++++++++++++++++++++++++----- 3 files changed, 84 insertions(+), 12 deletions(-) diff --git a/app/config.php b/app/config.php index 9a7b532f..242b2eae 100644 --- a/app/config.php +++ b/app/config.php @@ -269,12 +269,12 @@ return $parserFactory->create(ParserFactory::PREFER_PHP7); }, CodePatcher::class => function (ContainerInterface $c) { - $patch = (new Patch) + $patch = (new Patch()) ->withInsertion(new Insertion(Insertion::TYPE_BEFORE, 'ini_set("display_errors", "1");')) ->withInsertion(new Insertion(Insertion::TYPE_BEFORE, 'error_reporting(E_ALL);')) ->withInsertion(new Insertion(Insertion ::TYPE_BEFORE, 'date_default_timezone_set("Europe/London");')); - return new CodePatcher($c->get(Parser::class), new Standard, $patch); + return new CodePatcher($c->get(Parser::class), new Standard(), $c->get(LoggerInterface::class), $patch); }, FakerGenerator::class => function () { return FakerFactory::create(); diff --git a/src/CodePatcher.php b/src/CodePatcher.php index 150c4175..df4249ce 100644 --- a/src/CodePatcher.php +++ b/src/CodePatcher.php @@ -12,6 +12,7 @@ use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Exercise\SubmissionPatchable; use PhpSchool\PhpWorkshop\Patch\Transformer; +use Psr\Log\LoggerInterface; /** * Service to apply patches to a student's solution. Accepts a default patch via the constructor. @@ -30,6 +31,11 @@ class CodePatcher */ private $printer; + /** + * @var LoggerInterface + */ + private $logger; + /** * @var Patch|null */ @@ -45,12 +51,14 @@ class CodePatcher * * @param Parser $parser * @param Standard $printer + * @param LoggerInterface $logger * @param Patch|null $defaultPatch */ - public function __construct(Parser $parser, Standard $printer, Patch $defaultPatch = null) + public function __construct(Parser $parser, Standard $printer, LoggerInterface $logger, Patch $defaultPatch = null) { $this->parser = $parser; $this->printer = $printer; + $this->logger = $logger; $this->defaultPatch = $defaultPatch; } @@ -141,7 +149,10 @@ private function applyCodeInsertion(CodeInsertion $codeInsertion, array $stateme $codeToInsert = sprintf('parser->parse($codeToInsert) ?? []; } catch (Error $e) { - //we should probably log this and have a dev mode or something + $this->logger->critical( + 'Code Insertion could not be parsed: ' . $e->getMessage(), + ['code' => $codeInsertion->getCode()] + ); return $statements; } diff --git a/test/CodePatcherTest.php b/test/CodePatcherTest.php index 1093ca71..3db0d8a4 100644 --- a/test/CodePatcherTest.php +++ b/test/CodePatcherTest.php @@ -17,6 +17,8 @@ use PhpSchool\PhpWorkshopTest\Asset\PatchableExercise; use PHPUnit\Framework\TestCase; use PhpSchool\PhpWorkshop\CodeInsertion as Insertion; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; class CodePatcherTest extends TestCase { @@ -25,7 +27,12 @@ public function testDefaultPatchIsAppliedIfAvailable(): void $patch = (new Patch()) ->withInsertion(new Insertion(Insertion::TYPE_BEFORE, 'ini_set("display_errors", 1);')); - $patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard(), $patch); + $patcher = new CodePatcher( + (new ParserFactory())->create(ParserFactory::PREFER_PHP7), + new Standard(), + new NullLogger(), + $patch + ); $exercise = $this->createMock(ExerciseInterface::class); $expected = "create(ParserFactory::PREFER_PHP7), new Standard()); + $patcher = new CodePatcher( + (new ParserFactory())->create(ParserFactory::PREFER_PHP7), + new Standard(), + new NullLogger() + ); $exercise = $this->createMock(ExerciseInterface::class); $code = 'create(ParserFactory::PREFER_PHP7), new Standard()); - + $patcher = new CodePatcher( + (new ParserFactory())->create(ParserFactory::PREFER_PHP7), + new Standard(), + new NullLogger() + ); $exercise = $this->createMock(PatchableExercise::class); $exercise @@ -146,7 +160,11 @@ public function testBeforeInsertionInsertsAfterStrictTypesDeclaration(): void $code = 'withInsertion(new Insertion(Insertion::TYPE_BEFORE, '$before = "here";')); - $patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard()); + $patcher = new CodePatcher( + (new ParserFactory())->create(ParserFactory::PREFER_PHP7), + new Standard(), + new NullLogger() + ); $exercise = $this->createMock(PatchableExercise::class); @@ -174,7 +192,11 @@ public function testTransformerWithStrictTypes(): void ]; }); - $patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard()); + $patcher = new CodePatcher( + (new ParserFactory())->create(ParserFactory::PREFER_PHP7), + new Standard(), + new NullLogger() + ); $exercise = $this->createMock(PatchableExercise::class); @@ -202,7 +224,11 @@ public function testTransformerWhichAddsStrictTypesDoesNotResultInDoubleStrictTy ])]; }); - $patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard()); + $patcher = new CodePatcher( + (new ParserFactory())->create(ParserFactory::PREFER_PHP7), + new Standard(), + new NullLogger() + ); $exercise = $this->createMock(PatchableExercise::class); @@ -231,7 +257,11 @@ public function testAddingStrictTypesDeclareDoesNotBreakBeforeInsertion(): void }) ->withInsertion(new Insertion(Insertion::TYPE_BEFORE, '$before = "here";')); - $patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard()); + $patcher = new CodePatcher( + (new ParserFactory())->create(ParserFactory::PREFER_PHP7), + new Standard(), + new NullLogger() + ); $exercise = $this->createMock(PatchableExercise::class); @@ -245,4 +275,35 @@ public function testAddingStrictTypesDeclareDoesNotBreakBeforeInsertion(): void $patcher->patch($exercise, $code) ); } + + public function testExceptionIsLoggedIfCodeIsNotParseable(): void + { + $patcher = new CodePatcher( + (new ParserFactory())->create(ParserFactory::PREFER_PHP7), + new Standard(), + $logger = $this->createMock(LoggerInterface::class) + ); + + $exercise = $this->createMock(PatchableExercise::class); + + $patch = (new Patch())->withInsertion(new Insertion(Insertion::TYPE_BEFORE, '$before = "here"')); + + $exercise + ->expects($this->once()) + ->method('getPatch') + ->willReturn($patch); + + $logger + ->expects($this->once()) + ->method('critical') + ->with( + 'Code Insertion could not be parsed: Syntax error, unexpected EOF on line 1', + ['code' => '$before = "here"'] + ); + + $this->assertEquals( + "patch($exercise, '