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

Patching ranged slice of struct literals #130

Closed
mway opened this issue Jul 31, 2023 · 6 comments
Closed

Patching ranged slice of struct literals #130

mway opened this issue Jul 31, 2023 · 6 comments

Comments

@mway
Copy link
Member

mway commented Jul 31, 2023

I'm trying to write a patch to transform a ranged slice of (anonymous) struct literals. For example, I want to turn this:

for _, foo := range []struct {
  bar string
  baz string
} {
  {
    bar: "bar",
    baz: "baz",
  },
  // ...
} {
  // do something with foo
}

into this:

foos := []struct {
  bar string
  baz string
} {
  {
    bar: "bar",
    baz: "baz",
  },
  // ...
}

for _, foo := range foos {
  // do something with foo
}

I've tried minor variants of the following patch:

@@
var x identifier
@@
- for _, x := range []struct{ ... } {
  ...
- }
+ foos := []struct { ... }
+ for _, x := range foos {
  ...
+ }

which returns the following error:

0.minus.augmented:4:6: missing ',' before newline in composite literal (and 2 more errors)

@abhinav suggested adding a trailing ; to the elision, but that resulted in the same error.

I'm not sure if this is technically the same thing as #4, but I think for this case it doesn't seem practical to do it in two stages (e.g., a second match on the trailing } { would be extremely ambiguous).

Is this type of patch possible currently?

@mway
Copy link
Member Author

mway commented Jul 31, 2023

cc @abhinav @sywhang

@abhinav
Copy link
Contributor

abhinav commented Jul 31, 2023

@mway I misread the patch.
The issue was that the left and right sides of the patch did not independently make valid syntax. The []struct{ ... } is just the type, it was missing the []struct{ ... }{ a, b, c } bit.

Try this:

@@
var x identifier
@@
- for _, x := range []struct{ ... }{
+ foos := []struct { ... }{
    ...,
- } {
+ }
+ for _, x := range foos {
  ...
  }

It's a bit unreadable, but it appears to work on my local sample.

For a more readable version, the []struct{ ... }{ ... } would have to be all on one line, but right now it can't be because the ... matching is janky. I think there was an issue somewhere to give them names so that you could do something like:

@@
var x identifier
var fields, items dots // TODO: better name
@@
+foos := []struct{ fields }{ items }
-for _, x := range []struct{ fields }{ items } {
+for _, x := range foos {
  ...
  }

Which would eliminate all the bugginess with ... matching.

@abhinav
Copy link
Contributor

abhinav commented Jul 31, 2023

CC @lverma14

@mway
Copy link
Member Author

mway commented Jul 31, 2023

Thanks @abhinav, that worked. :)

As a minor thing, it does end up with the following:

foos := []struct{
  // ...
}{
  // ...
  { // last entry
    // ...
  }}

instead of

foos := []struct{
  // ...
}{
  // ...
  { // last entry
    // ...
  },
}

(note the }} instead of },\n}). This seems to be regardless of any attempt at "forcing" the trailing comma of the list initialization to be included (maybe an artifact of capturing from the ...,?). It wouldn't be a problem if this was something that gofmt or gofumpt corrected, but unfortunately they don't.

@abhinav
Copy link
Contributor

abhinav commented Jul 31, 2023

Yeah, my local experiment also showed the }}. For now, I would just sed the affected files to fix that. 😆

It's like that partly because ... is greedy about eating up comments and whitespace, so only what's necessary is left behind afterwards. There's definitely some room for improvement there.

@mway
Copy link
Member Author

mway commented Jul 31, 2023

Yeah, that's a reasonable workaround. Thanks for the help!

@mway mway closed this as completed Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants