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

Support Ruby 2.7.0 #34

Merged
merged 4 commits into from
Sep 6, 2022
Merged

Conversation

raxod502
Copy link
Contributor

@raxod502 raxod502 commented Sep 3, 2022

Similar to ruby-syntax-tree/syntax_tree#144, this package is also a dependency of running Prettier for Ruby code on Ubuntu 20.04.

Since @kddnewton was kind enough to add Ruby 2.7.0 support to syntax_tree proper in ruby-syntax-tree/syntax_tree#148, I figured I would try to update syntax_tree-rbs myself. Unfortunately, having little experience with Ruby, my work has proceeded very slowly. I get these errors when running the tests under 2.7.0:

186) Error:
RBSTest#test_class:
FrozenError: can't modify frozen String: "class Foo\nend\n"
    /home/raxod502/files/code/lib/syntax_tree-rbs/.bundle/vendor/ruby/2.7.0/gems/prettier_print-0.1.0/lib/prettier_print.rb:274:in `<<'
    /home/raxod502/files/code/lib/syntax_tree-rbs/.bundle/vendor/ruby/2.7.0/gems/prettier_print-0.1.0/lib/prettier_print.rb:761:in `block in flush'
    /home/raxod502/files/code/lib/syntax_tree-rbs/.bundle/vendor/ruby/2.7.0/gems/prettier_print-0.1.0/lib/prettier_print.rb:761:in `each'
    /home/raxod502/files/code/lib/syntax_tree-rbs/.bundle/vendor/ruby/2.7.0/gems/prettier_print-0.1.0/lib/prettier_print.rb:761:in `flush'
    /home/raxod502/files/code/lib/syntax_tree-rbs/lib/syntax_tree/rbs.rb:51:in `format'
    /home/raxod502/files/code/lib/syntax_tree-rbs/test/rbs_test.rb:343:in `assert_format'
    /home/raxod502/files/code/lib/syntax_tree-rbs/test/rbs_test.rb:163:in `test_class'

187) Error:
RBSTest#test_interface_5:
TypeError: no implicit conversion of Integer into Array
    /home/raxod502/files/code/lib/syntax_tree-rbs/.bundle/vendor/ruby/2.7.0/gems/prettier_print-0.1.0/lib/prettier_print.rb:782:in `-'
    /home/raxod502/files/code/lib/syntax_tree-rbs/.bundle/vendor/ruby/2.7.0/gems/prettier_print-0.1.0/lib/prettier_print.rb:782:in `flush'
    /home/raxod502/files/code/lib/syntax_tree-rbs/lib/syntax_tree/rbs.rb:51:in `format'
    /home/raxod502/files/code/lib/syntax_tree-rbs/test/rbs_test.rb:343:in `assert_format'
    /home/raxod502/files/code/lib/syntax_tree-rbs/test/rbs_test.rb:16:in `block (2 levels) in test_fixtures'

For some reason, the pretty-printing library seems like it's trying to modify one of the string literals that gets passed in from the test fixtures in syntax_tree-rbs, and I haven't been able to figure out why that's happening yet. The tests for prettier_print pass under 2.7.0, so I figure it isn't a problem within that library (or at least it's a bug that isn't known yet).

I'll continue working on this when I have time, but if anyone more familiar with Ruby wants to take a look, it'd probably speed things up.

@raxod502
Copy link
Contributor Author

raxod502 commented Sep 3, 2022

Whoopsie, I was just being silly. Once I fixed my first change, the errors I mentioned above went away. I'll see about updating documentation and build files.

@raxod502
Copy link
Contributor Author

raxod502 commented Sep 3, 2022

It looks like the CI here is configured to run using the workflow from main, but I ran the Rake tests locally using Ruby 2.7.0 and they passed. I also installed this branch into my Ubuntu 20.04 machine and confirmed that Prettier now works for Ruby code. I think this should be ready for review!

@kddnewton kddnewton merged commit 568684f into ruby-syntax-tree:main Sep 6, 2022
@kddnewton
Copy link
Member

Yeah looks good!

@raxod502 raxod502 deleted the rr-ruby-270 branch September 11, 2022 00:15
@raxod502
Copy link
Contributor Author

Mind tagging this as 0.5.1 so the new version can be installed by default?

@kddnewton
Copy link
Member

Yeah this is released!

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

Successfully merging this pull request may close these issues.

2 participants