build: Update Credo rules, add proper linting, remove stale deps from mix.lock

Author: atlas-oc

.credo.exs

@@ -31,7 +31,7 @@
           "apps/*/test/",
           "apps/*/web/"
         ],
-        excluded: [~r"/_build/", ~r"/deps/", ~r"/node_modules/"]
+        excluded: [~r"/_build/", ~r"/deps/", ~r"/node_modules/", "lib/nosedrum/"]
       },
       #
       # Load and configure plugins here:
@@ -88,7 +88,7 @@
           # 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: 2]},
+          {Credo.Check.Design.TagTODO, [exit_status: 0]},
           {Credo.Check.Design.TagFIXME, []},
 
           #
@@ -99,7 +99,6 @@
           {Credo.Check.Readability.LargeNumbers, []},
           {Credo.Check.Readability.MaxLineLength, [priority: :low, max_length: 130]},
           {Credo.Check.Readability.ModuleAttributeNames, []},
-          {Credo.Check.Readability.ModuleDoc, []},
           {Credo.Check.Readability.ModuleNames, []},
           {Credo.Check.Readability.ParenthesesInCondition, []},
           {Credo.Check.Readability.ParenthesesOnZeroArityDefs, []},
@@ -115,6 +114,8 @@
           {Credo.Check.Readability.UnnecessaryAliasExpansion, []},
           {Credo.Check.Readability.VariableNames, []},
           {Credo.Check.Readability.WithSingleClause, []},
+          {Credo.Check.Consistency.MultiAliasImportRequireUse, []},
+          {Credo.Check.Readability.ImplTrue, []},
 
           #
           ## Refactoring Opportunities
@@ -159,7 +160,11 @@
           {Credo.Check.Warning.UnusedRegexOperation, []},
           {Credo.Check.Warning.UnusedStringOperation, []},
           {Credo.Check.Warning.UnusedTupleOperation, []},
