Resolve Credo issues and enforce it in CI

.credo.exs

@@ -83,13 +83,9 @@
           #
           {Credo.Check.Design.AliasUsage,
            [priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]},
+          # TODO comments are acceptable in our codebase, but we want to see them locally
+          {Credo.Check.Design.TagTODO, [priority: :ignore]},
           {Credo.Check.Design.TagFIXME, []},
-          # You can also customize the exit_status of each check.
-          # If you don't want TODO comments to cause `mix credo` to fail, just
-          # set this value to 0 (zero).
-          #
-          {Credo.Check.Design.TagTODO, [exit_status: 0]},
-
           #
           ## Readability Checks
           #
@@ -99,8 +95,9 @@
           {Credo.Check.Readability.LargeNumbers, []},
           {Credo.Check.Readability.MaxLineLength, [priority: :low, max_length: 120]},
           {Credo.Check.Readability.ModuleAttributeNames, []},
-          {Credo.Check.Readability.ModuleDoc, []},
           {Credo.Check.Readability.ModuleNames, []},
+          # ModuleDoc is not required in our codebase, but we want to see them locally
+          {Credo.Check.Readability.ModuleDoc, [priority: :ignore]},
           {Credo.Check.Readability.MultiAlias, []},
           {Credo.Check.Readability.ParenthesesInCondition, []},
           {Credo.Check.Readability.ParenthesesOnZeroArityDefs, [parens: true]},

.github/workflows/ci.yml

@@ -112,8 +112,8 @@ jobs:
       - name: Check spelling
         run: mix spellweaver.check
 
