Merge pull request #100 from dak2/fix/global-variable-review-improvements

Add global variable ($var) type tracking

Author: dak2

core/src/analyzer/dispatch.rs

@@ -14,8 +14,9 @@ use ruby_prism::Node;
 use super::bytes_to_name;
 use super::calls::install_method_call;
 use super::variables::{
-    install_class_var_read, install_class_var_write, install_ivar_read, install_ivar_write,
-    install_local_var_read, install_local_var_write, install_self,
+    install_class_var_read, install_class_var_write, install_global_var_read,
+    install_global_var_write, install_ivar_read, install_ivar_write, install_local_var_read,
+    install_local_var_write, install_self,
 };
 
 /// Collect positional and keyword arguments from AST argument nodes.
@@ -81,6 +82,8 @@ pub(crate) enum NeedsChildKind<'a> {
     ClassVarWrite { cvar_name: String, value: Node<'a> },
     /// Local variable write: need to process value, then call finish_local_var_write
     LocalVarWrite { var_name: String, value: Node<'a> },
+    /// Global variable write: need to process value, then call install_global_var_write
+    GlobalVarWrite { gvar_name: String, value: Node<'a> },
     /// Method call: need to process receiver, then call finish_method_call
     MethodCall {
         receiver: Node<'a>,
@@ -142,6 +145,19 @@ pub(crate) fn dispatch_simple(genv: &mut GlobalEnv, lenv: &mut LocalEnv, node: &
         };
     }
 
+    // Global variable read: $name
+    // Ruby: uninitialized global variables are nil.
+    // Register the nil vertex so repeated reads of the same uninitialized
+    // variable reuse one vertex instead of allocating a new Source each time.
+    if let Some(gvar_read) = node.as_global_variable_read_node() {
+        let gvar_name = bytes_to_name(gvar_read.name().as_slice());
+        let vtx = install_global_var_read(genv, &gvar_name).unwrap_or_else(|| {
+            let nil_vtx = genv.new_source(Type::Nil);
+            install_global_var_write(genv, gvar_name, nil_vtx)
+        });
+        return DispatchResult::Vertex(vtx);
+    }
+
     // self
     if node.as_self_node().is_some() {
         return DispatchResult::Vertex(install_self(genv));
@@ -226,6 +242,15 @@ pub(crate) fn dispatch_needs_child<'a>(node: &Node<'a>, source: &str) -> Option<
         });
     }
 
+    // Global variable write: $name = value
+    if let Some(gvar_write) = node.as_global_variable_write_node() {
+        let gvar_name = bytes_to_name(gvar_write.name().as_slice());
+        return Some(NeedsChildKind::GlobalVarWrite {
+            gvar_name,
+            value: gvar_write.value(),
+        });
+    }
+
     // Local variable write: x = value
     if let Some(write_node) = node.as_local_variable_write_node() {
         let var_name = bytes_to_name(write_node.name().as_slice());
@@ -329,6 +354,10 @@ pub(crate) fn process_needs_child(
             let value_vtx = super::install::install_node(genv, lenv, changes, source, &value)?;
             Some(install_class_var_write(genv, cvar_name, value_vtx))
         }
+        NeedsChildKind::GlobalVarWrite { gvar_name, value } => {
+            let value_vtx = super::install::install_node(genv, lenv, changes, source, &value)?;
+            Some(install_global_var_write(genv, gvar_name, value_vtx))
+        }
         NeedsChildKind::LocalVarWrite { var_name, value } => {
             let value_vtx = super::install::install_node(genv, lenv, changes, source, &value)?;
             Some(finish_local_var_write(genv, lenv, changes, var_name, value_vtx))

core/src/analyzer/variables.rs

@@ -4,6 +4,7 @@
 //! - Local variable read/write (x, x = value)
 //! - Instance variable read/write (@name, @name = value)
 //! - Class variable read/write (@@name, @@name = value)
+//! - Global variable read/write ($var, $var = value)
 //! - self node handling
 
 use crate::env::{GlobalEnv, LocalEnv};
@@ -88,6 +89,22 @@ pub(crate) fn install_class_var_read(genv: &GlobalEnv, cvar_name: &str) -> Optio
     genv.scope_manager.lookup_class_var(cvar_name)
 }
 
+/// Install global variable write: $var = value
+///
+/// Delegates to [`GlobalEnv::set_global_var`] for edge behavior details.
+pub(crate) fn install_global_var_write(
+    genv: &mut GlobalEnv,
+    gvar_name: String,
+    value_vtx: VertexId,
+) -> VertexId {
+    genv.set_global_var(gvar_name, value_vtx)
+}
+
+/// Install global variable read: $var
+pub(crate) fn install_global_var_read(genv: &GlobalEnv, gvar_name: &str) -> Option<VertexId> {
+    genv.get_global_var(gvar_name)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -141,4 +158,52 @@ mod tests {
         // Second write returns the same VertexId (edge added, not overwritten)
         assert_eq!(vtx1, vtx2);
     }
+
+    #[test]
+    fn test_global_var_write_and_read() {
+        let mut genv = GlobalEnv::new();
+        let value_vtx = genv.new_source(Type::instance("String"));
+        let result_vtx = install_global_var_write(&mut genv, "$config".to_string(), value_vtx);
+        assert_eq!(result_vtx, value_vtx);
+
+        let read_vtx = install_global_var_read(&genv, "$config");
+        assert_eq!(read_vtx, Some(value_vtx));
+    }
+
+    #[test]
+    fn test_global_var_read_unregistered() {
+        let genv = GlobalEnv::new();
+        let read_vtx = install_global_var_read(&genv, "$unknown");
+        assert_eq!(read_vtx, None);
+    }
+
+    #[test]
+    fn test_global_var_write_twice_returns_same_vertex() {
+        let mut genv = GlobalEnv::new();
+        let vtx1 = genv.new_source(Type::instance("String"));
+        let first = install_global_var_write(&mut genv, "$data".to_string(), vtx1);
+
+        let vtx2 = genv.new_source(Type::instance("Integer"));
+        let second = install_global_var_write(&mut genv, "$data".to_string(), vtx2);
+
+        // Both writes should return the same VertexId (the first one registered)
+        assert_eq!(first, second);
+        // Read should also return the first vertex
+        assert_eq!(install_global_var_read(&genv, "$data"), Some(first));
+    }
+
+    #[test]
+    fn test_global_var_write_twice_propagates_via_vertex() {
+        let mut genv = GlobalEnv::new();
+        // Use a Vertex (not Source) as the initial value so type propagation works
+        let var_vtx = genv.new_vertex();
+        install_global_var_write(&mut genv, "$data".to_string(), var_vtx);
+
+        // Second write with a Source should propagate types into the Vertex
+        let str_src = genv.new_source(Type::instance("String"));
+        install_global_var_write(&mut genv, "$data".to_string(), str_src);
+
+        let types = genv.get_receiver_types(var_vtx).unwrap();
+        assert!(types.contains(&Type::instance("String")));
+    }
 }

core/src/env/global_env.rs

@@ -46,6 +46,9 @@ pub struct GlobalEnv {
 
     /// Module extensions: class_name → Vec<module_name> (in extend order)
     module_extensions: HashMap<String, Vec<String>>,
+
+    /// Global variables: $var_name → VertexId
+    global_variables: HashMap<String, VertexId>,
 }
 
 impl GlobalEnv {
@@ -59,6 +62,7 @@ impl GlobalEnv {
             module_inclusions: HashMap::new(),
             superclass_map: HashMap::new(),
             module_extensions: HashMap::new(),
+            global_variables: HashMap::new(),
         }
     }
 
@@ -234,6 +238,25 @@ impl GlobalEnv {
         );
     }
 
