Re-enable Windows CI

Author: vinistock

.github/workflows/ci.yml

@@ -27,9 +27,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        # TODO: Put back windows once we have figured how to compile Prism on it.
-        # os: [shopify-ubuntu-latest, macos-latest, shopify-windows-latest]
-        os: [shopify-ubuntu-latest, macos-latest]
+        os: [shopify-ubuntu-latest, macos-latest, shopify-windows-latest]
         ruby: ["3.4"]
     runs-on: ${{ matrix.os }}
     name: Ruby ${{ matrix.ruby }} on ${{ matrix.os }}

Gemfile

@@ -11,4 +11,8 @@ gem "minitest"
 gem "rubocop"
 gem "rubocop-shopify"
 gem "extconf_compile_commands_json"
-gem "ruby_memcheck"
+
+# Gems that aren't supported on Windows
+platforms :ruby do
+  gem "ruby_memcheck"
+end

Rakefile

@@ -4,7 +4,6 @@ require "bundler/gem_tasks"
 require "rubocop/rake_task"
 require "rake/extensiontask"
 require "rake/testtask"
-require "ruby_memcheck"
 
 GEMSPEC = Gem::Specification.load("saturn.gemspec")
 
@@ -19,7 +18,13 @@ test_config = lambda do |t|
   t.test_files = FileList["test/**/*_test.rb"]
 end
 Rake::TestTask.new(ruby_test: :compile, &test_config)
-namespace(:ruby_test) { RubyMemcheck::TestTask.new(valgrind: :compile, &test_config) }
+
+begin
+  require "ruby_memcheck"
+  namespace(:ruby_test) { RubyMemcheck::TestTask.new(valgrind: :compile, &test_config) }
+rescue LoadError
+  # ruby_memcheck is not available on Windows
+end
 
 RuboCop::RakeTask.new
 

lib/saturn/location.rb

@@ -24,7 +24,10 @@ def path
       uri = URI(@uri)
       raise Saturn::Error, "URI is not a file:// URI: #{@uri}" unless uri.scheme == "file"
 
-      uri.path
+      path = uri.path
+      # TODO: This has to go away once we have a proper URI abstraction
+      path.delete_prefix!("/") if Gem.win_platform?
+      path
     end
 
     #: (other: BasicObject) -> Integer

rust/saturn/src/indexing.rs

@@ -166,6 +166,8 @@ pub fn collect_file_paths(paths: Vec<String>) -> (Vec<String>, Vec<Errors>) {
 
 #[cfg(test)]
 mod tests {
+    use std::path::PathBuf;
+
     use super::*;
     use crate::test_utils::Context;
 
@@ -190,18 +192,21 @@ mod tests {
     #[test]
     fn collect_all_documents() {
         let context = Context::new();
-        context.touch("bar/baz.rb");
-        context.touch("bar/qux.rb");
-        context.touch("foo/bar.rb");
+        let baz = Path::new("bar").join("baz.rb");
+        let qux = Path::new("bar").join("qux.rb");
+        let bar = Path::new("foo").join("bar.rb");
+        context.touch(&baz);
+        context.touch(&qux);
+        context.touch(&bar);
 
         let (paths, errors) = collect_document_paths(&context, &["foo", "bar"]);
 
         assert_eq!(
             paths,
             vec![
-                "bar/baz.rb".to_string(),
-                "bar/qux.rb".to_string(),
-                "foo/bar.rb".to_string()
+                baz.to_str().unwrap().to_string(),
+                qux.to_str().unwrap().to_string(),
+                bar.to_str().unwrap().to_string()
             ]
         );
         assert!(errors.is_empty());
@@ -210,13 +215,19 @@ mod tests {
     #[test]
     fn collect_some_documents_based_on_paths() {
         let context = Context::new();
-        context.touch("bar/baz.rb");
-        context.touch("bar/qux.rb");
-        context.touch("foo/bar.rb");
+        let baz = Path::new("bar").join("baz.rb");
+        let qux = Path::new("bar").join("qux.rb");
+        let bar = Path::new("foo").join("bar.rb");
 
+        context.touch(&baz);
+        context.touch(&qux);
+        context.touch(&bar);
         let (paths, errors) = collect_document_paths(&context, &["bar"]);
 
-        assert_eq!(paths, vec!["bar/baz.rb".to_string(), "bar/qux.rb".to_string()]);
+        assert_eq!(
+            paths,
+            vec![baz.to_str().unwrap().to_string(), qux.to_str().unwrap().to_string()]
+        );
         assert!(errors.is_empty());
     }
 
@@ -239,30 +250,31 @@ mod tests {
                 .map(std::string::ToString::to_string)
                 .collect::<Vec<String>>(),
             vec![format!(
-                "File read error: Path '{}/non_existing_path' does not exist",
-                context.absolute_path().display()
+                "File read error: Path '{}' does not exist",
+                context.absolute_path_to("non_existing_path").display()
             )]
         );
     }
 
     #[test]
     fn index_relative_paths() {
+        let relative_path = Path::new("foo").join("bar.rb");
         let context = Context::new();
-        context.touch("foo/bar.rb");
+        context.touch(&relative_path);
 
         let working_directory = std::env::current_dir().unwrap();
+        let absolute_path = context.absolute_path_to("foo/bar.rb");
 
-        let dotdots = "../".repeat(working_directory.components().count());
+        let mut dots = PathBuf::from("..");
 
-        let relative_path = dotdots
-            + context
-                .absolute_path_to("foo/bar.rb")
-                .to_string_lossy()
-                .into_owned()
-                .as_str();
+        for _ in 0..working_directory.components().count() - 1 {
+            dots = dots.join("..");
+        }
+
+        let relative_to_pwd = &dots.join(absolute_path);
 
         let mut graph = Graph::new();
-        let errors = index_in_parallel(&mut graph, vec![relative_path]);
+        let errors = index_in_parallel(&mut graph, vec![relative_to_pwd.to_str().unwrap().to_string()]);
 
         assert!(errors.is_ok());
         assert_eq!(graph.documents().len(), 1);

test/definition_test.rb

@@ -77,7 +77,7 @@ def foo; end
       refute_nil(def_a)
       location = def_a.location
       refute_nil(location)
-      assert_equal("file://#{context.absolute_path_to("file1.rb")}", location.uri)
+      assert_equal(context.uri_to("file1.rb"), location.uri)
       assert_equal(context.absolute_path_to("file1.rb"), location.path)
       assert_equal(1, location.start_line)
       assert_equal(1, location.start_column)
@@ -88,7 +88,7 @@ def foo; end
       refute_nil(def_foo)
       location = def_foo.location
       refute_nil(location)
-      assert_equal("file://#{context.absolute_path_to("file1.rb")}", location.uri)
+      assert_equal(context.uri_to("file1.rb"), location.uri)
       assert_equal(context.absolute_path_to("file1.rb"), location.path)
       assert_equal(2, location.start_line)
       assert_equal(3, location.start_column)
@@ -104,27 +104,44 @@ def test_definition_comments
         # Multi-line comment
         class Foo
           # Method comment
-          def foo; end
+          def bar; end
         end
       RUBY
 
       graph = Saturn::Graph.new
       graph.index_all(context.glob("**/*.rb"))
 
-      comments = graph.documents.first.definitions.find { |d| d.name == "Foo" }.comments
+      foo_comments = graph.documents.first.definitions.find { |d| d.name == "Foo" }.comments
       assert_equal(
         [
           "# This is a class comment (#{context.absolute_path_to("file1.rb")}:1:1-1:26)",
           "# Multi-line comment (#{context.absolute_path_to("file1.rb")}:2:1-2:21)",
         ],
-        comments.map { |c| "#{c.string} (#{c.location})" },
+        foo_comments.map { |c| "#{c.string} (#{normalized_comment_location(c)})" },
       )
 
-      comments = graph.documents.first.definitions.find { |d| d.name == "foo" }.comments
+      bar_comments = graph.documents.first.definitions.find { |d| d.name == "bar" }.comments
       assert_equal(
         ["# Method comment (#{context.absolute_path_to("file1.rb")}:4:3-4:19)"],
-        comments.map { |c| "#{c.string} (#{c.location})" },
+        bar_comments.map { |c| "#{c.string} (#{normalized_comment_location(c)})" },
       )
     end
   end
+
+  private
+
+  # Comment locations on Windows include the carriage return. This means that the end column is off by one when compared
+  # to Unix locations. This method creates a fake adjusted location for Windows so that we can assert locations once
+  def normalized_comment_location(comment)
+    loc = comment.location
+    return loc unless Gem.win_platform?
+
+    Saturn::Location.new(
+      uri: loc.uri,
+      start_line: loc.start_line,
+      start_column: loc.start_column,
+      end_line: loc.end_line,
+      end_column: loc.end_column - 1,
+    )
+  end
 end

test/document_test.rb

@@ -23,7 +23,7 @@ def test_document_initialize_from_graph
 
       document = graph.documents.first
       assert_instance_of(Saturn::Document, document)
-      assert_equal("file://#{context.absolute_path}/file1.rb", document.uri)
+      assert_equal(context.uri_to("file1.rb"), document.uri)
     end
   end
 

test/helpers/context.rb

@@ -26,6 +26,14 @@ def absolute_path_to(relative_path)
         ::File.join(@absolute_path, relative_path)
       end
 
+      #: (String) -> String
+      def uri_to(relative_path)
+        path = absolute_path_to(relative_path)
+        # TODO: This has to go away once we have a proper URI abstraction
+        path.prepend("/") if Gem.win_platform?
+        URI::File.build(path: path).to_s
+      end
+
       #: (String relative_path, ?String contents, ?append: bool) -> void
       def write!(relative_path, contents = "", append: false)
         absolute_path = absolute_path_to(relative_path)