ZODB Components: Like Python Script, use Ace Editor annotations to check...

ZODB Components: Like Python Script, use Ace Editor annotations to check source code without saving.

Also, switch to Pylint for Python Script source code checking to use the same
code for both.

bt5/erp5_ace_editor/SkinTemplateItem/portal_skins/erp5_ace_editor/ace_editor_support.xml

@@ -414,15 +414,36 @@
     ace_editor.getSession().setMode(new PythonMode());\n
     ace_editor.getSession().setUseSoftTabs(true);\n
     ace_editor.getSession().setTabSize(2);\n
+\n
+    ace.require(\'ace/ext/language_tools\');\n
+    ace_editor.setOptions({ enableBasicAutocompletion: true, enableSnippets: true });\n
+\n
+    timer = 0;\n
+    function checkPythonSourceCode() {\n
+      if (timer) {\n
+        window.clearTimeout(timer);\n
+        timer = 0;\n
+      }\n
+      timer = window.setTimeout(function() {\n
+        $.post(\'${portal_url}/ERP5Site_checkPythonSourceCodeAsJSON\',\n
+          {\'data\': JSON.stringify(\n
+          { code: ace_editor.getSession().getValue() })},\n
+          function(data){\n
+            ace_editor.getSession().setAnnotations(data.annotations);\n
+          }\n
+        )\n
+      }, 500);\n
+    }\n
+\n
+    checkPythonSourceCode();\n
 \n
     var textarea = $(\'#${id}\');\n
     ace_editor.getSession().on(\'change\', function() {\n
       changed = true; // This is the dirty flag for onbeforeunload warning in erp5.js\n
       textarea.val(ace_editor.getSession().getValue());\n
+      checkPythonSourceCode();\n
     });\n
