Merge pull request #104 from github/aibaars/variables

Simple implementation of class and instance variables

ql/src/codeql_ruby/ast/Variable.qll

@@ -28,6 +28,9 @@ class VariableScope extends TScope {
     result = this.getAVariable() and
     result.getName() = name
   }
+
+  /** Gets the scope in which this scope is nested, if any. */
+  VariableScope getOuterScope() { result = enclosingScope(this.getScopeElement()) }
 }
 
 /** A variable declared in a scope. */
@@ -85,6 +88,23 @@ class GlobalVariable extends Variable, TGlobalVariable {
   final override GlobalVariableAccess getAnAccess() { result.getVariable() = this }
 }
 
+/** An instance variable. */
+class InstanceVariable extends Variable, TInstanceVariable {
+  override InstanceVariable::Range range;
+
+  /** Holds is this variable is a class instance variable. */
+  final predicate isClassInstanceVariable() { range.isClassInstanceVariable() }
+
+  final override InstanceVariableAccess getAnAccess() { result.getVariable() = this }
+}
+
+/** A class variable. */
+class ClassVariable extends Variable, TClassVariable {
+  override ClassVariable::Range range;
+
+  final override ClassVariableAccess getAnAccess() { result.getVariable() = this }
+}
+
 /** An access to a variable. */
 class VariableAccess extends Expr {
   override VariableAccess::Range range;
@@ -146,7 +166,6 @@ class VariableReadAccess extends VariableAccess {
 class LocalVariableAccess extends VariableAccess, @token_identifier {
   final override LocalVariableAccess::Range range;
 
-  /** Gets the variable this identifier refers to. */
   final override LocalVariable getVariable() { result = range.getVariable() }
 
   final override string getAPrimaryQlClass() {
@@ -177,11 +196,10 @@ class LocalVariableWriteAccess extends LocalVariableAccess, VariableWriteAccess
 /** An access to a local variable where the value is read. */
 class LocalVariableReadAccess extends LocalVariableAccess, VariableReadAccess { }
 
-/** An access to a local variable. */
+/** An access to a global variable. */
 class GlobalVariableAccess extends VariableAccess, @token_global_variable {
   final override GlobalVariableAccess::Range range;
 
-  /** Gets the variable this identifier refers to. */
   final override GlobalVariable getVariable() { result = range.getVariable() }
 
   final override string getAPrimaryQlClass() { result = "GlobalVariableAccess" }
@@ -192,3 +210,21 @@ class GlobalVariableWriteAccess extends GlobalVariableAccess, VariableWriteAcces
 
 /** An access to a global variable where the value is read. */
 class GlobalVariableReadAccess extends GlobalVariableAccess, VariableReadAccess { }
+
+/** An access to an instance variable. */
+class InstanceVariableAccess extends VariableAccess, @token_instance_variable {
+  final override InstanceVariableAccess::Range range;
+
+  final override InstanceVariable getVariable() { result = range.getVariable() }
+
+  final override string getAPrimaryQlClass() { result = "InstanceVariableAccess" }
+}
+
+/** An access to a class variable. */
+class ClassVariableAccess extends VariableAccess, @token_class_variable {
+  final override ClassVariableAccess::Range range;
+
+  final override ClassVariable getVariable() { result = range.getVariable() }
+
+  final override string getAPrimaryQlClass() { result = "ClassVariableAccess" }
+}

ql/src/codeql_ruby/ast/internal/Variable.qll

@@ -9,6 +9,45 @@ private Generated::AstNode parent(Generated::AstNode n) {
   not n = any(VariableScope s).getScopeElement()
 }
 
+private predicate instanceVariableAccess(
+  Generated::InstanceVariable var, string name, VariableScope scope, boolean instance
+) {
+  name = var.getValue() and
+  scope = enclosingModuleOrClass(var) and
+  if hasEnclosingMethod(var) then instance = true else instance = false
+}
+
+private predicate classVariableAccess(Generated::ClassVariable var, string name, VariableScope scope) {
+  name = var.getValue() and
+  scope = enclosingModuleOrClass(var)
+}
+
+predicate hasEnclosingMethod(Generated::AstNode node) {
+  exists(Callable method | parentCallableScope*(enclosingScope(node)) = TCallableScope(method) |
+    method instanceof Method or
+    method instanceof SingletonMethod
+  )
+}
+
+private TCallableScope parentCallableScope(TCallableScope scope) {
+  exists(Callable c |
+    scope = TCallableScope(c) and
+    not c instanceof Method and
+    not c instanceof SingletonMethod
+  |
+    result = scope.(VariableScope).getOuterScope()
+  )
+}
+
+private VariableScope parentScope(VariableScope scope) {
+  not scope instanceof ModuleOrClassScope and
+  result = scope.getOuterScope()
+}
+
+private ModuleOrClassScope enclosingModuleOrClass(Generated::AstNode node) {
+  result = parentScope*(enclosingScope(node))
+}
+
 private predicate parameterAssignment(
   CallableScope::Range scope, string name, Generated::Identifier i
 ) {
@@ -20,7 +59,6 @@ private predicate parameterAssignment(
 private predicate scopeDefinesParameterVariable(
   CallableScope::Range scope, string name, Generated::Identifier i
 ) {
-  parameterAssignment(scope, name, i) and
   // In case of overlapping parameter names (e.g. `_`), only the first
   // parameter will give rise to a variable
   i =
@@ -69,9 +107,6 @@ private class CapturingScope extends VariableScope {
     )
   }
 
-  /** Gets the scope in which this scope is nested, if any. */
-  private VariableScope getOuterScope() { result = enclosingScope(this.getScopeElement()) }
-
   /** Holds if this scope inherits `name` from an outer scope `outer`. */
   predicate inherits(string name, VariableScope outer) {
     not scopeDefinesParameterVariable(this, name, _) and
@@ -112,10 +147,25 @@ private module Cached {
   cached
   newtype TVariable =
     TGlobalVariable(string name) { name = any(Generated::GlobalVariable var).getValue() } or
+    TClassVariable(VariableScope scope, string name, Generated::AstNode decl) {
+      decl =
+        min(Generated::ClassVariable other |
+          classVariableAccess(other, name, scope)
+        |
+          other order by other.getLocation().getStartLine(), other.getLocation().getStartColumn()
+        )
+    } or
+    TInstanceVariable(VariableScope scope, string name, boolean instance, Generated::AstNode decl) {
+      decl =
+        min(Generated::InstanceVariable other |
+          instanceVariableAccess(other, name, scope, instance)
+        |
+          other order by other.getLocation().getStartLine(), other.getLocation().getStartColumn()
+        )
+    } or
     TLocalVariable(VariableScope scope, string name, Generated::Identifier i) {
       scopeDefinesParameterVariable(scope, name, i)
       or
-      scopeAssigns(scope, name, i) and
       i =
         min(Generated::Identifier other |
           scopeAssigns(scope, name, other)
@@ -307,6 +357,22 @@ private module Cached {
   predicate isCapturedAccess(LocalVariableAccess::Range access) {
     access.getVariable().getDeclaringScope() != enclosingScope(access)
   }
+
+  cached
+  predicate instanceVariableAccess(Generated::InstanceVariable var, InstanceVariable v) {
+    exists(string name, VariableScope scope, boolean instance |
+      v = TInstanceVariable(scope, name, instance, _) and
+      instanceVariableAccess(var, name, scope, instance)
+    )
+  }
+
+  cached
+  predicate classVariableAccess(Generated::ClassVariable var, ClassVariable variable) {
+    exists(VariableScope scope, string name |
+      variable = TClassVariable(scope, name, _) and
+      classVariableAccess(var, name, scope)
+    )
+  }
 }
 
 import Cached
@@ -416,6 +482,43 @@ module GlobalVariable {
   }
 }
 
+private class ModuleOrClassScope = TClassScope or TModuleScope or TTopLevelScope;
+
+module InstanceVariable {
+  class Range extends Variable::Range, TInstanceVariable {
+    private ModuleOrClassScope scope;
+    private boolean instance;
+    private string name;
+    private Generated::AstNode decl;
+
+    Range() { this = TInstanceVariable(scope, name, instance, decl) }
+
+    final override string getName() { result = name }
+
+    final predicate isClassInstanceVariable() { instance = false }
+
+    final override Location getLocation() { result = decl.getLocation() }
+
+    final override VariableScope getDeclaringScope() { result = scope }
+  }
+}
+
+module ClassVariable {
+  class Range extends Variable::Range, TClassVariable {
+    private ModuleOrClassScope scope;
+    private string name;
+    private Generated::AstNode decl;
+
+    Range() { this = TClassVariable(scope, name, decl) }
+
+    final override string getName() { result = name }
+
+    final override Location getLocation() { result = decl.getLocation() }
+
+    final override VariableScope getDeclaringScope() { result = scope }
+  }
+}
+
 module VariableAccess {
   abstract class Range extends Expr::Range {
     abstract Variable getVariable();
@@ -451,3 +554,23 @@ module GlobalVariableAccess {
     final override GlobalVariable getVariable() { result = variable }
   }
 }
+
+module InstanceVariableAccess {
+  class Range extends VariableAccess::Range, @token_instance_variable {
+    InstanceVariable variable;
+
+    Range() { instanceVariableAccess(this, variable) }
+
+    final override InstanceVariable getVariable() { result = variable }
+  }
+}
+
+module ClassVariableAccess {
+  class Range extends VariableAccess::Range, @token_class_variable {
+    ClassVariable variable;
+
+    Range() { classVariableAccess(this, variable) }
+
+    final override ClassVariable getVariable() { result = variable }
+  }
+}

ql/test/library-tests/variables/class_variables.rb

@@ -0,0 +1,29 @@
+@@x = 42
+
+p @@x
+
+def print
+ p @@x
+end
+
+class X
+  def b
+    p @@x
+  end
+  def self.s
+   p @@x
+  end
+end 
+
+class Y < BasicObject
+  @@x = 10
+end 
+
+module M
+  @@x = 12
+end
+
+module N
+  include M
+  p @@x
+end

ql/test/library-tests/variables/instance_variables.rb

@@ -0,0 +1,44 @@
+@top = 1
+
+def foo
+  @foo = 10
+end
+
+def print_foo
+  puts @foo
+end
+
+puts @top
+
+class X
+  @x = 10
+  def m()
+    @y = 7
+  end
+end
+
+module M
+ @m = 10
+ def n()
+   @n = 7
+ end
+end
+
+puts { 
+  @x = 100
+}
+
+def bar
+ 1.times { @x = 200 }
+end
+
+class C
+  @x = 42
+  def x 
+   def y
+     @x = 10
+   end
+   y
+   p @x
+  end
+ end

ql/test/library-tests/variables/varaccess.expected

@@ -1,4 +1,25 @@
 variableAccess
+| class_variables.rb:1:1:1:3 | @@x | class_variables.rb:1:1:1:3 | @@x | class_variables.rb:1:1:29:4 | top-level scope |
+| class_variables.rb:3:3:3:5 | @@x | class_variables.rb:1:1:1:3 | @@x | class_variables.rb:1:1:29:4 | top-level scope |
+| class_variables.rb:6:4:6:6 | @@x | class_variables.rb:1:1:1:3 | @@x | class_variables.rb:1:1:29:4 | top-level scope |
+| class_variables.rb:11:7:11:9 | @@x | class_variables.rb:11:7:11:9 | @@x | class_variables.rb:9:1:16:3 | class scope |
+| class_variables.rb:14:6:14:8 | @@x | class_variables.rb:11:7:11:9 | @@x | class_variables.rb:9:1:16:3 | class scope |
+| class_variables.rb:19:3:19:5 | @@x | class_variables.rb:19:3:19:5 | @@x | class_variables.rb:18:1:20:3 | class scope |
+| class_variables.rb:23:3:23:5 | @@x | class_variables.rb:23:3:23:5 | @@x | class_variables.rb:22:1:24:3 | module scope |
+| class_variables.rb:28:5:28:7 | @@x | class_variables.rb:28:5:28:7 | @@x | class_variables.rb:26:1:29:3 | module scope |
+| instance_variables.rb:1:1:1:4 | @top | instance_variables.rb:1:1:1:4 | @top | instance_variables.rb:1:1:44:4 | top-level scope |
+| instance_variables.rb:4:3:4:6 | @foo | instance_variables.rb:4:3:4:6 | @foo | instance_variables.rb:1:1:44:4 | top-level scope |
+| instance_variables.rb:8:8:8:11 | @foo | instance_variables.rb:4:3:4:6 | @foo | instance_variables.rb:1:1:44:4 | top-level scope |
+| instance_variables.rb:11:6:11:9 | @top | instance_variables.rb:1:1:1:4 | @top | instance_variables.rb:1:1:44:4 | top-level scope |
+| instance_variables.rb:14:3:14:4 | @x | instance_variables.rb:14:3:14:4 | @x | instance_variables.rb:13:1:18:3 | class scope |
+| instance_variables.rb:16:5:16:6 | @y | instance_variables.rb:16:5:16:6 | @y | instance_variables.rb:13:1:18:3 | class scope |
+| instance_variables.rb:21:2:21:3 | @m | instance_variables.rb:21:2:21:3 | @m | instance_variables.rb:20:1:25:3 | module scope |
+| instance_variables.rb:23:4:23:5 | @n | instance_variables.rb:23:4:23:5 | @n | instance_variables.rb:20:1:25:3 | module scope |
+| instance_variables.rb:28:3:28:4 | @x | instance_variables.rb:28:3:28:4 | @x | instance_variables.rb:1:1:44:4 | top-level scope |
+| instance_variables.rb:32:12:32:13 | @x | instance_variables.rb:32:12:32:13 | @x | instance_variables.rb:1:1:44:4 | top-level scope |
+| instance_variables.rb:36:3:36:4 | @x | instance_variables.rb:36:3:36:4 | @x | instance_variables.rb:35:1:44:4 | class scope |
+| instance_variables.rb:39:6:39:7 | @x | instance_variables.rb:39:6:39:7 | @x | instance_variables.rb:35:1:44:4 | class scope |
+| instance_variables.rb:42:6:42:7 | @x | instance_variables.rb:39:6:39:7 | @x | instance_variables.rb:35:1:44:4 | class scope |
 | nested_scopes.rb:5:3:5:3 | a | nested_scopes.rb:5:3:5:3 | a | nested_scopes.rb:4:1:39:3 | class scope |
 | nested_scopes.rb:7:5:7:5 | a | nested_scopes.rb:7:5:7:5 | a | nested_scopes.rb:6:3:37:5 | module scope |
 | nested_scopes.rb:9:7:9:7 | a | nested_scopes.rb:9:7:9:7 | a | nested_scopes.rb:8:5:35:7 | module scope |
@@ -221,6 +242,27 @@ implicitWrite
 | ssa.rb:64:8:64:8 | a |
 | ssa.rb:66:15:66:15 | a |
 readAccess
+| class_variables.rb:1:1:1:3 | @@x |
+| class_variables.rb:3:3:3:5 | @@x |
+| class_variables.rb:6:4:6:6 | @@x |
+| class_variables.rb:11:7:11:9 | @@x |
+| class_variables.rb:14:6:14:8 | @@x |
+| class_variables.rb:19:3:19:5 | @@x |
+| class_variables.rb:23:3:23:5 | @@x |
+| class_variables.rb:28:5:28:7 | @@x |
+| instance_variables.rb:1:1:1:4 | @top |
+| instance_variables.rb:4:3:4:6 | @foo |
+| instance_variables.rb:8:8:8:11 | @foo |
+| instance_variables.rb:11:6:11:9 | @top |
+| instance_variables.rb:14:3:14:4 | @x |
+| instance_variables.rb:16:5:16:6 | @y |
+| instance_variables.rb:21:2:21:3 | @m |
+| instance_variables.rb:23:4:23:5 | @n |
+| instance_variables.rb:28:3:28:4 | @x |
+| instance_variables.rb:32:12:32:13 | @x |
+| instance_variables.rb:36:3:36:4 | @x |
+| instance_variables.rb:39:6:39:7 | @x |
+| instance_variables.rb:42:6:42:7 | @x |
 | nested_scopes.rb:14:16:14:16 | a |
 | nested_scopes.rb:15:11:15:11 | a |
 | nested_scopes.rb:16:13:16:13 | a |

ql/test/library-tests/variables/variable.expected

@@ -1,5 +1,20 @@
+| class_variables.rb:1:1:1:3 | @@x |
+| class_variables.rb:11:7:11:9 | @@x |
+| class_variables.rb:19:3:19:5 | @@x |
+| class_variables.rb:23:3:23:5 | @@x |
+| class_variables.rb:28:5:28:7 | @@x |
 | file://:0:0:0:0 | $0 |
 | file://:0:0:0:0 | $global |
+| instance_variables.rb:1:1:1:4 | @top |
+| instance_variables.rb:4:3:4:6 | @foo |
+| instance_variables.rb:14:3:14:4 | @x |
+| instance_variables.rb:16:5:16:6 | @y |
+| instance_variables.rb:21:2:21:3 | @m |
+| instance_variables.rb:23:4:23:5 | @n |
+| instance_variables.rb:28:3:28:4 | @x |
+| instance_variables.rb:32:12:32:13 | @x |
+| instance_variables.rb:36:3:36:4 | @x |
+| instance_variables.rb:39:6:39:7 | @x |
 | nested_scopes.rb:5:3:5:3 | a |
 | nested_scopes.rb:7:5:7:5 | a |
 | nested_scopes.rb:9:7:9:7 | a |

ql/test/library-tests/variables/varscopes.expected

@@ -1,4 +1,25 @@
+| class_variables.rb:1:1:29:4 | top-level scope |
+| class_variables.rb:5:1:7:3 | method scope |
+| class_variables.rb:9:1:16:3 | class scope |
+| class_variables.rb:10:3:12:5 | method scope |
+| class_variables.rb:13:3:15:5 | method scope |
+| class_variables.rb:18:1:20:3 | class scope |
+| class_variables.rb:22:1:24:3 | module scope |
+| class_variables.rb:26:1:29:3 | module scope |
 | file://:0:0:0:0 | global scope |
+| instance_variables.rb:1:1:44:4 | top-level scope |
+| instance_variables.rb:3:1:5:3 | method scope |
+| instance_variables.rb:7:1:9:3 | method scope |
+| instance_variables.rb:13:1:18:3 | class scope |
+| instance_variables.rb:15:3:17:5 | method scope |
+| instance_variables.rb:20:1:25:3 | module scope |
+| instance_variables.rb:22:2:24:4 | method scope |
+| instance_variables.rb:27:6:29:1 | block scope |
+| instance_variables.rb:31:1:33:3 | method scope |
+| instance_variables.rb:32:10:32:21 | block scope |
+| instance_variables.rb:35:1:44:4 | class scope |
+| instance_variables.rb:37:3:43:5 | method scope |
+| instance_variables.rb:38:4:40:6 | method scope |
 | nested_scopes.rb:1:1:3:3 | method scope |
 | nested_scopes.rb:1:1:42:1 | top-level scope |
 | nested_scopes.rb:4:1:39:3 | class scope |