gclient eval: Get rid of global_scope, local_scope and functions other than Var.

Since we only support Var, and Var only does one thing, we hard code that
into the parser. Then, we don't need global_scope.

local_scope hasn't been needed for a while.

Bug: 760633
Change-Id: I21b171a8b71e7dcaf8e29c780ca88b9f46f368e8
Reviewed-on: https://chromium-review.googlesource.com/972611Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

gclient.py

@@ -765,16 +765,14 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
 
     local_scope = {}
     if deps_content:
-      global_scope = {
-        'Var': lambda var_name: '{%s}' % var_name,
-        'deps_os': {},
-      }
       # Eval the content.
       try:
         if self._get_option('validate_syntax', False):
-          local_scope = gclient_eval.Exec(
-              deps_content, global_scope, local_scope, filepath)
+          local_scope = gclient_eval.Exec(deps_content, filepath)
         else:
+          global_scope = {
+            'Var': lambda var_name: '{%s}' % var_name,
+          }
           exec(deps_content, global_scope, local_scope)
       except SyntaxError as e:
         gclient_utils.SyntaxErrorToError(filepath, e)
@@ -2880,14 +2878,12 @@ def CMDsetdep(parser, args):
                          'file in the current directory.')
   (options, args) = parser.parse_args(args)
 
-  global_scope = {'Var': lambda var: '{%s}' % var}
-
   if not os.path.isfile(options.deps_file):
     raise gclient_utils.Error(
         'DEPS file %s does not exist.' % options.deps_file)
   with open(options.deps_file) as f:
     contents = f.read()
-  local_scope = gclient_eval.Exec(contents, global_scope, {})
+  local_scope = gclient_eval.Exec(contents)
 
   for var in options.vars:
     name, _, value = var.partition('=')
@@ -2909,7 +2905,7 @@ def CMDsetdep(parser, args):
             % (name, package))
       gclient_eval.SetCIPD(local_scope, name, package, value)
     else:
-      gclient_eval.SetRevision(local_scope, global_scope, name, value)
+      gclient_eval.SetRevision(local_scope, name, value)
 
   with open(options.deps_file, 'w') as f:
     f.write(gclient_eval.RenderDEPSFile(local_scope))

gclient_eval.py

@@ -183,7 +183,7 @@ _GCLIENT_SCHEMA = schema.Schema(_NodeDictSchema({
 }))
 
 
-def _gclient_eval(node_or_string, global_scope, filename='<unknown>'):
+def _gclient_eval(node_or_string, filename='<unknown>'):
   """Safely evaluates a single expression. Returns the result."""
   _allowed_names = {'None': None, 'True': True, 'False': False}
   if isinstance(node_or_string, basestring):
@@ -209,16 +209,20 @@ def _gclient_eval(node_or_string, global_scope, filename='<unknown>'):
                 node.id, filename, getattr(node, 'lineno', '<unknown>')))
       return _allowed_names[node.id]
     elif isinstance(node, ast.Call):
