Skip to content

Commit 97f194c

Browse files
chore: raise PHPStan to level 9
Level 5 left the whole public surface untyped: every getter returned a bare array, so nothing described the shape of a placemark or a style to an IDE, and no analysis could catch a caller reading a key that is never set. It also hid three real defects. loadFromFile() passed the result of file_get_contents() straight on. An unreadable file, a permissions problem rather than a missing one, made that false, which became the empty string and surfaced as a parse error about the content instead of about the file. It now throws KmlParserException::failedToRead() with the path. Every xpath() call was iterated without checking for the false it returns on a malformed expression, and preg_split() the same. Both now fall back to an empty array. The KMZ limits were read from config with a blind (int) cast, so a non-numeric value silently became 0, which turns the limit off. That is the opposite of what someone setting a limit wants, so a value that is not a number now falls back to the documented default. Fixed at the source: the baseline is still empty, and there are no phpstan-ignore comments, no inline @var overrides and no casts added to quiet the analyser.
1 parent 1f057b6 commit 97f194c

8 files changed

Lines changed: 207 additions & 43 deletions

File tree

phpstan.neon.dist

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ includes:
22
- phpstan-baseline.neon
33

44
parameters:
5-
level: 5
5+
level: 9
66
paths:
77
- src
88
- config

src/Exceptions/KmlParserException.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ public static function fileNotFound(string $path): self
99
return new self("KML file not found: {$path}");
1010
}
1111

12+
public static function failedToRead(string $path): self
13+
{
14+
return new self("Unable to read KML file: {$path}");
15+
}
16+
1217
public static function noDataLoaded(): self
1318
{
1419
return new self('No KML data loaded');

src/KmlParser.php

Lines changed: 141 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@
1010
use PlinCode\KmlParser\Validators\KmlValidator;
1111
use SimpleXMLElement;
1212

13+
/**
14+
* @phpstan-import-type Position from ParsesCoordinates
15+
* @phpstan-import-type PolygonBoundaries from ParsesCoordinates
16+
*
17+
* @phpstan-type Placemark array<string, mixed>
18+
* @phpstan-type Geometry array<string, mixed>
19+
* @phpstan-type Style array<string, mixed>
20+
*/
1321
class KmlParser
1422
{
1523
use ParsesCoordinates;
@@ -25,7 +33,12 @@ class KmlParser
2533

2634
public function __construct()
2735
{
28-
$this->namespace = $this->packageConfig('kml-parser.namespace', $this->namespace);
36+
$namespace = $this->packageConfig('kml-parser.namespace', $this->namespace);
37+
38+
if (is_string($namespace) && $namespace !== '') {
39+
$this->namespace = $namespace;
40+
}
41+
2942
$this->validator = new KmlValidator($this->supportedNamespaces());
3043
}
3144

@@ -38,8 +51,17 @@ public function __construct()
3851
protected function supportedNamespaces(): array
3952
{
4053
$supported = $this->packageConfig('kml-parser.supported_namespaces', KmlValidator::DEFAULT_NAMESPACES);
54+
$supported = is_array($supported) ? $supported : [];
55+
56+
$namespaces = [$this->namespace];
4157

42-
return array_values(array_unique(array_merge([$this->namespace], (array) $supported)));
58+
foreach ($supported as $namespace) {
59+
if (is_string($namespace) && $namespace !== '') {
60+
$namespaces[] = $namespace;
61+
}
62+
}
63+
64+
return array_values(array_unique($namespaces));
4365
}
4466

4567
/**
@@ -53,7 +75,19 @@ public function loadFromFile(string $path): self
5375
throw KmlParserException::fileNotFound($path);
5476
}
5577

56-
return $this->loadFromString(file_get_contents($path));
78+
/*
79+
* The warning file_get_contents() raises carries less than the
80+
* exception below, and an application turning warnings into
81+
* exceptions would otherwise get that one instead of ours. The return
82+
* value is what is acted on.
83+
*/
84+
$content = @file_get_contents($path);
85+
86+
if ($content === false) {
87+
throw KmlParserException::failedToRead($path);
88+
}
89+
90+
return $this->loadFromString($content);
5791
}
5892

5993
/**
@@ -99,6 +133,8 @@ public function loadFromKmz(string $path): self
99133
/**
100134
* Get Placemarks Node from the KML
101135
*
136+
* @return list<Placemark>
137+
*
102138
* @throws Exception
103139
*/
104140
public function getPlacemarks(): array
@@ -108,7 +144,7 @@ public function getPlacemarks(): array
108144
}
109145

