From 4d7cad530a6c70f958092bd341160304f40e1b08 Mon Sep 17 00:00:00 2001 From: Corey Powell Date: Mon, 23 Sep 2024 09:14:45 -0500 Subject: [PATCH 1/3] 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) --- lib/geo/json/decoder.ex | 2 +- lib/geo/utils.ex | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/geo/json/decoder.ex b/lib/geo/json/decoder.ex index 8deba5e..ef9135d 100644 --- a/lib/geo/json/decoder.ex +++ b/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)}" diff --git a/lib/geo/utils.ex b/lib/geo/utils.ex index d769ab9..6e301d1 100644 --- a/lib/geo/utils.ex +++ b/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() + 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. From b75e80bcf7e9508cb32da2d27e1c258d4867bfd0 Mon Sep 17 00:00:00 2001 From: Corey Powell Date: Mon, 23 Sep 2024 10:52:01 -0500 Subject: [PATCH 2/3] Use no_return type to denote function with possible non-return --- lib/geo/utils.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/geo/utils.ex b/lib/geo/utils.ex index 6e301d1..ec062bf 100644 --- a/lib/geo/utils.ex +++ b/lib/geo/utils.ex @@ -15,7 +15,7 @@ defmodule Geo.Utils do end end - @spec string_to_float(String.t()) :: float() + @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} -> From 2befd05cafc211d474a9e9c1842b0fb567e3a8ce Mon Sep 17 00:00:00 2001 From: Corey Powell Date: Mon, 23 Sep 2024 10:56:01 -0500 Subject: [PATCH 3/3] 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 --- test/geo/json_test.exs | 96 ++++++++++++++++++++++++++++++++++------- test/geo/utils_test.exs | 17 ++++++++ 2 files changed, 97 insertions(+), 16 deletions(-) diff --git a/test/geo/json_test.exs b/test/geo/json_test.exs index 90726ff..3f73064 100644 --- a/test/geo/json_test.exs +++ b/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 diff --git a/test/geo/utils_test.exs b/test/geo/utils_test.exs index d1c14c8..77a7234 100644 --- a/test/geo/utils_test.exs +++ b/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)