Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add invalid-envvar checkers #1680

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 69 additions & 3 deletions pylint/checkers/stdlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import astroid
from astroid.bases import Instance
from astroid.node_classes import Const, Call
from pylint.interfaces import IAstroidChecker
from pylint.checkers import BaseChecker
from pylint.checkers import utils
Expand All @@ -23,6 +24,8 @@
OPEN_FILES = {'open', 'file'}
UNITTEST_CASE = 'unittest.case'
THREADING_THREAD = 'threading.Thread'
ENV_SETTERS = {'os.getenv'}

if sys.version_info >= (3, 0):
OPEN_MODULE = '_io'
else:
Expand Down Expand Up @@ -105,6 +108,16 @@ class StdlibChecker(BaseChecker):
'The warning is emitted when a threading.Thread class '
'is instantiated without the target function being passed. '
'By default, the first parameter is the group param, not the target param. '),
'E1507': ('"%s" does not support %s type argument',
'invalid-envvar-value',
'Env manipulation functions support only string type arguments. '
'See https://docs.python.org/3/library/os.html#os.putenv. '),
'W1508': ('"%s" default type is "%s". Expected str or None.',
'invalid-envvar-default',
'Env manipulation functions return None or str values. '
'Supplying anything different as a default may cause bugs. '
'See https://docs.python.org/3/library/os.html#os.putenv. '),

}

deprecated = {
Expand Down Expand Up @@ -193,15 +206,32 @@ def visit_call(self, node):
"""Visit a Call node."""
try:
for inferred in node.func.infer():

if inferred is astroid.Uninferable:
continue
if inferred.root().name == OPEN_MODULE:
elif inferred.root().name == OPEN_MODULE:
if getattr(node.func, 'name', None) in OPEN_FILES:
self._check_open_mode(node)
if inferred.root().name == UNITTEST_CASE:
elif inferred.root().name == UNITTEST_CASE:
self._check_redundant_assert(node, inferred)
if isinstance(inferred, astroid.ClassDef) and inferred.qname() == THREADING_THREAD:
elif isinstance(inferred, astroid.ClassDef) and inferred.qname() == THREADING_THREAD:
self._check_bad_thread_instantiation(node)
elif inferred.qname() in ENV_SETTERS:
if node.keywords:
kwargs = {keyword.arg: keyword.value for keyword in node.keywords}
else:
kwargs = None

if len(node.args) > 0:
self._check_invalid_envvar_value(node.args[0], inferred)
elif kwargs and 'key' in kwargs:
self._check_invalid_envvar_value(kwargs['key'], inferred)

if len(node.args) == 2:
self._check_invalid_envvar_default(node.args[1], inferred)
elif kwargs and 'default' in kwargs:
self._check_invalid_envvar_default(kwargs['default'], inferred)

self._check_deprecated_method(node, inferred)
except astroid.InferenceError:
return
Expand Down Expand Up @@ -287,6 +317,42 @@ def _check_open_mode(self, node):
self.add_message('bad-open-mode', node=node,
args=mode_arg.value)

def _check_invalid_envvar_value(self, call_arg, infer):
if isinstance(call_arg, Call):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have an utility called safe_infer which you should use instead.

call_arg = call_arg.inferred()

if not call_arg:
return

call_arg = call_arg[0]

if call_arg is astroid.Uninferable:
return

if isinstance(call_arg, Const):
if not isinstance(call_arg.value, (six.string_types, six.text_type)):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

six.string_types should be enough since it will contain both unicode and str on Python 2.

self.add_message('invalid-envvar-value', node=call_arg, args=(infer.qname(), call_arg.pytype()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the node that you are checking against, not the inferred node (e.g the node from the visit_ function)

else:
self.add_message('invalid-envvar-value', node=call_arg, args=(infer.qname(), call_arg.pytype()))

def _check_invalid_envvar_default(self, call_arg, infer):
if isinstance(call_arg, Call):
call_arg = call_arg.inferred()

if not call_arg:
return

call_arg = call_arg[0]

if call_arg is astroid.Uninferable:
return

if isinstance(call_arg, Const):
if not (isinstance(call_arg.value, (six.string_types, six.text_type)) or call_arg.value is None):
self.add_message('invalid-envvar-default', node=call_arg, args=(infer.qname(), call_arg.pytype()))
else:
self.add_message('invalid-envvar-default', node=call_arg, args=(infer.qname(), call_arg.pytype()))


def register(linter):
"""required method to auto register this checker """
Expand Down
75 changes: 75 additions & 0 deletions pylint/test/functional/invalid_envvar_value.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import os
from os import environ, getenv, putenv


def function_returning_list():
return []

def function_returning_none():
return None

def function_returning_string():
return "Result"

def function_returning_bytes():
return b"Result"

# --------------------------------------------------------------------------- #
# Testing getenv #
# --------------------------------------------------------------------------- #

getenv()

getenv(b"TEST")
getenv("TEST")
getenv(None)
getenv(["Crap"])
getenv(function_returning_bytes())
getenv(function_returning_list())
getenv(function_returning_none())
getenv(function_returning_string())

getenv(b"TEST", "default")
getenv("TEST", "default")
getenv(None, "default")
getenv(["Crap"], "default")
getenv(function_returning_bytes(), "default")
getenv(function_returning_list(), "default")
getenv(function_returning_none(), "default")
getenv(function_returning_string(), "default")

getenv(key=b"TEST")
getenv(key="TEST")
getenv(key=None)
getenv(key=["Crap"])
getenv(key=function_returning_bytes())
getenv(key=function_returning_list())
getenv(key=function_returning_none())
getenv(key=function_returning_string())

getenv('TEST', "value")
getenv('TEST', [])
getenv('TEST', None)
getenv('TEST', b"123")
getenv('TEST', function_returning_list())
getenv('TEST', function_returning_none())
getenv('TEST', function_returning_string())
getenv('TEST', function_returning_bytes())

getenv('TEST', default="value")
getenv('TEST', default=[])
getenv('TEST', default=None)
getenv('TEST', default=b"123")
getenv('TEST', default=function_returning_list())
getenv('TEST', default=function_returning_none())
getenv('TEST', default=function_returning_string())
getenv('TEST', default=function_returning_bytes())

getenv(key='TEST')
getenv(key='TEST', default="value")
getenv(key='TEST', default=b"value")
getenv(key='TEST', default=["Crap"])
getenv(key='TEST', default=function_returning_list())
getenv(key='TEST', default=function_returning_none())
getenv(key='TEST', default=function_returning_string())
getenv(key='TEST', default=function_returning_bytes())
5 changes: 5 additions & 0 deletions pylint/test/functional/invalid_envvar_value.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import os
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a look at the other test files from the functional directory. The .txt should be a mapping of the error occurrences from the .py file.


os.environ.get('TEST', []) # [invalid-envvar-value]
os.getenv('TEST', []) # [invalid-envvar-value]
os.putenv('TEST', []) # [invalid-envvar-value]