D4no0

D4no0

Credo rule to dissallow calls to specific functions

I have a project currently that I want to ban calls to some functions. Currently, all of those functions are either from a library or standard elixir library.

Since I have already a credo pipeline running, I was wondering if it would be possible to define a custom rule for this, or maybe there is already an existing rule for this (I could not find it).

Has anyone managed to do this successfully?

Marked As Solved

sodapopcan

sodapopcan

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

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.

Also Liked

sodapopcan

sodapopcan

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 :grimacing: :sweat_smile: 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 :upside_down_face: 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).

sodapopcan

sodapopcan

@D4no0 is talking about outright banning functions everywhere. This is certainly a linting concern. Boundary is used to describe application boundaries, what we’re talking about in this thread is stylistic choices of what functions can be used.

krasenyp

krasenyp

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.

D4no0

D4no0

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?

sodapopcan

sodapopcan

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.

Where Next?

Popular in Questions Top

lessless
I believe there are people here who are dealing with CSV files import on the daily basis, and since Excel is a really popular tool there ...
New
gshaw
What is the idiomatic way of matching for not nil in Elixir? E.g., First way: defp halt_if_not_signed_in(conn, signed_in_account) when...
New
myronmarston
The Elixir Typespec docs show the following syntax for keyword lists in typespecs: # ... | [key: type] # keyword lis...
New
alice
Hey, Just curious what are the main benefits of Elixir compared to Clojure? When is Elixir more useful than Clojure and vice versa? Th...
New
LegitStack
I’m trying to make a websocket server in Phoenix or raw Elixir. I heard about gun, I think I could use cowboy, but since I’m not that sma...
New
kostonstyle
Hi all I want to have a unix time, from the current time plus 1 hour. DateTime.now + 1 hour How to get it in elixir? Thanks
New
chrisalley
ExUnit now has describe blocks which is a welcome addition coming from RSpec. In the docs, it states that nested hierarchies of describe ...
New
Exadra37
Sometimes I want to check if the input into a function is not a blank string. My first approach: defmodule Example do def do_stuff(s...
New
skosch
To my knowledge, put_in, Map.update etc. all have the one limitation of not automatically creating intermediate keys when needed (for exa...
New
jay1
Why is it that the mnesia database isn’t the most preferred database for use in Elixir/Phoenix?
New

Other popular topics Top

chrismccord
Phoenix 1.4.0 released Phoenix 1.4 is out! This release ships with exciting new features, most notably with HTTP2 support, improved deve...
688 30048 115
New
peerreynders
Manning 2016 Halloween weekend sale via Deal of the Day Friday, October 28 - Half off all MEAPs - code WM102816LT Saturday, October 29 ...
326 29600 154
New
jerry
Good day to you all. I have been struggling to get a query involving like and ilike to work. Can anyone assist me on this, please? pro...
New
fireproofsocks
I’m working on defining a simple Ecto schema for a table (in PostGres), but I don’t see where I can define a column as NOT NULL. Conside...
New
myronmarston
The Elixir Typespec docs show the following syntax for keyword lists in typespecs: # ... | [key: type] # keyword lis...
New
danschultzer
None of the current solutions worked well for me, so I went ahead and built a user management system from scratch. This project took far...
548 27727 240
New
chensan
I have a User schema with a :from_id field set to type :string: defmodule TweetBot.Repo.Migrations.CreateUsers do use Ecto.Migration ...
New
chrisalley
ExUnit now has describe blocks which is a welcome addition coming from RSpec. In the docs, it states that nested hierarchies of describe ...
New
9mm
I am constructing a JSON object (map) and I need to conditionally set a field. I’m trying to write proper elixir-way code… and I’m at a l...
New
AstonJ
by Lance Halvorsen Elixir and Phoenix are generating tremendous excitement as an unbeatable platform for building modern web application...
460 27162 124
New

We're in Beta

About us Mission Statement