Skip to content

Prettier swallows parenthesis when doing pattern matching #1018

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

Closed
rindek opened this issue Oct 20, 2021 · 6 comments
Closed

Prettier swallows parenthesis when doing pattern matching #1018

rindek opened this issue Oct 20, 2021 · 6 comments

Comments

@rindek
Copy link

rindek commented Oct 20, 2021

Metadata

  • Operating system: macOS Catalina

  • Ruby version: 3.0.2

  • Node version: v14.17.6

  • @prettier/plugin-ruby or prettier gem version: @prettier/plugin-ruby@2.0.0-rc4

  • Options:

    • rubyArrayLiteral - true
    • rubyHashLabel - true
    • rubyModifier - true
    • rubySingleQuote - false
    • rubyToProc - false
    • trailingComma - "all"

Input

class Test
  def compose_words(words)
    case words
    in [first, second]
      words.join(" i ")
    in [first]
      words[0]
    in [*something, second, first]
      [something.join(", "), [second, first].join(" i ")].join(", ")
    else
      ""
    end
  end
end

Current output

class Test
  def compose_words(words)
    case words
    in first, second
      words.join(" i ")
    in first
      words[0]
    in *something, second, first
      [something.join(", "), [second, first].join(" i ")].join(", ")
    else
      ""
    end
  end
end

Expected output

class Test
  def compose_words(words)
    case words
    in [first, second]
      words.join(" i ")
    in [first]
      words[0]
    in [*something, second, first]
      [something.join(", "), [second, first].join(" i ")].join(", ")
    else
      ""
    end
  end
end

Removal of square parenthesis caused the code to be bugged

@kddnewton
Copy link
Member

Hmm looks like it creates a bug only in the [first] -> first case right?

@rindek
Copy link
Author

rindek commented Oct 20, 2021

No no, parenthesis must be maintained in order to properly match the array elements

Here is the tests how it should work:

  describe "#compose_words" do
    subject { compose_words(words) }

    context "no words" do
      let(:words) { [] }
      it { is_expected.to eq("") }
    end

    context "one word" do
      let(:words) { ["one"] }
      it { is_expected.to eq("one") }
    end

    context "two words" do
      let(:words) { %w[one two] }
      it { is_expected.to eq("one i two") }
    end

    context "three words" do
      let(:words) { %w[one two three] }
      it { is_expected.to eq("one, two i three") }
    end

    context "four words" do
      let(:words) { %w[one two three four] }
      it { is_expected.to eq("one, two, three i four") }
    end
  end

When parenthesis are there in every in place, then it all passes, if there is no parenthesis anywhere, it passes only two words example - for matching an array elements, [] must be there, as it is not case when but case in

@kddnewton
Copy link
Member

Doesn't this pass your tests?

class Test
  def compose_words(words)
    case words
    in first, second
      words.join(" i ")
    in [first]
      words[0]
    in *something, second, first
      [something.join(", "), [second, first].join(" i ")].join(", ")
    else
      ""
    end
  end
end

@rindek
Copy link
Author

rindek commented Oct 20, 2021

Oh right, it does!

@rindek
Copy link
Author

rindek commented Oct 20, 2021

TIL, I actually did not know that you can omit the parenthesis to properly match multiple array elems
But yes, it match only having one elem when prettier replaces [first] -> first it breaks

kddnewton added a commit that referenced this issue Oct 20, 2021

Verified

This commit was signed with the committer’s verified signature.
kddnewton Kevin Newton
@kddnewton kddnewton mentioned this issue Oct 20, 2021
kddnewton added a commit that referenced this issue Oct 20, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Fix #1018
@rindek
Copy link
Author

rindek commented Oct 20, 2021

Wow, that was fast! Big thanks @kddnewton :)

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

No branches or pull requests

2 participants