feat(ruby): add static type checking with Sorbet (#585)

Ruby, like most scripting languages, is not statically typed. While
great for initial productivity, it can make changes challenging since
errors are only caught at runtime, and only if that path is exercised.

There are two type checking solutions for Ruby:
[Sorbet](https://sorbet.org/docs/overview) and
[RBS/Steep](https://github.com/soutaro/steep). I tried both with the UDB
code base. Based on that evaluation, I determined that Sorbet was higher
quality, had much better documentation, and more mature. It's also in
commercial use so is likely to be supported long-term.

Sorbet uses _gradual type checking_: you can add type signatures to
subsets of the code, and Sorbet only checks what is known. Thus, we can
start using it without annotating the entire code base up front.

This PR highlights the utility of the tool: it found several bugs
already.

There is a *cost* to using Sorbet, mostly in terms of added development
time

* The signature syntax needs to be understood by developers.
* Some extra [assertions]https://sorbet.org/docs/type-assertions are
needed in code aside from the function signatures
* Though I found the tool is surprisingly helpful in telling you exactly
what assertion is needed (even with an autocorrect option)
 

Let's discuss more here, but based on this experiment my recommendation
is that we merge this PR.

Author: dhower-qc

.devcontainer/devcontainer.json

@@ -31,7 +31,8 @@
         "zhwu95.riscv",
         "mathematic.vscode-pdf",
         "CraigMaslowski.erb",
-        "HowerLimited.udb-extension-pack-vscode"
+        "HowerLimited.udb-extension-pack-vscode",
+        "sorbet.sorbet-vscode-extension"
       ]
     }
   },

.gitignore

@@ -21,3 +21,4 @@ __pycache__/
 .pytest_cache/
 *.bak
 *.log
+sorbet

.gitmodules

@@ -14,3 +14,6 @@
 [submodule "ext/riscv-tests"]
 	path = ext/riscv-tests
 	url = https://github.com/riscv-software-src/riscv-tests.git
+[submodule "ext/rbi-central"]
+	path = ext/rbi-central
+	url = https://github.com/Shopify/rbi-central

.pre-commit-config.yaml

@@ -119,7 +119,7 @@ repos:
     rev: v5.0.2
     hooks:
       - id: reuse-lint-file
-        exclude: COMMIT_EDITMSG
+        exclude: COMMIT_EDITMSG|MERGE_MSG
 
   - repo: https://github.com/alessandrojcm/commitlint-pre-commit-hook
     rev: v9.22.0

.sorbet-config

@@ -0,0 +1,10 @@
+--dir
+lib
+--dir
+sorbet/rbi
+--file
+Rakefile
+--file
+backends/ext_pdf_doc/idl_lexer.rb
+--file
+backends/cpp_hart_gen/lib/template_helpers.rb

.tapioca-config.yml

@@ -0,0 +1,27 @@
+gem:
+  outdir: .home/.sorbet/rbi/gems
+  file_header: true
+  exclude:
+    [
+      rdoc,
+      asciidoctor-pdf,
+      solargraph,
+      webrick,
+      ttfunk,
+      tapioca,
+      rubocop,
+      rubocop-minitest,
+      spoom,
+      rdbg,
+      parser,
+    ]
+dsl:
+  outdir: .home/.sorbet/rbi/dsl
+  # Add your `dsl` command parameters here:
+  #
+  # exclude:
+  # - SomeGeneratorName
+  # workers: 5
+annotations:
+  sources:
+    - ext/rbi-central

Gemfile

@@ -16,6 +16,7 @@ gem "pygments.rb"
 gem "rake", "~> 13.0"
 gem "rouge"
 gem "ruby-progressbar", "~> 1.13"
+gem "sorbet-runtime"
 gem "treetop", "1.6.12"
 gem "ttfunk", "1.7" # needed to avoid having asciidoctor-pdf dependencies pulling in a buggy version of ttunk (1.8)
 gem "webrick"
@@ -31,4 +32,7 @@ group :development do
   gem "ruby-prof"
   gem "ruby-prof-flamegraph", git: "https://github.com/oozou/ruby-prof-flamegraph.git", ref: "fc3c437", require: false
   gem "solargraph"
+  gem "sorbet"
+  gem "spoom"
+  gem "tapioca", require: false
 end

Gemfile.lock

@@ -65,6 +65,7 @@ GEM
       reline (>= 0.3.8)
     diff-lcs (1.6.0)
     drb (2.2.1)
+    erubi (1.13.1)
     hana (1.3.7)
     hashery (2.1.2)
     i18n (1.14.7)
@@ -89,6 +90,7 @@ GEM
     logger (1.6.6)
     matrix (0.4.2)
     minitest (5.25.5)
+    netrc (0.11.0)
     nkf (0.2.0)
     nokogiri (1.18.6-aarch64-linux-gnu)
       racc (~> 1.4)
@@ -126,6 +128,7 @@ GEM
       pdf-reader (~> 2.0)
       prawn (~> 2.2)
     prettyprint (0.2.0)
+    prism (1.4.0)
     psych (5.2.3)
       date
       stringio
@@ -134,6 +137,10 @@ GEM
     racc (1.8.1)
     rainbow (3.1.1)
     rake (13.2.1)
+    rbi (0.3.1)
+      prism (~> 1.0)
+      rbs (>= 3.4.4)
+      sorbet-runtime (>= 0.5.9204)
     rbs (3.9.1)
       logger
     rdbg (0.1.0)
@@ -189,7 +196,31 @@ GEM
       tilt (~> 2.0)
       yard (~> 0.9, >= 0.9.24)
       yard-solargraph (~> 0.1)
+    sorbet (0.5.11966)
+      sorbet-static (= 0.5.11966)
+    sorbet-runtime (0.5.11966)
+    sorbet-static (0.5.11966-aarch64-linux)
+    sorbet-static (0.5.11966-x86_64-linux)
+    sorbet-static-and-runtime (0.5.11966)
+      sorbet (= 0.5.11966)
+      sorbet-runtime (= 0.5.11966)
+    spoom (1.6.1)
+      erubi (>= 1.10.0)
+      prism (>= 0.28.0)
+      rbi (>= 0.2.3)
+      sorbet-static-and-runtime (>= 0.5.10187)
+      thor (>= 0.19.2)
     stringio (3.1.5)
+    tapioca (0.16.11)
+      benchmark
+      bundler (>= 2.2.25)
+      netrc (>= 0.11.0)
+      parallel (>= 1.21.0)
+      rbi (~> 0.2)
+      sorbet-static-and-runtime (>= 0.5.11087)
+      spoom (>= 1.2.0)
+      thor (>= 1.2.0)
+      yard-sorbet
     thor (1.3.2)
     tilt (2.6.0)
     treetop (1.6.12)
@@ -208,6 +239,9 @@ GEM
     yard (0.9.37)
     yard-solargraph (0.1.0)
       yard (~> 0.9)
+    yard-sorbet (0.9.0)
+      sorbet-runtime
+      yard
 
 PLATFORMS
   aarch64-linux-gnu
@@ -234,6 +268,10 @@ DEPENDENCIES
   ruby-prof-flamegraph!
   ruby-progressbar (~> 1.13)
   solargraph
+  sorbet
+  sorbet-runtime
+  spoom
+  tapioca
   treetop (= 1.6.12)
   ttfunk (= 1.7)
   webrick

Rakefile

@@ -1,4 +1,9 @@
 # frozen_string_literal: true
+# typed: true
+
+require "sorbet-runtime"
+T.bind(self, T.all(Rake::DSL, Object))
+extend T::Sig
 
 Encoding.default_external = "UTF-8"
 
@@ -37,8 +42,8 @@ directory "#{$root}/.stamps"
 
 # @param config_locator [String or Pathname]
 # @return [ConfiguredArchitecture]
+sig { params(config_locator: T.any(String, Pathname)).returns(ConfiguredArchitecture) }
 def cfg_arch_for(config_locator)
-  raise ArgumentError, "expecting String or Pathname" unless config_locator.is_a?(String) || config_locator.is_a?(Pathname)
   config_locator = config_locator.to_s
 
   $cfg_archs ||= {}
