Merge pull request #2561 from ruby/uk-typecheck-with-typed-true

Run Sorbet type-checking with `typed: true`

.github/workflows/main.yml

@@ -49,12 +49,12 @@ jobs:
       with:
         ruby-version: "3.3"
         bundler-cache: true
-    - name: Check annotations
-      run: |
-        bundle exec rake typecheck:tapioca typecheck:sorbet
-        bundle exec rake typecheck:steep
-        rm lib/prism/node.rb && CHECK_FIELD_KIND=true bundle exec rake
-
+    - name: Check Sorbet
+      run: bundle exec rake typecheck:tapioca typecheck:sorbet
+    - name: Check Steep
+      run: bundle exec rake typecheck:steep
+    - name: Check field kinds
+      run: rm lib/prism/node.rb && CHECK_FIELD_KIND=true bundle exec rake
   build:
     strategy:
       fail-fast: false

Rakefile

@@ -12,7 +12,7 @@ task default: [:compile, :test]
 require_relative "templates/template"
 
 desc "Generate all ERB template based files"
-task templates: Prism::TEMPLATES
+task templates: Prism::Template::TEMPLATES
 
 task make: [:templates] do
   sh(RUBY_PLATFORM.include?("openbsd") ? "gmake" : "make")
@@ -23,7 +23,7 @@ task make_no_debug: [:templates] do
 end
 
 # decorate the gem build task with prerequisites
-task build: [:check_manifest, :templates, :"typecheck:sorbet", :"typecheck:steep"]
+task build: [:check_manifest, :templates]
 
 # the C extension
 task "compile:prism" => ["templates"] # must be before the ExtensionTask is created
@@ -50,13 +50,13 @@ elsif RUBY_ENGINE == "jruby"
 end
 
 # So `rake clobber` will delete generated files
-CLOBBER.concat(Prism::TEMPLATES)
+CLOBBER.concat(Prism::Template::TEMPLATES)
 CLOBBER.concat(["build"])
 CLOBBER << "lib/prism/prism.#{RbConfig::CONFIG["DLEXT"]}"
 
-Prism::TEMPLATES.each do |filepath|
+Prism::Template::TEMPLATES.each do |filepath|
   desc "Generate #{filepath}"
   file filepath => ["templates/#{filepath}.erb", "templates/template.rb", "config.yml"] do |t|
-    Prism.template(t.name)
+    Prism::Template.render(t.name)
   end
 end

lib/prism/parse_result.rb

@@ -87,9 +87,9 @@ def find_line(byte_offset)
 
       while left <= right
         mid = left + (right - left) / 2
-        return mid if offsets[mid] == byte_offset
+        return mid if (offset = offsets[mid]) == byte_offset
 
-        if offsets[mid] < byte_offset
+        if offset < byte_offset
           left = mid + 1
         else
           right = mid - 1
@@ -262,7 +262,7 @@ def pretty_print(q)
 
     # Returns true if the given other location is equal to this location.
     def ==(other)
-      other.is_a?(Location) &&
+      Location === other &&
         other.start_offset == start_offset &&
         other.end_offset == end_offset
     end
@@ -276,13 +276,6 @@ def join(other)
 
       Location.new(source, start_offset, other.end_offset - start_offset)
     end
-
-    # Returns a null location that does not correspond to a source and points to
-    # the beginning of the file. Useful for when you want a location object but
-    # do not care where it points.
-    def self.null
-      new(nil, 0, 0) # steep:ignore
-    end
   end
 
   # This represents a comment that was encountered during parsing. It is the
@@ -541,7 +534,7 @@ def pretty_print(q)
 
     # Returns true if the given other token is equal to this token.
     def ==(other)
-      other.is_a?(Token) &&
+      Token === other &&
         other.type == type &&
         other.value == value
     end

lib/prism/parse_result/newlines.rb

@@ -58,6 +58,8 @@ def visit_statements_node(node)
 
     # Walk the tree and mark nodes that are on a new line.
     def mark_newlines!
+      value = self.value
+      raise "This method should only be called on a parse result that contains a node" unless Node === value
       value.accept(Newlines.new(Array.new(1 + source.offsets.size, false))) # steep:ignore
     end
   end

