I’m trying to change a create action in a Phoenix controller so that I can manipulate uploaded files (the purpose is also to learn Elixir as my knowledge is basic) but I’m getting an error and a warning with this piece of (simplified) code:
` def create(conn, %{"img" => img_params}) do
if upload = img_params["file"] do
filePath = "/path/to/file/uploaded/#{upload.filename}"
File.cp(upload.path, filePath)
end
img_params["path"] = filePath
end`
WARNING: the variable “filePath” is unsafe as it has been set inside a case/cond/receive/if/&&/||. Please explicitly return the variable value instead. For example:
`case int do
1 -> atom = :one
2 -> atom = :two
end`
should be written as
` atom =
case int do
1 -> :one
2 -> :two
end`
ERROR: cannot invoke remote function Access.get/2 inside match referring to this part of the code: img_params["path"] = filePath.
I must confess I don’t understand what the problems are.
WARNING: the variable “filePath” is unsafe as it has been set inside a case/cond/receive/if/&&/||. Please explicitly return the variable value instead.
This suggests to avoid binding a variable inside the if because the scoping might work differently from what you expect: that is, the variable defined inside the if body will leak to outside the if body. Take this minimal example:
if "something" do
a = 1
end
a
#=> 1
The compiler is suggesting to explicitly bind the variable outside of the if, for example:
file_path =
if upload = ... do
path = "/path/to..."
File.cp(...)
path
end
# ...
cannot invoke remote function Access.get/2 inside match referring to this part of the code:
img_params[“path”] = filePath.
This is saying that you cannot invoke a function call (foo[bar] compiles to a function call to the Access.get/2 function) inside a match. My guess is you’re trying to do assignment of a field in the img_params, but that doesn’t work in Elixir because data is immutable and you would have to return a whole new img_params to modify one field of it. In this case, you have a few alternatives but the basic idea is you want to update a map by changing the value of one of its fields. Here are some possible ways of doing that:
%{img_params | "path" => file_path} # if you know the "path" key is present, would raise otherwise
Map.update!(img_params, "path", file_path) # same as above
Map.put(img_params, "path", file_path) # replace if exists, put if doesn't exist
Thank you.
The error is now fully clear.
But regarding the warning, although I understand what you mean, I still don’t understand the need, because I want exactly that if a condition is true (and only then) to define and assign a value to a variable that can be accessed by other code.
So, is it more a code styling or there’s a good reason/use case for not doing it?
Well, the leaking assignment only happened in a few language constructs, but not in others. In a longer discussion to harmonize these variants, it has been decided to remove the “imperative assignment”. But to avoid breaking legacy code, it was only deprecated for now and these “imperative” assignments will get removed in elixir 2.0.
Your code currently lacks an important bit of information, what shall happen if there is no img_params["file"] available?
Of course it will break because filePath beeing not bound in the img_params["path"] = filePath line (assuming this were valid elixir). But the content of the stack trace might be misleading or missing some important bit of information.
Of course crashing is very idiomatic, but one should also always give a reason when crashing, so doing roughly this were much better (I think):
I understand that and you’re right (I only posted a small code snippet to focus on the issues).
My latest question was about the warning, not the error:
