Proposal: Restrict AliasUsage to a list of specific modules

[christiannelson]

Environment

• Credo version (mix credo -v): 0.6.1
• Erlang/Elixir version (elixir -v): 1.4.1
• Operating system: MacOS 10.12.3

Proposal

We like the AliasUsage check, but would like to restrict it so a set of modules. For example, we prefer the explicitness of Ecto.Changeset over Changeset, since Changeset comes from a library. We dislike seeing ProjectModule.<Module> in our application code (where ProjectModule is our app's top-level module.

We propose a configuration parameter so that the check could be limited to one or more specific modules names, like:

{Credo.Check.Design.AliasUsage, only: [ProjectModule, ModuleB]},
# OR
{Credo.Check.Design.AliasUsage, only: ProjectModule},

In the second case, AliasUsgae would only raise warnings when something like ProjectModule.User is encountered.

What do you think?

[rrrene]

@christiannelson I think this is a great addition to the AliasUsage check! :+1:

I would use a Regex instead of a List and match the encountered name against that.

# you can explicitly match a single module using ^ and $
{Credo.Check.Design.AliasUsage, only: ~r/^ProjectModule\.ModuleB$/]},

# or match a namespace by just matching the beginning of the name
{Credo.Check.Design.AliasUsage, only: ~r/^ProjectModule/]},

WDYT?

[christiannelson]

@rrrene That would certainly work for our use case, since we mostly want to discourage using our toplevel module throughout our application code.

What would flagging multiple modules look like? We could craft a regex using the OR operator like this: `~r/^HalfDome.|ProjectModule./

That would get cumbersome if someone curates a list of more than 2-3 modules to include. Do you think that's okay?

I personally find the or regex syntax a little bit of an eyesore. What if only could be a single regex or an list of them?

# A single regex...
{Credo.Check.Design.AliasUsage, only: ~r/^ProjectModule\.$/]}
# or a list of them...
{Credo.Check.Design.AliasUsage, only: [~r/^ProjectModule\./, ~r/^Ecto./]}

The first scenario is likely to be the most popular use case, but those users who want to include multiple can craft either a single large regex or a list of simpler ones.

I chatted with my team about this, and another "requirement" came up: only triggering the warning if the same prefix shows up more than once. I haven't thought too much about it yet, but do you think I should capture it in another "proposal" issue?

[rrrene]

What if only could be a single regex or an list of them?

We can do this. In fact, there is already for that called CodeHelper.matches/2 which provides this functionality.

I haven't thought too much about it yet, but do you think I should capture it in another "proposal" issue?

Please do! :+1:

[igas]

This looks great! ❤️ I have another use case for that feature. I have Faker.DateTime and it suggests to use alias, but in this case it'll be built in module. This regex will help to filter it out.

[rrrene]

@igas What you propose is already supposed to work today. We will just have to add DateTime to this list: https://github.com/rrrene/credo/blob/4bb23b87555dc196a7cabda9b6c0ebea7140bd17/lib/credo/check/design/alias_usage.ex#L51

If you would like to submit a PR updating that list of Elixir standard modules, I would gladly accept it! :+1: