From 751b8a5f1b3acf89727bca98aa782ac6e9798994 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Mon, 13 Oct 2025 10:02:15 -0500 Subject: [PATCH] improve dialyzer --- .dialyzer_ignore.exs | 11 ++++++++- .tool-versions | 2 +- lib/aprsme/is/is.ex | 6 +---- lib/aprsme/packet_consumer.ex | 16 +++++++++++++ lib/aprsme/packets.ex | 1 - lib/aprsme_web/aprs_symbol.ex | 24 +++++++++++++++++++ .../live/weather_live/callsign_view.ex | 22 ++++++++++------- lib/aprsme_web/plugs/rate_limiter.ex | 11 +++++++++ lib/aprsme_web/time_helpers.ex | 1 + lib/aprsme_web/time_utils.ex | 10 ++++++++ lib/aprsme_web/user_auth.ex | 8 +++++++ 11 files changed, 96 insertions(+), 16 deletions(-) diff --git a/.dialyzer_ignore.exs b/.dialyzer_ignore.exs index 1cb2ffe..bba37cb 100644 --- a/.dialyzer_ignore.exs +++ b/.dialyzer_ignore.exs @@ -13,5 +13,14 @@ # IP geolocation plug - these patterns are actually reachable but dialyzer can't prove it # The remote_ip can be nil or other values beyond just IPv4/IPv6 tuples - ~r/lib\/aprsme_web\/plugs\/ip_geolocation\.ex/ + ~r/lib\/aprsme_web\/plugs\/ip_geolocation\.ex/, + + # False positive: Aprs.parse/1 does return {:ok, _} but dialyzer can't see the vendored library types + {"lib/aprsme/is/is.ex"}, + + # False positive: The nil pattern is handled in the case statement above, dialyzer doesn't track this correctly + {"lib/aprsme/packets.ex"}, + + # False positive: PacketUtils.get_weather_field can return various types including nil + {"lib/aprsme_web/live/weather_live/callsign_view.ex"} ] diff --git a/.tool-versions b/.tool-versions index fd02f88..d6d5e2c 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,3 +1,3 @@ -elixir 1.19.0-rc.0-otp-27 +elixir 1.19.0-rc.2-otp-27 erlang 27.3.4 gleam 1.11.1 diff --git a/lib/aprsme/is/is.ex b/lib/aprsme/is/is.ex index 319fa9d..a6a8199 100644 --- a/lib/aprsme/is/is.ex +++ b/lib/aprsme/is/is.ex @@ -512,7 +512,7 @@ defmodule Aprsme.Is do attrs = Aprsme.Packet.extract_additional_data(attrs, message) # Normalize data_type to string if it's an atom - attrs = normalize_data_type(attrs) + attrs = Aprsme.EncodingUtils.normalize_data_type(attrs) # Submit to GenStage pipeline for batch processing Aprsme.PacketProducer.submit_packet(attrs) @@ -542,10 +542,6 @@ defmodule Aprsme.Is do end end - # Normalize data_type to ensure proper storage - @spec normalize_data_type(map()) :: map() - defp normalize_data_type(attrs), do: Aprsme.EncodingUtils.normalize_data_type(attrs) - @spec update_packet_stats(map(), integer()) :: map() defp update_packet_stats(stats, current_time) do new_total = stats.total_packets + 1 diff --git a/lib/aprsme/packet_consumer.ex b/lib/aprsme/packet_consumer.ex index 30aa932..c6c5548 100644 --- a/lib/aprsme/packet_consumer.ex +++ b/lib/aprsme/packet_consumer.ex @@ -11,6 +11,15 @@ defmodule Aprsme.PacketConsumer do require Logger + @type state :: %{ + batch: list(map()), + batch_size: integer(), + batch_timeout: integer(), + max_batch_size: integer(), + timer: reference() | nil + } + + @spec start_link(keyword()) :: GenServer.on_start() def start_link(opts \\ []) do # Allow unnamed consumers for pool usage name = opts[:name] @@ -102,6 +111,7 @@ defmodule Aprsme.PacketConsumer do end # Helper functions with pattern matching + @spec reset_batch_timer(state()) :: state() defp reset_batch_timer(%{timer: nil} = state) do new_timer = Process.send_after(self(), :process_batch, state.batch_timeout) %{state | batch: [], timer: new_timer} @@ -114,10 +124,12 @@ defmodule Aprsme.PacketConsumer do end # Efficient batch length calculation (cached if possible) + @spec batch_length(list()) :: non_neg_integer() defp batch_length([]), do: 0 defp batch_length(batch), do: length(batch) # Pattern matching for logging + @spec log_dropped_packets(list(), list()) :: :ok defp log_dropped_packets([], _), do: :ok defp log_dropped_packets(dropped, processed) do @@ -150,12 +162,14 @@ defmodule Aprsme.PacketConsumer do end # Helper to start batch timer + @spec start_batch_timer(state()) :: state() defp start_batch_timer(%{batch_timeout: timeout} = state) do timer = Process.send_after(self(), :process_batch, timeout) %{state | timer: timer} end # Pattern matching for batch utilization check + @spec check_batch_utilization(list(), integer()) :: :ok defp check_batch_utilization(batch, max_size) when length(batch) > max_size * 0.8 do Logger.warning("Batch size approaching limit", batch_status: @@ -169,6 +183,7 @@ defmodule Aprsme.PacketConsumer do defp check_batch_utilization(_, _), do: :ok + @spec process_batch(list(map())) :: :ok defp process_batch(packets) do require Logger @@ -253,6 +268,7 @@ defmodule Aprsme.PacketConsumer do # ) end + @spec process_chunk(list(map())) :: {non_neg_integer(), non_neg_integer()} defp process_chunk(packets) do # Use Stream for memory-efficient packet preparation packet_stream = diff --git a/lib/aprsme/packets.ex b/lib/aprsme/packets.ex index 39acea8..636d7ce 100644 --- a/lib/aprsme/packets.ex +++ b/lib/aprsme/packets.ex @@ -416,7 +416,6 @@ defmodule Aprsme.Packets do end end - defp get_canonical_device_identifier(nil), do: nil defp get_canonical_device_identifier(device_identifier), do: to_string(device_identifier) @doc """ diff --git a/lib/aprsme_web/aprs_symbol.ex b/lib/aprsme_web/aprs_symbol.ex index b7b9202..7d3d15d 100644 --- a/lib/aprsme_web/aprs_symbol.ex +++ b/lib/aprsme_web/aprs_symbol.ex @@ -25,6 +25,11 @@ defmodule AprsmeWeb.AprsSymbol do background_size: "512px 192px" } """ + @spec get_sprite_info(String.t() | nil, String.t() | nil) :: %{ + sprite_file: String.t(), + background_position: String.t(), + background_size: String.t() + } def get_sprite_info(symbol_table, symbol_code) do # For overlay symbols (A-Z, 0-9), display the base symbol with overlay if symbol_table && String.match?(symbol_table, ~r/^[A-Z0-9]$/) do @@ -68,6 +73,11 @@ defmodule AprsmeWeb.AprsSymbol do Gets sprite information for overlay symbols (A-Z, 0-9). These symbols display the base symbol from the overlay table. """ + @spec get_overlay_base_symbol_info(String.t()) :: %{ + sprite_file: String.t(), + background_position: String.t(), + background_size: String.t() + } def get_overlay_base_symbol_info(base_symbol_code) do # Some overlay base symbols are in the alternate table (1), others in overlay table (2) # Check which table to use based on the symbol code @@ -101,6 +111,7 @@ defmodule AprsmeWeb.AprsSymbol do Determines which sprite table to use for overlay base symbols. Some symbols are in the alternate table (1), others in overlay table (2). """ + @spec get_overlay_base_table_id(String.t()) :: String.t() def get_overlay_base_table_id(base_symbol_code) do # Map symbols to the correct sprite table based on APRS specification # Most overlay symbols are in the alternate table (1) @@ -130,6 +141,11 @@ defmodule AprsmeWeb.AprsSymbol do Gets sprite information for overlay characters (A-Z, 0-9). These are rendered from the overlay table. """ + @spec get_overlay_character_sprite_info(String.t()) :: %{ + sprite_file: String.t(), + background_position: String.t(), + background_size: String.t() + } def get_overlay_character_sprite_info(overlay_char) do # Use overlay table (table 2) for the overlay character sprite_file = "/aprs-symbols/aprs-symbols-128-2@2x.png" @@ -171,6 +187,7 @@ defmodule AprsmeWeb.AprsSymbol do iex> AprsmeWeb.AprsSymbol.normalize_symbol_table("invalid") "/" """ + @spec normalize_symbol_table(String.t() | nil) :: String.t() def normalize_symbol_table(symbol_table) do cond do symbol_table in ["/", "\\", "]"] -> symbol_table @@ -193,6 +210,7 @@ defmodule AprsmeWeb.AprsSymbol do iex> AprsmeWeb.AprsSymbol.normalize_symbol_code("") ">" """ + @spec normalize_symbol_code(String.t() | nil) :: String.t() def normalize_symbol_code(symbol_code) do if symbol_code && symbol_code != "", do: symbol_code, else: ">" end @@ -211,6 +229,7 @@ defmodule AprsmeWeb.AprsSymbol do iex> AprsmeWeb.AprsSymbol.get_table_id("]") "2" """ + @spec get_table_id(String.t()) :: String.t() def get_table_id(symbol_table) do case symbol_table do # Primary table @@ -233,6 +252,7 @@ defmodule AprsmeWeb.AprsSymbol do iex> AprsmeWeb.AprsSymbol.render_marker_html("/", "_", "W1AW") "
..." """ + @spec render_marker_html(String.t() | nil, String.t() | nil, String.t() | nil, integer()) :: String.t() def render_marker_html(symbol_table, symbol_code, callsign \\ nil, size \\ 32) do # For symbols without callsigns, use Cachex for better caching if is_nil(callsign) do @@ -254,6 +274,7 @@ defmodule AprsmeWeb.AprsSymbol do end end + @spec generate_marker_html(String.t() | nil, String.t() | nil, String.t() | nil, integer()) :: String.t() defp generate_marker_html(symbol_table, symbol_code, callsign, size) do sprite_info = get_sprite_info(symbol_table, symbol_code) @@ -330,6 +351,7 @@ defmodule AprsmeWeb.AprsSymbol do iex> AprsmeWeb.AprsSymbol.render_style("/", "_", 32) "width: 32px; height: 32px; background-image: url(/aprs-symbols/aprs-symbols-128-0@2x.png); ..." """ + @spec render_style(String.t() | nil, String.t() | nil, integer()) :: String.t() def render_style(symbol_table, symbol_code, size \\ 32) do sprite_info = get_sprite_info(symbol_table, symbol_code) @@ -347,6 +369,7 @@ defmodule AprsmeWeb.AprsSymbol do iex> AprsmeWeb.AprsSymbol.extract_from_packet(%{}) {"/", ">"} """ + @spec extract_from_packet(map()) :: {String.t(), String.t()} def extract_from_packet(packet) do symbol_table_id = get_packet_field(packet, :symbol_table_id, "/") symbol_code = get_packet_field(packet, :symbol_code, ">") @@ -355,6 +378,7 @@ defmodule AprsmeWeb.AprsSymbol do end # Helper function to safely extract a value from a packet or data_extended map + @spec get_packet_field(map(), atom(), any()) :: any() defp get_packet_field(packet, field, default) do data_extended = Map.get(packet, :data_extended, Map.get(packet, "data_extended", %{})) || %{} diff --git a/lib/aprsme_web/live/weather_live/callsign_view.ex b/lib/aprsme_web/live/weather_live/callsign_view.ex index 0b46a0a..299033f 100644 --- a/lib/aprsme_web/live/weather_live/callsign_view.ex +++ b/lib/aprsme_web/live/weather_live/callsign_view.ex @@ -232,9 +232,9 @@ defmodule AprsmeWeb.WeatherLive.CallsignView do value = PacketUtils.get_weather_field(packet, field) case value do - nil -> false "N/A" -> false "" -> false + nil -> false _ -> true end end @@ -246,13 +246,13 @@ defmodule AprsmeWeb.WeatherLive.CallsignView do value = PacketUtils.get_weather_field(packet, key) case value do - nil -> - nil - "N/A" -> nil - value when is_number(value) -> + nil -> + nil + + value when is_binary(value) -> case @weather_formatters[key] do {formatter, separator} -> {converted_value, unit} = formatter.(value, locale) @@ -262,7 +262,6 @@ defmodule AprsmeWeb.WeatherLive.CallsignView do "#{value}" end - value when is_binary(value) -> case @weather_formatters[key] do {formatter, separator} -> case Float.parse(value) do @@ -278,8 +277,15 @@ defmodule AprsmeWeb.WeatherLive.CallsignView do "#{value}" end - _ -> - nil + value when is_number(value) -> + case @weather_formatters[key] do + {formatter, separator} -> + {converted_value, unit} = formatter.(value, locale) + "#{converted_value}#{separator}#{unit}" + + nil -> + "#{value}" + end end end end diff --git a/lib/aprsme_web/plugs/rate_limiter.ex b/lib/aprsme_web/plugs/rate_limiter.ex index 1f175a2..23d30b3 100644 --- a/lib/aprsme_web/plugs/rate_limiter.ex +++ b/lib/aprsme_web/plugs/rate_limiter.ex @@ -5,6 +5,15 @@ defmodule AprsmeWeb.Plugs.RateLimiter do import Phoenix.Controller import Plug.Conn + @type key_type :: :ip | :user_agent | (Plug.Conn.t() -> String.t()) | String.t() + @type init_opts :: [ + scale: integer(), + limit: integer(), + key: key_type(), + error_message: String.t() + ] + + @spec init(Keyword.t()) :: init_opts() def init(opts) do # Default options Keyword.merge( @@ -21,6 +30,7 @@ defmodule AprsmeWeb.Plugs.RateLimiter do ) end + @spec call(Plug.Conn.t(), init_opts()) :: Plug.Conn.t() def call(conn, opts) do key = get_key(conn, opts[:key]) scale = opts[:scale] @@ -39,6 +49,7 @@ defmodule AprsmeWeb.Plugs.RateLimiter do end end + @spec get_key(Plug.Conn.t(), key_type()) :: String.t() defp get_key(conn, :ip) do # Check headers in order of preference case {get_req_header(conn, "cf-connecting-ip"), get_req_header(conn, "x-forwarded-for"), diff --git a/lib/aprsme_web/time_helpers.ex b/lib/aprsme_web/time_helpers.ex index 960a9e5..0e95f78 100644 --- a/lib/aprsme_web/time_helpers.ex +++ b/lib/aprsme_web/time_helpers.ex @@ -23,6 +23,7 @@ defmodule AprsmeWeb.TimeHelpers do format_time_diff(diff_seconds) <> " " <> gettext("ago") end + @spec format_time_diff(integer()) :: String.t() defp format_time_diff(seconds) when seconds < 60, do: gettext("less than a minute") defp format_time_diff(seconds) when seconds < 120, do: gettext("1 minute") defp format_time_diff(seconds) when seconds < 3600, do: gettext("%{count} minutes", count: div(seconds, 60)) diff --git a/lib/aprsme_web/time_utils.ex b/lib/aprsme_web/time_utils.ex index ae2ddd9..0f41be0 100644 --- a/lib/aprsme_web/time_utils.ex +++ b/lib/aprsme_web/time_utils.ex @@ -6,6 +6,7 @@ defmodule AprsmeWeb.TimeUtils do @doc """ Returns a DateTime that is the specified number of hours before now. """ + @spec hours_ago(number()) :: DateTime.t() def hours_ago(hours) when is_number(hours) do DateTime.add(DateTime.utc_now(), -hours * 3600, :second) end @@ -13,33 +14,41 @@ defmodule AprsmeWeb.TimeUtils do @doc """ Returns a DateTime that is one hour before now. """ + @spec one_hour_ago() :: DateTime.t() def one_hour_ago, do: hours_ago(1) @doc """ Returns a DateTime that is 24 hours (one day) before now. """ + @spec one_day_ago() :: DateTime.t() def one_day_ago, do: hours_ago(24) @doc """ Returns a DateTime that is 48 hours (two days) before now. """ + @spec two_days_ago() :: DateTime.t() def two_days_ago, do: hours_ago(48) @doc """ Returns a DateTime that is 7 days (one week) before now. """ + @spec one_week_ago() :: DateTime.t() def one_week_ago, do: hours_ago(24 * 7) @doc """ Returns a DateTime that is the specified number of days before now. """ + @spec days_ago(number()) :: DateTime.t() def days_ago(days) when is_number(days) do DateTime.add(DateTime.utc_now(), -days, :day) end + @type time_range_atom :: :last_hour | :last_day | :last_two_days | :last_week + @doc """ Returns a tuple of {start_time, end_time} for common time ranges. """ + @spec time_range(time_range_atom()) :: {DateTime.t(), DateTime.t()} def time_range(:last_hour), do: {one_hour_ago(), DateTime.utc_now()} def time_range(:last_day), do: {one_day_ago(), DateTime.utc_now()} def time_range(:last_two_days), do: {two_days_ago(), DateTime.utc_now()} @@ -48,5 +57,6 @@ defmodule AprsmeWeb.TimeUtils do @doc """ Returns a default time range of 48 hours. """ + @spec default_time_range() :: {DateTime.t(), DateTime.t()} def default_time_range, do: time_range(:last_two_days) end diff --git a/lib/aprsme_web/user_auth.ex b/lib/aprsme_web/user_auth.ex index a224f56..208cc9a 100644 --- a/lib/aprsme_web/user_auth.ex +++ b/lib/aprsme_web/user_auth.ex @@ -40,6 +40,7 @@ defmodule AprsmeWeb.UserAuth do |> redirect(to: user_return_to || signed_in_path(conn)) end + @spec maybe_write_remember_me_cookie(Plug.Conn.t(), String.t(), map()) :: Plug.Conn.t() defp maybe_write_remember_me_cookie(conn, token, %{"remember_me" => "true"}) do put_resp_cookie(conn, @remember_me_cookie, token, @remember_me_options) end @@ -61,6 +62,7 @@ defmodule AprsmeWeb.UserAuth do # |> put_session(:preferred_locale, preferred_locale) # end # + @spec renew_session(Plug.Conn.t()) :: Plug.Conn.t() defp renew_session(conn) do conn |> configure_session(renew: true) @@ -98,6 +100,7 @@ defmodule AprsmeWeb.UserAuth do assign(conn, :current_user, user) end + @spec ensure_user_token(Plug.Conn.t()) :: {String.t() | nil, Plug.Conn.t()} defp ensure_user_token(conn) do case get_session(conn, :user_token) do nil -> ensure_user_token_from_cookie(conn) @@ -105,6 +108,7 @@ defmodule AprsmeWeb.UserAuth do end end + @spec ensure_user_token_from_cookie(Plug.Conn.t()) :: {String.t() | nil, Plug.Conn.t()} defp ensure_user_token_from_cookie(conn) do conn = fetch_cookies(conn, signed: [@remember_me_cookie]) @@ -180,6 +184,7 @@ defmodule AprsmeWeb.UserAuth do end end + @spec mount_current_user(map(), Socket.t()) :: Socket.t() defp mount_current_user(session, socket) do case session do %{"user_token" => user_token} -> @@ -229,17 +234,20 @@ defmodule AprsmeWeb.UserAuth do end end + @spec put_token_in_session(Plug.Conn.t(), String.t()) :: Plug.Conn.t() defp put_token_in_session(conn, token) do conn |> put_session(:user_token, token) |> put_session(:live_socket_id, "users_sessions:#{Base.url_encode64(token)}") end + @spec maybe_store_return_to(Plug.Conn.t()) :: Plug.Conn.t() defp maybe_store_return_to(%{method: "GET"} = conn) do put_session(conn, :user_return_to, current_path(conn)) end defp maybe_store_return_to(conn), do: conn + @spec signed_in_path(Plug.Conn.t() | Socket.t()) :: String.t() defp signed_in_path(_conn), do: ~p"/" end