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

feature request: automatically migrate struct update syntax into pattern matching #199

Open
SteffenDE opened this issue Nov 9, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@SteffenDE
Copy link

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
Copy link
Contributor

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

@novaugust novaugust added the enhancement New feature or request label Nov 9, 2024
@josevalim
Copy link

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?

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

No branches or pull requests

3 participants