Add a presubmit check for boolean policy polarity.

chrome/app/policy/syntax_check_policy_template_json.py

 #!/usr/bin/env python
 # Copyright (c) 2012 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
@@ skipping 12 matching lines @@
 TYPE_TO_SCHEMA = {
   'int': 'integer',
   'list': 'array',
   'dict': 'object',
   'main': 'boolean',
   'string': 'string',
   'int-enum': 'integer',
   'string-enum': 'string',
 }
 
+# List of boolean policies that have been introduced with negative polarity in
+# the past and should not trigger the negative polarity check.
+LEGACY_INVERTED_POLARITY_WHITELIST = [
+    'DeveloperToolsDisabled',
+    'DeviceAutoUpdateDisabled',
+    'Disable3DAPIs',
+    'DisableAuthNegotiateCnameLookup',
+    'DisablePluginFinder',
+    'DisablePrintPreview',
+    'DisableSafeBrowsingProceedAnyway',
+    'DisableScreenshots',
+    'DisableSpdy',
+    'DisableSSLRecordSplitting',
+    'ExternalStorageDisabledforrealz',
+    'GDataDisabled',
+    'GDataDisabledOverCellular',
+    'SavingBrowserHistoryDisabled',
+    'SyncDisabled',
+]
+
 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
 
@@ skipping 77 matching lines @@
     '''
     for i in range(len(policy_ids)):
       if (i + 1) not in policy_ids:
         self._Error('No policy with id: %s' % (i + 1))
 
   def _CheckPolicySchema(self, policy, policy_type):
     '''Checks that the 'schema' field matches the 'type' field.'''
     self._CheckContains(policy, 'schema', dict)
     if isinstance(policy.get('schema'), dict):
       self._CheckContains(policy['schema'], 'type', str)
-      if policy['schema'].get('type') != TYPE_TO_SCHEMA[policy_type]:
+      schema_type = policy['schema'].get('type')
+      if schema_type != TYPE_TO_SCHEMA[policy_type]:
         self._Error('Schema type must match the existing type for policy %s' %
                     policy.get('name'))
 
+      # Checks that boolean policies are not negated (which makes them harder to
+      # reason about).
+      if (schema_type == 'boolean' and
+          "disable" in policy.get('name').lower() and

[Joao da Silva, 2012/08/06 21:49:05]

Nit: 'disable' with single quotes

[Mattias Nissler (ping if slow), 2012/08/07 08:20:07]

Done.

+          policy.get('name') not in LEGACY_INVERTED_POLARITY_WHITELIST):
+        self._Error(('Boolean policy %s uses negative polarity, please make ' +
+                     'new boolean policies follow the XYZEnabled pattern. ' +
+                     'See also http://crbug.com/85687') % policy.get('name'))
+
+
   def _CheckPolicy(self, policy, is_in_group, policy_ids):
     if not isinstance(policy, dict):
       self._Error('Each policy must be a dictionary.', 'policy', None, policy)
       return
 
     # There should not be any unknown keys in |policy|.
     for key in policy:
       if key not in ('name', 'type', 'caption', 'desc', 'device_only',
                      'supported_on', 'label', 'policies', 'items',
                      'example_value', 'features', 'deprecated', 'future',
@@ skipping 290 matching lines @@
     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__':
   sys.exit(PolicyTemplateChecker().Run(sys.argv))