lib/prism/polyfill/string.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+# Polyfill for String#unpack1 with the offset parameter.
+if String.instance_method(:unpack1).parameters.none? { |_, name| name == :offset }
+  String.prepend(
+    Module.new {
+      def unpack1(format, offset: 0) # :nodoc:
+        offset == 0 ? super(format) : self[offset..].unpack1(format) # steep:ignore
+      end
+    }
+  )
+end

lib/prism/translation/parser/compiler.rb

@@ -1806,7 +1806,7 @@ def visit_block(call, block)
 
         # Visit a heredoc that can be either a string or an xstring.
         def visit_heredoc(node)
-          children = []
+          children = Array.new
           node.parts.each do |part|
             pushing =
               if part.is_a?(StringNode) && part.unescaped.include?("\n")

lib/prism/translation/parser/lexer.rb

@@ -302,7 +302,7 @@ def to_a
                 index += 1
               end
             when :tFID
-              if !tokens.empty? && tokens[-1][0] == :kDEF
+              if !tokens.empty? && tokens.dig(-1, 0) == :kDEF
                 type = :tIDENTIFIER
               end
             end

lib/prism/translation/ripper.rb

@@ -1007,16 +1007,16 @@ def visit_call_node(node)
 
             bounds(last_argument.location)
             on_assign(call, value)
-          when :-@, :+@, :~@
+          when :-@, :+@, :~
             receiver = visit(node.receiver)
 
             bounds(node.location)
             on_unary(node.name, receiver)
-          when :!@
+          when :!
             receiver = visit(node.receiver)
 
             bounds(node.location)
-            on_unary(node.message == "not" ? :not : :!@, receiver)
+            on_unary(node.message == "not" ? :not : :!, receiver)
           when *BINARY_OPERATORS
             receiver = visit(node.receiver)
             value = visit(node.arguments.arguments.first)

prism.gemspec

@@ -87,6 +87,7 @@ Gem::Specification.new do |spec|
     "lib/prism/parse_result/comments.rb",
     "lib/prism/parse_result/newlines.rb",
     "lib/prism/pattern.rb",
+    "lib/prism/polyfill/string.rb",
     "lib/prism/serialize.rb",
     "lib/prism/translation.rb",
     "lib/prism/translation/parser.rb",

rakelib/typecheck.rake

@@ -24,7 +24,53 @@ namespace :typecheck do
 
   desc "Typecheck with Sorbet"
   task sorbet: :templates do
-    Process.wait(fork { exec Gem.bin_path("sorbet", "srb"), "typecheck" })
+    polyfills = Dir["lib/prism/polyfill/**/*.rb"]
+    gem_rbis = Dir["sorbet/rbi/**/*.rbi"]
+
+    File.write("sorbet/typed_overrides.yml", ERB.new(<<~YAML, trim_mode: "-").result(binding))
+      false:
+        - ./lib/prism/debug.rb
+        - ./lib/prism/lex_compat.rb
+        - ./lib/prism/node_ext.rb
+        - ./lib/prism/parse_result.rb
+        - ./lib/prism/visitor.rb
+        - ./lib/prism/translation/ripper.rb
+        - ./lib/prism/translation/ripper/ripper_compiler.rb
+        - ./lib/prism/translation/ripper/sexp.rb
+        - ./lib/prism/translation/ruby_parser.rb
+        # We want to treat all polyfill files as "typed: false"
+      <% polyfills.each do |file| -%>
+        - ./<%= file %>
+      <% end -%>
+        # We want to treat all RBI files as "typed: false"
+      <% gem_rbis.each do |file| -%>
+        - ./<%= file %>
+      <% end -%>
+    YAML
+
+    File.write("sorbet/config", <<~CONFIG)
+      --dir=.
+      --ignore=tmp/
+      --ignore=vendor/
+      --ignore=ext/
+      --ignore=test/
+      --ignore=rakelib/
+      --ignore=Rakefile
+      # Treat all files as "typed: true" by default
+      --typed=true
+      # Use the typed-override file to revert some files to "typed: false"
+      --typed-override=sorbet/typed_overrides.yml
+      # We want to permit initializing a class by constant assignment
+      --suppress-error-code=4022
+      # We want to permit redefining the existing method as a method alias
+      --suppress-error-code=5037
+      # We want to permit changing the type of a variable in a loop
+      --suppress-error-code=7001
+    CONFIG
+
+    Process.wait(fork do
+      exec "#{::Gem::Specification.find_by_name("sorbet-static").full_gem_path}/libexec/sorbet"
+    end)
   end
 
   desc "Typecheck with Steep"