110146
$placemarks = [];
111-
$placemarksXml = $this->xml->xpath('//kml:Placemark');
147+
$placemarksXml = $this->xml->xpath('//kml:Placemark') ?: [];
112148

113149
foreach ($placemarksXml as $placemarkXml) {
114150
$placemark = [
@@ -225,6 +261,8 @@ protected function parseMultiGeometry(SimpleXMLElement $multiGeometry): array
225261
/**
226262
* Get Style Node from the KML
227263
*
264+
* @return array<string, Style>
265+
*
228266
* @throws Exception
229267
*/
230268
public function getStyles(): array
@@ -234,7 +272,7 @@ public function getStyles(): array
234272
}
235273

236274
$styles = [];
237-
$stylesXml = $this->xml->xpath('//kml:Style');
275+
$stylesXml = $this->xml->xpath('//kml:Style') ?: [];
238276

239277
foreach ($stylesXml as $styleXml) {
240278
$id = (string) $styleXml->attributes()->id;
@@ -376,6 +414,8 @@ protected function parsePolyStyle(SimpleXMLElement $polyStyle): array
376414
/**
377415
* Get StyleMap Node from the KML
378416
*
417+
* @return array<string, array{id: string, pairs: array<string, string>}>
418+
*
379419
* @throws Exception
380420
*/
381421
public function getStyleMaps(): array
@@ -385,7 +425,7 @@ public function getStyleMaps(): array
385425
}
386426

387427
$styleMaps = [];
388-
$styleMapsXml = $this->xml->xpath('//kml:StyleMap');
428+
$styleMapsXml = $this->xml->xpath('//kml:StyleMap') ?: [];
389429

390430
foreach ($styleMapsXml as $styleMapXml) {
391431
$id = (string) $styleMapXml->attributes()->id;
@@ -410,6 +450,8 @@ public function getStyleMaps(): array
410450
/**
411451
* Convert data to GeoJSON format
412452
*
453+
* @return array{type: string, features: list<array<string, mixed>>}
454+
*
413455
* @throws Exception
414456
*/
415457
public function toGeoJson(): array
@@ -457,67 +499,129 @@ public function toGeoJson(): array
457499
* A KML MultiGeometry maps onto a GeoJSON GeometryCollection, which nests
458500
* the same way, so this recurses alongside parseMultiGeometry().
459501
*
460-
* @param array<string, mixed> $geometry
502+
* @param array<mixed> $geometry
461503
* @return array<string, mixed>|null
462504
*/
463505
protected function toGeoJsonGeometry(array $geometry): ?array
464506
{
465-
return match ($geometry['type'] ?? null) {
507+
$type = $geometry['type'] ?? null;
508+
509+
if ($type === GeometryType::MULTI_GEOMETRY->value) {
510+
return [
511+
'type' => 'GeometryCollection',
512+
'geometries' => $this->toGeoJsonGeometries($geometry['geometries'] ?? []),
513+
];
514+
}
515+
516+
$coordinates = $geometry['coordinates'] ?? null;
517+
518+
if (! is_array($coordinates)) {
519+
return null;
520+
}
521+
522+
return match ($type) {
466523
GeometryType::POINT->value => [
467524
'type' => 'Point',
468-
'coordinates' => $this->toGeoJsonPosition($geometry['coordinates']),
525+
'coordinates' => $this->toGeoJsonPosition($coordinates),
469526
],
470527
GeometryType::LINE_STRING->value => [
471528
'type' => 'LineString',
472-
'coordinates' => array_map(
473-
fn (array $position) => $this->toGeoJsonPosition($position),
474-
$geometry['coordinates'],
475-
),
529+
'coordinates' => $this->toGeoJsonPositions($coordinates),
476530
],
477531
GeometryType::POLYGON->value => [
478532
'type' => 'Polygon',
479-
'coordinates' => $this->toGeoJsonRings($geometry['coordinates']),
480-
],
481-
GeometryType::MULTI_GEOMETRY->value => [
482-
'type' => 'GeometryCollection',
483-
'geometries' => array_values(array_filter(array_map(
484-
fn (array $child) => $this->toGeoJsonGeometry($child),
485-
$geometry['geometries'],
486-
))),
533+
'coordinates' => $this->toGeoJsonRings($coordinates),
487534
],
488535
default => null,
489536
};
490537
}
491538

492539
/**
493-
* @param array{longitude: float, latitude: float, altitude: float} $position
494-
* @return array<int, float>
540+
* @return list<array<string, mixed>>
541+
*/
542+
protected function toGeoJsonGeometries(mixed $geometries): array
543+
{
544+
if (! is_array($geometries)) {
545+
return [];
546+
}
547+
548+
$converted = [];
549+
550+
foreach ($geometries as $child) {
551+
if (! is_array($child)) {
552+
continue;
553+
}
554+
555+
$geometry = $this->toGeoJsonGeometry($child);
556+
557+
if ($geometry !== null) {
558+
$converted[] = $geometry;
559+
}
560+
}
561+
562+
return $converted;
563+
}
564+
565+
/**
566+
* @param array<mixed> $positions
567+
* @return list<list<float>>
568+
*/
569+
protected function toGeoJsonPositions(array $positions): array
570+
{
571+
$converted = [];
572+
573+
foreach ($positions as $position) {
574+
if (is_array($position)) {
575+
$converted[] = $this->toGeoJsonPosition($position);
576+
}
577+
}
578+
579+
return $converted;
580+
}
581+
582+
/**
583+
* A coordinate map as parsePointCoordinates() produces it, turned into the
584+
* GeoJSON position order. Anything not numeric reads as 0.0 rather than
585+
* throwing, so one malformed coordinate cannot take a whole document down.
586+
*
587+
* @param array<mixed> $position
588+
* @return list<float>
495589
*/
496590
protected function toGeoJsonPosition(array $position): array
497591
{
498-
return [$position['longitude'], $position['latitude'], $position['altitude']];
592+
return [
593+
$this->toFloat($position['longitude'] ?? null),
594+
$this->toFloat($position['latitude'] ?? null),
595+
$this->toFloat($position['altitude'] ?? null),
596+
];
597+
}
598+
599+
protected function toFloat(mixed $value): float
600+
{
601+
return is_numeric($value) ? (float) $value : 0.0;
499602
}
500603

501604
/**
502605
* GeoJSON puts the outer ring first and every inner ring after it.
503606
*
504-
* @param array{outerBoundary: array<int, array<string, float>>, innerBoundaries: array<int, array<int, array<string, float>>>} $boundaries
505-
* @return array<int, array<int, array<int, float>>>
607+
* @param array<mixed> $boundaries
608+
* @return list<list<list<float>>>
506609
*/
507610
protected function toGeoJsonRings(array $boundaries): array
508611
{
509-
$rings = [
510-
array_map(
511-
fn (array $position) => $this->toGeoJsonPosition($position),
512-
$boundaries['outerBoundary'],
513-
),
514-
];
612+
$outer = $boundaries['outerBoundary'] ?? [];
613+
$inner = $boundaries['innerBoundaries'] ?? [];
614+
615+
$rings = [is_array($outer) ? $this->toGeoJsonPositions($outer) : []];
515616

516-
foreach ($boundaries['innerBoundaries'] as $innerBoundary) {
517-
$rings[] = array_map(
518-
fn (array $position) => $this->toGeoJsonPosition($position),
519-
$innerBoundary,
520-
);
617+
if (! is_array($inner)) {
618+
return $rings;
619+
}
620+
621+
foreach ($inner as $innerBoundary) {
622+
if (is_array($innerBoundary)) {
623+
$rings[] = $this->toGeoJsonPositions($innerBoundary);
624+
}
521625
}
522626

523627
return $rings;

src/KmzExtractor.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,8 @@ protected function open(string $path): ZipArchive
128128
*/
129129
protected function guardArchive(ZipArchive $zip): void
130130
{
131-
$maxEntries = (int) $this->packageConfig('kml-parser.max_archive_entries', self::DEFAULT_MAX_ENTRIES);
132-
$maxSize = (int) $this->packageConfig('kml-parser.max_uncompressed_size', self::DEFAULT_MAX_UNCOMPRESSED_SIZE);
131+
$maxEntries = $this->configuredLimit('kml-parser.max_archive_entries', self::DEFAULT_MAX_ENTRIES);
132+
$maxSize = $this->configuredLimit('kml-parser.max_uncompressed_size', self::DEFAULT_MAX_UNCOMPRESSED_SIZE);
133133

134134
if ($maxEntries > 0 && $zip->numFiles > $maxEntries) {
135135
throw KmzExtractorException::tooManyEntries($zip->numFiles, $maxEntries);
@@ -154,6 +154,18 @@ protected function guardArchive(ZipArchive $zip): void
154154
}
155155
}
156156

157+
/**
158+
* A limit that is not a number is a misconfiguration, and silently reading
159+
* it as 0 would turn the limit off, which is the opposite of what someone
160+
* setting it wants. The documented default is used instead.
161+
*/
162+
protected function configuredLimit(string $key, int $default): int
163+
{
164+
$value = $this->packageConfig($key, $default);
165+
166+
return is_numeric($value) ? (int) $value : $default;
167+
}
168+
157169
/**
158170
* @throws KmzExtractorException
159171
*/

src/Traits/ParsesCoordinates.php

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@
44

55
use SimpleXMLElement;
66

7+
/**
8+
* @phpstan-type Position array{longitude: float, latitude: float, altitude: float}
9+
* @phpstan-type PolygonBoundaries array{outerBoundary: list<Position>, innerBoundaries: list<list<Position>>}
10+
*/
711
trait ParsesCoordinates
812
{
913
/**
10-
* @return array{longitude: float, latitude: float, altitude: float}
14+
* @return Position
1115
*/
1216
protected function parsePointCoordinates(string $coordinates): array
1317
{
@@ -20,10 +24,13 @@ protected function parsePointCoordinates(string $coordinates): array
2024
];
2125
}
2226

27+
/**
28+
* @return list<Position>
29+
*/
2330
protected function parseLineStringCoordinates(string $coordinates): array
2431
{
2532
$coords = [];
26-
$points = preg_split('/\s+/', trim($coordinates));
33+
$points = preg_split('/\s+/', trim($coordinates)) ?: [];
2734

2835
foreach ($points as $point) {
2936
if (empty(trim($point))) {
@@ -43,6 +50,9 @@ protected function parseLineStringCoordinates(string $coordinates): array
4350
return $coords;
4451
}
4552

53+
/**
54+
* @return PolygonBoundaries
55+
*/
4656
protected function parsePolygonCoordinates(SimpleXMLElement $polygon): array
4757
{
4858
$result = [

src/Validators/KmlValidator.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ protected function validateGeometryCoordinates(SimpleXMLElement $geometry, strin
167167
throw new KmlException('Empty coordinates in geometry');
168168
}
169169

170-
$coords = preg_split('/\s+/', trim($coordinates));
170+
$coords = preg_split('/\s+/', trim($coordinates)) ?: [];
171171
foreach ($coords as $coord) {
172172
if (empty(trim($coord))) {
173173
continue;

0 commit comments

Comments
 (0)