Edit: Sorry, I didn’t read carefully, my bad
1 Like
@Eiji true, you could define a generic module that could handle this, however …
With the same argument you would remove all of the shortcut syntax and only have fully qualified function names (no sigels, no `[..|..], …).
Even though it might be worthwhile to have those functions for newbies, it would be very cumbersome for those common cases.
2 Likes
Yes and no. Look that depending on how you write the code it may be less obvious:
user_input
|> Code.eval_string()
|> then(fn {struct, _bindings} -> struct end)
|> then(&apply(SomeModule, :update, [&1])
This way all compilers and tools would have to parse every string to support it and I guess there are much more cases that makes things like that complex.
Sigils are macros and that’s off-topic.
We have a functions for them and they’re called hd(list) and tl(list). Both could be used in pattern-matching instead of syntax notation. [head | tail] notation is for a List and not a sub-type (like structs are for maps), so it’s not the best example here.
Look that by default every struct does not implement access behaviour and there are hex packages to fix that and I wanted to propose something exactly like that i.e. extend the specific struct with the functions that are known well from a Map module (like merge, put or update), but with a help of metaprogramming we can make them more helpful for LSP user, so that the editor could propose the keys based on the oldest known patterns in the language like pattern-matching and spec.
From a credo user perspective this also reduces module dependencies, so that your module does not depend on Map and MyStruct, but only MyStruct. The code is still clear and we don’t lose any editor features.
Slightly off topic, but personally I believe the best practice is to have all structure mutations encapsulated in the respective functions and then invoke those functions from outside of the structure module (instead of updating the structure directly), e.g
ideally:
defmodule Foo do
defstruct [ :a, :b]
def update_a( foo, updater) do
Map.update!( foo, :a, updater)
end
def update_b( foo, updater) do
Map.update!( foo, :b, updater)
end
end
or at least
defmodule Foo do
defstruct [ :a, :b]
def update( foo, field, updater) do
Map.update!( foo, field, updater)
end
def update( foo, changes) do
Map.merge( foo, changes)
end
end
This approach enables far easier and less error prone refactoring and debugging as well as having the abstraction in question have the last say on validating the mutation intent.
My 2c
1 Like
This only gives readability. I made one step forward in my example. The pattern-matching in function heads and specifications should be more than enough for LSP and other tools to make autocomplete working. This is very easy solution and gives people more opportunity to care about docs and specs.
1 Like
Couldn’t the type system catch this bug without the pattern match, inferring from the update syntax that user is %User{}?
100% agreed
2 Likes
Unfortunately not in all scenarios. In this particular example yes, it would, but once you add conditionals and other constructs, it gets blurrier. Here is an example of a warning that would not be caught with the current inference but would with pattern matching:
def absolute_z(condition, point) do
point.w
if condition do
%Point{point | z: abs(point.z)}
else
point
end
end
Any conditional is problematic because we can only assume that point is a %Point{} in a specific branch, so we cannot propagate the inference up. If we assumed it to be a Point on all branches, it could lead to false positives. If you do %Point{} = point, it always works.
3 Likes
Also people sometimes tends to overuse some patterns like binding different values to the “same” variable.
user = struct(User, user)
# instead of
user = struct(User, user_params)
It’s the same when we pattern match in the if branch:
def absolute_z(condition, point) do
point.w
if condition do
%Point{} = point
%{point | z: abs(point.z)}
else
point
end
end
but I get the point
3 Likes
This made me think that pattern matching inside a branch (actually after the first usage of a variable in the scope) should be warned for variables from the outer scope in the same way struct updates are warned.
I am positive this is doable.
1 Like
Putting more emphasis on the match operator is preferable to relying on a special update syntax. There is some debate in this thread on plausible interpretations of the map/struct update syntax as a human reader. Pattern matching lacks this ambiguity.
I’m also on the fence.
Deprecation has impact on existing libraries, LLMs, LSPs, etc; and may or may not reduce the language surface.
The arguments for preferring the simpler map update syntax are also good, and would probably make sense in the introductory guides in the Elixir docs.
For now, I think I’d find ways to “suggest” the new syntax (doc updates, LSP hints), but not produce compiler warnings, and evaluate this again before v1.20.
Of all arguments this one have no sense. Some LLMs are so “smart” that they are trying to use square brackets instead of do … end block in Elixir. The language is really well documented, so any tool that at least want to pretend it’s smart should generate a valid code depending on the latest stable version that matches mix.exs version requirement. Any kind of algorithm/tool that does not do that is not good enough to even mention it.
Also keep in mind that often many chatbots are learned based on very old data like few years ago which means few stable versions back and bringing new feature every few years is terrible idea. Some sites correctly warns about false positives when using chatbots which is nothing surprising having in mind that chatbots are prepared for conversation purposes and therefore does not need to be as strict as we expect from for example LSP.
Existing hex packages for now would just warn a deprecation warning. Nobody should complain about that. Community and especially business rarely depends on abandoned projects. Teams behind LSP implementations and editor plugins are doing amazing work already, so I consider it a problem at all.
Why we should postpone warning message that would not affect the code at all? Fixing it in every repository is trivial and there is no sense of forcing “warnings as errors” for dependency compilation and even if many projects are already updated or even didn’t had such usage.
1 Like
I think the type system should be able to detect this as well. Effectively this would be the same as code:
def trim_address(user) do
tmp1 = user.adress
tmp2 = String.trim(tmp1)
%User{user | address: tmp2}
end
In tmp1 line we infer the user variable has to be a map with an adress field. When we reach the update line, we can see a type error - the user variable can’t be of the %User struct if it has an adress field. The error is reported on a different line, than if we asserted the type upfront, but it should still be an error.
This is informed by how eqWAlizer works, which might be different for semantic subtyping.
As a second point, I also have concern about the migration. Imagine I had a code like:
def foo(x, :update) do
%Foo{x | y: :updated}
end
def foo(x, _) do
x
end
With the migration as suggested, it would be changed to:
def foo(%Foo{} = x, :update) do
%{x | y: :updated}
end
def foo(x, _) do
x
end
but this has suddenly subtly different semantics: if anything other than the %Foo struct is passed, the old code would crash, the new code will silently fail the first clause and pass it through. This can be “remedied” with the separate assertion like:
def foo(x, :update) do
%Foo{} = x
%{x | y: :updated}
end
def foo(x, _) do
x
end
but that’s becoming quite verbose quickly.
8 Likes
Yes, in fact it does in main. 
I was concise in the original description as to not go too much into the typing aspects, but there are cases where it is ambiguous and we cannot, I gave one example here: Changes to the struct update syntax - #35 by josevalim
Good point. The migration would definitely require some care in certain cases such as the one you presented.
3 Likes
In the example you talk about propagating type information backward from the update - I agree that wouldn’t work. But propagating type information down from the field access should work just fine - and that’s the behaviour I talked about. In the example you give the at the first line the point.w expression should give a type of “has w field” to the point variable. When we reach the update expression, the type-checker should fail to unify the type of expected %Point struct with the “has w field” type since the struct doesn’t have a w field.
As I said, the error would be in a less helpful location, but there should be an error regardless.
For reference, here’s the snippet:
def absolute_z(condition, point) do
point.w
if condition do
%Point{point | z: abs(point.z)}
else
point
end
end
2 Likes
Yes, thank you, you are correct that indeed we propagate the point.w down and we already do that today! 
There are other cases where we would not be able to detect though, such as Map.get(point, :z) not being able to say it will always return default, while it would with pattern matching? But in a nutshell we will always catch more as we get smarter but there will always be something that we won’t be able to.
4 Likes
There are reasons you wouldn’t want that. One primary one is that you may have a function that accepts different structs that have the foo field. The map update syntax allows polymorphism across multiple struct types. Otherwise you’d have to do something like:
def put_foo(%module{foo: _} = thing, foo) do
%module{thing | foo: foo}
end
Whereas you can write:
def put_foo(%{foo: _} = thing, foo) do
%{thing | foo: foo}
end
5 Likes
I recently tried Elixir 1.19-rc0 on our codebase, and it produced ~500 warnings, which the formatter sadly can not fix easily.
The changes necessary are rather mechanical, and I consider it a pity that we can not disable the warning during a migration phase.
We usually do build with --warnings-as-errors and even if we didn’t, we fear that there are more important warnings hidden in the stream of warnings.
Despite my personal opinions about the update and tooling support, this change will massively slow down our adoption of 1.19 despite all the nice features it would add.
Going through those 500 places all at once will be annoying, and probably none of us seniors want to do that, neither do we want to have our junior (soon to come) through this monotony.
So enabling this warning gradually would massively help us (and probably other teams with larger code bases) to adopt Elixir 1.19.
It would be sad if the adoption of such a featurefull release would be delayed because of this one warning.
And even though I generally prefer the proposal from rc.1, and it would make it easier to change (only one line touched per warning), it still would be 500 warnings…
2 Likes
