[FEATURE REQUEST] Incorporate lint for log messages in __virtual__

[oeuftete]

Is your feature request related to a problem? Please describe.

Community contributors (and maybe Salt devs, too) add well-meaning logs to __virtual__ methods, typically to highlight that a module could not be loaded. However, these messages will show up very frequently in the logs, which is especially problematic if the logs are errors (see, e.g. https://github.com/saltstack/salt/pull/57723). The supported solution is to return a tuple (False, 'the log message') from __virtual__.

Describe the solution you'd like

Use pylint to detect use of logging in __virtual__ methods. Enforce this via CI and in pre-commit.

Describe alternatives you've considered

Status quo.

[oeuftete]

Support was added for this in https://github.com/saltstack/salt-pylint/pull/37. However, I ran into a couple of obstacles trying to add it.

Salt's dunders blow up the check

E.g. https://github.com/saltstack/salt/blob/b95213ec903402f25c1e0aeb3990fe8452ab63ce/salt/modules/linux_service.py#L18-L56

$ pylint --rcfile=.pylintrc --disable=I --rcfile=.pylintrc --disable=I salt/modules/linux_service.py
Traceback (most recent call last):
  File "/home/ken/.virtualenvs/salt/bin/pylint", line 8, in <module>
    sys.exit(run_pylint())
  File "/home/ken/.virtualenvs/salt/lib/python3.6/site-packages/pylint/__init__.py", line 23, in run_pylint
    PylintRun(sys.argv[1:])
  File "/home/ken/.virtualenvs/salt/lib/python3.6/site-packages/pylint/lint.py", line 1731, in __init__
    linter.check(args)
  File "/home/ken/.virtualenvs/salt/lib/python3.6/site-packages/pylint/lint.py", line 1004, in check
    self._do_check(files_or_modules)
  File "/home/ken/.virtualenvs/salt/lib/python3.6/site-packages/pylint/lint.py", line 1165, in _do_check
    self.check_astroid_module(ast_node, walker, rawcheckers, tokencheckers)
  File "/home/ken/.virtualenvs/salt/lib/python3.6/site-packages/pylint/lint.py", line 1252, in check_astroid_module
    walker.walk(ast_node)
  File "/home/ken/.virtualenvs/salt/lib/python3.6/site-packages/pylint/utils/ast_walker.py", line 77, in walk
    self.walk(child)
  File "/home/ken/.virtualenvs/salt/lib/python3.6/site-packages/pylint/utils/ast_walker.py", line 74, in walk
    callback(astroid)
  File "/home/ken/.virtualenvs/salt/lib/python3.6/site-packages/saltpylint/virt.py", line 52, in visit_functiondef
    for inferred in functions.func.expr.infer():
  File "/home/ken/.virtualenvs/salt/lib/python3.6/site-packages/astroid/decorators.py", line 131, in raise_if_nothing_inferred
    yield next(generator)
  File "/home/ken/.virtualenvs/salt/lib/python3.6/site-packages/astroid/decorators.py", line 92, in wrapped
    generator = _func(node, context, **kwargs)
  File "/home/ken/.virtualenvs/salt/lib/python3.6/site-packages/astroid/inference.py", line 195, in infer_name
    name=self.name, scope=self.scope(), context=context
astroid.exceptions.NameInferenceError: '__grains__' not found in <FunctionDef.__virtual__ l.18 at 0x7f37b4840eb8>.

Log message in missing import blocks aren't found

https://github.com/saltstack/salt/blob/b95213ec903402f25c1e0aeb3990fe8452ab63ce/salt/returners/appoptics_return.py#L83-L108

$ pylint --rcfile=.pylintrc --disable=I --rcfile=.pylintrc --disable=I salt/returners/appoptics_return.py 

------------------------------------
Your code has been rated at 10.00/10

If I add:

    log.info('foo')

at the top of __virtual__, though:

$ pylint --rcfile=.pylintrc --disable=I --rcfile=.pylintrc --disable=I salt/returners/appoptics_return.py 
************* Module salt.returners.appoptics_return
salt/returners/appoptics_return.py:98: [E1401(log-in-virtual), __virtual__] Log statement detected inside __virtual__ function. Remove it.

-----------------------------------
Your code has been rated at 9.29/10

[oeuftete]

Opened https://github.com/saltstack/salt-pylint/issues/42, which hopefully will help move this forward. This is blocked until that's fixed.