Credo.Check.Design.AliasUsage doesn't warn when unaliased modules are used without function calls

[pdgonzalez872]

Hi @rrrene!

First off, thanks for Credo!

Precheck

Current master/main.

Expected outcome

Warn when modules are not aliased. More details below.

Actual outcome

It doesn't warn.

More details

I wanted to note a curious behavior from the current implementation of AliasUsage: It only works when modules are making function calls. If you reference the module itself you won't get a warning. This showed up in one of our implementation switches and I wondered why Credo didn't complain. I imagine more folks will start to run into this as Mox (I love Mox) is used for testing and this type of switch is more common. I wrote a test that shows the behavior.

The real question here is: would you expect it to warn in this case? If so, I'd be happy to try to fix this. If not, no worries, maybe some docs on this would help clear things up. I'd be happy either way.

Here is a commit with a failing test, I pushed it up to a branch here if you'd like to check it out:

• branch: https://github.com/pdgonzalez872/credo/commits/pg-alias-usage-01
• commit: https://github.com/pdgonzalez872/credo/commit/967a663a42e56c5612de1a0e7b8085b6dafa5f8f

Here is the commit for simplicity sake:

commit 967a663a42e56c5612de1a0e7b8085b6dafa5f8f
Author: Paulo Daniel Gonzalez <[email protected]>
Date:   Thu Mar 24 10:31:18 2022 -0500

    Add failing test

diff --git a/test/credo/check/design/alias_usage_test.exs b/test/credo/check/design/alias_usage_test.exs
index 189e4ace..496b8d71 100644
--- a/test/credo/check/design/alias_usage_test.exs
+++ b/test/credo/check/design/alias_usage_test.exs
@@ -211,6 +211,23 @@ defmodule Credo.Check.Design.AliasUsageTest do
     |> assert_issue()
   end

+  test "it should report a violation even when there is no function call" do
+    """
+    defmodule CredoSampleModule do
+      def call do
+        impl().something
+      end
+
+      def impl do
+        Application.get_env(:some_third_party, :live_impl, Credo.LiveImpl)
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> assert_issue()
+  end
+
   test "it should report if configured to complain start at a certain depth" do
     """
     defmodule CredoSampleModule do

You will get a failing test when running mix test test/credo/check/design/alias_usage_test.exs

Thanks again!

PG

[rrrene]

You are right.

I would happily merge a PR for this 😄

[pdgonzalez872]

Excellent @rrrene. I won't be able to tackle this for a little while, if anyone wants to take a stab no hard feelings :laughing: