Merge pull request #319 from twm/kwargs-287

Reject invalid keyword arguments with TypeError

src/treq/client.py

@@ -143,12 +143,29 @@ def delete(self, url, **kwargs):
         kwargs.setdefault('_stacklevel', 3)
         return self.request('DELETE', url, **kwargs)
 
-    def request(self, method, url, **kwargs):
+    def request(
+        self,
+        method,
+        url,
+        *,
+        params=None,
+        headers=None,
+        data=None,
+        files=None,
+        json=_NOTHING,
+        auth=None,
+        cookies=None,
+        allow_redirects=True,
+        browser_like_redirects=False,
+        unbuffered=False,
+        reactor=None,
+        timeout=None,
+        _stacklevel=2,
+    ):
         """
         See :func:`treq.request()`.
         """
         method = method.encode('ascii').upper()
-        stacklevel = kwargs.pop('_stacklevel', 2)
 
         if isinstance(url, DecodedURL):
             parsed_url = url.encoded_url
@@ -163,35 +180,27 @@ def request(self, method, url, **kwargs):
 
         # Join parameters provided in the URL
         # and the ones passed as argument.
-        params = kwargs.pop('params', None)
         if params:
             parsed_url = parsed_url.replace(
                 query=parsed_url.query + tuple(_coerced_query_params(params))
             )
 
         url = parsed_url.to_uri().to_text().encode('ascii')
 
-        headers = self._request_headers(kwargs.pop('headers', None), stacklevel + 1)
+        headers = self._request_headers(headers, _stacklevel + 1)
 
-        bodyProducer, contentType = self._request_body(
-            data=kwargs.pop('data', None),
-            files=kwargs.pop('files', None),
-            json=kwargs.pop('json', _NOTHING),
-            stacklevel=stacklevel + 1,
-        )
+        bodyProducer, contentType = self._request_body(data, files, json,
+                                                       stacklevel=_stacklevel + 1)
         if contentType is not None:
             headers.setRawHeaders(b'Content-Type', [contentType])
 
-        cookies = kwargs.pop('cookies', {})
-
         if not isinstance(cookies, CookieJar):
             cookies = cookiejar_from_dict(cookies)
 
         cookies = merge_cookies(self._cookiejar, cookies)
         wrapped_agent = CookieAgent(self._agent, cookies)
 
-        browser_like_redirects = kwargs.pop('browser_like_redirects', False)
-        if kwargs.pop('allow_redirects', True):
+        if allow_redirects:
             if browser_like_redirects:
                 wrapped_agent = BrowserLikeRedirectAgent(wrapped_agent)
             else:
@@ -200,18 +209,15 @@ def request(self, method, url, **kwargs):
         wrapped_agent = ContentDecoderAgent(wrapped_agent,
                                             [(b'gzip', GzipDecoder)])
 
-        auth = kwargs.pop('auth', None)
         if auth:
             wrapped_agent = add_auth(wrapped_agent, auth)
 
         d = wrapped_agent.request(
             method, url, headers=headers,
             bodyProducer=bodyProducer)
 
-        reactor = kwargs.pop('reactor', None)
         if reactor is None:
             from twisted.internet import reactor
-        timeout = kwargs.pop('timeout', None)
         if timeout:
             delayedCall = reactor.callLater(timeout, d.cancel)
 
@@ -222,20 +228,9 @@ def gotResult(result):
 
             d.addBoth(gotResult)
 
-        if not kwargs.pop('unbuffered', False):
+        if not unbuffered:
             d.addCallback(_BufferedResponse)
 
-        if kwargs:
-            warnings.warn(
-                (
-                    "Got unexpected keyword argument: {}."
-                    " treq will ignore this argument,"
-                    " but will raise TypeError in the next treq release."
-                ).format(", ".join(repr(k) for k in kwargs)),
-                DeprecationWarning,
-                stacklevel=stacklevel,
-            )
-
         return d.addCallback(_Response, cookies)
 
     def _request_headers(self, headers, stacklevel):

src/treq/test/test_api.py

@@ -101,25 +101,15 @@ def test_request_invalid_param(self):
         This test verifies that stacklevel is set appropriately when issuing
         the warning.
         """
-        self.failureResultOf(
+        with self.assertRaises(TypeError) as c:
             treq.request(
                 "GET",
                 "https://foo.bar",
                 invalid=True,
                 pool=SyntacticAbominationHTTPConnectionPool(),
             )
-        )
 
-        [w] = self.flushWarnings([self.test_request_invalid_param])
-        self.assertEqual(DeprecationWarning, w["category"])
-        self.assertEqual(
-            (
-                "Got unexpected keyword argument: 'invalid'."
-                " treq will ignore this argument,"
-                " but will raise TypeError in the next treq release."
-            ),
-            w["message"],
-        )
+        self.assertIn("invalid", str(c.exception))
 
     def test_post_json_with_data(self):
         """

src/treq/test/test_client.py

@@ -655,19 +655,15 @@ def test_request_dict_headers_invalid_values(self):
 
     def test_request_invalid_param(self):
         """
-        `HTTPClient.request()` warns that invalid parameters are ignored and
-        that this is deprecated.
+        `HTTPClient.request()` rejects invalid keyword parameters with
+        `TypeError`.
         """
-        self.client.request('GET', b'http://example.com', invalid=True)
-
-        [w] = self.flushWarnings([self.test_request_invalid_param])
-        self.assertEqual(
-            (
-                "Got unexpected keyword argument: 'invalid'."
-                " treq will ignore this argument,"
-                " but will raise TypeError in the next treq release."
-            ),
-            w['message'],
+        self.assertRaises(
+            TypeError,
+            self.client.request,
+            "GET",
+            b"http://example.com",
+            invalid=True,
         )
 
     @with_clock

src/treq/test/test_multipart.py

@@ -78,20 +78,26 @@ def test_unknownLength(self):
         passed as a parameter without either a C{seek} or C{tell} method,
         its C{length} attribute is set to C{UNKNOWN_LENGTH}.
         """
-        class HasSeek:
+        class CantTell:
             def seek(self, offset, whence):
-                pass
+                """
+                A C{seek} method that is never called because there is no
+                matching C{tell} method.
+                """
 
-        class HasTell:
+        class CantSeek:
             def tell(self):
-                pass
+                """
+                A C{tell} method that is never called because there is no
+                matching C{seek} method.
+                """
 
         producer = MultiPartProducer(
-            {"f": ("name", None, FileBodyProducer(HasSeek()))})
+            {"f": ("name", None, FileBodyProducer(CantTell()))})
         self.assertEqual(UNKNOWN_LENGTH, producer.length)
 
         producer = MultiPartProducer(
-            {"f": ("name", None, FileBodyProducer(HasTell()))})
+            {"f": ("name", None, FileBodyProducer(CantSeek()))})
         self.assertEqual(UNKNOWN_LENGTH, producer.length)
 
     def test_knownLengthOnFile(self):