Commit fbeff3f4 authored by Jérome Perrin's avatar Jérome Perrin

More pylint integration

Since early days of @arnau work on ERP5 components, we have pylint integrated as a way to check the code in components and in python scripts but nothing prevents us from commiting code with lint errors, leaving the problem for the next developer. 

For this, I extended [`CodingStyleTestCase`](https://lab.nexedi.com/nexedi/erp5/blob/88e4b34ff67aa07447f81c8af836feba607843e8/product/ERP5Type/tests/CodingStyleTestCase.py) that was introduced for very similar purpose so that it also  use our tool to check python code. This way,  we can have failing tests when the code do not pass pylint.

What's still not good is that if we want a business template to be tested, we have to explicitly add a test for this business template, but it's also the case for all our "static" tests (naming conventions, HTML validation etc).


A missing feature was that there was no way to get an overview of all the reported problems on a business template. For this, I added a report action on business template that uses the same code to display errors in all *python* code from a business template:

![ERP5_pylint_business_template](/uploads/13613199227884f0340b8f1c40ca4418/ERP5_pylint_business_template.png)
( the lines have direct links to edit python code )

Because `testERP5WebCodingStyle` was already here checking `erp5_web` (well not really because it was no longer using `CodingStyleTestCase` correctly) , I fixed all problems reported by pylint in erp5_web. In reality it only took a few minutes to do this.

PS: I don't know exactly how we could integrate javascript linter in all this. We have a [`test_javascript_lint`](https://lab.nexedi.com/nexedi/erp5/blob/master/product/ERP5/tests/testXHTML.py#L171), but it does not test much of the code written recently because it only checks portal skins. @romain and @vincentB maybe have plans for this. Anyway, it's out of the scope of this.

/cc @kazuhiko @vpelletier @tc 

/reviewed-on nexedi/erp5!308
parents f74f420e 8637adef
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="ActionInformation" module="Products.CMFCore.ActionInformation"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>action</string> </key>
<value>
<persistent> <string encoding="base64">AAAAAAAAAAI=</string> </persistent>
</value>
</item>
<item>
<key> <string>categories</string> </key>
<value>
<tuple>
<string>action_type/object_report</string>
</tuple>
</value>
</item>
<item>
<key> <string>category</string> </key>
<value> <string>object_report</string> </value>
</item>
<item>
<key> <string>condition</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>description</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>icon</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>check_python_code</string> </value>
</item>
<item>
<key> <string>permissions</string> </key>
<value>
<tuple>
<string>Manage portal</string>
</tuple>
</value>
</item>
<item>
<key> <string>priority</string> </key>
<value> <float>12.0</float> </value>
</item>
<item>
<key> <string>title</string> </key>
<value> <string>Check Python Code</string> </value>
</item>
<item>
<key> <string>visible</string> </key>
<value> <int>1</int> </value>
</item>
</dictionary>
</pickle>
</record>
<record id="2" aka="AAAAAAAAAAI=">
<pickle>
<global name="Expression" module="Products.CMFCore.Expression"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>text</string> </key>
<value> <string>string:${object_url}/BusinessTemplate_viewCheckPythonCodeDialog</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
"""Return python source code in business template
"""
import json
class Message:
"""A python code linter message, with a link to edit the source code.
Supports both being displayed in a listbox and being printed.
"""
def __init__(self, location, message, edit_url):
self.location = location
self.message = message
self.edit_url = edit_url
def getListItemUrl(self, *args, **kw):
return self.edit_url
def __repr__(self):
return "{}:{}".format(self.location, self.message)
portal = context.getPortalObject()
line_list = []
def checkPythonScript(script_instance, script_path):
"""Check a python script, adding messages to global `line_list`
"""
# printed is from RestrictedPython.RestrictionMutator the rest comes from
# RestrictedPython.Utilities.utility_builtins
extra_builtins = ['printed', 'same_type', 'string', 'sequence', 'random',
'DateTime', 'whrandom', 'reorder', 'sets', 'test', 'math']
for annotation in json.loads(portal.ERP5Site_checkPythonSourceCodeAsJSON(
{'bound_names': extra_builtins +
script_instance.getBindingAssignments().getAssignedNamesInOrder(),
'params': script_instance.params(),
'code': unicode(script_instance.read(), 'utf8')
}))['annotations']:
annotation["script_path"] = script_path
line_list.append(
Message(
location="{script_path}:{row}:{column}".format(**annotation),
message=annotation['text'],
edit_url="{script_path}/manage_main?line={row}".format(**annotation),))
def checkComponent(component_instance):
"""Check a component, adding messages to global `line_list`
"""
for annotation in json.loads(portal.ERP5Site_checkPythonSourceCodeAsJSON(
{'code': unicode(component_instance.getTextContent(), 'utf8')}))['annotations']:
annotation['component_path'] = component_instance.getRelativeUrl()
line_list.append(
Message(
location="{component_path}:{row}:{column}".format(**annotation),
message=annotation["text"],
edit_url="{component_path}?line={row}".format(**annotation),))
# Check scripts
script_container_list = []
for skin_id in context.getTemplateSkinIdList():
script_container_list.append(portal.portal_skins[skin_id])
for workflow_id in context.getTemplateWorkflowIdList():
script_container_list.append(portal.portal_workflow[workflow_id])
for script_container in script_container_list:
for script_path, script_instance in portal.ZopeFind(
script_container,
obj_metatypes=['Script (Python)'],
search_sub=1):
checkPythonScript(script_instance, "%s/%s" % (
portal.portal_url.getRelativeUrl(script_container), script_path))
# Check components
for component_id in (
context.getTemplateExtensionIdList() +
context.getTemplateDocumentIdList() +
context.getTemplateTestIdList()):
checkComponent(portal.portal_components[component_id])
return line_list
......@@ -50,11 +50,11 @@
</item>
<item>
<key> <string>_params</string> </key>
<value> <string></string> </value>
<value> <string>*args, **kw</string> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>ERP5Site_checkPythonScriptsWithPyflakes</string> </value>
<value> <string>BusinessTemplate_getPythonSourceCodeMessageList</string> </value>
</item>
</dictionary>
</pickle>
......
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="ERP5 Form" module="erp5.portal_type"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>_bind_names</string> </key>
<value>
<object>
<klass>
<global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
</klass>
<tuple/>
<state>
<dictionary>
<item>
<key> <string>_asgns</string> </key>
<value>
<dictionary/>
</value>
</item>
</dictionary>
</state>
</object>
</value>
</item>
<item>
<key> <string>_objects</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>action</string> </key>
<value> <string>BusinessTemplate_viewCheckPythonCodeDialog</string> </value>
</item>
<item>
<key> <string>description</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>edit_order</string> </key>
<value>
<list/>
</value>
</item>
<item>
<key> <string>encoding</string> </key>
<value> <string>UTF-8</string> </value>
</item>
<item>
<key> <string>enctype</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>group_list</string> </key>
<value>
<list>
<string>left</string>
<string>right</string>
<string>center</string>
<string>bottom</string>
<string>hidden</string>
</list>
</value>
</item>
<item>
<key> <string>groups</string> </key>
<value>
<dictionary>
<item>
<key> <string>bottom</string> </key>
<value>
<list>
<string>listbox</string>
</list>
</value>
</item>
<item>
<key> <string>center</string> </key>
<value>
<list/>
</value>
</item>
<item>
<key> <string>hidden</string> </key>
<value>
<list/>
</value>
</item>
<item>
<key> <string>left</string> </key>
<value>
<list/>
</value>
</item>
<item>
<key> <string>right</string> </key>
<value>
<list/>
</value>
</item>
</dictionary>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>BusinessTemplate_viewCheckPythonCodeDialog</string> </value>
</item>
<item>
<key> <string>method</string> </key>
<value> <string>POST</string> </value>
</item>
<item>
<key> <string>name</string> </key>
<value> <string>BusinessTemplate_viewCheckPythonCodeDialog</string> </value>
</item>
<item>
<key> <string>pt</string> </key>
<value> <string>form_dialog</string> </value>
</item>
<item>
<key> <string>row_length</string> </key>
<value> <int>4</int> </value>
</item>
<item>
<key> <string>stored_encoding</string> </key>
<value> <string>UTF-8</string> </value>
</item>
<item>
<key> <string>title</string> </key>
<value> <string>Check Python Code</string> </value>
</item>
<item>
<key> <string>unicode_mode</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>update_action</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>update_action_title</string> </key>
<value> <string></string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
"""Runs pylint/pyflakes on all python scripts.
TODO / BUGS:
* script containing only a comment cannot be parsed
* wouldn't it be better to use this on a business template to check scripts
from the business template ?
"""
import json
def checkPythonSourceCode(script_instance):
# printed is from RestrictedPython.RestrictionMutator the rest comes from
# RestrictedPython.Utilities.utility_builtins
extra_builtins = ['printed', 'same_type', 'string', 'sequence', 'random',
'DateTime', 'whrandom', 'reorder', 'sets', 'test', 'math']
return json.loads(context.ERP5Site_checkPythonSourceCodeAsJSON(
{'bound_names': extra_builtins + script_instance.getBindingAssignments().getAssignedNamesInOrder(),
'params': script_instance.params(),
'code': unicode(script_instance.read(), 'utf8')
}))['annotations']
for script_container in (context.portal_skins, context.portal_workflow):
for script_path, script_instance in context.ZopeFind(script_container, obj_metatypes=['Script (Python)'], search_sub=1):
for annotation in checkPythonSourceCode(script_instance):
annotation["script_path"] = "%s/%s" % (script_container.getId(), script_path)
print "{script_path}:{row}:{column}:{text}".format(**annotation)
return printed
Business Template | check_python_code
Missing Category Document Constraint | predicate
Missing Category Document Constraint | view
portal_actions | consistency
......
......@@ -12,6 +12,8 @@ Export the web page and its components to a single (m)html file.
TODO: export same components into one mhtml attachment if possible.
"""
# ERP5 web uses format= argument, which is also a python builtin
# pylint: disable=redefined-builtin
from zExceptions import Unauthorized
from base64 import b64encode, b64decode
......
......@@ -50,7 +50,7 @@
</item>
<item>
<key> <string>_params</string> </key>
<value> <string>group_by_criterion=[\'left\', \'right\', \'center\', \'bottom\', \'hidden\']</string> </value>
<value> <string>group_by_criterion=(\'left\', \'right\', \'center\', \'bottom\', \'hidden\')</string> </value>
</item>
<item>
<key> <string>id</string> </key>
......
......@@ -20,11 +20,6 @@
"""
from Products.ZSQLCatalog.SQLCatalog import SimpleQuery, ComplexQuery
if portal is None: portal = context.getPortalObject()
portal_catalog = portal.portal_catalog
# The list of portal types here should be large enough to include
# all portal_types defined in the various sections so that
# href tags which point to a document by reference can still work.
valid_portal_type_list = portal.getPortalDocumentTypeList()
# Find the applicable language
if language is None:
......
......@@ -3,7 +3,8 @@ Export the web page and its components to a single (m)html file.
see Base_convertHtmlToSingleFile for documentation
"""
# ERP5 web uses format= argument, which is also a python builtin
# pylint: disable=redefined-builtin
data = context.Base_convertHtmlToSingleFile(
context.getTextContent(""),
allow_script=allow_script,
......
......@@ -5,6 +5,8 @@
"""
# Initialise result
sub_field_list = []
if default_sub_field_property_dict is None:
default_sub_field_property_dict = {}
# Maximum size of the MultiListField
default_sub_field_property_dict.update({
......@@ -17,7 +19,7 @@ default_sub_field_property_dict.update({
})
z = 0
for i in range(1):
for _ in range(1):
new_dict = default_sub_field_property_dict.copy()
new_dict['title'] = '&nbsp;'
new_dict['key'] = str(z)
......
......@@ -50,7 +50,7 @@
</item>
<item>
<key> <string>_params</string> </key>
<value> <string>item_list, value_list, default_sub_field_property_dict={}, is_right_display=0</string> </value>
<value> <string>item_list, value_list, default_sub_field_property_dict=None, is_right_display=0</string> </value>
</item>
<item>
<key> <string>id</string> </key>
......
request = context.REQUEST
request_form = context.REQUEST.form
request_form = request.form
from ZTUtils import make_query
portal = context.getPortalObject()
title = context.getTitle('Unknown')
......
......@@ -5,6 +5,9 @@
category -- the category to use
"""
# our pylint integration does not support global variabled in zope python script.
# pylint: disable=global-variable-undefined
from ZODB.POSException import ConflictError
portal = context.getPortalObject()
translateString = context.Base_translateString
......@@ -20,7 +23,7 @@ def getNiceID(s):
s = s.lower()
s = s.split()
s = '-'.join(s)
s = filter(lambda c: c in valid_char, s)
s = [c for c in s if c in valid_char]
s = s.replace('_', '-')
return s
......@@ -77,7 +80,7 @@ def createWebSectionFromCategoryValue(container, category, depth, section_id=Non
new_section.updateLocalRolesOnSecurityGroups()
except ConflictError:
raise
except:
except Exception:
failed_list.append(category.getRelativeUrl())
else:
new_section = container[section_id]
......
......@@ -24,11 +24,6 @@
portal skins. It is recommended to use the first approach
to host multiple sites on a single ERP5Site instance.
"""
portal_object = context.getPortalObject()
# First find the Web Section or Web Site we belong to
current_section = context.getWebSectionValue()
# First get all the applicable references
# There might be more than one reference due to security differences
# (ex. a default restricted web page and a default public web page)
......
request = context.REQUEST
portal = context.getPortalObject()
title = context.getTitle('Unknown')
translateString = context.Base_translateString
......
......@@ -10,11 +10,11 @@
- you need to adjust group, function and site to your needs
"""
# pylint: disable=unreachable
# since the following code is just an example, we simply raise an exception so that
# it is not executed actually.
raise NotImplementedError
from Products.Formulator.Errors import ValidationError, FormValidationError
portal = context.getPortalObject()
translateString = context.Base_translateString
website = context.getWebSiteValue()
......@@ -25,7 +25,7 @@ result, result_type = context.Base_edit(form_id, silent_mode=1, field_prefix='yo
# Return if not appropriate
if result_type != 'edit':
return result
kw, encapsulated_editor_list = result
kw, _ = result
# Set default values
person_group = kw.get('group', None)
......@@ -55,11 +55,11 @@ person.validate()
#person.immediateReindexObject()
# Create default career
career = person.newContent(portal_type='Career',
id='default_career',
group=person_group,
function=person_function,
role=person_role)
person.newContent(portal_type='Career',
id='default_career',
group=person_group,
function=person_function,
role=person_role)
# Create assignment
assignment = person.newContent(portal_type='Assignment',
group=person_group,
......
......@@ -4,4 +4,4 @@
"""
site = context.getWebSiteValue()
section_list = site.contentValues(portal_type='Web Section', sort_on='int_index', checked_permission='View')
return filter(lambda x: x.isVisible(), section_list)
return [x for x in section_list if x.isVisible()]
......@@ -4,7 +4,7 @@
website = context.getWebSiteValue()
REQUEST = context.REQUEST
if REQUEST.has_key('portal_skin'):
context.portal_skins.clearSkinCookie()
context.portal_skins.clearSkinCookie()
REQUEST.RESPONSE.expireCookie('__ac', path='/')
msg = context.Base_translateString('You have been logged out. Thank you for using this website.')
return website.Base_redirect(form_id, keep_items = {'portal_status_message' : msg}, **kw)
......@@ -7,16 +7,16 @@
from Products.CMFCore.WorkflowCore import WorkflowException
history = {}
workflow_id_list = [workflow_id for workflow_id, workflow_state in context.getWorkflowStateItemList()]
workflow_id_list = [workflow_id for workflow_id, _ in context.getWorkflowStateItemList()]
for wf_id in workflow_id_list:
try:
history[wf_id]=context.Base_getWorkflowHistoryItemList(workflow_id=wf_id)
history[wf_id] = context.Base_getWorkflowHistoryItemList(workflow_id=wf_id)
except WorkflowException:
# some workflow don't have history
pass
# some workflow don't have history
pass
event_list = []
for worrkflow_id in history.keys():
event_list += history[worrkflow_id]
for workflow_id in history.keys():
event_list += history[workflow_id]
if sort: event_list.sort(key=lambda x:x.time, reverse=True)
return event_list
......@@ -6,11 +6,11 @@
"""
def getInlinePage(reference, language):
if reference:
page = context.getDocumentValue(reference)
if page is not None:
return page.asStrippedHTML()
return None
if reference:
page = context.getDocumentValue(reference)
if page is not None:
return page.asStrippedHTML()
return None
from Products.ERP5Type.Cache import CachingMethod
web_site_url = context.getWebSectionValue().absolute_url()
......
......@@ -9,8 +9,6 @@
Result is cached for high performance.
"""
web_site_value = context.getWebSiteValue()
web_site_url = web_site_value.absolute_url()
web_section_value = context.getWebSectionValue()
web_section_url = web_section_value.absolute_url()
context = web_section_value
......
......@@ -7,9 +7,7 @@
from zExceptions import Unauthorized
# Initialize some useful variables
request = context.REQUEST
portal = context.getPortalObject()
website = context.getWebSiteValue()
user = portal.portal_membership.getAuthenticatedMember()
user_preference = None
portal_preferences = portal.portal_preferences
......
object = state_change['object']
document = state_change['object']
workflow_tool = object.getPortalObject().portal_workflow
workflow_tool = document.getPortalObject().portal_workflow
if workflow_tool.isTransitionPossible(object, 'expire'):
object.expire()
if workflow_tool.isTransitionPossible(document, 'expire'):
document.expire()
if workflow_tool.isTransitionPossible(object, 'expire_protected'):
object.expireProtected()
if workflow_tool.isTransitionPossible(document, 'expire_protected'):
document.expireProtected()
if workflow_tool.isTransitionPossible(object, 'expire_published'):
object.expirePublished()
if workflow_tool.isTransitionPossible(document, 'expire_published'):
document.expirePublished()
extension.erp5.WebUtility
\ No newline at end of file
......@@ -30,13 +30,16 @@
import unittest
from Products.ERP5Type.tests.CodingStyleTestCase import CodingStyleTestCase
class CodingStyle(CodingStyleTestCase):
class ERP5WebCodingStyle(CodingStyleTestCase):
"""
Check consistency of erp5_web business template code
"""
def getTitle(self):
return "erp5_web CodingStyle"
def getTestedBusinessTemplateList(self):
return ('erp5_web', )
def getBusinessTemplateList(self):
"""
Return the list of required business templates.
......@@ -47,9 +50,10 @@ class CodingStyle(CodingStyleTestCase):
'erp5_ingestion_mysql_innodb_catalog',
'erp5_ingestion',
'erp5_crm',
'erp5_administration',
)
def test_suite():
suite = unittest.TestSuite()
suite.addTest(unittest.makeSuite(CodingStyle))
suite.addTest(unittest.makeSuite(ERP5WebCodingStyle))
return suite
......@@ -75,11 +75,12 @@ def getSkinPrefixList(self):
# Add other prefix
skin_prefix_list.extend((
'Base',
'Entity',
'NotificationTool',
'ERP5Site',
'ERP5Type',
'Entity', # A base class for Person / Organisation
'Form', # Acceptable for ERP5 Forms which will soon become portal types too
'Brain', # Catalog brains
))
return set(skin_prefix_list)
......
......@@ -32,18 +32,16 @@ from Testing import ZopeTestCase
from Acquisition import aq_base
class CodingStyleTestCase(ERP5TypeTestCase):
"""XXX
"""Test case to test coding style in business templates.
Subclasses must override:
* getBusinessTemplateList to list business template to install.
* getTestedBusinessTemplateList to list business templates to test.
"""
manager_username = 'zope'
manager_password = 'zope'
website_id = 'test'
def getTitle(self):
"""
Override this method in implementation class.
"""
raise NotImplementedError
def getBusinessTemplateList(self):
"""
Return the list of required business templates.
......@@ -89,8 +87,22 @@ class CodingStyleTestCase(ERP5TypeTestCase):
skin = portal_skins[skin_id]
for document in skin.objectValues():
if getattr(aq_base(document), 'checkConsistency', None) is not None:
message_list.extend(document.checkConsistency())
message_list.extend(document.checkConsistency())
# Return results
if len(message_list):
raise self.failureException('\n'.join(map(lambda x: repr(x), message_list)))
def test_PythonSourceCode(self):
"""test python script from the tested business templates.
reuses BusinessTemplate_getPythonSourceCodeMessageList from erp5_administration
"""
if 'erp5_administration' not in self.getBusinessTemplateList():
self.skipTest('erp5_administration needs be installed to check python source code')
self.maxDiff = None
for business_template in self.portal.portal_templates.contentValues():
if business_template.getTitle() in self.getTestedBusinessTemplateList():
self.assertEqual([], business_template.BusinessTemplate_getPythonSourceCodeMessageList())
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment