diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index e6c4ffd54a..574a7cebfa 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -88,6 +88,7 @@ contributors: * Add consider-merging-isinstance, superfluous-else-return * Fix consider-using-ternary for 'True and True and True or True' case * Add bad-docstring-quotes and docstring-first-line-empty + * Add missing-timeout - Ville Skyttä - Pierre-Yves David - David Shea : invalid sequence and slice index diff --git a/doc/data/messages/m/missing-timeout/bad.py b/doc/data/messages/m/missing-timeout/bad.py new file mode 100644 index 0000000000..52444f28aa --- /dev/null +++ b/doc/data/messages/m/missing-timeout/bad.py @@ -0,0 +1,3 @@ +import requests + +requests.post("http://localhost") # [missing-timeout] diff --git a/doc/data/messages/m/missing-timeout/details.rst b/doc/data/messages/m/missing-timeout/details.rst new file mode 100644 index 0000000000..6926338c26 --- /dev/null +++ b/doc/data/messages/m/missing-timeout/details.rst @@ -0,0 +1,10 @@ +You can add new methods that should have a defined ```timeout`` argument as qualified names +in the ``timeout-methods`` option, for example: + +* ``requests.api.get`` +* ``requests.api.head`` +* ``requests.api.options`` +* ``requests.api.patch`` +* ``requests.api.post`` +* ``requests.api.put`` +* ``requests.api.request`` diff --git a/doc/data/messages/m/missing-timeout/good.py b/doc/data/messages/m/missing-timeout/good.py new file mode 100644 index 0000000000..dbeb51255e --- /dev/null +++ b/doc/data/messages/m/missing-timeout/good.py @@ -0,0 +1,3 @@ +import requests + +requests.post("http://localhost", timeout=10) diff --git a/doc/whatsnew/2/2.15/index.rst b/doc/whatsnew/2/2.15/index.rst index 46fa821929..970102ea81 100644 --- a/doc/whatsnew/2/2.15/index.rst +++ b/doc/whatsnew/2/2.15/index.rst @@ -15,6 +15,9 @@ Summary -- Release highlights New checkers ============ +Added new checker ``missing-timeout`` to warn of default timeout values that could cause +a program to be hanging indefinitely. + Removed checkers ================ diff --git a/pylint/checkers/method_args.py b/pylint/checkers/method_args.py new file mode 100644 index 0000000000..d9593a58f2 --- /dev/null +++ b/pylint/checkers/method_args.py @@ -0,0 +1,86 @@ +# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html +# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE +# Copyright (c) https://github.com/PyCQA/pylint/blob/main/CONTRIBUTORS.txt + +"""Variables checkers for Python code.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from astroid import arguments, nodes + +from pylint.checkers import BaseChecker, utils +from pylint.interfaces import INFERENCE + +if TYPE_CHECKING: + from pylint.lint import PyLinter + + +class MethodArgsChecker(BaseChecker): + """BaseChecker for method_args. + + Checks for + * missing-timeout + """ + + name = "method_args" + msgs = { + "W3101": ( + "Missing timeout argument for method '%s' can cause your program to hang indefinitely", + "missing-timeout", + "Used when a method needs a 'timeout' parameter in order to avoid waiting " + "for a long time. If no timeout is specified explicitly the default value " + "is used. For example for 'requests' the program will never time out " + "(i.e. hang indefinitely).", + ), + } + options = ( + ( + "timeout-methods", + { + "default": ( + "requests.api.delete", + "requests.api.get", + "requests.api.head", + "requests.api.options", + "requests.api.patch", + "requests.api.post", + "requests.api.put", + "requests.api.request", + ), + "type": "csv", + "metavar": "", + "help": "List of qualified names (i.e., library.method) which require a timeout parameter " + "e.g. 'requests.api.get,requests.api.post'", + }, + ), + ) + + @utils.only_required_for_messages("missing-timeout") + def visit_call(self, node: nodes.Call) -> None: + """Check if the call needs a timeout parameter based on package.func_name + configured in config.timeout_methods. + + Package uses inferred node in order to know the package imported. + """ + inferred = utils.safe_infer(node.func) + call_site = arguments.CallSite.from_call(node) + if ( + inferred + and not call_site.has_invalid_keywords() + and inferred.qname() in self.config.timeout_methods + ): + keyword_arguments = [keyword.arg for keyword in node.keywords] + keyword_arguments.extend(call_site.keyword_arguments) + if "timeout" not in keyword_arguments: + self.add_message( + "missing-timeout", + node=node, + args=(node.func.as_string(),), + confidence=INFERENCE, + ) + + +def register(linter: PyLinter) -> None: + linter.register_checker(MethodArgsChecker(linter)) diff --git a/requirements_test_min.txt b/requirements_test_min.txt index 386f2605cb..f93c408ae1 100644 --- a/requirements_test_min.txt +++ b/requirements_test_min.txt @@ -5,3 +5,4 @@ typing-extensions~=4.2 pytest~=7.1 pytest-benchmark~=3.4 pytest-timeout~=2.1 +requests diff --git a/tests/functional/m/missing/missing_timeout.py b/tests/functional/m/missing/missing_timeout.py new file mode 100644 index 0000000000..13ff35f862 --- /dev/null +++ b/tests/functional/m/missing/missing_timeout.py @@ -0,0 +1,85 @@ +"""Tests for missing-timeout.""" + +# pylint: disable=consider-using-with,import-error,no-member,no-name-in-module,reimported + +import requests +from requests import ( + delete, + delete as delete_r, + get, + get as get_r, + head, + head as head_r, + options, + options as options_r, + patch, + patch as patch_r, + post, + post as post_r, + put, + put as put_r, + request, + request as request_r, +) + +# requests without timeout +requests.delete("http://localhost") # [missing-timeout] +requests.get("http://localhost") # [missing-timeout] +requests.head("http://localhost") # [missing-timeout] +requests.options("http://localhost") # [missing-timeout] +requests.patch("http://localhost") # [missing-timeout] +requests.post("http://localhost") # [missing-timeout] +requests.put("http://localhost") # [missing-timeout] +requests.request("call", "http://localhost") # [missing-timeout] + +delete_r("http://localhost") # [missing-timeout] +get_r("http://localhost") # [missing-timeout] +head_r("http://localhost") # [missing-timeout] +options_r("http://localhost") # [missing-timeout] +patch_r("http://localhost") # [missing-timeout] +post_r("http://localhost") # [missing-timeout] +put_r("http://localhost") # [missing-timeout] +request_r("call", "http://localhost") # [missing-timeout] + +delete("http://localhost") # [missing-timeout] +get("http://localhost") # [missing-timeout] +head("http://localhost") # [missing-timeout] +options("http://localhost") # [missing-timeout] +patch("http://localhost") # [missing-timeout] +post("http://localhost") # [missing-timeout] +put("http://localhost") # [missing-timeout] +request("call", "http://localhost") # [missing-timeout] + +kwargs_wo_timeout = {} +post("http://localhost", **kwargs_wo_timeout) # [missing-timeout] + +# requests valid cases +requests.delete("http://localhost", timeout=10) +requests.get("http://localhost", timeout=10) +requests.head("http://localhost", timeout=10) +requests.options("http://localhost", timeout=10) +requests.patch("http://localhost", timeout=10) +requests.post("http://localhost", timeout=10) +requests.put("http://localhost", timeout=10) +requests.request("call", "http://localhost", timeout=10) + +delete_r("http://localhost", timeout=10) +get_r("http://localhost", timeout=10) +head_r("http://localhost", timeout=10) +options_r("http://localhost", timeout=10) +patch_r("http://localhost", timeout=10) +post_r("http://localhost", timeout=10) +put_r("http://localhost", timeout=10) +request_r("call", "http://localhost", timeout=10) + +delete("http://localhost", timeout=10) +get("http://localhost", timeout=10) +head("http://localhost", timeout=10) +options("http://localhost", timeout=10) +patch("http://localhost", timeout=10) +post("http://localhost", timeout=10) +put("http://localhost", timeout=10) +request("call", "http://localhost", timeout=10) + +kwargs_timeout = {'timeout': 10} +post("http://localhost", **kwargs_timeout) diff --git a/tests/functional/m/missing/missing_timeout.txt b/tests/functional/m/missing/missing_timeout.txt new file mode 100644 index 0000000000..f3c633c2a1 --- /dev/null +++ b/tests/functional/m/missing/missing_timeout.txt @@ -0,0 +1,25 @@ +missing-timeout:26:0:26:35::Missing timeout argument for method 'requests.delete' can cause your program to hang indefinitely:INFERENCE +missing-timeout:27:0:27:32::Missing timeout argument for method 'requests.get' can cause your program to hang indefinitely:INFERENCE +missing-timeout:28:0:28:33::Missing timeout argument for method 'requests.head' can cause your program to hang indefinitely:INFERENCE +missing-timeout:29:0:29:36::Missing timeout argument for method 'requests.options' can cause your program to hang indefinitely:INFERENCE +missing-timeout:30:0:30:34::Missing timeout argument for method 'requests.patch' can cause your program to hang indefinitely:INFERENCE +missing-timeout:31:0:31:33::Missing timeout argument for method 'requests.post' can cause your program to hang indefinitely:INFERENCE +missing-timeout:32:0:32:32::Missing timeout argument for method 'requests.put' can cause your program to hang indefinitely:INFERENCE +missing-timeout:33:0:33:44::Missing timeout argument for method 'requests.request' can cause your program to hang indefinitely:INFERENCE +missing-timeout:35:0:35:28::Missing timeout argument for method 'delete_r' can cause your program to hang indefinitely:INFERENCE +missing-timeout:36:0:36:25::Missing timeout argument for method 'get_r' can cause your program to hang indefinitely:INFERENCE +missing-timeout:37:0:37:26::Missing timeout argument for method 'head_r' can cause your program to hang indefinitely:INFERENCE +missing-timeout:38:0:38:29::Missing timeout argument for method 'options_r' can cause your program to hang indefinitely:INFERENCE +missing-timeout:39:0:39:27::Missing timeout argument for method 'patch_r' can cause your program to hang indefinitely:INFERENCE +missing-timeout:40:0:40:26::Missing timeout argument for method 'post_r' can cause your program to hang indefinitely:INFERENCE +missing-timeout:41:0:41:25::Missing timeout argument for method 'put_r' can cause your program to hang indefinitely:INFERENCE +missing-timeout:42:0:42:37::Missing timeout argument for method 'request_r' can cause your program to hang indefinitely:INFERENCE +missing-timeout:44:0:44:26::Missing timeout argument for method 'delete' can cause your program to hang indefinitely:INFERENCE +missing-timeout:45:0:45:23::Missing timeout argument for method 'get' can cause your program to hang indefinitely:INFERENCE +missing-timeout:46:0:46:24::Missing timeout argument for method 'head' can cause your program to hang indefinitely:INFERENCE +missing-timeout:47:0:47:27::Missing timeout argument for method 'options' can cause your program to hang indefinitely:INFERENCE +missing-timeout:48:0:48:25::Missing timeout argument for method 'patch' can cause your program to hang indefinitely:INFERENCE +missing-timeout:49:0:49:24::Missing timeout argument for method 'post' can cause your program to hang indefinitely:INFERENCE +missing-timeout:50:0:50:23::Missing timeout argument for method 'put' can cause your program to hang indefinitely:INFERENCE +missing-timeout:51:0:51:35::Missing timeout argument for method 'request' can cause your program to hang indefinitely:INFERENCE +missing-timeout:54:0:54:45::Missing timeout argument for method 'post' can cause your program to hang indefinitely:INFERENCE