[PERF] use is_message_enabled for all lints

[fernandahf]

Currently, there are some lints that are not using the following validation:

pylint-odoo/pylint_odoo/checkers/no_modules.py

Line 872 in 5c15bca

if self.linter.is_message_enabled('copy-wo-api-one'):

That generates all code in those lints, even if they are disabled, are checked.

Example:

pylint-odoo/pylint_odoo/checkers/no_modules.py

Lines 593 to 751 in 5c15bca

	def visit_call(self, node):
	infer_node = utils.safe_infer(node.func)
	if utils.is_builtin_object(infer_node) and infer_node.name == 'print':
	self.add_message('print-used', node=node)
	if ('fields' == self.get_func_lib(node.func) and
	isinstance(node.parent, astroid.Assign) and
	isinstance(node.parent.parent, astroid.ClassDef)):
	args = misc.join_node_args_kwargs(node)
	index = 0
	field_name = ''
	if (isinstance(node.parent, astroid.Assign) and
	node.parent.targets and
	isinstance(node.parent.targets[0], astroid.AssignName)):
	field_name = (node.parent.targets[0].name
	.replace('_', ' '))
	is_related = bool([1 for kw in node.keywords or [] if kw.arg == 'related'])
	for argument in args:
	argument_aux = argument
	# Check this 'name = fields.Char("name")'
	if (not is_related and isinstance(argument, astroid.Const) and
	(index ==
	FIELDS_METHOD.get(argument.parent.func.attrname, 0)) and
	(argument.value in
	[field_name.capitalize(), field_name.title()])):
	self.add_message('attribute-string-redundant', node=node)
	if isinstance(argument, astroid.Keyword):
	argument_aux = argument.value
	deprecated = self.config.deprecated_field_parameters
	if argument.arg in ['compute', 'search', 'inverse'] and \
	isinstance(argument_aux, astroid.Const) and \
	isinstance(argument_aux.value, string_types) and \
	not argument_aux.value.startswith(
	'_' + argument.arg + '_'):
	self.add_message('method-' + argument.arg,
	node=argument_aux)
	# Check if the param string is equal to the name
	# of variable
	elif not is_related and argument.arg == 'string' and \
	(isinstance(argument_aux, astroid.Const) and
	argument_aux.value in
	[field_name.capitalize(), field_name.title()]):
	self.add_message(
	'attribute-string-redundant', node=node)
	elif (argument.arg in deprecated):
	self.add_message(
	'renamed-field-parameter', node=node,
	args=(argument.arg, deprecated[argument.arg])
	)
	if isinstance(argument_aux, astroid.Call) and \
	isinstance(argument_aux.func, astroid.Name) and \
	argument_aux.func.name == '_':
	self.add_message('translation-field', node=argument_aux)
	index += 1
	# Check cr.commit()
	if isinstance(node, astroid.Call) and \
	isinstance(node.func, astroid.Attribute) and \
	node.func.attrname == 'commit' and \
	self.get_cursor_name(node.func) in self.config.cursor_expr:
	self.add_message('invalid-commit', node=node)
	if (isinstance(node, astroid.Call) and
	isinstance(node.func, astroid.Attribute) and
	node.func.attrname == 'with_context' and
	not node.keywords and node.args):
	# with_context(**ctx) is considered a keywords
	# So, if only one args is received it is overridden
	self.add_message('context-overridden', node=node,
	args=(node.args[0].as_string(),))
	# Call the message_post()
	base_dirname = os.path.basename(os.path.normpath(
	os.path.dirname(self.linter.current_file)))
	if (base_dirname != 'tests' and isinstance(node, astroid.Call) and
	isinstance(node.func, astroid.Attribute) and
	node.func.attrname == 'message_post'):
	for arg in itertools.chain(node.args, node.keywords or []):
	if isinstance(arg, astroid.Keyword):
	keyword = arg.arg
	value = arg.value
	else:
	keyword = ''
	value = arg
	if keyword and keyword not in ('subject', 'body'):
	continue
	as_string = ''
	# case: message_post(body='String')
	if isinstance(value, astroid.Const):
	as_string = value.as_string()
	# case: message_post(body='String %s' % (...))
	elif (isinstance(value, astroid.BinOp)
	and value.op == '%'
	and isinstance(value.left, astroid.Const)
	# The right part is translatable only if it's a
	# function or a list of functions
	and not (
	isinstance(value.right, (
	astroid.Call, astroid.Tuple, astroid.List))
	and all([
	isinstance(child, astroid.Call)
	for child in getattr(value.right, 'elts', [])
	]))):
	as_string = value.left.as_string()
	# case: message_post(body='String {...}'.format(...))
	elif (isinstance(value, astroid.Call)
	and isinstance(value.func, astroid.Attribute)
	and isinstance(value.func.expr, astroid.Const)
	and value.func.attrname == 'format'):
	as_string = value.func.expr.as_string()
	if as_string:
	keyword = keyword and '%s=' % keyword
	self.add_message(
	'translation-required', node=node,
	args=('message_post', keyword, as_string))
	# Call _(...) with variables into the term to be translated
	if (isinstance(node.func, astroid.Name)
	and node.func.name == '_'
	and node.args):
	wrong = ''
	right = ''
	arg = node.args[0]
	# case: _('...' % (variables))
	if isinstance(arg, astroid.BinOp) and arg.op == '%':
	wrong = '%s %% %s' % (
	arg.left.as_string(), arg.right.as_string())
	right = '_(%s) %% %s' % (
	arg.left.as_string(), arg.right.as_string())
	# Case: _('...'.format(variables))
	elif (isinstance(arg, astroid.Call)
	and isinstance(arg.func, astroid.Attribute)
	and isinstance(arg.func.expr, astroid.Const)
	and arg.func.attrname == 'format'):
	self.add_message('str-format-used', node=node)
	wrong = arg.as_string()
	params_as_string = ', '.join([
	x.as_string()
	for x in itertools.chain(arg.args, arg.keywords or [])])
	right = '_(%s).format(%s)' % (
	arg.func.expr.as_string(), params_as_string)
	if wrong and right:
	self.add_message(
	'translation-contains-variable', node=node,
	args=(wrong, right))
	# translation-positional-used: Check "string to translate"
	# to check "%s %s..." used where the position can't be changed
	str2translate = arg.as_string()
	printf_args = (
	misc.WrapperModuleChecker.
	_get_printf_str_args_kwargs(str2translate))
	if isinstance(printf_args, tuple) and len(printf_args) >= 2:
	# Return tuple for %s and dict for %(varname)s
	# Check just the following cases "%s %s..."
	self.add_message('translation-positional-used',
	node=node, args=(str2translate,))
	# SQL Injection
	if self._check_sql_injection_risky(node):
	self.add_message('sql-injection', node=node)

[antonag32]

Working on the issue, however before fixing any code I am thinking of ways we can objectively measure the code quality, or performance loss/increase.

Right now it seems there are to ways to be sure the checks won't run if the message is enabled, one of them is using @utils.check_messages or @utils.only_required_for_messages. The other one is with the checks proposed in the OP:
if self.linter.is_message_enabled('copy-wo-api-one'):

The only problem with @utils.only_required_for_messages is that it only works when one message is used, when multiple messages are included as arguments, the method can be called if one of them is enabled, while all the others are disabled, causing the issue we are trying to fix.

The first method of devising the code quality and finding all messages that are added without checking is by statical analysis of the code, this will be done with a pylint checker (writing a pylint checker to check pylint checkers 😎 ) currently developed at https://github.com/antonag32/lilint.

Will be writing my thoughts and the progress fixing this issue so feel free to bring me back to reality if I overthink too much.

[antonag32]

Apparently I overthought too much too fast, lilint will stay for posterity since it was too young to die but there are more important lints to write, so I will just do this by hand and hope to get as much add_message nodes as possible.