75-critical-unexpected-conversion-when-storing-$x-and-$y (#80)

* New interfaces: geodetic and cartesian interfaces.
* Splitting geodetic and cartesian coordinates
* New internal exception: RangeException
* Fix #79
* Fix #76 
* Fix #77 
* Update full.yaml : CI updated to avoid double checks on pushes and pull-requests
* Update paambaati/codeclimate-action version

.github/workflows/full.yaml

@@ -1,8 +1,21 @@
 name: Full tests
 on:
+  schedule:
+    - cron: '0 17 * * 4'
   push:
+    branches:
+      - main
+      - dev
+    paths-ignore:
+      - '**.md'
+      - 'LICENSE'
   pull_request:
-    branches: [ dev, main ]
+    branches:
+      - main
+      - dev
+    paths-ignore:
+      - '**.md'
+      - 'LICENSE'
 
 permissions:
   contents: read
@@ -90,7 +103,7 @@ jobs:
 
       - name: Run test suite and with coverage for codeclimate for PHP 8.3 and doctrine/orm ^3.1 only
         if: ${{ env.HAS_CC_SECRET == 'true' }}
-        uses: paambaati/codeclimate-action@v5.0.0
+        uses: paambaati/codeclimate-action@v6.0.0
         env:
            CC_TEST_REPORTER_ID: ${{secrets.CC_TEST_REPORTER_ID}}
         with:

composer.json

@@ -26,7 +26,7 @@
         "php": "^8.1",
         "ext-json": "*",
         "ext-mbstring": "*",
-        "longitude-one/geo-parser": "^3.0.0",
+        "longitude-one/geo-parser": "^3.0.1",
         "longitude-one/wkt-parser": "^3.0.0",
         "longitude-one/wkb-parser": "^3.0.0",
         "doctrine/orm": "^2.19|^3.1"

lib/LongitudeOne/Spatial/Exception/InvalidValueException.php

@@ -25,4 +25,8 @@
  */
 class InvalidValueException extends \Exception implements ExceptionInterface
 {
+    public const OUT_OF_RANGE_LATITUDE = 'Out of range latitude value, latitude must be between -90 and 90, got "%s".';
+    public const OUT_OF_RANGE_LONGITUDE = 'Out of range longitude value, longitude must be between -180 and 180, got "%s".';
+    public const OUT_OF_RANGE_MINUTE = 'Out of range minute value, minute must be between 0 and 59, got "%s".';
+    public const OUT_OF_RANGE_SECOND = 'Out of range second value, second must be between 0 and 59, got "%s".';
 }

lib/LongitudeOne/Spatial/Exception/RangeException.php

@@ -0,0 +1,30 @@
+<?php
+/**
+ * This file is part of the doctrine spatial extension.
+ *
+ * PHP          8.1 | 8.2 | 8.3
+ * Doctrine ORM 2.19 | 3.1
+ *
+ * Copyright Alexandre Tranchant <[email protected]> 2017-2024
+ * Copyright Longitude One 2020-2024
+ * Copyright 2015 Derek J. Lambert
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ *
+ */
+
+declare(strict_types=1);
+
+namespace LongitudeOne\Spatial\Exception;
+
+/**
+ * Range Exception class.
+ *
+ * This exception is thrown when a geodesic coordinate is out of range.
+ *
+ * @internal the library uses this exception internally and is always caught to throw a more explicit InvalidValueException
+ */
+final class RangeException extends \Exception implements ExceptionInterface
+{
+}

lib/LongitudeOne/Spatial/PHP/Types/AbstractPoint.php

@@ -18,18 +18,19 @@
 
 namespace LongitudeOne\Spatial\PHP\Types;
 
-use LongitudeOne\Geo\String\Exception\RangeException;
+use LongitudeOne\Geo\String\Exception\RangeException as GeoParserRangeException;
 use LongitudeOne\Geo\String\Exception\UnexpectedValueException;
 use LongitudeOne\Geo\String\Parser;
 use LongitudeOne\Spatial\Exception\InvalidValueException;
+use LongitudeOne\Spatial\Exception\RangeException;
 
 /**
  * Abstract point object for POINT spatial types.
  *
  * @see https://stackoverflow.com/questions/7309121/preferred-order-of-writing-latitude-longitude-tuples
  * @see https://docs.geotools.org/latest/userguide/library/referencing/order.html
  */
-abstract class AbstractPoint extends AbstractGeometry
+abstract class AbstractPoint extends AbstractGeometry implements PointInterface
 {
     /**
      * The X coordinate or the longitude.
@@ -96,79 +97,67 @@ public function getY(): float|int
     /**
      * Latitude fluent setter.
      *
-     * @param string $latitude the new latitude of point
+     * @param float|int|string $latitude the new latitude of point
      *
      * @throws InvalidValueException when latitude is not valid
      */
-    public function setLatitude(string $latitude): self
+    public function setLatitude(float|int|string $latitude): static
     {
-        return $this->setY($latitude);
+        try {
+            $geodesicCoordinate = $this->setGeodesicCoordinate($latitude, -90, 90);
+        } catch (RangeException $e) {
+            throw new InvalidValueException(sprintf(InvalidValueException::OUT_OF_RANGE_LATITUDE, $latitude), $e->getCode(), $e);
+        }
+
+        $this->y = $geodesicCoordinate;
+
+        return $this;
     }
 
     /**
      * Longitude setter.
      *
-     * @param string $longitude the new longitude
+     * @param float|int|string $longitude the new longitude
      *
      * @throws InvalidValueException when longitude is not valid
      */
-    public function setLongitude(string $longitude): self
+    public function setLongitude(float|int|string $longitude): static
     {
-        return $this->setX($longitude);
+        try {
+            $geodesicCoordinate = $this->setGeodesicCoordinate($longitude, -180, 180);
+        } catch (RangeException $e) {
+            throw new InvalidValueException(sprintf(InvalidValueException::OUT_OF_RANGE_LONGITUDE, $longitude), $e->getCode(), $e);
+        }
+
+        $this->x = $geodesicCoordinate;
+
+        return $this;
     }
 
     /**
      * X setter. (Latitude setter).
      *
-     * @param string $x the new X
+     * @param float|int|string $x the new X
      *
      * @throws InvalidValueException when x is not valid
      */
-    public function setX(string $x): self
+    public function setX(float|int|string $x): static
     {
-        $parser = new Parser($x);
-
-        try {
-            $x = $parser->parse();
-        } catch (RangeException $e) {
-            throw new InvalidValueException($e->getMessage(), $e->getCode(), $e);
-        } catch (UnexpectedValueException $e) {
-            throw new InvalidValueException(sprintf('Invalid coordinate value, got "%s".', $x), $e->getCode(), $e);
-        }
-
-        if (is_array($x)) {
-            throw new InvalidValueException('Invalid coordinate value, coordinate cannot be an array.');
-        }
-
-        $this->x = $x;
+        $this->x = $this->setCartesianCoordinate($x);
 
         return $this;
     }
 
     /**
      * Y setter. Longitude Setter.
      *
-     * @param string $y the new Y value
+     * @param float|int|string $y the new Y value
      *
      * @throws InvalidValueException when Y is invalid, not in valid range
      */
-    public function setY(string $y): self
+    public function setY(float|int|string $y): static
     {
-        $parser = new Parser($y);
-
-        try {
-            $y = $parser->parse();
-        } catch (RangeException $e) {
-            throw new InvalidValueException($e->getMessage(), $e->getCode(), $e);
-        } catch (UnexpectedValueException $e) {
-            throw new InvalidValueException(sprintf('Invalid coordinate value, got "%s".', $y), $e->getCode(), $e);
-        }
-
-        if (is_array($y)) {
-            throw new InvalidValueException('Invalid coordinate value, coordinate cannot be an array.');
-        }
-
-        $this->y = $y;
+        $this->y = $this->setCartesianCoordinate($y);
 
         return $this;
     }
@@ -184,23 +173,6 @@ public function toArray(): array
         return [$this->x, $this->y];
     }
 
-    /**
-     * Abstract point internal constructor.
-     *
-     * @param string   $x    X, longitude
-     * @param string   $y    Y, latitude
-     * @param null|int $srid Spatial Reference System Identifier
-     *
-     * @throws InvalidValueException if x or y are invalid
-     */
-    protected function construct(string $x, string $y, ?int $srid = null): void
-    {
-        $this->setX($x)
-            ->setY($y)
-            ->setSrid($srid)
-        ;
-    }
-
     /**
      * Validate arguments.
      *
@@ -216,6 +188,11 @@ protected function validateArguments(array $argv, string $caller): array
         $argc = count($argv);
 
         if (1 == $argc && is_array($argv[0])) {
+            $count = count($argv[0]);
+            if ($count < 2 || $count > 3) {
+                throw $this->createException($argv[0], $caller, true);
+            }
+
             foreach ($argv[0] as $value) {
                 if (is_numeric($value) || is_string($value)) {
                     continue;
@@ -251,13 +228,34 @@ protected function validateArguments(array $argv, string $caller): array
         throw $this->createException($argv, $caller);
     }
 
+    /**
+     * Check the range of a coordinate.
+     *
+     * @param float|int $coordinate the coordinate to check
+     * @param int       $min        the minimum accepted value
+     * @param int       $max        the maximum accepted value
+     *
+     * @return float|int $coordinate or throw a RangeException
+     *
+     * @throws RangeException when coordinate is out of range fixed by min and max
+     */
+    private function checkRange(float|int $coordinate, int $min, int $max): float|int
+    {
+        if ($coordinate < $min || $coordinate > $max) {
+            throw new RangeException(sprintf('Coordinate must be comprised between %d and %d, got "%s".', $min, $max, $coordinate));
+        }
+
+        return $coordinate;
+    }
+
     /**
      * Create a fluent message for InvalidException.
      *
-     * @param mixed[] $argv   the arguments
-     * @param string  $caller the method calling the method calling exception :)
+     * @param mixed[] $argv     the arguments
+     * @param string  $caller   the method calling the method calling exception :)
+     * @param bool    $subArray when the first argument was a subarray converted into an array
      */
-    private function createException(array $argv, string $caller): InvalidValueException
+    private function createException(array $argv, string $caller, bool $subArray = false): InvalidValueException
     {
         array_walk($argv, function (&$value) {
             if (is_numeric($value) || is_string($value)) {
@@ -267,11 +265,107 @@ private function createException(array $argv, string $caller): InvalidValueExcep
             $value = gettype($value);
         });
 
+        $message = 'Invalid parameters passed to %s::%s: %s';
+        if ($subArray) {
+            $message = 'Invalid parameters passed to %s::%s: array(%s)';
+        }
+
         return new InvalidValueException(sprintf(
-            'Invalid parameters passed to %s::%s: %s',
-            $this::class,
+            $message,
+            static::class,
             $caller,
             implode(', ', $argv)
         ));
     }
+
+    /**
+     * Use the longitude-one/geo-parser to parse a coordinate.
+     *
+     * @param string $coordinate the coordinate to parse
+     *
+     * @throws InvalidValueException when coordinate is invalid
+     */
+    private function geoParse(string $coordinate): float|int
+    {
+        try {
+            $parser = new Parser($coordinate);
+
+            $parsedCoordinate = $parser->parse();
+        } catch (GeoParserRangeException $e) {
+            $message = match ($e->getCode()) {
+                GeoParserRangeException::LATITUDE_OUT_OF_RANGE => sprintf(InvalidValueException::OUT_OF_RANGE_LATITUDE, $coordinate),
+                GeoParserRangeException::LONGITUDE_OUT_OF_RANGE => sprintf(InvalidValueException::OUT_OF_RANGE_LONGITUDE, $coordinate),
+                GeoParserRangeException::MINUTES_OUT_OF_RANGE => sprintf(InvalidValueException::OUT_OF_RANGE_MINUTE, $coordinate),
+                GeoParserRangeException::SECONDS_OUT_OF_RANGE => sprintf(InvalidValueException::OUT_OF_RANGE_SECOND, $coordinate),
+                default => $e->getMessage(),
+            };
+
+            throw new InvalidValueException($message, $e->getCode(), $e);
+        } catch (UnexpectedValueException $e) {
+            throw new InvalidValueException(sprintf('Invalid coordinate value, got "%s".', $coordinate), $e->getCode(), $e);
+        }
+
+        if (is_array($parsedCoordinate)) {
+            throw new InvalidValueException('Invalid coordinate value, coordinate cannot be an array.');
+        }
+
+        return $parsedCoordinate;
+    }
+
+    /**
+     * Set a cartesian coordinate.
+     * Abscissa or ordinate.
+     *
+     * @param float|int|string $coordinate the coordinate to set
+     *
+     * @throws InvalidValueException when coordinate is invalid, RangeException is never thrown
+     */
+    private function setCartesianCoordinate(float|int|string $coordinate): float|int
+    {
+        if (is_integer($coordinate) || is_float($coordinate)) {
+            // We don't check the range of the value.
+            return $coordinate;
+        }
+
+        // $y is a string, let's use the geo-parser.
+        return $this->geoParse($coordinate);
+    }
+
+    /**
+     * Set a geodesic coordinate.
+     * Latitude or longitude.
+     *
+     * @param float|int|string $coordinate the coordinate to set
+     * @param int              $min        the minimum value
+     * @param int              $max        the maximum value
+     *
+     * @throws InvalidValueException|RangeException when coordinate is invalid or out of range
+     */
+    private function setGeodesicCoordinate(float|int|string $coordinate, int $min, int $max): float|int
+    {
+        if (is_integer($coordinate) || is_float($coordinate)) {
+            // We check the range of the value.
+            return $this->checkRange($coordinate, $min, $max);
+        }
+
+        // $y is a string, let's use the geo-parser.
+        $parsedCoordinate = $this->geoParse($coordinate);
+
+        if ($parsedCoordinate < $min || $parsedCoordinate > $max) {
+            throw new RangeException(sprintf('Coordinate must be comprised between %d and %d, got "%s".', $min, $max, $coordinate));
+        }
+
+        return $parsedCoordinate;
+    }
+
+    /**
+     * Abstract point internal constructor.
+     *
+     * @param string   $x    X, longitude
+     * @param string   $y    Y, latitude
+     * @param null|int $srid Spatial Reference System Identifier
+     *
+     * @throws InvalidValueException if x or y are invalid
+     */
+    abstract protected function construct(string $x, string $y, ?int $srid = null): void;
 }