Use a custom string_to_float/1 function instead of plain String.to_fl…

…oat for ensure_numeric (#221)

* Use a custom string_to_float/1 function instead of plain String.to_float

The latter doesn't work when the textual representation is an integer, Float.parse/1 however works with it but might have its own side effects that haven't been tested for (likely support for things like underscores and other elixir-esque formatting)

* Use no_return type to denote function with possible non-return

* Added some tests for string_to_integer

And a decode test to ensure it works as intended with integers

EDIT: You know I should read properly next time
EDIT.2: Made the formatter happy

lib/geo/json/decoder.ex

@@ -262,7 +262,7 @@ defmodule Geo.JSON.Decoder do
 
       str when is_binary(str) ->
         try do
-          String.to_float(str)
+          Geo.Utils.string_to_float!(str)
         catch
           ArgumentError ->
             raise ArgumentError, "expected a numeric coordinate, got the string #{inspect(str)}"

lib/geo/utils.ex

@@ -1,6 +1,31 @@
 defmodule Geo.Utils do
   @moduledoc false
 
+  @spec string_to_float(String.t()) :: {:ok, float()} | {:error, term}
+  def string_to_float(str) when is_binary(str) do
+    case Float.parse(str) do
+      :error ->
+        {:error, :bad_arg}
+
+      {flt, ""} ->
+        {:ok, flt}
+
+      {_flt, _rest} ->
+        {:error, :bad_arg}
+    end
+  end
+
+  @spec string_to_float!(String.t()) :: float() | no_return()
+  def string_to_float!(str) when is_binary(str) do
+    case string_to_float(str) do
+      {:ok, flt} ->
+        flt
+
+      {:error, :bad_arg} ->
+        raise ArgumentError, "given string is not a textual representation of a float"
+    end
+  end
+
   @doc """
   Turns a hex string or an integer of base 16 into its floating point
   representation.

test/geo/json_test.exs

@@ -57,6 +57,45 @@ defmodule Geo.JSON.Test do
     assert_geojson_equal(exjson, new_exjson)
   end
 
+  test "GeoJson to Point (with integer components) and back" do
+    json = """
+      {
+        "type": "Point",
+        "coordinates": [100, 0]
+      }
+    """
+
+    exjson = Jason.decode!(json)
+    geom = Jason.decode!(json) |> Geo.JSON.decode!()
+
+    assert(geom.coordinates == {100.0, 0.0})
+
+    new_exjson = Geo.JSON.encode!(geom)
+    assert_geojson_equal(exjson, new_exjson)
+  end
+
+  test "GeoJson to Point (with string:integer components) and back" do
+    json = """
+      {
+        "type": "Point",
+        "coordinates": ["100", "0"]
+      }
+    """
+
+    exjson =
+      %{
+        "type" => "Point",
+        "coordinates" => [100.0, 0.0]
+      }
+
+    geom = Jason.decode!(json) |> Geo.JSON.decode!()
+
+    assert(geom.coordinates == {100.0, 0.0})
+
+    new_exjson = Geo.JSON.encode!(geom)
+    assert_geojson_equal(exjson, new_exjson)
+  end
+
   test "GeoJson Point without coordinates" do
     json = "{ \"type\": \"Point\", \"coordinates\": [] }"
     exjson = Jason.decode!(json)
@@ -387,23 +426,48 @@ defmodule Geo.JSON.Test do
     assert geom.geometries == []
   end
 
-  test "Decode seamlessly converts coordinates that are numbers-as-strings" do
-    check all(
-            x <- float(),
-            y <- float()
-          ) do
-      json = """
-        {
-          "properties": {},
-          "geometry": {
-            "type": "Point",
-            "coordinates": ["#{x}", "#{y}"]
-          },
-          "type": "Feature"
-        }
-      """
+  describe "decode seamlessly converts coordinates that are numbers-as-strings" do
+    test "works with floats" do
+      check all(
+              x <- float(),
+              y <- float()
+            ) do
+        json = """
+          {
+            "properties": {},
+            "geometry": {
+              "type": "Point",
+              "coordinates": ["#{x}", "#{y}"]
+            },
+            "type": "Feature"
+          }
+        """
+
+        assert %Geo.Point{coordinates: {^x, ^y}} = Jason.decode!(json) |> Geo.JSON.decode!()
+      end
+    end
 
-      assert %Geo.Point{coordinates: {^x, ^y}} = Jason.decode!(json) |> Geo.JSON.decode!()
+    test "works with integers" do
+      check all(
+              x <- integer(),
+              y <- integer()
+            ) do
+        json = """
+          {
+            "properties": {},
+            "geometry": {
+              "type": "Point",
+              "coordinates": ["#{x}", "#{y}"]
+            },
+            "type": "Feature"
+          }
+        """
+
+        # float coercion
+        fx = 0.0 + x
+        fy = 0.0 + y
+        assert %Geo.Point{coordinates: {^fx, ^fy}} = Jason.decode!(json) |> Geo.JSON.decode!()
+      end
     end
   end
 

test/geo/utils_test.exs

@@ -2,6 +2,23 @@ defmodule Geo.Utils.Test do
   use ExUnit.Case, async: true
   use ExUnitProperties
 
+  describe "string_to_float/1" do
+    test "can convert a textual float" do
+      assert {:ok, +0.0} == Geo.Utils.string_to_float("0.0")
+      assert {:ok, 12.34} == Geo.Utils.string_to_float("12.34")
+    end
+
+    test "can convert a textual integer" do
+      assert {:ok, +0.0} == Geo.Utils.string_to_float("0")
+      assert {:ok, 12.0} == Geo.Utils.string_to_float("12")
+    end
+
+    test "can handle badly formatted float" do
+      assert {:error, :bad_arg} == Geo.Utils.string_to_float("0.x")
+      assert {:error, :bad_arg} == Geo.Utils.string_to_float("11f")
+    end
+  end
+
   test "Hex String to Float Conversion" do
     assert(Geo.Utils.hex_to_float("40000000") == 2.0)
     assert(Geo.Utils.hex_to_float("C0000000") == -2.0)