Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Question] What to do about used/unused variable warnings? #31

Open
coladarci opened this issue Feb 10, 2019 · 8 comments
Open

[Question] What to do about used/unused variable warnings? #31

coladarci opened this issue Feb 10, 2019 · 8 comments

Comments

@coladarci
Copy link

Forgive me for using this as a venue to get a recommendation, but I'm having a really hard time even forming my question in other venues, so I thought I'd try it out here since this library is what spurred this question.

My goal here is to explore adding permission checks to my context functions.

@decorate permit(:create_thing)
def create_thing(_user, params), do:
   %Thing{} |> Thing.changset(params) |> Repo.insert()

I have setup permit to use the first argument provided to the decorated function (the user):

def permit(action, body, %{args: [user, _params]}) do
   with :ok -> Permissions.can(user, action) do
     unquote(body)
   end
end

Note that in the above use-case, the user is actually only used in the decorated code, NOT the function being decorated. This confuses the compiler and gives warnings (if it's underscored, I'm told that it's being used even though it's underscored. If it's NOT underscored, I get warnings saying it's unused).

I have to note that this entire thing smells bad because my best case scenario here is that a seemingly unused arg is actually being used (or vice-versa). This makes me seriously question my API choices, but I think I'm onto a very powerful use of this library.

SO, my question is:

  1. Is what I'm running into something that feels like a bug somewhere? Is it a common issue?
  2. Seeing what I'm attempting to accomplish, am I missing anything obvious that might be a better API approach?

Thanks for a great library - appreciate all the work that went into it!

@geonnave
Copy link

+1, I am also having this issue. To complement, if I remove the _ from the beginning of the variable name, the compiler complains that the variable is unused; but if I add the _, it will say that it is used (probably because of the macro using it) and should not have a _.

Weird behaviours! 🙃

@coladarci
Copy link
Author

Yeah - basically, I realized that I was in a lose-lose here... the idea that you write code that uses things that look like they aren't used is terrible :) ha.. was still curious if anyone else solved a similar use-case, but this is not the right venue for that, I suspect.

@arjan
Copy link
Owner

arjan commented Feb 26, 2019

I will have a look.. has been some time since I wrote this, I'll see if I can make the generated code play nicer with the compiler.

@arjan
Copy link
Owner

arjan commented Feb 27, 2019

This is more complicated than I expected. Tried detecting and renaming the variables on compile time but that does not seem to help much.

@BenMorganIO
Copy link

I'm experiencing this issue as well....

@arjan
Copy link
Owner

arjan commented Aug 5, 2019

I have not continued with this yet, like I said, renaming the variables on compiletime does not work, it seem we are in a catch-22 here.

A solution would be to not include this unused argument in the function declaration, but instead let the decorator automatically add this "context" argument as the first (or last?) argument.

This would require a different annotation though, for instance called @decorate_with_context:

@decorate_with_context permit(:create_thing)
def create_thing(params), do:
   %Thing{} |> Thing.changset(params) |> Repo.insert()

This would NOT define create_thing/1, but create_thing/2 with the first argument passed through into the decorator.

def permit(action, body, %{args: [user, _params]}) do
  quote do
    with :ok -> Permissions.can(user, action) do
      unquote(body)
    end
  end
end

@robacourt
Copy link

I have this issue too. The workaround I've used may be useful for others if you know the type of your unused variable.

Instead of:

@decorate permit(:create_thing)
def create_thing(_user, params), do: ...

If you know that user is a map you can do:

@decorate permit(:create_thing)
def create_thing(%{}, params), do:

Not great, but if you have warnings as errors as we do, this workaround helped.

@Arp-G
Copy link

Arp-G commented Aug 25, 2023

Using Kernel.binding/1 I was able to get rid of the un-used variable warnings, It's not the best way and seems like a hack, but it's all I would find for now.

Thanks to this PR for the idea

Example usage:

@decorate with_fallback()
def handle_event("accept_event", _, %{assigns: assigns} = socket) do

Then

def with_fallback(opts, body, _context) do
  quote do
    try do
      unquote(body)
    catch
      %DataSource.Error{} = error ->
      # handle error
      socket = Keyword.get(binding(), :socket)
      {:noreply, socket}
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants