[WIP] Enforce consistent module attribute ordering

[sascha-wolf]

This PR is a work-in-progress (it does not work yet) and an evolution of #588.

Instead of strictly enforcing the styleguide it instead captures the ordering of the module attributes in the whole project, and enforces consistency across the codebase.

Due to the nature of the check it does this a bit different than all other consistency checks. The other consistency checks act on a line-by-line basis but this check looks at the whole module.

It's also special in the way that not all modules necessarily use all module attributes but can still be considered valid. For example:

defmodule Module1 do
  @moduledoc false
  
  use AnotherModule
  
  alias SomeThing
  
  require Logger
end

Has the module attribute order [:moduledoc, :use, :alias, :require], while this module:

defmodule Module2 do
  use AnotherModule

  import MoreFunctions
  
  alias Stuff
  alias MoreStuff
end

Has the order [:use, :import, :alias].

These two orders do not contradict but rather compliment each other. The correct general order can be inferred: [:moduledoc, :use, :import, :alias, :require]

As such the check introduces a new optional transform_frequencies/1 callback for Consistency checks. This callback is used in the check to merge orders as seen above, which is a surprisingly hard problem. All of this "merging" happens in the Merge module which has been extracted to ease readability (although the code itself is not really readable.

---

In some cases the check can not determine the preferred order, take the example from above but now imagine Module2 to have a @type at the end:

defmodule Module2 do
  use AnotherModule

  import MoreFunctions
  
  alias Stuff
  alias MoreStuff

  @type my_type :: any()
end

In this case the two orders look like this:

Module1: [:moduledoc, :use,          :alias, :require]
Module2: [            :use, :import, :alias, :type]

As visible it's impossible to determine the correct order from just looking at these two modules. Instead this is handled by checking the order of attributes of all other modules in the order of frequency in the hope that one contains both attributes (type and require) to determine the correct order.

If this fails too the check falls back to the order listed in the community styleguide and prints a warning.

[sascha-wolf]

@rrrene Please let's use this draft PR to have a discussion about the approach here.

I'm not really happy with the whole merging implementation (especially with the usage of throw) but I'm not certain how this could be approached differently. Maybe by using an Agent?

[rrrene]

@sascha-wolf Thx for putting this together! 👍 I think it enables us to have a deeper discussion surrounding a check for module attribute/macro order.

This PR raises the question if this can be molded into a sensible consistency check at all, since we can't know if this can be kept consistent across "real" codebases. I've module attributes that need to be set before a use can be used.

The problem is that we will only know once we have a working version of this 😄

As for the combinatorial problem you mentioned:

A:  [:moduledoc,  :use,           :alias, :require]
B:  [             :use, :import,  :alias          ]
X1: [:moduledoc,  :use, :import,  :alias, :require]

Basically A and B are valid orders if the "rule" is X1. So the problem becomes to find an X1 for all combinations we found in the codebase.

This is why

As visible it's impossible to determine the correct order from just looking at these two modules.

is asking the wrong question. We are not looking for the "correct" order, we are looking for one valid order that all combinations can be mapped to:

A:  [:moduledoc, :use,          :alias, :require]
B:  [            :use, :import, :alias, :type]
X1: [:moduledoc, :use, :import, :alias, :require, :type]
X2: [:moduledoc, :use, :import, :alias, :type, :require]

Both X1 and X2 are valid orders that A and B can map to. If we modify B, only X2 remains valid (and becomes X1, but that's irrelevant):

A:  [:moduledoc, :use,          :alias, :require]
B:  [            :use, :import, :alias, :type, :require]
X1: [:moduledoc, :use, :import, :alias, :type, :require]

Hope this helps to clarify the point of consistency for this case. If you have any questions, feel free to ask 👍

P.S. As an aside:

If this fails too the check falls back to the order listed in the community styleguide and prints a warning.

This should not happen. Consistency checks by nature do not bother the user unless something inconsistent can be detected in the present code (and this exception would yell at the user about a style guide, which Credo does not aim to implement).