Skip to content

Commit 8f58d23

Browse files
ArtARTs36ArtARTs36
authored
[Bug] Fix NativeJsonDecoder (#30)
* Add DirectoryTest * move test * Add MemoryRequestFetcherTest * Add SymfonyYamlDecoderTest * fix phpstan issues for 9 level * Add NativeJsonDecoderTest * up version * up config
1 parent 55f9c1c commit 8f58d23

File tree

18 files changed

+256
-17
lines changed

18 files changed

+256
-17
lines changed

‎.mr-linter.yml

+8
Original file line numberDiff line numberDiff line change
@@ -39,5 +39,13 @@ rules:
3939
$all:
4040
isStudlyCase: true
4141

42+
- definition: "Description must have list of fixed bugs"
43+
rules:
44+
description:
45+
contains: "Fixed\n"
46+
when:
47+
labels:
48+
has: "Bug"
49+
4250
credentials:
4351
github_actions: "env(MR_LINTER_GITHUB_HTTP_TOKEN)"

‎src/Infrastructure/Container/MapContainer.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function has(string $id): bool
3636
}
3737

3838
/**
39-
* @template V object
39+
* @template V as object
4040
* @param class-string<V> $id
4141
* @param V $object
4242
*/

‎src/Infrastructure/RequestFetcher/MemoryRequestFetcher.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
use ArtARTs36\MergeRequestLinter\Domain\Request\MergeRequest;
66
use ArtARTs36\MergeRequestLinter\Domain\Request\MergeRequestFetcher;
77