@@ -163,6 +168,11 @@ namespace :serve do
   end
 end
 
+sig { params(test_files: T::Array[String]).returns(String) }
+def make_test_cmd(test_files)
+  "-Ilib:test -w -e 'require \"minitest/autorun\"; #{test_files.map{ |f| "require \"#{f}\""}.join("; ")}' --"
+end
+
 namespace :test do
 
   # "Run the cross-validation against LLVM"
@@ -175,18 +185,20 @@ namespace :test do
   end
   # "Run the IDL compiler test suite"
   task :idl_compiler do
-    t = Minitest::TestTask.new(:lib_test)
-    t.test_globs = ["#{$root}/lib/idl/tests/test_*.rb"]
-    t.process_env
-    ruby t.make_test_cmd
+    test_files = Dir["#{$root}/lib/idl/tests/test_*.rb"]
+    ruby make_test_cmd(test_files)
   end
 
   # "Run the Ruby library test suite"
   task :lib do
-    t = Minitest::TestTask.new(:lib_test)
-    t.test_globs = ["#{$root}/lib/test/test_*.rb"]
-    t.process_env
-    ruby t.make_test_cmd
+    test_files = Dir["#{$root}/lib/test/test_*.rb"]
+
+    ruby make_test_cmd(test_files)
+  end
+
+  desc "Type-check the Ruby library"
+  task :sorbet do
+    sh "srb tc @.sorbet-config"
   end
 end
 
@@ -208,13 +220,13 @@ namespace :test do
 
     cfg_arch = cfg_arch_for("_")
     insts = cfg_arch.instructions
-    failed = false
+    failed = T.let(false, T::Boolean)
     insts.each_with_index do |inst, idx|
       [32, 64].each do |xlen|
         next unless inst.defined_in_base?(xlen)
 
         (idx...insts.size).each do |other_idx|
-          other_inst = insts[other_idx]
+          other_inst = T.must(insts[other_idx])
           next unless other_inst.defined_in_base?(xlen)
           next if other_inst == inst
 
@@ -236,13 +248,13 @@ namespace :test do
 
     cfg_arch = cfg_arch_for("_")
     csrs = cfg_arch.csrs
-    failed = false
+    failed = T.let(false, T::Boolean)
     csrs.each_with_index do |csr, idx|
       [32, 64].each do |xlen|
         next unless csr.defined_in_base?(xlen)
 
         (idx...csrs.size).each do |other_idx|
-          other_csr = csrs[other_idx]
+          other_csr = T.must(csrs[other_idx])
           next unless other_csr.defined_in_base?(xlen)
           next if other_csr == csr
 
@@ -284,7 +296,6 @@ def insert_warning(str, from)
   first_line = lines.shift
   lines.unshift(first_line, "\n# WARNING: This file is auto-generated from #{Pathname.new(from).relative_path_from($root)}").join("")
 end
-private :insert_warning
 
 (3..31).each do |hpm_num|
   file "#{$root}/arch/csr/Zihpm/mhpmcounter#{hpm_num}.yaml" => [
@@ -437,6 +448,8 @@ namespace :test do
     Rake::Task["test:idl_compiler"].invoke
     $logger.info "Running test:lib"
     Rake::Task["test:lib"].invoke
+    $logger.info "UPDATE: Running test:sorbet"
+    Rake::Task["test:sorbet"].invoke
     $logger.info "Running test:schema"
     Rake::Task["test:schema"].invoke
     $logger.info "UPDATE: Running test:idl for rv32"

backends/cpp_hart_gen/templates/csrs_impl.hxx.erb

@@ -56,11 +56,7 @@ void <%= name_of(:csr_field, cfg_arch, csr.name, field.name) %><SocType>::reset(
   <%- end -%>
   <%- else -%>
   auto val_fn = [this]() ->
-    <%- if field.could_be_undefined? -%>
     PossiblyUnknownBits<<%= max_width + 1 %>>
-    <%- else -%>
-    Bits<<%= max_width + 1 %>>
-    <%- end -%>
   {
     <%= ast.gen_cpp(cfg_arch.symtab, 8) %>
   };