-          {Credo.Check.Warning.UnsafeExec, []}
+          {Credo.Check.Warning.UnsafeExec, []},
+          {Credo.Check.Warning.LeakyEnvironment, []},
+          {Credo.Check.Warning.MapGetUnsafePass, []},
+          {Credo.Check.Warning.MixEnv, []},
+          {Credo.Check.Warning.UnsafeToAtom, []}
         ],
         disabled: [
           #
@@ -169,14 +174,13 @@
           # Controversial and experimental checks (opt-in, just move the check to `:enabled`
           #   and be sure to use `mix credo --strict` to see low priority checks)
           #
-          {Credo.Check.Consistency.MultiAliasImportRequireUse, []},
-          {Credo.Check.Consistency.UnusedVariableNames, []},
-          {Credo.Check.Design.DuplicatedCode, []},
-          {Credo.Check.Design.SkipTestWithoutComment, []},
+          {Credo.Check.Consistency.UnusedVariableNames, [force: :meaningful]},
           {Credo.Check.Readability.AliasAs, []},
           {Credo.Check.Readability.BlockPipe, []},
-          {Credo.Check.Readability.ImplTrue, []},
+          {Credo.Check.Readability.ModuleDoc, []},
           {Credo.Check.Readability.MultiAlias, []},
+          {Credo.Check.Design.DuplicatedCode, []},
+          {Credo.Check.Design.SkipTestWithoutComment, []},
           {Credo.Check.Readability.NestedFunctionCalls, []},
           {Credo.Check.Readability.OneArityFunctionInPipe, []},
           {Credo.Check.Readability.SeparateAliasRequire, []},
@@ -198,11 +202,7 @@
           {Credo.Check.Refactor.PipeChainStart, []},
           {Credo.Check.Refactor.RejectFilter, []},
           {Credo.Check.Refactor.VariableRebinding, []},
-          {Credo.Check.Warning.LazyLogging, []},
-          {Credo.Check.Warning.LeakyEnvironment, []},
-          {Credo.Check.Warning.MapGetUnsafePass, []},
-          {Credo.Check.Warning.MixEnv, []},
-          {Credo.Check.Warning.UnsafeToAtom, []}
+          {Credo.Check.Warning.LazyLogging, []}
 
           # {Credo.Check.Refactor.MapInto, []},
 

.github/workflows/build_prs.yml

@@ -0,0 +1,46 @@
+name: Build PRs
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+
+jobs:
+  lint_pr:
+      name: Lint PR
+      runs-on: ubuntu-latest
+      steps:
+        - name: Checkout repo
+          uses: actions/checkout@v4
+        
+        - name: Set up Elixir
+          uses: erlef/setup-beam@v1
+          with:
+            otp-version: 27
+            elixir-version: 1.19.5
+
+        - uses: actions/cache@v4
+          env:
+            base-key: 27-1.19
+          with:
+            path: |
+              _build
+              deps
+            # Generate a hash of the OTP version and Elixir version
+            key: ${{ env.base-key }}-mix-${{ hashFiles('**/mix.lock') }}
+            restore-keys: ${{ env.base-key }}-mix
+
+        - name: Install dependencies
+          run: mix deps.get
+        
+        - name: Compile dependencies
+          run: mix deps.compile
+        
+        - name: Compile project
+          run: mix compile --strict
+
+        - name: Lint (Credo)
+          run: mix credo --strict --ignore 'TagTODO'
+        
+        - name: Lint (mix format)
+          run: mix format --check-formatted

.github/workflows/lint.yml

@@ -23,7 +23,7 @@ defmodule Cluster.Strategy.Tailscale do
 
   def start_link(args), do: GenServer.start_link(__MODULE__, args)
 
-  @impl true
+  @impl GenServer
   def init([%State{meta: nil} = state]) do
     init([%State{state | :meta => MapSet.new()}])
   end
@@ -32,7 +32,7 @@ defmodule Cluster.Strategy.Tailscale do
     {:ok, load(state)}
   end
 
-  @impl true
+  @impl GenServer
   def handle_info(:timeout, state) do
     handle_info(:load, state)
   end

lib/cluster/strategy/tailscale.ex

@@ -100,7 +100,7 @@ defmodule Octocon.Accounts do
   def region_for_user(system_identity) do
     case get_user_registry(system_identity) do
       nil -> nil
-      %UserRegistry{region: region} -> String.to_atom(region)
+      %UserRegistry{region: region} -> String.to_existing_atom(region)
     end
   end
 

lib/octocon/accounts.ex

@@ -24,7 +24,7 @@ defmodule Octocon.Application do
 
   require Logger
 
-  @impl true
+  @impl Application
   def start(_type, _args) do
     group = Octocon.RPC.NodeTracker.current_group()
     Logger.warning("Starting node of type: #{group}")
@@ -44,13 +44,13 @@ defmodule Octocon.Application do
 
   # Tell Phoenix to update the endpoint configuration
   # whenever the application is updated.
-  @impl true
+  @impl Application
   def config_change(changed, _new, removed) do
     OctoconWeb.Endpoint.config_change(changed, removed)
     :ok
   end
 
-  defp global_children() do
+  defp global_children do
     [
       # Telemetry
       OctoconWeb.Telemetry,
@@ -106,7 +106,7 @@ defmodule Octocon.Application do
 
   defp group_children(_), do: []
 
-  defp generate_libcluster_child() do
+  defp generate_libcluster_child do
     node_list = Application.get_env(:octocon, :node_list, nil)
     use_tailscale = Application.get_env(:octocon, :use_tailscale, false)
 

lib/octocon/application.ex

@@ -38,7 +38,7 @@ defmodule Octocon.ChannelBlacklists do
 
   This should always be backed by a `OctoconDiscord.ChannelBlacklistManager` cache, as it is a latency-sensitive operation.
   """
-  def is_channel_blacklisted?(channel_id) do
+  def channel_blacklisted?(channel_id) do
     from(c in ChannelBlacklist, where: c.channel_id == ^to_string(channel_id))
     |> Repo.all_regional(@region_specifier)
     |> case do

lib/octocon/channel_blacklists.ex

@@ -8,7 +8,7 @@ defmodule Octocon.ClusterUtils do
   @doc """
   Check if the current node is a primary node.
   """
-  def is_primary?, do: NodeTracker.is_primary?()
+  def primary?, do: NodeTracker.primary?()
 
   def current_db_region do
     Application.get_env(:octocon, :current_db_region)
@@ -28,7 +28,7 @@ defmodule Octocon.ClusterUtils do
   def primary_nodes(true) do
     other_nodes = primary_nodes(false)
 
-    if is_primary?() do
+    if primary?() do
       [Node.self() | other_nodes]
     else
       other_nodes
@@ -41,7 +41,7 @@ defmodule Octocon.ClusterUtils do
   """
   def run_on_all_primary_nodes(fun) do
     # If we are a primary node, run the function locally as well
-    if NodeTracker.is_primary?() do
+    if NodeTracker.primary?() do
       fun.()
     end
 
@@ -84,7 +84,7 @@ defmodule Octocon.ClusterUtils do
 
   def check_primary(node) do
     Octocon.RPC.NodeTracker.rpc(node, fn ->
-      NodeTracker.is_primary?()
+      NodeTracker.primary?()
     end)
   end
 end

lib/octocon/cluster_utils.ex

@@ -8,13 +8,15 @@ defmodule Octocon.FCM do
 
   import Ecto.Query, warn: false
 
-  alias Octocon.Accounts
-  alias Octocon.Alters
-  alias Octocon.Alters.Alter
-  alias Octocon.Friendships.Friendship
-  alias Octocon.NotificationTokens
-  alias Octocon.NotificationTokens.NotificationToken
-  alias Octocon.Repo
+  alias Octocon.{
+    Accounts,
+    Alters,
+    Alters.Alter,
+    Friendships.Friendship,
+    NotificationTokens,
+    NotificationTokens.NotificationToken,
+    Repo
+  }
 
   @size_limit 500
 

lib/octocon/fcm.ex

@@ -10,15 +10,16 @@ defmodule Octocon.Friendships do
   @query_concurrency 10
 
   import Ecto.Query, warn: false
-  alias Octocon.Alters.Alter
-  alias Octocon.Repo
 
-  alias Octocon.Accounts
-
-  alias Octocon.Friendships.Friendship
-  alias Octocon.Fronts
-
-  alias Octocon.Fronts.CurrentFront
+  alias Octocon.{
+    Accounts,
+    Alters.Alter,
+    Friendships.Friendship,
+    Friendships.Request,
+    Fronts,
+    Fronts.CurrentFront,
+    Repo
+  }
 
   @doc """
   Returns the friendship status between two users with the given identities, or `nil` if no friendship exists.
@@ -292,8 +293,6 @@ defmodule Octocon.Friendships do
     Friendship.changeset(friendship, attrs)
   end
 
-  alias Octocon.Friendships.Request
-
   @doc """
   Returns the friend request between two users with the given identities, or `nil` if no request exists.
   """
@@ -417,9 +416,7 @@ defmodule Octocon.Friendships do
     user_id = Accounts.id_from_system_identity(user_identity, :system)
     friend_id = Accounts.id_from_system_identity(friend_identity, :system)
 
-    if not friendship_exists?(user_identity, friend_identity) do
-      {:error, :not_friends}
-    else
+    if friendship_exists?(user_identity, friend_identity) do
       from(
         f in Friendship,
         where: f.user_id == ^user_id and f.friend_id == ^friend_id
@@ -445,6 +442,8 @@ defmodule Octocon.Friendships do
       end)
 
       :ok
+    else
+      {:error, :not_friends}
     end
   end