Syntax checker for policy_templates.json

chrome/app/policy/syntax_check_policy_template_json.py

Index: chrome/app/policy/syntax_check_policy_template_json.py
diff --git a/chrome/app/policy/syntax_check_policy_template_json.py b/chrome/app/policy/syntax_check_policy_template_json.py
new file mode 100644
index 0000000000000000000000000000000000000000..3bbbd01ab9bff94934712c23adaf78f156f442b0
--- /dev/null
+++ b/chrome/app/policy/syntax_check_policy_template_json.py
@@ -0,0 +1,393 @@
+#!/usr/bin/python2
+# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+'''
+Checks a policy_templates.json file for conformity to its syntax specification.
+'''
+
+import json
+import optparse
+import os
+import re
+import sys
+
+
+LEADING_WHITESPACE = re.compile('^([ \t]*)')
+TRAILING_WHITESPACE = re.compile('.*?([ \t]+)$')
+
+
+class PolicyTemplateChecker(object):
+
+  def __init__(self):
+    self.error_count = 0
+    self.warning_count = 0
+    self.num_policies = 0
+    self.num_groups = 0
+    self.num_policies_in_groups = 0
+    self.options = None
+
+  def _Error(self, message, parent_element=None, identifier=None,
+             offending_snippet=None):
+    self.error_count += 1
+    error = ''
+    if identifier is not None and parent_element is not None:
+      error += 'In %s %s: ' % (parent_element, identifier)
+    print error + 'Error: ' + message
+    if offending_snippet is not None:
+      print '  Offending:', json.dumps(offending_snippet, indent=2)
+
+  def _CheckContains(self, container, key, value_type,
+                     optional=False,
+                     parent_element='policy',
+                     container_name=None,
+                     identifier=None,
+                     offending='__CONTAINER__'):

[gfeher, 2011/01/14 13:25:07]

Nit: =None is the usual pattern here.

[Jakob Kummerow, 2011/01/14 14:41:55]

Yes, and I'm using =None elsewhere, but I can't use it in this case because None
is precisely the custom value that's passed in to override the default behavior.

+    '''
+    Checks |container| for presence of |key| with value of type |value_type|.
+    
+    The other parameters are needed to generate, if applicable, an appropriate
+    human-readable error message of the following form:
+
+    In |parent_element| |identifier|:
+      (if the key is not present):
+      Error: |container_name| must have a |value_type| named |key|.
+      Offending snippet: |offending| (if specified, defaults to |container|)

[gfeher, 2011/01/14 13:25:07]

"if not specified"?

[Jakob Kummerow, 2011/01/14 14:41:55]

s/,/;/ has the same effect, wouldn't you agree? The sentence is supposed to be
short for:
|offending| if |offending| is specified; otherwise defaults to |container|