-      - name: Run Credo (won't fail the build)
-        run: mix credo --mute-exit-status
+      - name: Run Credo
+        run: mix credo --min-priority low
 
       - name: DB Setup
         run: mix ecto.migrate.reset

lib/nerves_hub/accounts.ex

@@ -11,11 +11,11 @@ defmodule NervesHub.Accounts do
   alias NervesHub.Accounts.OrgUser
   alias NervesHub.Accounts.RemoveAccount
   alias NervesHub.Accounts.User
+  alias NervesHub.Accounts.UserNotifier
   alias NervesHub.Accounts.UserToken
   alias NervesHub.Devices
   alias NervesHub.Devices.Device
   alias NervesHub.Products.Product
-  alias NervesHub.Accounts.UserNotifier
 
   alias NervesHub.Repo
 

lib/nerves_hub/accounts/user_notifier.ex

@@ -7,8 +7,8 @@ defmodule NervesHub.Accounts.UserNotifier do
   alias NervesHub.Emails.ConfirmationTemplate
   alias NervesHub.Emails.LoginWithGoogleReminderTemplate
   alias NervesHub.Emails.OrgUserAddedTemplate
-  alias NervesHub.Emails.PasswordResetTemplate
   alias NervesHub.Emails.PasswordResetConfirmationTemplate
+  alias NervesHub.Emails.PasswordResetTemplate
   alias NervesHub.Emails.PasswordUpdatedTemplate
   alias NervesHub.Emails.TellOrgUserAddedTemplate
   alias NervesHub.Emails.TellOrgUserInvitedTemplate
@@ -17,6 +17,7 @@ defmodule NervesHub.Accounts.UserNotifier do
   alias NervesHub.Emails.WelcomeTemplate
 
   alias NervesHub.SwooshMailer, as: Mailer
+  alias Phoenix.HTML
 
   def deliver_confirmation_instructions(user, confirmation_url) do
     assigns = %{
@@ -207,7 +208,7 @@ defmodule NervesHub.Accounts.UserNotifier do
 
     text =
       module.text_render(assigns)
-      |> Phoenix.HTML.Safe.to_iodata()
+      |> HTML.Safe.to_iodata()
       |> IO.iodata_to_binary()
 
     {html, text}

lib/nerves_hub/application.ex

@@ -3,6 +3,8 @@ defmodule NervesHub.Application do
 
   require Logger
 
+  alias NervesHub.Telemetry.Customizations, as: TelemetryCustomizations
+
   def start(_type, _args) do
     case System.cmd("fwup", ["--version"], env: []) do
       {_, 0} ->
@@ -57,8 +59,7 @@ defmodule NervesHub.Application do
       :ok = :httpc.set_option(:ipfamily, :inet6fb4)
     end
 
-    :ok = NervesHub.Telemetry.Customizations.setup()
-
+    :ok = TelemetryCustomizations.setup()
     :ok = OpentelemetryBandit.setup()
     :ok = OpentelemetryPhoenix.setup(adapter: :bandit)
     :ok = OpentelemetryOban.setup(trace: [:jobs])

lib/nerves_hub/certificate.ex

@@ -1,4 +1,6 @@
 defmodule NervesHub.Certificate do
+  alias X509.Certificate.Extension, as: X509Extension
+
   import X509.ASN1,
     only: [
       extension: 1,
@@ -17,7 +19,7 @@ defmodule NervesHub.Certificate do
   def get_aki(otp_certificate) do
     otp_certificate
     |> X509.Certificate.extensions()
-    |> X509.Certificate.Extension.find(:authority_key_identifier)
+    |> X509Extension.find(:authority_key_identifier)
     |> extension()
     |> Keyword.get(:extnValue)
     |> authority_key_identifier()
@@ -27,7 +29,7 @@ defmodule NervesHub.Certificate do
   def get_ski(otp_certificate) do
     otp_certificate
     |> X509.Certificate.extensions()
-    |> X509.Certificate.Extension.find(:subject_key_identifier)
+    |> X509Extension.find(:subject_key_identifier)
     |> case do
       nil ->
         nil

lib/nerves_hub/device_ssl_transport.ex

@@ -13,18 +13,20 @@ defmodule NervesHub.DeviceSSLTransport do
 
   @behaviour ThousandIsland.Transport
 
+  alias ThousandIsland.Transports.SSL, as: SSLTransport
+
   @impl ThousandIsland.Transport
-  defdelegate listen(port, user_options), to: ThousandIsland.Transports.SSL
+  defdelegate listen(port, user_options), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate accept(listener_socket), to: ThousandIsland.Transports.SSL
+  defdelegate accept(listener_socket), to: SSLTransport
 
   @impl ThousandIsland.Transport
   def handshake(socket) do
     if NervesHub.RateLimit.increment() do
       :telemetry.execute([:nerves_hub, :rate_limit, :accepted], %{count: 1})
 
-      ThousandIsland.Transports.SSL.handshake(socket)
+      SSLTransport.handshake(socket)
     else
       :telemetry.execute([:nerves_hub, :rate_limit, :rejected], %{count: 1})
 
@@ -33,50 +35,50 @@ defmodule NervesHub.DeviceSSLTransport do
   end
 
   @impl ThousandIsland.Transport
-  defdelegate upgrade(socket, opts), to: ThousandIsland.Transports.SSL
+  defdelegate upgrade(socket, opts), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate controlling_process(socket, pid), to: ThousandIsland.Transports.SSL
+  defdelegate controlling_process(socket, pid), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate recv(socket, length, timeout), to: ThousandIsland.Transports.SSL
+  defdelegate recv(socket, length, timeout), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate send(socket, data), to: ThousandIsland.Transports.SSL
+  defdelegate send(socket, data), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate sendfile(socket, filename, offset, length), to: ThousandIsland.Transports.SSL
+  defdelegate sendfile(socket, filename, offset, length), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate getopts(socket, options), to: ThousandIsland.Transports.SSL
+  defdelegate getopts(socket, options), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate setopts(socket, options), to: ThousandIsland.Transports.SSL
+  defdelegate setopts(socket, options), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate shutdown(socket, way), to: ThousandIsland.Transports.SSL
+  defdelegate shutdown(socket, way), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate close(socket), to: ThousandIsland.Transports.SSL
+  defdelegate close(socket), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate sockname(socket), to: ThousandIsland.Transports.SSL
+  defdelegate sockname(socket), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate peername(socket), to: ThousandIsland.Transports.SSL
+  defdelegate peername(socket), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate peercert(socket), to: ThousandIsland.Transports.SSL
+  defdelegate peercert(socket), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate secure?(), to: ThousandIsland.Transports.SSL
+  defdelegate secure?(), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate getstat(socket), to: ThousandIsland.Transports.SSL
+  defdelegate getstat(socket), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate negotiated_protocol(socket), to: ThousandIsland.Transports.SSL
+  defdelegate negotiated_protocol(socket), to: SSLTransport
 
   @impl ThousandIsland.Transport
-  defdelegate connection_information(socket), to: ThousandIsland.Transports.SSL
+  defdelegate connection_information(socket), to: SSLTransport
 end

lib/nerves_hub/devices.ex

@@ -37,6 +37,7 @@ defmodule NervesHub.Devices do
   alias NervesHub.Products.Product
   alias NervesHub.Repo
   alias NervesHub.TaskSupervisor, as: Tasks
+  alias Phoenix.Channel.Server, as: ChannelServer
 
   def get_device(device_id) when is_integer(device_id) do
     Repo.get(Device, device_id)
@@ -668,13 +669,9 @@ defmodule NervesHub.Devices do
 
     {:ok, device} = update_device(device, %{firmware_validation_status: "validated"})
 
-    Phoenix.Channel.Server.broadcast_from!(
-      NervesHub.PubSub,
-      self(),
-      "device:#{device.identifier}:internal",
-      "firmware:validated",
-      %{}
-    )
+    event = "firmware:validated"
+    topic = "device:#{device.identifier}:internal"
+    ChannelServer.broadcast_from!(NervesHub.PubSub, self(), topic, event, %{})
 
     {:ok, device}
   end
@@ -1021,13 +1018,8 @@ defmodule NervesHub.Devices do
       firmware_uuid: firmware_uuid
     }
 
-    _ =
-      Phoenix.Channel.Server.broadcast(
-        NervesHub.PubSub,
-        "orchestrator:deployment:#{device.deployment_id}",
-        "device-online",
-        payload
-      )
+    topic = "orchestrator:deployment:#{device.deployment_id}"
+    _ = ChannelServer.broadcast(NervesHub.PubSub, topic, "device-online", payload)
 
     :ok
   end
@@ -1037,13 +1029,8 @@ defmodule NervesHub.Devices do
   end
 
   def deployment_device_updated(device) do
-    _ =
-      Phoenix.Channel.Server.broadcast(
-        NervesHub.PubSub,
-        "orchestrator:deployment:#{device.deployment_id}",
-        "device-updated",
-        %{}
-      )
+    topic = "orchestrator:deployment:#{device.deployment_id}"
+    _ = ChannelServer.broadcast(NervesHub.PubSub, topic, "device-updated", %{})
 
     :ok
   end
@@ -1185,12 +1172,9 @@ defmodule NervesHub.Devices do
       {:ok, device} = result ->
         _ =
           if device.deployment_id do
-            Phoenix.Channel.Server.broadcast(
-              NervesHub.PubSub,
-              "orchestrator:deployment:#{device.deployment_id}",
-              "device-updated",
-              %{}
-            )
+            event = "device-updated"
+            topic = "orchestrator:deployment:#{device.deployment_id}"
+            ChannelServer.broadcast(NervesHub.PubSub, topic, event, %{})
           end
 
         result

lib/nerves_hub/devices/log_lines.ex

@@ -6,6 +6,7 @@ defmodule NervesHub.Devices.LogLines do
   alias NervesHub.AnalyticsRepo
   alias NervesHub.Devices.Device
   alias NervesHub.Devices.LogLine
+  alias Phoenix.Channel.Server, as: ChannelServer
 
   import Ecto.Query
 
@@ -53,13 +54,8 @@ defmodule NervesHub.Devices.LogLines do
       {:ok, log_line} ->
         _ = AnalyticsRepo.insert_all(LogLine, [changeset.changes], settings: [async_insert: 1])
 
-        _ =
-          Phoenix.Channel.Server.broadcast(
-            NervesHub.PubSub,
-            "device:#{device.identifier}:internal",
-            "logs:received",
-            log_line
-          )
+        topic = "device:#{device.identifier}:internal"
+        _ = ChannelServer.broadcast(NervesHub.PubSub, topic, "logs:received", log_line)
 
         {:ok, log_line}
 

lib/nerves_hub/emails/closing_block.ex

@@ -2,15 +2,17 @@ defmodule NervesHub.Emails.ClosingBlock do
   use MjmlEEx.Component, mode: :runtime
   use NervesHubWeb, :html
 
+  alias Phoenix.HTML
+
   @impl MjmlEEx.Component
   def render(_assigns) do
     """
     <mj-section padding-top="0px">
       <mj-column>
         <mj-text font-size="20px" font-family="sans-serif">
-        #{support_section() |> Phoenix.HTML.Safe.to_iodata()}
+        #{HTML.Safe.to_iodata(support_section())}
         </mj-text>
-        #{signoff() |> Phoenix.HTML.Safe.to_iodata()}
+        #{HTML.Safe.to_iodata(signoff())}
       </mj-column>
     </mj-section>
     """

lib/nerves_hub/extensions.ex

@@ -15,6 +15,8 @@ defmodule NervesHub.Extensions do
     critical firmware update.
   """
 
+  alias Phoenix.Channel.Server, as: ChannelServer
+
   @callback handle_in(event :: String.t(), Phoenix.Channel.payload(), Phoenix.Socket.t()) ::
               {:noreply, Phoenix.Socket.t()}
               | {:noreply, Phoenix.Socket.t(), timeout() | :hibernate}
@@ -77,15 +79,8 @@ defmodule NervesHub.Extensions do
   end
 
   def broadcast_extension_event(target, event, extension) do
-    Phoenix.Channel.Server.broadcast_from!(
-      NervesHub.PubSub,
-      self(),
-      topic(target),
-      event,
-      %{
-        "extensions" => [extension]
-      }
-    )
+    params = %{"extensions" => [extension]}
+    ChannelServer.broadcast_from!(NervesHub.PubSub, self(), topic(target), event, params)
   end
 
   defp topic(%NervesHub.Devices.Device{} = device), do: "device:#{device.id}:extensions"

lib/nerves_hub/extensions/geo.ex

@@ -2,6 +2,7 @@ defmodule NervesHub.Extensions.Geo do
   @behaviour NervesHub.Extensions
 
   alias NervesHub.Devices.Connections
+  alias Phoenix.Channel.Server, as: ChannelServer
 
   @impl NervesHub.Extensions
   def description() do
@@ -49,14 +50,9 @@ defmodule NervesHub.Extensions.Geo do
   @impl NervesHub.Extensions
   def handle_in("location:update", location, socket) do
     Connections.merge_update_metadata(socket.assigns.reference_id, %{location: location})
-
-    _ =
-      Phoenix.Channel.Server.broadcast(
-        NervesHub.PubSub,
-        "device:#{socket.assigns.device.identifier}:internal",
-        "location:updated",
-        location
-      )
+    event = "location:updated"
+    topic = "device:#{socket.assigns.device.identifier}:internal"
+    _ = ChannelServer.broadcast(NervesHub.PubSub, topic, event, location)
 
     {:noreply, socket}
   end

lib/nerves_hub/extensions/health.ex

@@ -5,6 +5,7 @@ defmodule NervesHub.Extensions.Health do
   alias NervesHub.Devices.HealthStatus
   alias NervesHub.Devices.Metrics
   alias NervesHub.Helpers.Logging
+  alias Phoenix.Channel.Server, as: ChannelServer
 
   require Logger
 
@@ -109,7 +110,6 @@ defmodule NervesHub.Extensions.Health do
 
   defp device_internal_broadcast!(device, event, payload) do
     topic = "device:#{device.id}:extensions"
-
-    Phoenix.Channel.Server.broadcast_from!(NervesHub.PubSub, self(), topic, event, payload)
+    ChannelServer.broadcast_from!(NervesHub.PubSub, self(), topic, event, payload)
   end
 end