+    // ===== Global Variables =====
+
+    /// Set a global variable. If it already exists, add an edge from value_vtx
+    /// to the existing vertex so types propagate. Otherwise, register value_vtx directly.
+    pub fn set_global_var(&mut self, name: String, value_vtx: VertexId) -> VertexId {
+        if let Some(&existing_vtx) = self.global_variables.get(&name) {
+            self.add_edge(value_vtx, existing_vtx);
+            existing_vtx
+        } else {
+            self.global_variables.insert(name, value_vtx);
+            value_vtx
+        }
+    }
+
+    /// Get the vertex for a global variable, if it has been registered.
+    pub fn get_global_var(&self, name: &str) -> Option<VertexId> {
+        self.global_variables.get(name).copied()
+    }
+
     // ===== Type Errors =====
 
     /// Record a type error (undefined method)

test/global_variable_test.rb

@@ -0,0 +1,114 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class GlobalVariableTest < Minitest::Test
+  include CLITestHelper
+
+  # ============================================
+  # No Error
+  # ============================================
+
+  def test_global_variable_basic
+    source = <<~RUBY
+      $config = "production"
+      $config.upcase
+    RUBY
+    assert_no_check_errors(source)
+  end
+
+  def test_global_variable_type_error
+    source = <<~RUBY
+      $count = 42
+      $count.upcase
+    RUBY
+    assert_check_error(source, method_name: 'upcase', receiver_type: 'Integer')
+  end
+
+  def test_global_variable_in_method
+    source = <<~RUBY
+      class App
+        def setup
+          $logger = "Logger instance"
+        end
+
+        def run
+          $logger.upcase
+        end
+      end
+    RUBY
+    assert_no_check_errors(source)
+  end
+
+  # ============================================
+  # Error Detection
+  # ============================================
+
+  def test_global_variable_across_methods_type_error
+    source = <<~RUBY
+      class App
+        def setup
+          $counter = 0
+        end
+
+        def process
+          $counter.upcase
+        end
+      end
+    RUBY
+    assert_check_error(source, method_name: 'upcase', receiver_type: 'Integer')
+  end
+
+  def test_global_variable_top_level
+    source = <<~RUBY
+      $name = "Alice"
+      $age = 30
+
+      def greet
+        $name.upcase
+      end
+
+      def show_age
+        $age.upcase
+      end
+    RUBY
+    assert_check_error(source, method_name: 'upcase', receiver_type: 'Integer')
+  end
+
+  def test_global_variable_type_error_in_same_method
+    source = <<~RUBY
+      def run
+        $value = 100
+        $value.upcase
+      end
+    RUBY
+    assert_check_error(source, method_name: 'upcase', receiver_type: 'Integer')
+  end
+
+  def test_global_variable_across_classes
+    source = <<~RUBY
+      class Writer
+        def write
+          $shared = "data"
+        end
+      end
+
+      class Reader
+        def read
+          $shared.upcase
+        end
+      end
+    RUBY
+    assert_no_check_errors(source)
+  end
+
+  def test_multiple_global_variables_independent
+    source = <<~RUBY
+      $name = "Alice"
+      $count = 42
+      $name.upcase
+      $count.to_s
+    RUBY
+    assert_no_check_errors(source)
+  end
+end