Skip to content

Commit f889737

Browse files
AydinHassanAydinHassan
authored
Merge pull request #222 from php-school/log-code-parse-errors-from-patches
Log code parse errors from patches
2 parents b5db77f + 865b8f8 commit f889737

File tree

3 files changed

+84
-12
lines changed

3 files changed

+84
-12
lines changed

‎app/config.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,12 +274,12 @@
274274
return $parserFactory->create(ParserFactory::PREFER_PHP7);
275275
},
276276
CodePatcher::class => function (ContainerInterface $c) {
277-
$patch = (new Patch)
277+
$patch = (new Patch())
278278
->withInsertion(new Insertion(Insertion::TYPE_BEFORE, 'ini_set("display_errors", "1");'))
279279
->withInsertion(new Insertion(Insertion::TYPE_BEFORE, 'error_reporting(E_ALL);'))
280280
->withInsertion(new Insertion(Insertion ::TYPE_BEFORE, 'date_default_timezone_set("Europe/London");'));
281281

282-
return new CodePatcher($c->get(Parser::class), new Standard, $patch);
282+
return new CodePatcher($c->get(Parser::class), new Standard(), $c->get(LoggerInterface::class), $patch);
283283
},
284284
FakerGenerator::class => function () {
285285
return FakerFactory::create();

‎src/CodePatcher.php‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface;
1313
use PhpSchool\PhpWorkshop\Exercise\SubmissionPatchable;
1414
use PhpSchool\PhpWorkshop\Patch\Transformer;
15+
use Psr\Log\LoggerInterface;
1516

1617
/**
1718
* Service to apply patches to a student's solution. Accepts a default patch via the constructor.
@@ -30,6 +31,11 @@ class CodePatcher
3031
*/
3132
private $printer;
3233

34+
/**
35+
* @var LoggerInterface
36+
*/
37+
private $logger;
38+
3339
/**
3440
* @var Patch|null
3541
*/
@@ -45,12 +51,14 @@ class CodePatcher
4551
*
4652
* @param Parser $parser
4753
* @param Standard $printer
54+
* @param LoggerInterface $logger
4855
* @param Patch|null $defaultPatch
4956
*/
50-
public function __construct(Parser $parser, Standard $printer, Patch $defaultPatch = null)
57+
public function __construct(Parser $parser, Standard $printer, LoggerInterface $logger, Patch $defaultPatch = null)
5158
{
5259
$this->parser = $parser;
5360
$this->printer = $printer;
61+
$this->logger = $logger;
5462
$this->defaultPatch = $defaultPatch;
5563
}
5664

@@ -141,7 +149,10 @@ private function applyCodeInsertion(CodeInsertion $codeInsertion, array $stateme
141149
$codeToInsert = sprintf('<?php %s', preg_replace('/^\s*<\?php/', '', $codeToInsert));
142150
$additionalStatements = $this->parser->parse($codeToInsert) ?? [];
143151
} catch (Error $e) {
144-
//we should probably log this and have a dev mode or something
152+
$this->logger->critical(
153+
'Code Insertion could not be parsed: ' . $e->getMessage(),
154+
['code' => $codeInsertion->getCode()]
155+
);
145156
return $statements;
146157
}
147158

‎test/CodePatcherTest.php‎

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
use PhpSchool\PhpWorkshopTest\Asset\PatchableExercise;
1818
use PHPUnit\Framework\TestCase;
1919
use PhpSchool\PhpWorkshop\CodeInsertion as Insertion;
20+
use Psr\Log\LoggerInterface;
21+
use Psr\Log\NullLogger;
2022

2123
class CodePatcherTest extends TestCase
2224
{
@@ -25,7 +27,12 @@ public function testDefaultPatchIsAppliedIfAvailable(): void
2527
$patch = (new Patch())
2628
->withInsertion(new Insertion(Insertion::TYPE_BEFORE, 'ini_set("display_errors", 1);'));
2729

28-
$patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard(), $patch);
30+
$patcher = new CodePatcher(
31+
(new ParserFactory())->create(ParserFactory::PREFER_PHP7),
32+
new Standard(),
33+
new NullLogger(),
34+
$patch
35+
);
2936
$exercise = $this->createMock(ExerciseInterface::class);
3037

3138
$expected = "<?php\n\nini_set(\"display_errors\", 1);\n\$original = true;";
@@ -34,7 +41,11 @@ public function testDefaultPatchIsAppliedIfAvailable(): void
3441

3542
public function testPatcherDoesNotApplyPatchIfNotPatchableExercise(): void
3643
{
37-
$patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard());
44+
$patcher = new CodePatcher(
45+
(new ParserFactory())->create(ParserFactory::PREFER_PHP7),
46+
new Standard(),
47+
new NullLogger()
48+
);
3849
$exercise = $this->createMock(ExerciseInterface::class);
3950

4051
$code = '<?php $original = true;';
@@ -46,8 +57,11 @@ public function testPatcherDoesNotApplyPatchIfNotPatchableExercise(): void
4657
*/
4758
public function testPatcher(string $code, Patch $patch, string $expectedResult): void
4859
{
49-
$patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard());
50-
60+
$patcher = new CodePatcher(
61+
(new ParserFactory())->create(ParserFactory::PREFER_PHP7),
62+
new Standard(),
63+
new NullLogger()
64+
);
5165
$exercise = $this->createMock(PatchableExercise::class);
5266

5367
$exercise
@@ -146,7 +160,11 @@ public function testBeforeInsertionInsertsAfterStrictTypesDeclaration(): void
146160
$code = '<?php declare(strict_types=1); $original = true;';
147161
$patch = (new Patch())->withInsertion(new Insertion(Insertion::TYPE_BEFORE, '$before = "here";'));
148162

149-
$patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard());
163+
$patcher = new CodePatcher(
164+
(new ParserFactory())->create(ParserFactory::PREFER_PHP7),
165+
new Standard(),
166+
new NullLogger()
167+
);
150168

