Repository: prettier/plugin-ruby (javascript)

Issue:
TypeError: childNodes.forEach is not a function
## Metadata

Prettier 2.2.1, @prettier/plugin-ruby 1.5.0, default options.

## Input

```ruby
[
  ['先生小'], #
  ['小'],
]
```

```
[error] /tmp/foo.rb: TypeError: childNodes.forEach is not a function
[error]     at getSortedChildNodes (~/.config/yarn/global/node_modules/prettier/index.js:13931:14)
[error]     at decorateComment (~/.config/yarn/global/node_modules/prettier/index.js:13947:22)
[error]     at decorateComment (~/.config/yarn/global/node_modules/prettier/index.js:13963:7)
[error]     at decorateComment (~/.config/yarn/global/node_modules/prettier/index.js:13963:7)
[error]     at decorateComment (~/.config/yarn/global/node_modules/prettier/index.js:13963:7)
[error]     at decorateComment (~/.config/yarn/global/node_modules/prettier/index.js:13963:7)
[error]     at decorateComment (~/.config/yarn/global/node_modules/prettier/index.js:13963:7)
[error]     at ~/.config/yarn/global/node_modules/prettier/index.js:14041:5
[error]     at Array.forEach (<anonymous>)
[error]     at Object.attach (~/.config/yarn/global/node_modules/prettier/index.js:14028:12)
```

Method chaining and conversion to conditional modifier
## Metadata

* Ruby version: ruby 2.6.3p62
* `@prettier/plugin-ruby` or `prettier` gem version: 1.5.0
* Options: defaults

## Input

```ruby
def foo
  unless false
    true
  end.to_s
end

pp foo
```

(Prints `"true"`)

## Current output

```ruby
def foo
  true unless false.to_s
end

pp foo
```

(Prints `nil`)

## Expected output

Not sure, maybe:

```ruby
def foo
  (true unless false).to_s
end

pp foo
```



Discussion:
```
$ ruby --version
ruby 2.6.3p62 (2019-04-16 revision 67580) [universal.x86_64-darwin19]
```


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,11 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a
 
 ## [Unreleased]
 
+### Changed
+
+- [#799](https://github.com/prettier/plugin-ruby/issues/799) - jscheid, kddeisz - Multi-byte characters shouldn't give invalid source string bounds.
+- [#801](https://github.com/prettier/plugin-ruby/issues/801) - jscheid, kddeisz - When converting a conditional to the modifier form, make sure to add parentheses if there is a chained method call.
+
 ## [1.5.0] - 2021-01-21
 
 ### Added
diff --git a/bin/sexp b/bin/sexp
--- a/bin/sexp
+++ b/bin/sexp
@@ -20,7 +20,15 @@ PP.prepend(
   end
 )
 
-source = File.file?(ARGV[0]) ? File.read(ARGV[0]) : ARGV[0].gsub('\\n', "\n")
+source =
+  if !ARGV[0]
+    File.read('test.rb')
+  elsif File.file?(ARGV[0])
+    File.read(ARGV[0])
+  else
+    ARGV[0].gsub('\\n', "\n")
+  end
+
 parsed = Prettier::Parser.new(source).parse
 
 puts '=== SOURCE === '
diff --git a/src/ruby/parser.rb b/src/ruby/parser.rb
--- a/src/ruby/parser.rb
+++ b/src/ruby/parser.rb
@@ -21,7 +21,40 @@ module Prettier
 end
 
 class Prettier::Parser < Ripper
-  attr_reader :source, :lines, :scanner_events, :line_counts
+  # Represents a line in the source. If this class is being used, it means that
+  # every character in the string is 1 byte in length, so we can just return the
+  # start of the line + the index.
+  class SingleByteString
+    def initialize(start)
+      @start = start
+    end
+
+    def [](byteindex)
+      @start + byteindex
+    end
+  end
+
+  # Represents a line in the source. If this class is being used, it means that
+  # there are characters in the string that are multi-byte, so we will build up
+  # an array of indices, such that array[byteindex] will be equal to the index
+  # of the character within the string.
+  class MultiByteString
+    def initialize(start, line)
+      @indices = []
+
+      line
+        .each_char
+        .with_index(start) do |char, index|
+          char.bytesize.times { @indices << index }
+        end
+    end
+
+    def [](byteindex)
+      @indices[byteindex]
+    end
+  end
+
+  attr_reader :source, :lines, :scanner_events
 
   # This is an attr_accessor so Stmts objects can grab comments out of this
   # array and attach them to themselves.
@@ -40,9 +73,23 @@ def initialize(source, *args)
     @heredocs = []
 
     @scanner_events = []
-    @line_counts = [0]
+    @line_counts = []
+
+    # Here we're going to build up a list of SingleByteString or MultiByteString
+    # objects. They're each going to represent a string in the source. They are
+    # used by the `char_pos` method to determine where we are in the source
+    # string.
+    last_index = 0
 
-    @source.lines.each { |line| @line_counts << @line_counts.last + line.size }
+    @source.lines.each do |line|
+      if line.size == line.bytesize
+        @line_counts << SingleByteString.new(last_index)
+      else
+        @line_counts << MultiByteString.new(last_index, line)
+      end
+
+      last_index += line.size
+    end
   end
 
   def self.parse(source)
@@ -60,7 +107,7 @@ def self.parse(source)
   # this line, then we add the number of columns into this line that we've gone
   # through.
   def char_pos
-    line_counts[lineno - 1] + column
+    @line_counts[lineno - 1][column]
   end
 
   # As we build up a list of scanner events, we'll periodically need to go
diff --git a/src/utils/inlineEnsureParens.js b/src/utils/inlineEnsureParens.js
--- a/src/utils/inlineEnsureParens.js
+++ b/src/utils/inlineEnsureParens.js
@@ -1,4 +1,11 @@
-const needsParens = ["args", "assign", "assoc_new", "massign", "opassign"];
+const needsParens = [
+  "args",
+  "assign",
+  "assoc_new",
+  "call",
+  "massign",
+  "opassign"
+];
 
 // If you have a modifier statement (for instance an inline if statement or an
 // inline while loop) there are times when you need to wrap the entire statement


Test patch:
diff --git a/test/js/ruby/comments.test.js b/test/js/ruby/comments.test.js
--- a/test/js/ruby/comments.test.js
+++ b/test/js/ruby/comments.test.js
@@ -237,4 +237,15 @@ describe("comments", () => {
       return expect(content).toMatchFormat();
     });
   });
+
+  test("works with multi-byte characters", () => {
+    const content = ruby(`
+      [
+        ['先生小'], #
+        ['小']
+      ]
+    `);
+
+    return expect(content).toMatchFormat();
+  });
 });
diff --git a/test/js/ruby/nodes/conditionals.test.js b/test/js/ruby/nodes/conditionals.test.js
--- a/test/js/ruby/nodes/conditionals.test.js
+++ b/test/js/ruby/nodes/conditionals.test.js
@@ -166,6 +166,16 @@ describe("conditionals", () => {
         expect(`hash[:key] = ${keyword} false then :value end`).toChangeFormat(
           `hash[:key] = (:value ${keyword} false)`
         ));
+
+      test("wraps inline version with calls", () => {
+        const content = ruby(`
+          if true
+            false
+          end.to_s
+        `);
+
+        return expect(content).toChangeFormat("(false if true).to_s");
+      });
     });
   });