-      if not isinstance(node.func, ast.Name):
+      if not isinstance(node.func, ast.Name) or node.func.id != 'Var':
         raise ValueError(
-            'invalid call: func should be a name (file %r, line %s)' % (
+            'Var is the only allowed function (file %r, line %s)' % (
                 filename, getattr(node, 'lineno', '<unknown>')))
-      if node.keywords or node.starargs or node.kwargs:
+      if node.keywords or node.starargs or node.kwargs or len(node.args) != 1:
         raise ValueError(
-            'invalid call: use only regular args (file %r, line %s)' % (
+            'Var takes exactly one argument (file %r, line %s)' % (
                 filename, getattr(node, 'lineno', '<unknown>')))
-      args = map(_convert, node.args)
-      return global_scope[node.func.id](*args)
+      arg = _convert(node.args[0])
+      if not isinstance(arg, basestring):
+        raise ValueError(
+            'Var\'s argument must be a variable name (file %r, line %s)' % (
+                filename, getattr(node, 'lineno', '<unknown>')))
+      return '{%s}' % arg
     elif isinstance(node, ast.BinOp) and isinstance(node.op, ast.Add):
       return _convert(node.left) + _convert(node.right)
     elif isinstance(node, ast.BinOp) and isinstance(node.op, ast.Mod):
@@ -231,7 +235,7 @@ def _gclient_eval(node_or_string, global_scope, filename='<unknown>'):
   return _convert(node_or_string)
 
 
-def Exec(content, global_scope, local_scope, filename='<unknown>'):
+def Exec(content, filename='<unknown>'):
   """Safely execs a set of assignments. Mutates |local_scope|."""
   node_or_string = ast.parse(content, filename=filename, mode='exec')
   if isinstance(node_or_string, ast.Expression):
@@ -249,7 +253,7 @@ def Exec(content, global_scope, local_scope, filename='<unknown>'):
         raise ValueError(
             'invalid assignment: target should be a name (file %r, line %s)' % (
                 filename, getattr(node, 'lineno', '<unknown>')))
-      value = _gclient_eval(node.value, global_scope, filename=filename)
+      value = _gclient_eval(node.value, filename=filename)
 
       if target.id in defined_variables:
         raise ValueError(
@@ -449,7 +453,7 @@ def SetCIPD(gclient_dict, dep_name, package_name, new_version):
   packages[0]._SetNode('version', new_version, node)
 
 
-def SetRevision(gclient_dict, global_scope, dep_name, new_revision):
+def SetRevision(gclient_dict, dep_name, new_revision):
   if not isinstance(gclient_dict, _NodeDict) or gclient_dict.tokens is None:
     raise ValueError(
         "Can't use SetRevision for the given gclient dict. It contains no "
@@ -473,7 +477,7 @@ def SetRevision(gclient_dict, global_scope, dep_name, new_revision):
       SetVar(gclient_dict, node.args[0].s, new_revision)
     else:
       _UpdateAstString(tokens, node, new_revision)
-      value = _gclient_eval(dep_node, global_scope)
+      value = _gclient_eval(dep_node)
       dep_dict._SetNode(dep_key, value, dep_node)
 
   if isinstance(gclient_dict['deps'][dep_name], _NodeDict):

tests/gclient_eval_unittest.py

@@ -60,44 +60,44 @@ vars = {
 
 class GClientEvalTest(unittest.TestCase):
   def test_str(self):
-    self.assertEqual('foo', gclient_eval._gclient_eval('"foo"', {}))
+    self.assertEqual('foo', gclient_eval._gclient_eval('"foo"'))
 
   def test_tuple(self):
-    self.assertEqual(('a', 'b'), gclient_eval._gclient_eval('("a", "b")', {}))
+    self.assertEqual(('a', 'b'), gclient_eval._gclient_eval('("a", "b")'))
 
   def test_list(self):
-    self.assertEqual(['a', 'b'], gclient_eval._gclient_eval('["a", "b"]', {}))
+    self.assertEqual(['a', 'b'], gclient_eval._gclient_eval('["a", "b"]'))
 
   def test_dict(self):
-    self.assertEqual({'a': 'b'}, gclient_eval._gclient_eval('{"a": "b"}', {}))
+    self.assertEqual({'a': 'b'}, gclient_eval._gclient_eval('{"a": "b"}'))
 
   def test_name_safe(self):
-    self.assertEqual(True, gclient_eval._gclient_eval('True', {}))
+    self.assertEqual(True, gclient_eval._gclient_eval('True'))
 
   def test_name_unsafe(self):
     with self.assertRaises(ValueError) as cm:
-      gclient_eval._gclient_eval('UnsafeName', {'UnsafeName': 'foo'})
+      gclient_eval._gclient_eval('UnsafeName')
     self.assertIn('invalid name \'UnsafeName\'', str(cm.exception))
 
   def test_call(self):
     self.assertEqual(
-        'bar',
-        gclient_eval._gclient_eval('Foo("bar")', {'Foo': lambda x: x}))
+        '{bar}',
+        gclient_eval._gclient_eval('Var("bar")'))
 
   def test_plus(self):
-    self.assertEqual('foo', gclient_eval._gclient_eval('"f" + "o" + "o"', {}))
+    self.assertEqual('foo', gclient_eval._gclient_eval('"f" + "o" + "o"'))
 
   def test_format(self):
-    self.assertEqual('foo', gclient_eval._gclient_eval('"%s" % "foo"', {}))
+    self.assertEqual('foo', gclient_eval._gclient_eval('"%s" % "foo"'))
 
   def test_not_expression(self):
     with self.assertRaises(SyntaxError) as cm:
-      gclient_eval._gclient_eval('def foo():\n  pass', {})
+      gclient_eval._gclient_eval('def foo():\n  pass')
     self.assertIn('invalid syntax', str(cm.exception))
 
   def test_not_whitelisted(self):
     with self.assertRaises(ValueError) as cm:
-      gclient_eval._gclient_eval('[x for x in [1, 2, 3]]', {})
+      gclient_eval._gclient_eval('[x for x in [1, 2, 3]]')
     self.assertIn(
         'unexpected AST node: <_ast.ListComp object', str(cm.exception))
 
@@ -105,42 +105,36 @@ class GClientEvalTest(unittest.TestCase):
     for test_case in itertools.permutations(range(4)):
       input_data = ['{'] + ['"%s": "%s",' % (n, n) for n in test_case] + ['}']
       expected = [(str(n), str(n)) for n in test_case]
-      result = gclient_eval._gclient_eval(''.join(input_data), {})
+      result = gclient_eval._gclient_eval(''.join(input_data))
       self.assertEqual(expected, result.items())
 
 
 class ExecTest(unittest.TestCase):
   def test_multiple_assignment(self):
     with self.assertRaises(ValueError) as cm:
-      gclient_eval.Exec('a, b, c = "a", "b", "c"', {}, {})
+      gclient_eval.Exec('a, b, c = "a", "b", "c"')
     self.assertIn(
         'invalid assignment: target should be a name', str(cm.exception))
 
   def test_override(self):
     with self.assertRaises(ValueError) as cm:
-      gclient_eval.Exec('a = "a"\na = "x"', {}, {})
+      gclient_eval.Exec('a = "a"\na = "x"')
     self.assertIn(
         'invalid assignment: overrides var \'a\'', str(cm.exception))
 
   def test_schema_wrong_type(self):
     with self.assertRaises(schema.SchemaError):
-      gclient_eval.Exec('include_rules = {}', {}, {}, '<string>')
+      gclient_eval.Exec('include_rules = {}', '<string>')
 
   def test_recursedeps_list(self):
-    local_scope = {}
     local_scope = gclient_eval.Exec(
         'recursedeps = [["src/third_party/angle", "DEPS.chromium"]]',
-        {}, local_scope,
         '<string>')
     self.assertEqual(
         {'recursedeps': [['src/third_party/angle', 'DEPS.chromium']]},
         local_scope)
 
   def test_var(self):
-    local_scope = {}
-    global_scope = {
-        'Var': lambda var_name: '{%s}' % var_name,
-    }
     local_scope = gclient_eval.Exec('\n'.join([
         'vars = {',
         '  "foo": "bar",',
@@ -148,14 +142,14 @@ class ExecTest(unittest.TestCase):
         'deps = {',
         '  "a_dep": "a" + Var("foo") + "b",',
         '}',
-    ]), global_scope, local_scope, '<string>')
+    ]), '<string>')
     self.assertEqual({
         'vars': collections.OrderedDict([('foo', 'bar')]),
         'deps': collections.OrderedDict([('a_dep', 'a{foo}b')]),
     }, local_scope)
 
   def test_empty_deps(self):
-    local_scope = gclient_eval.Exec('deps = {}', {}, {}, '<string>')
+    local_scope = gclient_eval.Exec('deps = {}', '<string>')
     self.assertEqual({'deps': {}}, local_scope)
 
 
@@ -211,7 +205,7 @@ class EvaluateConditionTest(unittest.TestCase):
 
 class SetVarTest(unittest.TestCase):
   def testSetVar(self):
-    local_scope = gclient_eval.Exec(_SAMPLE_DEPS_FILE, {'Var': str}, {})
+    local_scope = gclient_eval.Exec(_SAMPLE_DEPS_FILE)
 
     gclient_eval.SetVar(local_scope, 'dep_2_rev', 'c0ffee')
     result = gclient_eval.RenderDEPSFile(local_scope)
@@ -223,7 +217,7 @@ class SetVarTest(unittest.TestCase):
 
 class SetCipdTest(unittest.TestCase):
   def testSetCIPD(self):
-    local_scope = gclient_eval.Exec(_SAMPLE_DEPS_FILE, {'Var': str}, {})
+    local_scope = gclient_eval.Exec(_SAMPLE_DEPS_FILE)
 
     gclient_eval.SetCIPD(
         local_scope, 'src/cipd/package', 'another/cipd/package', '6.789')
@@ -234,27 +228,25 @@ class SetCipdTest(unittest.TestCase):
 
 class SetRevisionTest(unittest.TestCase):
   def setUp(self):
-    self.global_scope = {'Var': str}
-    self.local_scope = gclient_eval.Exec(
-        _SAMPLE_DEPS_FILE, self.global_scope, {})
+    self.local_scope = gclient_eval.Exec(_SAMPLE_DEPS_FILE)
 
   def testSetRevision(self):
     gclient_eval.SetRevision(
-        self.local_scope, self.global_scope, 'src/dep', 'deadfeed')
+        self.local_scope, 'src/dep', 'deadfeed')
     result = gclient_eval.RenderDEPSFile(self.local_scope)
 
     self.assertEqual(result, _SAMPLE_DEPS_FILE.replace('deadbeef', 'deadfeed'))
 
   def testSetRevisionInUrl(self):
     gclient_eval.SetRevision(
-        self.local_scope, self.global_scope, 'src/dep_3', '0ff1ce')
+        self.local_scope, 'src/dep_3', '0ff1ce')
     result = gclient_eval.RenderDEPSFile(self.local_scope)
 
     self.assertEqual(result, _SAMPLE_DEPS_FILE.replace('5p1e5', '0ff1ce'))
 
   def testSetRevisionInVars(self):
     gclient_eval.SetRevision(
-        self.local_scope, self.global_scope, 'src/android/dep_2', 'c0ffee')
+        self.local_scope, 'src/android/dep_2', 'c0ffee')
     result = gclient_eval.RenderDEPSFile(self.local_scope)
 
     self.assertEqual(result, _SAMPLE_DEPS_FILE.replace('1ced', 'c0ffee'))