Credo rule to dissallow calls to specific functions

I haven’t seen such rule and don’t know how to implement it but I’m curious about your use case as I too want to do something similar. In my case it’s related to capability-based development.

I made one to ban assign/2. then is next

defmodule GelaSkins.Credo.NoCallsToAssign2 do
  @moduledoc """
  Checks that `Phoenix.LiveView.assign/2` can't be called.
  """

  # you can configure the basics of your check via the `use Credo.Check` call
  use Credo.Check,
    base_priority: :high,
    category: :custom,
    exit_status: 0,
    explanations: [
      check: """
      Always use `assign/3` in favour of `assign/2`.

      This forces a pipeline when using multiple assigns.  The advantage here is
      that assigns may be added, removed, and re-ordered easily (no commas to deal
      with) making diffs nicer.  It also means you must explicitly name the
      parameters accepted by LiveComponents.  You cannot simply do
      `assign(socket, assigns)`.
      """
    ]

  @doc false
  @impl true
  def run(%SourceFile{} = source_file, params \\ []) do
    # IssueMeta helps us pass down both the source_file and params of a check
    # run to the lower levels where issues are created, formatted and returned
    issue_meta = IssueMeta.for(source_file, params)

    # Finally, we can run our custom made analysis.
    # In this example, we look for lines in source code matching our regex:
    Credo.Code.prewalk(source_file, &traverse(&1, &2, [], issue_meta))
  end

  defp traverse({:|>, _, [{:socket, _, _}, {:assign, meta, [_]}]} = ast, issues, [], issue_meta) do
    {ast, issues ++ [issue_for(:assign, meta[:line], issue_meta)]}
  end

  defp traverse({:assign, meta, [{:socket, _, _}, [_]]} = ast, issues, [], issue_meta) do
    {ast, issues ++ [issue_for(:assign, meta[:line], issue_meta)]}
  end

  defp traverse(ast, issues, _, _issue_meta) do
    {ast, issues}
  end

  defp issue_for(trigger, line_no, issue_meta) do
    format_issue(
      issue_meta,
      message: "Only use assign/3",
      line_no: line_no,
      trigger: trigger
    )
  end
end

It also checks for a single pipe into assign. I can’t remember why I did this instead of using the existing credo rule but I assume it’s because sometimes I’m ok with single pipes. This was a while ago.

Nice! This is exactly what I was looking for.

Do you think it’s possible to define all the banned functions in a single check or there are advantages to having them separated?

I’d say it’s definitely fine to do all of them in a single check. Certainly less code. I think the only advantage in using multiple checks is giving more details about why specific functions are banned per error as opposed to just a big “don’t a or b or c or d” but if you aren’t distributing it then one big one is fine.

Rereading this a perhaps slightly clearer way to write the check would be:

  defp traverse({:assign, meta, [args, [_]]} = ast, issues, [], issue_meta)
       when elem(args, 0) == :socket and len(args) == 3 do
    {ast, issues ++ [issue_for(:assign, meta[:line], issue_meta)]}
  end

EDIT: I royally hecked up my refactor there—it made no sense as I was using elem on a list and && in a guard Fixed it, but now it’s only maybe marginally better.

EDIT 2: I should really stop answering questions first thing in the morning. I re-edited it, not that is really matters I also realized the way I had it there it’s possible to get around the rule by not calling the socket variable socket, so perhaps you don’t even want to check for that. Ok, I’m really done now (probably, lol).

Last update: I release I was wrong, based on the first arg to issue_for you can determine which function was called and tailor the error there. So really it just seems to be about the moduledoc as well as being able to easily turn certain ones on and off, but in your situation I would say there are no disadvantages.

Do you need to have a Credo rule for this or is the goal just to block calls to those functions? If you don’t need to necessarily use Credo, take a look at the Boundary library.

I am already using Boundary, but that is only good to organize the application code. I need specifically to block functionality either from standard or external libraries.

Doesn’t it actually raise warnings at compilation time? I imagine that could be paired with mix compile --warning-as-errors, no!? From the roadmap, it seems it also supports validating calls to external deps.

I think theoretically you could do that, however I think that using Boundary in this case is not a good fit, especially since I have multiple top boundaries in my application.

I personally use Boundary to organize business logic in well-structured module hierarchies. Banning function calls falls in the lint region of the project, where this is more of a enforceable guideline to follow for existing and new code.

I mean, that’s a core feature of the library, but since it’s a matter of preference, I’m glad you found something that works for you with Credo. Good luck with it