rbi/prism/compiler.rbi

@@ -9,6 +9,6 @@ class Prism::Compiler
   sig { params(nodes: T::Array[T.nilable(Prism::Node)]).returns(T::Array[T.nilable(Result)]) }
   def visit_all(nodes); end
 
-  sig { params(node: T.nilable(Prism::Node)).returns(T::Array[T.nilable(Result)]) }
+  sig { params(node: Prism::Node).returns(T::Array[T.nilable(Result)]) }
   def visit_child_nodes(node); end
 end

rbi/prism/translation/ripper.rbi

@@ -1,6 +1,6 @@
 # typed: strict
 
-class Prism::Translation::Ripper < Prism::Translation::RipperCompiler
+class Prism::Translation::Ripper < Prism::Compiler
   Result = type_member
 
   sig { returns(T::Boolean) }
@@ -9,11 +9,11 @@ class Prism::Translation::Ripper < Prism::Translation::RipperCompiler
   sig { returns(T.nilable(Result)) }
   def parse; end
 
-  sig { params(source: String).returns(T.untyped) }
-  def self.sexp_raw(source); end
+  sig { params(source: String, filename: String, lineno: Integer, raise_errors: T.untyped).returns(T.untyped) }
+  def self.sexp_raw(source, filename = "-", lineno = 1, raise_errors: false); end
 
-  sig { params(source: String).returns(T.untyped) }
-  def self.sexp(source); end
+  sig { params(source: String, filename: String, lineno: Integer, raise_errors: T.untyped).returns(T.untyped) }
+  def self.sexp(source, filename = "-", lineno = 1, raise_errors: false); end
 end
 
 class Prism::Translation::Ripper::SexpBuilder < Prism::Translation::Ripper

sig/prism/parse_result.rbs

@@ -43,8 +43,6 @@ module Prism
     def deconstruct_keys: (Array[Symbol] keys) -> { start_offset: Integer, end_offset: Integer }
     def pretty_print: (untyped q) -> untyped
     def join: (Location other) -> Location
-
-    def self.null: () -> Location
   end
 
   class Comment

templates/ext/prism/api_node.c.erb