-    ace.require(\'ace/ext/language_tools\');\n
-    ace_editor.setOptions({ enableBasicAutocompletion: true, enableSnippets: true });\n
-    \n
+\n
     /* Only display the source code saving button if the main save button is\n
      * displayed. This specific save button allows to save without reloading the\n
      * page (and thus keep the cursor position and mode (maximize/fullscreen)\n

bt5/erp5_ace_editor/bt/change_log

+2014-02-24 arnaud.fontaine
+* Add annotations support to check source code without saving.
+
 2013-08-22 jerome
 * Update to 45d3068aa7190f08396bcfe134e505fe144c1ccb ( package 10.28.2013 )
 

bt5/erp5_ace_editor/bt/revision

-19
\ No newline at end of file
+20
\ No newline at end of file

bt5/erp5_administration/ExtensionTemplateItem/portal_components/extension.erp5.ERP5Administration.py

@@ -136,16 +136,12 @@ def checkConversionToolAvailability(self):
   result.edit(severity=severity)
   active_process.activateResult(result)
 
-def runPyflakes(script_code, script_path):
-  # TODO: reuse _runPyflakes ...
-  from pyflakes.api import check
-  from pyflakes import reporter
-  from StringIO import StringIO
-  stream = StringIO()
-  check(script_code, script_path, reporter.Reporter(stream, stream))
-  return stream.getvalue()
-
-def runPyflakesOnPythonScript(self, data):
+from Products.ERP5Type.Utils import checkPythonSourceCode
+
+def checkPythonSourceCodeAsJSON(self, data):
+  """
+  Check Python source suitable for Ace Editor and return a JSON object
+  """
   import json
 
   # XXX data is encoded as json, because jQuery serialize lists as []
@@ -153,56 +149,35 @@ def runPyflakesOnPythonScript(self, data):
     data = json.loads(data)
 
   # data contains the code, the bound names and the script params. From this
-  # we reconstruct a function that can be parsed with pyflakes.
-  code = data
+  # we reconstruct a function that can be checked
   def indent(text):
     return ''.join(("  " + line) for line in text.splitlines(True))
 
-  bound_names = data['bound_names']
-  signature_parts = data['bound_names']
-  if data['params']:
-    signature_parts += [data['params']]
-  signature = ", ".join(signature_parts)
+  is_python_script = 'bound_names' in data
+  if is_python_script:
+    signature_parts = data['bound_names']
+    if data['params']:
+      signature_parts += [data['params']]
+    signature = ", ".join(signature_parts)
+
+    function_name = "function_name"
+    body = "def %s(%s):\n%s" % (function_name,
+                                signature,
+                                indent(data['code']) or "  pass")
+  else:
+    body = data['code']
 
-  function_name = "function_name"
-  body = "def %s(%s):\n%s" % (function_name,
-                              signature,
-                              indent(data['code']) or "  pass")
+  message_list = checkPythonSourceCode(body)
+  for message_dict in message_list:
+    if is_python_script:
+      message_dict['row'] = message_dict['row'] - 2
+    else:
+      message_dict['row'] = message_dict['row'] - 1
 
-  error_list = _runPyflakes(body, lineno_offset=-1)
+    if message_dict['type'] in ('E', 'F'):
+      message_dict['type'] = 'error'
+    else:
+      message_dict['type'] = 'warning'
 
   self.REQUEST.RESPONSE.setHeader('content-type', 'application/json')
-  return json.dumps(dict(annotations=error_list))
-
-
-def _runPyflakes(code, lineno_offset=0):
-  import pyflakes.api
-  error_list = []
-
-  class Reporter(object):
-    def unexpectedError(self, filename, msg):
-        error_list.append(
-         { 'row': 0,
-           'column': 0,
-           'text': msg,
-           'type': 'error' }
-        )
-
-    def syntaxError(self, filename, msg, lineno, offset, text):
-        error_list.append(
-         { 'row': lineno - 1 + lineno_offset,
-           'column': offset,
-           'text': msg + (text and ": " + text or ''),
-           'type': 'error' }
-        )
-
-    def flake(self, message):
-      error_list.append(
-         { 'row': message.lineno - 1 + lineno_offset,
-           'column': getattr(message, 'col', 0),
-           'text': message.message % message.message_args,
-           'type': 'warning' }
-      )
-
-  pyflakes.api.check(code, '', Reporter())
-  return error_list
+  return json.dumps(dict(annotations=message_list))
\ No newline at end of file

bt5/erp5_administration/SkinTemplateItem/portal_skins/erp5_administration/ERP5Site_checkPythonScriptAsJSON.xml → bt5/erp5_administration/SkinTemplateItem/portal_skins/erp5_administration/ERP5Site_checkPythonSourceCodeAsJSON.xml

@@ -8,7 +8,7 @@
       <dictionary>
         <item>
             <key> <string>_function</string> </key>
-            <value> <string>runPyflakesOnPythonScript</string> </value>
+            <value> <string>checkPythonSourceCodeAsJSON</string> </value>
         </item>
         <item>
             <key> <string>_module</string> </key>
@@ -16,7 +16,7 @@
         </item>
         <item>
             <key> <string>id</string> </key>
-            <value> <string>ERP5Site_checkPythonScriptAsJSON</string> </value>
+            <value> <string>ERP5Site_checkPythonSourceCodeAsJSON</string> </value>
         </item>
         <item>
             <key> <string>title</string> </key>

bt5/erp5_administration/bt/change_log

+2014-02-24 arnaud.fontaine
+* Use Pylint to check source code to unify ZODB Components and Python Script source code checking.
+
 2013-09-03 arnaud.fontaine
 * ZODB Components: Workflow History must always be kept, so avoid an extra step for developers.
 

bt5/erp5_administration/bt/revision

-158
\ No newline at end of file
+159
\ No newline at end of file

product/ERP5/bootstrap/erp5_core/WorkflowTemplateItem/portal_workflow/dynamic_class_generation_interaction_workflow/scripts/Component_checkSourceCodeAndValidateAfterModified.xml

@@ -52,7 +52,19 @@
             <key> <string>_body</string> </key>
             <value> <string>obj = state_change[\'object\']\n
 \n
-error_list, warning_list = obj.checkSourceCode()\n
+error_list = []\n
+warning_list = []\n
+for message_dict in obj.checkSourceCode():\n
+  message = \'%s:%3d,%3d: %s\' % (message_dict[\'type\'],\n
+                                  message_dict[\'row\'],\n
+                                  message_dict[\'column\'],\n
+                                  message_dict[\'text\'])\n
+\n
+  if message_dict[\'type\'] in (\'F\', \'E\'):\n
+    error_list.append(message)\n
+  else:\n
+    warning_list.append(message)\n
+\n
 obj.setTextContentWarningMessageList(warning_list)\n
 obj.setTextContentErrorMessageList(error_list)\n
 \n

product/ERP5/bootstrap/erp5_core/bt/change_log

+2014-02-24 arnaud.fontaine
+* Follow API changes to unify ZODB Components and Python Script source code checking.
+
 2014-02-19 Arnaud Fontaine
 * ZODB Components: Remove ClassTool and DocumentationHelper relying on it.
 

product/ERP5/bootstrap/erp5_core/bt/revision

-41152
\ No newline at end of file
+41153
\ No newline at end of file

product/ERP5Type/Utils.py

@@ -419,6 +419,116 @@ def fill_args_from_request(*optional_args):
     return decorator(function)
   return decorator
 
+_pylint_message_re = re.compile(
+  '^(?P<type>[CRWEF]):\s*(?P<row>\d+),\s*(?P<column>\d+):\s*(?P<message>.*)$')
+
+def checkPythonSourceCode(source_code_str):
+  """
+  Check source code with pylint or compile() builtin if not available.
+
+  TODO-arnau: Get rid of NamedTemporaryFile (require a patch on pylint to
+              allow passing a string) and this should probably return a proper
+              ERP5 object rather than a dict...
+  """
+  if not source_code_str:
+    return []
+
+  try:
+    from pylint.lint import Run
+    from pylint.reporters.text import TextReporter
+  except ImportError, error:
+    try:
+      compile(source_code_str, '<string>', 'exec')
+      return []
+    except Exception, error:
+      if isinstance(error, SyntaxError):
+        message = {'type': 'F',
+                   'row': error.lineno,
+                   'column': error.offset,
+                   'text': error.message}
+      else:
+        message = {'type': 'F',
+                   'row': -1,
+                   'column': -1,
+                   'text': str(error)}
+
+      return [message]
+
+  import cStringIO
+  import tempfile
+  import sys
+
+  #import time
+  #started = time.time()
+  message_list = []
+  output_file = cStringIO.StringIO()
+
+  # pylint prints directly on stderr/stdout (only reporter content matters)
+  stderr = sys.stderr
+  stdout = sys.stdout
+  try:
+    sys.stderr = cStringIO.StringIO()
+    sys.stdout = cStringIO.StringIO()
+
+    with tempfile.NamedTemporaryFile() as input_file:
+      input_file.write(source_code_str)
+      input_file.seek(0)
+
+      Run([input_file.name, '--reports=n', '--indent-string="  "', '--zope=y',
+           # Disable Refactoring and Convention messages which are too verbose
+           # TODO-arnau: Should perphaps check ERP5 Naming Conventions?
+           '--disable=R,C',
+           # 'String statement has no effect': eg docstring at module level
+           '--disable=W0105',
+           # 'Using possibly undefined loop variable %r': Spurious warning
+           # (loop variables used after the loop)
+           '--disable=W0631',
+           # 'fixme': No need to display TODO/FIXME entry in warnings
+           '--disable=W0511',
+           # 'Unused argument %r': Display for readability or when defining abstract methods
+           '--disable=W0613',
+           # 'Catching too general exception %s': Too coarse
+           # TODO-arnau: Should consider raise in except
+           '--disable=W0703',
+           # 'Used * or ** magic': commonly used in ERP5
+           '--disable=W0142',
+           # 'Class has no __init__ method': Spurious warning
+           '--disable=W0232',
+           # 'Attribute %r defined outside __init__': Spurious warning
+           '--disable=W0201',
+           # Dynamic class generation so some attributes may not be found
+           # TODO-arnau: Enable it properly would require inspection API
+           # '%s %r has no %r member'
+           '--disable=E1101,E1103',
+           # 'No name %r in module %r'
+           '--disable=E0611',
+           # map and filter should not be considered bad as in some cases
+           # map is faster than its recommended replacement (list
+           # comprehension)
+           '--bad-functions=apply,input',
+           # 'Access to a protected member %s of a client class'
+           '--disable=W0212',
+           # string module does not only contain deprecated functions...
+           '--deprecated-modules=regsub,TERMIOS,Bastion,rexec'],
+          reporter=TextReporter(output_file), exit=False)
+
+    output_file.reset()
+    for line in output_file:
+      match_obj = _pylint_message_re.match(line)
+      if match_obj:
+        message_list.append({'type': match_obj.group('type'),
+                             'row': int(match_obj.group('row')),
+                             'column': int(match_obj.group('column')),
+                             'text': match_obj.group('message')})
+
+  finally:
+    output_file.close()
+    sys.stderr = stderr
+    sys.stdout = stdout
+
+  #LOG('Utils', INFO, 'Checking time (pylint): %.2f' % (time.time() - started))
+  return message_list
+
 #####################################################
 # Globals initialization
 #####################################################

product/ERP5Type/mixin/component.py

@@ -41,10 +41,7 @@ from Products.ERP5Type.ConsistencyMessage import ConsistencyMessage
 from zLOG import LOG, INFO
 
 from ExtensionClass import ExtensionClass
-from Products.ERP5Type.Utils import convertToUpperCase
-
-import re
-pylint_message_re = re.compile('^(?P<type>[CRWEF]):\s*\d+,\s*\d+:\s*.*$')
+from Products.ERP5Type.Utils import convertToUpperCase, checkPythonSourceCode
 
 class RecordablePropertyMetaClass(ExtensionClass):
   """
@@ -279,109 +276,10 @@ class ComponentMixin(PropertyRecordableMixin, Base):
   security.declareProtected(Permissions.ModifyPortalContent, 'checkSourceCode')
   def checkSourceCode(self):
     """
-    Check source code with pylint
-
-    TODO-arnau: Get rid of NamedTemporaryFile (require a patch on pylint to
-                allow passing a string)
+    Check Component source code through Pylint or compile() builtin if not
+    available
     """
-    source_code = self.getTextContent()
-    # checkConsistency() ensures that it cannot happen once validated/modified
-    if not source_code:
-      return [], []
-
-    try:
-      from pylint.lint import Run
-      from pylint.reporters.text import TextReporter
-    except ImportError, error:
-      try:
-        compile(source_code, '<string>', 'exec')
-        return [], []
-      except Exception, error:
-        if isinstance(error, SyntaxError):
-          error = '%4d, %4d: %s' % (error.lineno,
-                                    error.offset,
-                                    error.message)
-
-        return ['F: %s' % error], []
-
-    import cStringIO
-    import tempfile
-    import sys
-
-    #import time
-    #started = time.time()
-    error_list = []
-    warning_list = []
-    output_file = cStringIO.StringIO()
-
-    # pylint prints directly on stderr/stdout (only reporter content matters)
-    stderr = sys.stderr
-    stdout = sys.stdout
-    try:
-      sys.stderr = cStringIO.StringIO()
-      sys.stdout = cStringIO.StringIO()
-
-      with tempfile.NamedTemporaryFile() as input_file:
-        input_file.write(source_code)
-        input_file.seek(0)
-
-        Run([input_file.name, '--reports=n', '--indent-string="  "', '--zope=y',
-             # Disable Refactoring and Convention messages which are too verbose
-             # TODO-arnau: Should perphaps check ERP5 Naming Conventions?
-             '--disable=R,C',
-             # 'String statement has no effect': eg docstring at module level
-             '--disable=W0105',
-             # 'Using possibly undefined loop variable %r': Spurious warning
-             # (loop variables used after the loop)
-             '--disable=W0631',
-             # 'fixme': No need to display TODO/FIXME entry in warnings
-             '--disable=W0511',
-             # 'Unused argument %r': Display for readability or when defining abstract methods
-             '--disable=W0613',
-             # 'Catching too general exception %s': Too coarse
-             # TODO-arnau: Should consider raise in except
-             '--disable=W0703',
-             # 'Used * or ** magic': commonly used in ERP5
-             '--disable=W0142',
-             # 'Class has no __init__ method': Spurious warning
-             '--disable=W0232',
-             # 'Attribute %r defined outside __init__': Spurious warning
-             '--disable=W0201',
-             # Dynamic class generation so some attributes may not be found
-             # TODO-arnau: Enable it properly would require inspection API
-             # '%s %r has no %r member'
-             '--disable=E1101,E1103',
-             # 'No name %r in module %r'
-             '--disable=E0611',
-             # map and filter should not be considered bad as in some cases
-             # map is faster than its recommended replacement (list
-             # comprehension)
-             '--bad-functions=apply,input',
-             # 'Access to a protected member %s of a client class'
-             '--disable=W0212',
-             # string module does not only contain deprecated functions...
-             '--deprecated-modules=regsub,TERMIOS,Bastion,rexec'],
-            reporter=TextReporter(output_file), exit=False)
-
-      output_file.reset()
-      for line in output_file:
-        message_obj = pylint_message_re.match(line)
-        if message_obj:
-          line = line.strip()
-          if line[0] in ('E', 'F'):
-            error_list.append(line)
-          else:
-            warning_list.append(line)
-
-    finally:
-      output_file.close()
-      sys.stderr = stderr
-      sys.stdout = stdout
-
-      #LOG('component', INFO, 'Checking time (pylint): %.2f' % (time.time() -
-      #                                                         started))
-
-    return error_list, warning_list
+    return checkPythonSourceCode(self.getTextContent())
 
   security.declareProtected(Permissions.ModifyPortalContent, 'PUT')
   def PUT(self, REQUEST, RESPONSE):

product/ERP5Type/patches/AceEditorZMI.py

@@ -98,7 +98,7 @@ $(document).ready(function() {
           timer = 0;
         }
         timer = window.setTimeout(function() {
-          $.post('%(portal_url)s/ERP5Site_checkPythonScriptAsJSON',
+          $.post('%(portal_url)s/ERP5Site_checkPythonSourceCodeAsJSON',
             {'data': JSON.stringify(
             { code: editor.getSession().getValue(),
               bound_names: %(bound_names)s,

product/ERP5Type/tests/testDynamicClassGeneration.py

@@ -1627,17 +1627,17 @@ class _TestZodbComponent(SecurityTestCase):
     self.assertEqual(component.checkConsistency(), [])
     self.assertEqual(component.getTextContentErrorMessageList(), [])
     self.assertEqual(component.getTextContentWarningMessageList(),
-                      ["W:  1, 0: Unused import sys (unused-import)"])
+                      ["W:  1,  0: Unused import sys (unused-import)"])
 
     component.setTextContent('import unexistent_module')
     self.tic()
     self.assertEqual(
       [m.getMessage().translate() for m in component.checkConsistency()],
-      ["Error in Source Code: F:  1, 0: Unable to import 'unexistent_module' (import-error)"])
+      ["Error in Source Code: F:  1,  0: Unable to import 'unexistent_module' (import-error)"])
     self.assertEqual(component.getTextContentErrorMessageList(),
-                      ["F:  1, 0: Unable to import 'unexistent_module' (import-error)"])
+                      ["F:  1,  0: Unable to import 'unexistent_module' (import-error)"])
     self.assertEqual(component.getTextContentWarningMessageList(),
-                      ["W:  1, 0: Unused import unexistent_module (unused-import)"])
+                      ["W:  1,  0: Unused import unexistent_module (unused-import)"])
 
     valid_code = 'def foobar():\n  return 42'
     ComponentTool.reset = assertResetCalled
@@ -1669,15 +1669,15 @@ class _TestZodbComponent(SecurityTestCase):
        [],
        []),
       ('def foobar(*args, **kwargs)\n  return 42',
-       ["Error in Source Code: E:  1, 0: invalid syntax (syntax-error)"],
-       ["E:  1, 0: invalid syntax (syntax-error)"],
+       ["Error in Source Code: E:  1,  0: invalid syntax (syntax-error)"],
+       ["E:  1,  0: invalid syntax (syntax-error)"],
        []),
       # Make sure that foobar NameError is at the end to make sure that after
       # defining foobar function, it is not available at all
       ('foobar',
-       ["Error in Source Code: E:  1, 0: Undefined variable 'foobar' (undefined-variable)"],
-       ["E:  1, 0: Undefined variable 'foobar' (undefined-variable)"],
-       ["W:  1, 0: Statement seems to have no effect (pointless-statement)"]))
+       ["Error in Source Code: E:  1,  0: Undefined variable 'foobar' (undefined-variable)"],
+       ["E:  1,  0: Undefined variable 'foobar' (undefined-variable)"],
+       ["W:  1,  0: Statement seems to have no effect (pointless-statement)"]))
 
     for (invalid_code,
          check_consistency_list,