feature request: automatically migrate struct update syntax into pattern matching

[SteffenDE]

Relates to: elixir-lang/elixir#13974

It is planned to deprecate the struct update syntax %MyStruct{x | ...} in a future Elixir release. I asked José if this is a feature they're planning to integrate into mix format --migrate, but it is not likely at the moment.

Since styler also added an automatic rewrite for unless to !if, this would be a nice feature, although it's probably it bit more complicated.

[novaugust]

thanks for bringing this up @SteffenDE!
styler definitely does more than a few deprecation rewrites, so i'm happy to add another. i wouldn't be surprised if formatter itself starts doing more of them though, since they did add the unless => if! rewrite on their end too.

(the following is me thinking out loud, pardon me)

for this new deprecation, jose gives this:

# bad
for entry <- conf.entries do
  %UploadEntry{entry | preflighted?: entry.preflighted? || entry.ref in refs}
end

# good
for %UploadEntry{} = entry <- conf.entries do
  %{entry | preflighted?: entry.preflighted? || entry.ref in refs}
end

making this change with styler is trivial, both to detect and to fix:

-  %UploadEntry{entry | preflighted?: entry.preflighted? || entry.ref in refs}
+  %{entry | preflighted?: entry.preflighted? || entry.ref in refs}

whereas this one i'd have to spend time experimenting with to see how do-able it is:

-for entry <- conf.entries do
+for %UploadEntry{} = entry <- conf.entries do

as it'd require backtracking in the ast upon encountering the %Struct{var | ..} syntax to find where var was defined. i don't think there's anywhere equivalent to that in styler's existing behaviours...

anyways, the second change isn't necessary as part of removing the deprecated syntax. but it sure would be nice to do for folks who are concerned about the type warnings that structs provide

[josevalim]

Unfortunately I would say that the second part is essential. So if we can't do it automatically, it may be best to ask users to do it. Perhaps what Styler could do is to remove the update syntax IF the variable has been bound to the same struct before. This way, Styler can remove the redundant information, and users are only required to refactor it when it is not redundant?

[novaugust]

fun as it would've been, i never found the time to walk backwards through the AST in a context-aware way searching for a variable's assignment location to determine whether it was already pattern matching against a struct. further, there are scenarios where that would be impossible to accomplish, like this:

x = foo()

if bar do
  %A{x | ...}
else
  %B{x | ...}
end

so given that, I'm just going to ship an update that deletes this syntax when encountered. looking at our own codebase at frame, it seems that when authors care enough about struct "types" that they're using the struct update syntax, you're guaranteed that they also used the struct match when assigning the variable

[novaugust]

apologies, i thought this deprecation shipped in 1.18 but i see it's actually 1.19. there's still time!

still no promises i'll get to it 🤞

[novaugust]

time or not, i'm just going to ship deletion sans assignment checking with version 1.4 because we want it internally :) sorry folks
6896d97