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

Use prism v0.26.0 #1953

Merged
merged 1 commit into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ PATH
specs:
ruby-lsp (0.16.5)
language_server-protocol (~> 3.17.0)
prism (>= 0.23.0, < 0.25)
prism (>= 0.23.0, < 0.27)
sorbet-runtime (>= 0.5.10782)

GEM
Expand Down Expand Up @@ -45,7 +45,7 @@ GEM
ast (~> 2.4.1)
racc
prettier_print (1.2.1)
prism (0.24.0)
prism (0.26.0)
psych (5.1.2)
stringio
racc (1.7.3)
Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_indexer/lib/ruby_indexer/collector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Collector

LEAVE_EVENT = T.let(Object.new.freeze, Object)

sig { params(index: Index, parse_result: Prism::ParseResult, file_path: String).void }
sig { params(index: Index, parse_result: Prism::ParseResult[Prism::ProgramNode], file_path: String).void }
def initialize(index, parse_result, file_path)
@index = index
@file_path = file_path
Expand Down
6 changes: 3 additions & 3 deletions lib/ruby_lsp/document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class Document

abstract!

sig { returns(Prism::ParseResult) }
sig { returns(Prism::ParseResult[Prism::ProgramNode]) }
attr_reader :parse_result

sig { returns(String) }
Expand All @@ -31,7 +31,7 @@ def initialize(source:, version:, uri:, encoding: Encoding::UTF_8)
@version = T.let(version, Integer)
@uri = T.let(uri, URI::Generic)
@needs_parsing = T.let(true, T::Boolean)
@parse_result = T.let(parse, Prism::ParseResult)
@parse_result = T.let(parse, Prism::ParseResult[Prism::ProgramNode])
end

sig { returns(Prism::ProgramNode) }
Expand Down Expand Up @@ -93,7 +93,7 @@ def push_edits(edits, version:)
@cache.clear
end

sig { abstract.returns(Prism::ParseResult) }
sig { abstract.returns(Prism::ParseResult[Prism::ProgramNode]) }
def parse; end

sig { returns(T::Boolean) }
Expand Down
8 changes: 8 additions & 0 deletions lib/ruby_lsp/internal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
require "prism/visitor"
require "language_server-protocol"

# Prism v0.26.0 introduced generics for ParseResult, but it causes some problems so the intention is to remove it.
# Once that is done, we can remove this patch.
module Prism
class ParseResult
extend T::Generic
end
end

require "ruby-lsp"
require "ruby_lsp/base_server"
require "ruby_indexer/ruby_indexer"
Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_lsp/listeners/document_link.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def extract_document_link(node)
match = comment.location.slice.match(%r{source://.*#\d+$})
return unless match

uri = T.cast(URI(T.must(match[0])), URI::Source)
uri = T.cast(URI(match[0]), URI::Source)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorbet was complaining about the T.must on untyped, but I'm not sure why it wasn't previously.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(previously it was T.nilable(String).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should cast location.slice as String instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(@st0012 is investigating the underlying cause).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was caused by Tapioca dropping signatures of attr_reader methods (issue). I've opened ruby/prism#2719 to convert them into normal method declarations (which Tapioca does anyway).

I also tested that when generating the rbi file from that branch, this change will not be necessary.

gem_version = resolve_version(uri)
return if gem_version.nil?

Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_lsp/ruby_document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

module RubyLsp
class RubyDocument < Document
sig { override.returns(Prism::ParseResult) }
sig { override.returns(Prism::ParseResult[Prism::ProgramNode]) }
def parse
return @parse_result unless @needs_parsing

Expand Down
2 changes: 1 addition & 1 deletion ruby-lsp.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Gem::Specification.new do |s|
s.require_paths = ["lib"]

s.add_dependency("language_server-protocol", "~> 3.17.0")
s.add_dependency("prism", ">= 0.23.0", "< 0.25")
s.add_dependency("prism", ">= 0.23.0", "< 0.27")
s.add_dependency("sorbet-runtime", ">= 0.5.10782")

s.required_ruby_version = ">= 3.0"
Expand Down
Loading
Loading