Refactor complex macros and enforce Credo in CI

Author: Taras Tyshko

.credo.exs

@@ -84,12 +84,6 @@
           {Credo.Check.Design.AliasUsage,
            [priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]},
           {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,7 +93,6 @@
           {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, []},
           {Credo.Check.Readability.MultiAlias, []},
           {Credo.Check.Readability.ParenthesesInCondition, []},
@@ -170,6 +163,14 @@
           {Credo.Check.Warning.WrongTestFileExtension, []}
         ],
         disabled: [
+          #
+          # TODO comments are acceptable in our codebase
+          {Credo.Check.Design.TagTODO, []},
+
+          #
+          # ModuleDoc is not required in our codebase
+          {Credo.Check.Readability.ModuleDoc, []},
+
           #
           # Checks scheduled for next check update (opt-in for now)
           {Credo.Check.Refactor.UtcNowTruncate, []},

.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
 
       - name: DB Setup
         run: mix ecto.migrate.reset

lib/nerves_hub/accounts.ex

@@ -3,19 +3,18 @@ defmodule NervesHub.Accounts do
 
   alias Ecto.Changeset
   alias Ecto.Multi
-
   alias NervesHub.Accounts.Invite
   alias NervesHub.Accounts.Org
   alias NervesHub.Accounts.OrgKey
   alias NervesHub.Accounts.OrgMetric
   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

@@ -1,7 +1,7 @@
 defmodule NervesHub.Application do
   use Application
-
   require Logger
+  alias NervesHub.Telemetry.Customizations
 
   def start(_type, _args) do
     case System.cmd("fwup", ["--version"], env: []) do
@@ -53,8 +53,7 @@ defmodule NervesHub.Application do
       :ok = :httpc.set_option(:ipfamily, :inet6fb4)
     end
 
-    :ok = NervesHub.Telemetry.Customizations.setup()
-
+    :ok = Customizations.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
+
   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)
+    |> Extension.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)
+    |> Extension.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
+
   @impl ThousandIsland.Transport
-  defdelegate listen(port, user_options), to: ThousandIsland.Transports.SSL
+  defdelegate listen(port, user_options), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate accept(listener_socket), to: ThousandIsland.Transports.SSL
+  defdelegate accept(listener_socket), to: SSL
 
   @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)
+      SSL.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: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate controlling_process(socket, pid), to: ThousandIsland.Transports.SSL
+  defdelegate controlling_process(socket, pid), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate recv(socket, length, timeout), to: ThousandIsland.Transports.SSL
+  defdelegate recv(socket, length, timeout), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate send(socket, data), to: ThousandIsland.Transports.SSL
+  defdelegate send(socket, data), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate sendfile(socket, filename, offset, length), to: ThousandIsland.Transports.SSL
+  defdelegate sendfile(socket, filename, offset, length), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate getopts(socket, options), to: ThousandIsland.Transports.SSL
+  defdelegate getopts(socket, options), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate setopts(socket, options), to: ThousandIsland.Transports.SSL
+  defdelegate setopts(socket, options), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate shutdown(socket, way), to: ThousandIsland.Transports.SSL
+  defdelegate shutdown(socket, way), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate close(socket), to: ThousandIsland.Transports.SSL
+  defdelegate close(socket), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate sockname(socket), to: ThousandIsland.Transports.SSL
+  defdelegate sockname(socket), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate peername(socket), to: ThousandIsland.Transports.SSL
+  defdelegate peername(socket), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate peercert(socket), to: ThousandIsland.Transports.SSL
+  defdelegate peercert(socket), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate secure?(), to: ThousandIsland.Transports.SSL
+  defdelegate secure?(), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate getstat(socket), to: ThousandIsland.Transports.SSL
+  defdelegate getstat(socket), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate negotiated_protocol(socket), to: ThousandIsland.Transports.SSL
+  defdelegate negotiated_protocol(socket), to: SSL
 
   @impl ThousandIsland.Transport
-  defdelegate connection_information(socket), to: ThousandIsland.Transports.SSL
+  defdelegate connection_information(socket), to: SSL
 end

lib/nerves_hub/devices.ex

@@ -34,6 +34,7 @@ defmodule NervesHub.Devices do
   alias NervesHub.Products.Product
   alias NervesHub.Repo
   alias NervesHub.TaskSupervisor, as: Tasks
+  alias Phoenix.Channel.Server
 
   def get_device(device_id) when is_integer(device_id) do
     Repo.get(Device, device_id)
@@ -995,13 +996,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}"
+    _ = Server.broadcast(NervesHub.PubSub, topic, "device-online", payload)
 
     :ok
   end
@@ -1011,13 +1007,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}"
+    _ = Server.broadcast(NervesHub.PubSub, topic, "device-updated", %{})
 
     :ok
   end
@@ -1159,12 +1150,8 @@ 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",
-              %{}
-            )
+            topic = "orchestrator:deployment:#{device.deployment_id}"
+            Server.broadcast(NervesHub.PubSub, topic, "device-updated", %{})
           end
 
         result
@@ -1640,13 +1627,8 @@ defmodule NervesHub.Devices do
 
     payload = %{inflight_update: inflight_update, update_payload: update_payload}
 
-    _ =
-      Phoenix.Channel.Server.broadcast(
-        NervesHub.PubSub,
-        "device:#{device_id}",
-        "update-scheduled",
-        payload
-      )
+    topic = "device:#{device_id}"
+    _ = Server.broadcast(NervesHub.PubSub, topic, "update-scheduled", payload)
 
     :ok
   end

lib/nerves_hub/devices/connections.ex

@@ -7,6 +7,7 @@ defmodule NervesHub.Devices.Connections do
   alias NervesHub.Devices.Device
   alias NervesHub.Devices.DeviceConnection
   alias NervesHub.Repo
+  alias Phoenix.Channel.Server
 
   @doc """
   Get all connections for a device.
@@ -90,20 +91,11 @@ defmodule NervesHub.Devices.Connections do
     {1, nil} =
       DeviceConnection
       |> where([dc], dc.id == ^id)
-      |> Repo.update_all(
-        set: [
-          status: "connected",
-          last_seen_at: DateTime.utc_now()
-        ]
-      )
+      |> Repo.update_all(set: [status: "connected", last_seen_at: DateTime.utc_now()])
 
-    Phoenix.Channel.Server.broadcast_from!(
-      NervesHub.PubSub,
-      self(),
-      "device:#{device.identifier}:internal",
-      "connection:heartbeat",
-      %{}
-    )
+    event = "connection:heartbeat"
+    topic = "device:#{device.identifier}:internal"
+    Server.broadcast_from!(NervesHub.PubSub, self(), topic, event, %{})
   end
 
   @doc """

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
 
   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"
+        _ = Server.broadcast(NervesHub.PubSub, topic, "logs:received", log_line)
 
         {:ok, log_line}