+      (if the value does not have the required type):
+      Error: Value of |key| must be a |value_type|.
+      Offending snippet: |container[key]|
+
+    Returns: |container[key]| if the key is present, None otherwise.
+    '''
+    if identifier is None:
+      identifier = container.get('name')
+    if container_name is None:
+      container_name = parent_element
+    if offending == '__CONTAINER__':
+      offending = container
+    if key not in container:
+      if optional:
+        return
+      else:
+        self._Error('%s must have a %s "%s".' %
+                            (container_name.title(), value_type.__name__, key),
+                            container_name, identifier, offending)
+      return None
+    value = container[key]
+    if not isinstance(value, value_type):
+      self._Error('Value of "%s" must be a %s.' %
+                          (key, value_type.__name__),
+                          container_name, identifier, value)
+    return value
+
+  def _CheckPolicy(self, policy, may_contain_groups):
+    if not isinstance(policy, dict):
+      self._Error('Each policy must be a dictionary.', 'policy', None, policy)
+      return
+
+    # Each policy must have a name.
+    self._CheckContains(policy, 'name', str)
+
+    # Each policy must have a type.
+    policy_type = self._CheckContains(policy, 'type', str)
+    if policy_type not in ('group', 'main', 'string', 'int', 'list', 'int-enum',
+                           'string-enum'):
+      self._Error('Policy type must be either of: group, main, string, int, '
+                  'list, int-enum, string-enum', 'policy', policy, policy_type)
+      return  # Can't continue for unsupported type.
+
+    # Each policy must have a caption message.
+    self._CheckContains(policy, 'caption', str)
+
+    # Each policy must have a description message.
+    self._CheckContains(policy, 'desc', str)
+
+    if policy_type == 'group':
+
+      # Groups must not be nested.
+      if not may_contain_groups:
+        self._Error('Policy groups must not be nested.', 'policy', policy)
+
+      # Each policy group must have a list of policies.
+      policies = self._CheckContains(policy, 'policies', list)
+      if policies is not None:
+        for nested_policy in policies:
+          self._CheckPolicy(nested_policy, False)
+
+      # Statistics.
+      self.num_groups += 1
+    else:  # type != group
+
+      # Each policy must have a supported_on list.
+      supported_on = self._CheckContains(policy, 'supported_on', list)
+      if supported_on is not None:
+        for s in supported_on:
+          if not isinstance(s, str):
+            self._Error('Entries in "supported_on" must be strings.', 'policy',
+                              policy, supported_on)
+
+      # Each policy must have a 'features' dict.
+      self._CheckContains(policy, 'features', dict)
+
+      # Each policy must have an 'example_value' of appropriate type.
+      if policy_type == 'main':
+        value_type = bool
+      elif policy_type in ('string', 'string-enum'):
+        value_type = str
+      elif policy_type in ('int', 'int-enum'):
+        value_type = int
+      elif policy_type == 'list':
+        value_type = list
+      else:
+        raise NotImplementedError('Unimplemented policy type: %s' % policy_type)
+      self._CheckContains(policy, 'example_value', value_type)
+
+      # Statistics.
+      self.num_policies += 1
+      if not may_contain_groups:
+        self.num_policies_in_groups += 1
+
+    # If 'deprecated' is present, it must be a bool.
+    self._CheckContains(policy, 'deprecated', bool, True)
+
+    # If 'label' is present, it must be a string.
+    self._CheckContains(policy, 'label', str, True)

[gfeher, 2011/01/14 13:25:07]

Nit/suggestion: move these two up next to 'caption' and 'desc'?

[Jakob Kummerow, 2011/01/14 14:41:55]

Done.

+
+    if policy_type in ('int-enum', 'string-enum'):
+
+      # Enums must contain a list of items.
+      items = self._CheckContains(policy, 'items', list)
+      if items is not None:
+        if len(items) < 1:
+          self._Error('"items" must not be empty.', 'policy', policy, items)
+        for item in items:
+
+          # Each item must have a name.
+          self._CheckContains(item, 'name', str, container_name='item',
+                              identifier=policy.get('name'))
+
+          # Each item must have a value of the correct type.
+          self._CheckContains(item, 'value', value_type, container_name='item',
+                              identifier=policy.get('name'))
+
+          # Each item must have a caption.
+          self._CheckContains(item, 'caption', str, container_name='item',
+                              identifier=policy.get('name'))
+
+    # There should not be any unknown keys in |policy|.
+    for key in policy:
+      if key not in ('name', 'type', 'caption', 'desc', 'supported_on',
+                     'label', 'policies', 'items', 'example_value', 'features',
+                     'deprecated'):
+        self.warning_count += 1
+        print ('In policy %s: Warning: Unknown key: %s' %
+               (policy.get('name'), key))

[gfeher, 2011/01/14 13:25:07]

My general suggestion is that you should move the checks that are
type-independent next to each other, e.g.: at the beginning of this method. (Or
probably a separate method, _CheckCommonPolicyKeys)

[Jakob Kummerow, 2011/01/14 14:41:55]

Done.

+
+  def _CheckMessage(self, key, value):
+    # |key| must be a string, |value| a dict.
+    if not isinstance(key, str):
+      self._Error('Each message key must be a string.', 'message', key, key)
+      return
+
+    if not isinstance(value, dict):
+      self._Error('Each message must be a dictionary.', 'message', key, value)
+      return
+
+    # Each message must have a desc.
+    self._CheckContains(value, 'desc', str, parent_element='message',
+                        identifier=key)
+
+    # Each message must have a text.
+    self._CheckContains(value, 'text', str, parent_element='message',
+                        identifier=key)
+
+    # There should not be any unknown keys in |value|.
+    for vkey in value:
+      if vkey not in ('desc', 'text'):
+        self.warning_count += 1
+        print 'In message %s: Warning: Unknown key: %s' % (key, vkey)
+
+  def _CheckPlaceholder(self, placeholder):
+    if not isinstance(placeholder, dict):
+      self._Error('Each placeholder must be a dictionary.',
+                  'placeholder', None, placeholder)
+      return
+
+    # Each placeholder must have a 'key'.
+    key = self._CheckContains(placeholder, 'key', str,
+                              parent_element='placeholder')
+
+    # Each placeholder must have a 'value'.
+    self._CheckContains(placeholder, 'value', str, parent_element='placeholder',
+                        identifier=key)
+
+    # There should not be any unknown keys in |placeholder|.
+    for k in placeholder:
+      if k not in ('key', 'value'):
+        self.warning_count += 1
+        name = str(placeholder.get('key'), placeholder)
+        print 'In placeholder %s: Warning: Unknown key: %s' % (name, k)
+
+  def _LeadingWhitespace(self, line):
+    match = LEADING_WHITESPACE.match(line)
+    if match:
+      return match.group(1)
+    return ''
+
+  def _TrailingWhitespace(self, line):
+    match = TRAILING_WHITESPACE.match(line)
+    if match:
+      return match.group(1)
+    return ''
+
+  def _LineError(self, message, line_number):
+    self.error_count += 1
+    print 'In line %d: Error: %s' % (line_number, message)
+
+  def _LineWarning(self, message, line_number):
+    self.warning_count += 1
+    print ('In line %d: Warning: Automatically fixing formatting: %s'
+           % (line_number, message))
+
+  def _CheckFormat(self, filename):
+    if self.options.fix:
+      fixed_lines = []
+    with open(filename) as f:
+      indent = 0
+      line_number = 0
+      for line in f:
+        line_number += 1
+        line = line.rstrip('\n')
+        # Check for trailing whitespace.
+        trailing_whitespace = self._TrailingWhitespace(line)
+        if len(trailing_whitespace) > 0:
+          if self.options.fix:
+            line = line.rstrip()
+            self._LineWarning('Trailing whitespace.', line_number)
+          else:
+            self._LineError('Trailing whitespace.', line_number)
+        if len(line) == 0:
+          if self.options.fix:
+            fixed_lines += ['\n']
+          continue
+        if len(line) == len(trailing_whitespace):
+          continue
+        # Check for correct amount of leading whitespace.
+        leading_whitespace = self._LeadingWhitespace(line)
+        if leading_whitespace.count('\t') > 0:
+          if self.options.fix:
+            line = leading_whitespace.replace('\t', '  ') + line.lstrip()
+            self._LineWarning('Tab character found.', line_number)
+          else:
+            self._LineError('Tab character found.', line_number)
+        if line[len(leading_whitespace)] in (']', '}'):
+          indent -= 2
+        if line[0] != '#':  # Ignore 0-indented comments.
+          if len(leading_whitespace) != indent:
+            if self.options.fix:
+              line = ' ' * indent + line.lstrip()
+              self._LineWarning('Indentation should be ' + str(indent) +
+                                ' spaces.', line_number)
+            else:
+              self._LineError('Bad indentation. Should be ' + str(indent) +
+                              ' spaces.', line_number)
+        if line[-1] in ('[', '{'):
+          indent += 2
+        if self.options.fix:
+          fixed_lines.append(line + '\n')
+
+    # If --fix is specified: backup the file (deleting any existing backup),
+    # then write the fixed version with the old filename.
+    if self.options.fix:
+      if self.options.backup:
+        backupfilename = filename + '.bak'
+        if os.path.exists(backupfilename):
+          os.remove(backupfilename)
+        os.rename(filename, backupfilename)
+      with open(filename, 'w') as f:
+        f.writelines(fixed_lines)
+
+  def Main(self, filename, options):
+    try:
+      with open(filename) as f:
+        data = eval(f.read())
+    except:
+      import traceback
+      traceback.print_exc(file=sys.stdout)
+      self._Error('Invalid JSON syntax.')
+      return
+    if data == None:
+      self._Error('Invalid JSON syntax.')
+      return
+    self.options = options
+
+    # First part: check JSON structure.
+
+    # Check policy definitions.
+    policy_definitions = self._CheckContains(data, 'policy_definitions', list,
+                                             parent_element=None,
+                                             container_name='The root element',
+                                             offending=None)
+    if policy_definitions is not None:
+      for policy in policy_definitions:
+        self._CheckPolicy(policy, True)
+
+    # Check (non-policy-specific) message definitions.
+    messages = self._CheckContains(data, 'messages', dict,
+                                   parent_element=None,
+                                   container_name='The root element',
+                                   offending=None)
+    if messages is not None:
+      for message in messages:
+        self._CheckMessage(message, messages[message])
+
+    # Check placeholders.
+    placeholders = self._CheckContains(data, 'placeholders', list,
+                                       parent_element=None,
+                                       container_name='The root element',
+                                       offending=None)
+    if placeholders is not None:
+      for placeholder in placeholders:
+        self._CheckPlaceholder(placeholder)
+
+    # Second part: check formatting.
+    self._CheckFormat(filename)
+
+    # Third part: summary and exit.
+    print ('Finished. %d errors, %d warnings.' %
+        (self.error_count, self.warning_count))
+    if self.options.stats:
+      if self.num_groups > 0:
+        print ('%d policies, %d of those in %d groups (containing on '
+               'average %.1f policies).' %
+               (self.num_policies, self.num_policies_in_groups, self.num_groups,
+                 (1.0 * self.num_policies_in_groups / self.num_groups)))
+      else:
+        print self.num_policies, 'policies, 0 policy groups.'
+    if self.error_count > 0:
+      return 1
+    return 0
+
+  def Run(self, argv, filename=None):
+    parser = optparse.OptionParser(
+        usage='usage: %prog [options] filename',
+        description='Syntax check a policy_templates.json file.')
+    parser.add_option('--fix', action='store_true',
+                      help='Automatically fix formatting.')
+    parser.add_option('--backup', action='store_true',
+                      help='Create backup of original file (before fixing).')
+    parser.add_option('--stats', action='store_true',
+                      help='Generate statistics.')
+    (options, args) = parser.parse_args(argv)
+    if filename is None:
+      if len(args) != 2:
+        parser.print_help()
+        sys.exit(1)
+      filename = args[1]
+    return self.Main(filename, options)
+
+
+if __name__ == '__main__':
+  checker = PolicyTemplateChecker()
+  sys.exit(checker.Run(sys.argv))