8-
class MemoryRequestFetcher implements MergeRequestFetcher
8+
final class MemoryRequestFetcher implements MergeRequestFetcher
99
{
1010
public function __construct(
1111
private MergeRequest $request,

‎src/Infrastructure/Text/Decoder/NativeJsonDecoder.php

+5-1
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,15 @@ class NativeJsonDecoder implements TextDecoder
99
{
1010
public function decode(string $content): array
1111
{
12-
$data = \json_decode($content);
12+
$data = \json_decode($content, true);
1313
if (\JSON_ERROR_NONE !== \json_last_error()) {
1414
throw new TextDecodingException('json_decode error: ' . \json_last_error_msg());
1515
}
1616

17+
if (! is_array($data)) {
18+
throw new TextDecodingException('JSON content invalid');
19+
}
20+
1721
return $data;
1822
}
1923
}

‎src/Infrastructure/Text/Decoder/SymfonyYamlDecoder.php

+8-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
use Symfony\Component\Yaml\Exception\ParseException;
88
use Symfony\Component\Yaml\Parser;
99

10-
class SymfonyYamlDecoder implements TextDecoder
10+
final class SymfonyYamlDecoder implements TextDecoder
1111
{
1212
public function __construct(
1313
private readonly Parser $parser = new Parser(),
@@ -18,7 +18,13 @@ public function __construct(
1818
public function decode(string $content): array
1919
{
2020
try {
21-
return $this->parser->parse($content);
21+
$parsed = $this->parser->parse($content);
22+
23+
if (! is_array($parsed)) {
24+
throw new TextDecodingException('Invalid yaml string');
25+
}
26+
27+
return $parsed;
2228
} catch (ParseException $e) {
2329
throw new TextDecodingException($e->getMessage(), previous: $e);
2430
}

‎src/Presentation/Console/Command/Command.php

+30
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,36 @@ final protected function getWorkDir(InputInterface $input): string
2020
return (new WorkDirResolver())->resolve($input);
2121
}
2222

23+
final protected function getStringOptionFromInput(InputInterface $input, string $key): ?string
24+
{
25+
$option = $input->getOption($key);
26+
27+
if ($option === null) {
28+
return null;
29+
}
30+
31+
if (! is_string($option)) {
32+
throw new \RuntimeException(sprintf('Input option "%s" must be string', $key));
33+
}
34+
35+
return $option;
36+
}
37+
38+
protected function getBoolFromOption(InputInterface $input, string $key, bool $default = false): bool
39+
{
40+
$option = $input->getOption($key);
41+
42+
if ($option === null) {
43+
return $default;
44+
}
45+
46+
if (! is_bool($option)) {
47+
throw new \RuntimeException(sprintf('Input option "%s" must be bool', $key));
48+
}
49+
50+
return $option;
51+
}
52+
2353
private function addWorkDirOption(): void
2454
{
2555
$this

‎src/Presentation/Console/Command/InstallCommand.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
4646

4747
private function resolveConfigFormat(InputInterface $input): ConfigFormat
4848
{
49-
if ($input->getOption('format') !== null) {
49+
if ($input->getOption('format') !== null && is_string($input->getOption('format'))) {
5050
return ConfigFormat::from($input->getOption('format'));
5151
}
5252

‎src/Presentation/Console/Command/LintCommand.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
6767

6868
$result = $this->handler->handle(new LintTask(
6969
$this->getWorkDir($input),
70-
$input->getOption('config'),
70+
$this->getStringOptionFromInput($input, 'config'),
7171
));
7272

7373
$style->newLine(2);
@@ -76,7 +76,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
7676
$this->notePrinter->print(new SymfonyTablePrinter($style), $result->notes);
7777
}
7878

79-
$this->printMetrics($style, $result, $input->getOption('metrics'));
79+
$this->printMetrics($style, $result, $this->getBoolFromOption($input, 'metrics'));
8080

8181
if ($result->isFail()) {
8282
$style->error(sprintf('Found %d notes', $result->notes->count()));

‎src/Presentation/Console/Interaction/WorkDirResolver.php

+11-3
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,16 @@ public function resolve(InputInterface $input): string
2727

2828
private function getFromInput(InputInterface $input): ?string
2929
{
30-
return $input->hasOption(self::OPTION_NAME) ?
31-
$input->getOption(self::OPTION_NAME) :
32-
null;
30+
$option = $input->getOption(self::OPTION_NAME);
31+
32+
if (empty($option)) {
33+
return null;
34+
}
35+
36+
if (! is_string($option)) {
37+
throw new \RuntimeException(sprintf('Option "%s" must be string', self::OPTION_NAME));
38+
}
39+
40+
return $option;
3341
}
3442
}

‎src/Presentation/Console/Output/ConsoleLogger.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public function __construct(
2020

2121
public function log($level, \Stringable|string $message, array $context = []): void
2222
{
23-
if ($this->willBeLogged($level)) {
23+
if (is_string($level) && $this->willBeLogged($level)) {
2424
$this->output->write("\n");
2525
$this->logger->log($level, $message, $context);
2626
}

‎src/Shared/DataStructure/Arrayee.php

+3-2
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,9 @@ public function implode(string $sep): string
6868
}
6969

7070
/**
71-
* @param callable(V): mixed $mapper
72-
* @return array<mixed>
71+
* @template M
72+
* @param callable(V): M $mapper
73+
* @return array<M>
7374
*/
7475
public function mapToArray(callable $mapper): array
7576
{

‎src/Shared/File/Directory.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ public function __construct(
1212

1313
public function pathTo(string $filename): string
1414
{
15-
return $this->directory . DIRECTORY_SEPARATOR . $filename;
15+
return rtrim($this->directory, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR . $filename;
1616
}
1717

1818
public function __toString(): string

‎src/Version.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99
final class Version
1010
{
11-
public const VERSION = '0.8.0';
11+
public const VERSION = '0.8.1';
1212

1313
private function __construct()
1414
{

‎tests/Unit/Application/Rule/Rules/UpdatedPhpConstantCheckerTest.php renamed to ‎tests/Unit/Application/Rule/Rules/HasChangesRule/UpdatedPhpConstantCheckerTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22

3-
namespace ArtARTs36\MergeRequestLinter\Tests\Unit\Application\Rule\Rules;
3+
namespace ArtARTs36\MergeRequestLinter\Tests\Unit\Application\Rule\Rules\HasChangesRule;
44

55
use ArtARTs36\MergeRequestLinter\Application\Rule\Rules\HasChangesRule\NeedFileChange;
66
use ArtARTs36\MergeRequestLinter\Application\Rule\Rules\HasChangesRule\UpdatedPhpConstantChecker;

‎tests/Unit/Infrastructure/RequestFetcher/MemoryRequestFetcherTest.php

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
namespace ArtARTs36\MergeRequestLinter\Tests\Unit\Infrastructure\RequestFetcher;
4+
5+
use ArtARTs36\MergeRequestLinter\Infrastructure\RequestFetcher\MemoryRequestFetcher;
6+
use ArtARTs36\MergeRequestLinter\Tests\TestCase;
7+
8+
final class MemoryRequestFetcherTest extends TestCase
9+
{
10+
/**
11+
* @covers \ArtARTs36\MergeRequestLinter\Infrastructure\RequestFetcher\MemoryRequestFetcher::fetch
12+
* @covers \ArtARTs36\MergeRequestLinter\Infrastructure\RequestFetcher\MemoryRequestFetcher::__construct
13+
*/
14+
public function testFetch(): void
15+
{
16+
$fetcher = new MemoryRequestFetcher($request = $this->makeMergeRequest());
17+
18+
self::assertEquals($request, $fetcher->fetch());
19+
}
20+
21+
/**
22+
* @covers \ArtARTs36\MergeRequestLinter\Infrastructure\RequestFetcher\MemoryRequestFetcher::useRequest
23+
* @covers \ArtARTs36\MergeRequestLinter\Infrastructure\RequestFetcher\MemoryRequestFetcher::fetch
24+
* @covers \ArtARTs36\MergeRequestLinter\Infrastructure\RequestFetcher\MemoryRequestFetcher::__construct
25+
*/
26+
public function testUseRequest(): void
27+
{
28+
$fetcher = new MemoryRequestFetcher($this->makeMergeRequest());
29+
30+
$fetcher->useRequest($request2 = $this->makeMergeRequest());
31+
32+
self::assertEquals($request2, $fetcher->fetch());
33+
}
34+
}

‎tests/Unit/Infrastructure/Text/Decoder/NativeJsonDecoderTest.php

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
namespace ArtARTs36\MergeRequestLinter\Tests\Unit\Infrastructure\Text\Decoder;
4+
5+
use ArtARTs36\MergeRequestLinter\Infrastructure\Text\Decoder\NativeJsonDecoder;
6+
use ArtARTs36\MergeRequestLinter\Tests\TestCase;
7+
8+
final class NativeJsonDecoderTest extends TestCase
9+
{
10+
public function providerForTestDecode(): array
11+
{
12+
return [
13+
[
14+
'{"key": "value"}',
15+
['key' => 'value'],
16+
]
17+
];
18+
}
19+
20+
/**
21+
* @covers \ArtARTs36\MergeRequestLinter\Infrastructure\Text\Decoder\NativeJsonDecoder::decode
22+
* @dataProvider providerForTestDecode
23+
*/
24+
public function testDecode(string $json, array $expected): void
25+
{
26+
$decoder = new NativeJsonDecoder();
27+
28+
self::assertEquals($expected, $decoder->decode($json));
29+
}
30+
31+
/**
32+
* @covers \ArtARTs36\MergeRequestLinter\Infrastructure\Text\Decoder\NativeJsonDecoder::decode
33+
*/
34+
public function testDecodeOnJsonError(): void
35+
{
36+
self::expectExceptionMessage('json_decode error:');
37+
38+
$decoder = new NativeJsonDecoder();
39+
40+
$decoder->decode('');
41+
}
42+
}

‎tests/Unit/Infrastructure/Text/Decoder/SymfonyYamlDecoderTest.php

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php
2+
3+
namespace ArtARTs36\MergeRequestLinter\Tests\Unit\Infrastructure\Text\Decoder;
4+
5+
use ArtARTs36\MergeRequestLinter\Infrastructure\Text\Decoder\SymfonyYamlDecoder;
6+
use ArtARTs36\MergeRequestLinter\Tests\TestCase;
7+
8+
final class SymfonyYamlDecoderTest extends TestCase
9+
{
10+
/**
11+
* @covers \ArtARTs36\MergeRequestLinter\Infrastructure\Text\Decoder\SymfonyYamlDecoder::decode
12+
*/
13+
public function testDecode(): void
14+
{
15+
$decoder = new SymfonyYamlDecoder();
16+
17+
self::assertEquals([1, 2, 3], $decoder->decode('
18+
- 1
19+
- 2
20+
- 3
21+
'));
22+
}
23+
24+
/**
25+
* @covers \ArtARTs36\MergeRequestLinter\Infrastructure\Text\Decoder\SymfonyYamlDecoder::decode
26+
*/
27+
public function testDecodeOnInvalidYamlString(): void
28+
{
29+
self::expectExceptionMessage('Invalid yaml string');
30+
31+
$decoder = new SymfonyYamlDecoder();
32+
33+
$decoder->decode('');
34+
}
35+
36+
public function providerForTestDecodeOnParseException(): array
37+
{
38+
return [
39+
[
40+
'- e
41+
a',
42+
'Unable to parse at line 2 (near "a").',
43+
]
44+
];
45+
}
46+
47+
/**
48+
* @covers \ArtARTs36\MergeRequestLinter\Infrastructure\Text\Decoder\SymfonyYamlDecoder::decode
49+
* @dataProvider providerForTestDecodeOnParseException
50+
*/
51+
public function testDecodeOnParseException(string $yaml, string $expectedExceptionMessage): void
52+
{
53+
self::expectExceptionMessage($expectedExceptionMessage);
54+
55+
$decoder = new SymfonyYamlDecoder();
56+
57+
$decoder->decode($yaml);
58+
}
59+
}

‎tests/Unit/Shared/File/DirectoryTest.php

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
namespace ArtARTs36\MergeRequestLinter\Tests\Unit\Shared\File;
4+
5+
use ArtARTs36\MergeRequestLinter\Shared\File\Directory;
6+
use ArtARTs36\MergeRequestLinter\Tests\TestCase;
7+
8+
final class DirectoryTest extends TestCase
9+
{
10+
public function providerForTestPathTo(): array
11+
{
12+
return [
13+
[
14+
'/var/web',
15+
'file.txt',
16+
'/var/web/file.txt',
17+
],
18+
[
19+
'/var/web/',
20+
'file.txt',
21+
'/var/web/file.txt',
22+
],
23+
];
24+
}
25+
26+
/**
27+
* @covers \ArtARTs36\MergeRequestLinter\Shared\File\Directory::__construct
28+
* @covers \ArtARTs36\MergeRequestLinter\Shared\File\Directory::pathTo
29+
* @dataProvider providerForTestPathTo
30+
*/
31+
public function testPathTo(string $dir, string $file, string $expected): void
32+
{
33+
$directory = new Directory($dir);
34+
35+
self::assertEquals($expected, $directory->pathTo($file));
36+
}
37+
38+
/**
39+
* @covers \ArtARTs36\MergeRequestLinter\Shared\File\Directory::__toString
40+
*/
41+
public function testToString(): void
42+
{
43+
$directory = new Directory('/var/web');
44+
45+
self::assertEquals('/var/web', (string) $directory);
46+
}
47+
}

0 commit comments

Comments
 (0)