151169
$exercise = $this->createMock(PatchableExercise::class);
152170

@@ -174,7 +192,11 @@ public function testTransformerWithStrictTypes(): void
174192
];
175193
});
176194

177-
$patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard());
195+
$patcher = new CodePatcher(
196+
(new ParserFactory())->create(ParserFactory::PREFER_PHP7),
197+
new Standard(),
198+
new NullLogger()
199+
);
178200

179201
$exercise = $this->createMock(PatchableExercise::class);
180202

@@ -202,7 +224,11 @@ public function testTransformerWhichAddsStrictTypesDoesNotResultInDoubleStrictTy
202224
])];
203225
});
204226

205-
$patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard());
227+
$patcher = new CodePatcher(
228+
(new ParserFactory())->create(ParserFactory::PREFER_PHP7),
229+
new Standard(),
230+
new NullLogger()
231+
);
206232

207233
$exercise = $this->createMock(PatchableExercise::class);
208234

@@ -231,7 +257,11 @@ public function testAddingStrictTypesDeclareDoesNotBreakBeforeInsertion(): void
231257
})
232258
->withInsertion(new Insertion(Insertion::TYPE_BEFORE, '$before = "here";'));
233259

234-
$patcher = new CodePatcher((new ParserFactory())->create(ParserFactory::PREFER_PHP7), new Standard());
260+
$patcher = new CodePatcher(
261+
(new ParserFactory())->create(ParserFactory::PREFER_PHP7),
262+
new Standard(),
263+
new NullLogger()
264+
);
235265

236266
$exercise = $this->createMock(PatchableExercise::class);
237267

@@ -245,4 +275,35 @@ public function testAddingStrictTypesDeclareDoesNotBreakBeforeInsertion(): void
245275
$patcher->patch($exercise, $code)
246276
);
247277
}
278+
279+
public function testExceptionIsLoggedIfCodeIsNotParseable(): void
280+
{
281+
$patcher = new CodePatcher(
282+
(new ParserFactory())->create(ParserFactory::PREFER_PHP7),
283+
new Standard(),
284+
$logger = $this->createMock(LoggerInterface::class)
285+
);
286+
287+
$exercise = $this->createMock(PatchableExercise::class);
288+
289+
$patch = (new Patch())->withInsertion(new Insertion(Insertion::TYPE_BEFORE, '$before = "here"'));
290+
291+
$exercise
292+
->expects($this->once())
293+
->method('getPatch')
294+
->willReturn($patch);
295+
296+
$logger
297+
->expects($this->once())
298+
->method('critical')
299+
->with(
300+
'Code Insertion could not be parsed: Syntax error, unexpected EOF on line 1',
301+
['code' => '$before = "here"']
302+
);
303+
304+
$this->assertEquals(
305+
"<?php\n\n\$original = true;",
306+
$patcher->patch($exercise, '<?php $original = true;')
307+
);
308+
}
248309
}

0 commit comments

Comments
 (0)