@@ -130,15 +130,15 @@ pm_ast_new(const pm_parser_t *parser, const pm_node_t *node, rb_encoding *encodi
 
             switch (PM_NODE_TYPE(node)) {
                 <%- nodes.each do |node| -%>
-                <%- if node.fields.any? { |field| [Prism::NodeField, Prism::OptionalNodeField, Prism::NodeListField].include?(field.class) } -%>
+                <%- if node.fields.any? { |field| [Prism::Template::NodeField, Prism::Template::OptionalNodeField, Prism::Template::NodeListField].include?(field.class) } -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                 case <%= node.type %>: {
                     pm_<%= node.human %>_t *cast = (pm_<%= node.human %>_t *) node;
                     <%- node.fields.each do |field| -%>
                     <%- case field -%>
-                    <%- when Prism::NodeField, Prism::OptionalNodeField -%>
+                    <%- when Prism::Template::NodeField, Prism::Template::OptionalNodeField -%>
                     pm_node_stack_push(&node_stack, (pm_node_t *) cast-><%= field.name %>);
-                    <%- when Prism::NodeListField -%>
+                    <%- when Prism::Template::NodeListField -%>
                     for (size_t index = 0; index < cast-><%= field.name %>.size; index++) {
                         pm_node_stack_push(&node_stack, (pm_node_t *) cast-><%= field.name %>.nodes[index]);
                     }
@@ -159,7 +159,7 @@ pm_ast_new(const pm_parser_t *parser, const pm_node_t *node, rb_encoding *encodi
                 <%- nodes.each do |node| -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                 case <%= node.type %>: {
-                    <%- if node.fields.any? { |field| ![Prism::NodeField, Prism::OptionalNodeField, Prism::FlagsField].include?(field.class) } -%>
+                    <%- if node.fields.any? { |field| ![Prism::Template::NodeField, Prism::Template::OptionalNodeField, Prism::Template::FlagsField].include?(field.class) } -%>
                     pm_<%= node.human %>_t *cast = (pm_<%= node.human %>_t *) node;
                     <%- end -%>
                     VALUE argv[<%= node.fields.length + 2 %>];
@@ -170,50 +170,50 @@ pm_ast_new(const pm_parser_t *parser, const pm_node_t *node, rb_encoding *encodi
 
                     // <%= field.name %>
                     <%- case field -%>
-                    <%- when Prism::NodeField, Prism::OptionalNodeField -%>
+                    <%- when Prism::Template::NodeField, Prism::Template::OptionalNodeField -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                     argv[<%= index %>] = rb_ary_pop(value_stack);
-                    <%- when Prism::NodeListField -%>
+                    <%- when Prism::Template::NodeListField -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                     argv[<%= index %>] = rb_ary_new_capa(cast-><%= field.name %>.size);
                     for (size_t index = 0; index < cast-><%= field.name %>.size; index++) {
                         rb_ary_push(argv[<%= index %>], rb_ary_pop(value_stack));
                     }
-                    <%- when Prism::StringField -%>
+                    <%- when Prism::Template::StringField -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                     argv[<%= index %>] = pm_string_new(&cast-><%= field.name %>, encoding);
-                    <%- when Prism::ConstantField -%>
+                    <%- when Prism::Template::ConstantField -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                     assert(cast-><%= field.name %> != 0);
                     argv[<%= index %>] = rb_id2sym(constants[cast-><%= field.name %> - 1]);
-                    <%- when Prism::OptionalConstantField -%>
+                    <%- when Prism::Template::OptionalConstantField -%>
                     argv[<%= index %>] = cast-><%= field.name %> == 0 ? Qnil : rb_id2sym(constants[cast-><%= field.name %> - 1]);
-                    <%- when Prism::ConstantListField -%>
+                    <%- when Prism::Template::ConstantListField -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                     argv[<%= index %>] = rb_ary_new_capa(cast-><%= field.name %>.size);
                     for (size_t index = 0; index < cast-><%= field.name %>.size; index++) {
                         assert(cast-><%= field.name %>.ids[index] != 0);
                         rb_ary_push(argv[<%= index %>], rb_id2sym(constants[cast-><%= field.name %>.ids[index] - 1]));
                     }
-                    <%- when Prism::LocationField -%>
+                    <%- when Prism::Template::LocationField -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                     argv[<%= index %>] = pm_location_new(parser, cast-><%= field.name %>.start, cast-><%= field.name %>.end);
-                    <%- when Prism::OptionalLocationField -%>
+                    <%- when Prism::Template::OptionalLocationField -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                     argv[<%= index %>] = cast-><%= field.name %>.start == NULL ? Qnil : pm_location_new(parser, cast-><%= field.name %>.start, cast-><%= field.name %>.end);
-                    <%- when Prism::UInt8Field -%>
+                    <%- when Prism::Template::UInt8Field -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                     argv[<%= index %>] = UINT2NUM(cast-><%= field.name %>);
-                    <%- when Prism::UInt32Field -%>
+                    <%- when Prism::Template::UInt32Field -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                     argv[<%= index %>] = ULONG2NUM(cast-><%= field.name %>);
-                    <%- when Prism::FlagsField -%>
+                    <%- when Prism::Template::FlagsField -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                     argv[<%= index %>] = ULONG2NUM(node->flags & ~PM_NODE_FLAG_COMMON_MASK);
-                    <%- when Prism::IntegerField -%>
+                    <%- when Prism::Template::IntegerField -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                     argv[<%= index %>] = pm_integer_new(&cast-><%= field.name %>);
-                    <%- when Prism::DoubleField -%>
+                    <%- when Prism::Template::DoubleField -%>
 #line <%= __LINE__ + 1 %> "<%= File.basename(__FILE__) %>"
                     argv[<%= index %>] = DBL2NUM(cast-><%= field.name %>);
                     <%- else -%>