feat!: Enforcing usage of keyword-only arguments in some functions (#656)

BREAKING CHANGE: Enforcing using keyword-only arguments in functions :
  - `copy()` in `base_item.py` `file.py`
  - `create_shared_link()` in `item.py`, `web_link.py`
  - `get_shared_link()` in `item.py`, `web_link.py`
  - `remove_shared_link()` in `item.py`
  - `get()` in `base_object.py`, `item.py`
  - `update_info()` in `base_object.py`, `collaboration.py`, `item.py`, `metadata_template.py`
  - `delete()` in `base_object.py`, `folder.py`, `item.py`, `user.py`
Closes: SDK-1904

Author: lukaszsocha2

boxsdk/object/base_endpoint.py

@@ -34,17 +34,14 @@ def translator(self) -> 'Translator':
         """
         return self._session.translator
 
-    def get_url(self, endpoint: str, *args: Any) -> str:
+    def get_url(self, *args: Any) -> str:
         """
         Return the URL used to access the endpoint.
 
-        :param endpoint:
-            The name of the endpoint.
         :param args:
-            Additional parts of the endpoint URL.
+            Parts of the endpoint URL.
         """
-        # pylint:disable=no-self-use
-        return self._session.get_url(endpoint, *args)
+        return self._session.get_url(*args)
 
     def clone(self, session: 'Session' = None) -> 'BaseEndpoint':
         """

boxsdk/object/base_item.py

@@ -15,7 +15,7 @@
 class BaseItem(BaseObject):
 
     @api_call
-    def copy(self, parent_folder: 'Folder', name: str = None) -> 'BaseItem':
+    def copy(self, *, parent_folder: 'Folder', name: str = None, **_kwargs) -> 'BaseItem':
         """Copy the item to the given folder.
 
         :param parent_folder:
@@ -52,7 +52,7 @@ def move(self, parent_folder: 'Folder', name: str = None) -> 'BaseItem':
         }
         if name is not None:
             data['name'] = name
-        return self.update_info(data)
+        return self.update_info(data=data)
 
     @api_call
     def rename(self, name: str) -> 'BaseItem':
@@ -65,7 +65,7 @@ def rename(self, name: str) -> 'BaseItem':
         data = {
             'name': name,
         }
-        return self.update_info(data)
+        return self.update_info(data=data)
 
     @api_call
     def create_shared_link(self, **kwargs: Any) -> Any:
@@ -102,7 +102,7 @@ def create_shared_link(self, **kwargs: Any) -> Any:
         data = {'shared_link': shared_link}
         update_info_kwargs = {'etag': kwargs.get('etag')} if kwargs.get('etag') is not None else {}
 
-        return self.update_info(data, **update_info_kwargs)
+        return self.update_info(data=data, **update_info_kwargs)
 
     @api_call
     def get_shared_link(self, **kwargs: Any) -> str:
@@ -131,7 +131,7 @@ def remove_shared_link(self, **kwargs: Any) -> bool:
         data = {'shared_link': None}
         update_info_kwargs = {'etag': kwargs.get('etag')} if kwargs.get('etag') is not None else {}
 
-        item = self.update_info(data, **update_info_kwargs)
+        item = self.update_info(data=data, **update_info_kwargs)
         return item.shared_link is None  # pylint:disable=no-member
 
     @api_call
@@ -149,7 +149,7 @@ def add_to_collection(self, collection: 'Collection') -> 'BaseItem':
         data = {
             'collections': collections
         }
-        return self.update_info(data)
+        return self.update_info(data=data)
 
     @api_call
     def remove_from_collection(self, collection: 'Collection') -> 'BaseItem':
@@ -166,7 +166,7 @@ def remove_from_collection(self, collection: 'Collection') -> 'BaseItem':
         data = {
             'collections': updated_collections
         }
-        return self.update_info(data)
+        return self.update_info(data=data)
 
     @staticmethod
     def validate_item_id(item_id: Union[str, int]) -> None:

boxsdk/object/base_object.py

@@ -1,7 +1,7 @@
 # coding: utf-8
 
 import json
-from typing import TYPE_CHECKING, Any, Iterable, Optional
+from typing import TYPE_CHECKING, Any, Iterable, Optional, Union, List
 
 from .base_endpoint import BaseEndpoint
 from .base_api_json_object import BaseAPIJSONObject
@@ -40,7 +40,6 @@ def get_url(self, *args: Any) -> str:
         Base class override.
         Return the given object's URL, appending any optional parts as specified by args.
         """
-        # pylint:disable=arguments-differ
         return super().get_url(f'{self._item_type}s', self._object_id, *args)
 
     def get_type_url(self) -> str:
@@ -57,7 +56,7 @@ def object_id(self) -> str:
         return self._object_id
 
     @api_call
-    def get(self, fields: Iterable[str] = None, headers: dict = None) -> Any:
+    def get(self, *, fields: Iterable[str] = None, headers: dict = None, **_kwargs) -> Any:
         """
         Get information about the object, specified by fields. If fields is None, return the default fields.
 
@@ -79,7 +78,8 @@ def get(self, fields: Iterable[str] = None, headers: dict = None) -> Any:
     @api_call
     def update_info(
             self,
-            data: dict,
+            *,
+            data: Union[dict, List[dict]],
             params: Optional[dict] = None,
             headers: Optional[dict] = None,
             **kwargs: Any
@@ -109,19 +109,18 @@ def update_info(
             Construct the new object with all the default attributes that are
             returned from the endpoint.
         """
-        # pylint:disable=no-else-return
         url = self.get_url()
         box_response = self._session.put(url, data=json.dumps(data), params=params, headers=headers, **kwargs)
         if 'expect_json_response' in kwargs and not kwargs['expect_json_response']:
             return box_response.ok
-        else:
-            return self.translator.translate(
-                session=self._session,
-                response_object=box_response.json(),
-            )
+
+        return self.translator.translate(
+            session=self._session,
+            response_object=box_response.json(),
+        )
 
     @api_call
-    def delete(self, params: Optional[dict] = None, headers: Optional[dict] = None) -> bool:
+    def delete(self, *, params: Optional[dict] = None, headers: Optional[dict] = None, **_kwargs) -> bool:
         """ Delete the object.
 
         :param params:
@@ -134,8 +133,7 @@ def delete(self, params: Optional[dict] = None, headers: Optional[dict] = None)
             :class:`BoxAPIException` in case of unexpected errors.
         """
         url = self.get_url()
-        # ??? There's a question about why params forces a default to {}, while headers doesn't. Looking for
-        # confirmation that the below is correct.
+
         box_response = self._session.delete(url, expect_json_response=False, params=params or {}, headers=headers)
         return box_response.ok
 

boxsdk/object/collaboration.py

@@ -1,5 +1,5 @@
 # coding: utf-8
-from typing import Optional
+from typing import Optional, Any
 
 from boxsdk.object.base_object import BaseObject
 from boxsdk.util.text_enum import TextEnum
@@ -34,8 +34,10 @@ class Collaboration(BaseObject):
     @api_call
     def update_info(
             self,
+            *,
             role: Optional[CollaborationRole] = None,
-            status: Optional[CollaborationStatus] = None
+            status: Optional[CollaborationStatus] = None,
+            **kwargs: Any
     ) -> 'BaseObject':
         """Edit an existing collaboration on Box
 
@@ -56,9 +58,9 @@ def update_info(
         if status:
             data['status'] = status
         if role == CollaborationRole.OWNER:
-            return super().update_info(data=data, expect_json_response=False)
+            return super().update_info(data=data, expect_json_response=False, **kwargs)
 
-        return super().update_info(data=data)
+        return super().update_info(data=data, **kwargs)
 
     @api_call
     def accept(self) -> 'BaseObject':

boxsdk/object/comment.py

@@ -47,4 +47,4 @@ def edit(self, message: str) -> 'Comment':
             The content of the reply comment.
         """
         data = self.construct_params_from_message(message)
-        return self.update_info(data)
+        return self.update_info(data=data)

boxsdk/object/events.py

@@ -60,7 +60,6 @@ class Events(BaseEndpoint):
 
     def get_url(self, *args: Any) -> str:
         """Base class override."""
-        # pylint:disable=arguments-differ
         return super().get_url('events', *args)
 
     @api_call

boxsdk/object/file.py

@@ -344,7 +344,7 @@ def lock(self, prevent_download: bool = False, expire_time: Optional[str] = None
         }
         if expire_time is not None:
             data['lock']['expires_at'] = expire_time
-        return self.update_info(data)
+        return self.update_info(data=data)
 
     @api_call
     def unlock(self) -> 'File':
@@ -355,7 +355,7 @@ def unlock(self) -> 'File':
             A new :class:`File` instance reflecting that the file has been unlocked.
         """
         data = {'lock': None}
-        return self.update_info(data)
+        return self.update_info(data=data)
 
     @api_call
     def get_shared_link_download_url(
@@ -681,7 +681,14 @@ def get_thumbnail_representation(self, dimensions: str, extension: str = 'png')
         return b''
 
     @api_call
-    def copy(self, parent_folder: 'Folder', name: Optional[str] = None, file_version: 'FileVersion' = None) -> 'File':
+    def copy(
+            self,
+            *,
+            parent_folder: 'Folder',
+            name: Optional[str] = None,
+            file_version: 'FileVersion' = None,
+            **_kwargs
+    ) -> 'File':
         # pylint: disable=arguments-differ
         """Copy the item to the given folder.
 
@@ -694,6 +701,7 @@ def copy(self, parent_folder: 'Folder', name: Optional[str] = None, file_version
         :returns:
             The copy of the file
         """
+        # pylint: disable=arguments-differ
         url = self.get_url('copy')
         data = {
             'parent': {'id': parent_folder.object_id}

boxsdk/object/folder.py

@@ -503,7 +503,13 @@ def create_web_link(
         )
 
     @api_call
-    def delete(self, recursive: bool = True, etag: Optional[str] = None) -> bool:
+    def delete(
+            self,
+            *,
+            recursive: bool = True,
+            etag: Optional[str] = None,
+            **kwargs
+    ) -> bool:
         """Base class override. Delete the folder.
 
         :param recursive:
@@ -515,7 +521,7 @@ def delete(self, recursive: bool = True, etag: Optional[str] = None) -> bool:
         :raises: :class:`BoxAPIException` if the specified etag doesn't match the latest version of the folder.
         """
         # pylint:disable=arguments-differ,arguments-renamed
-        return super().delete({'recursive': recursive}, etag)
+        return super().delete(params={'recursive': recursive}, etag=etag, **kwargs)
 
     @api_call
     def get_metadata_cascade_policies(

boxsdk/object/item.py

@@ -99,7 +99,7 @@ def _preflight_check(
         return response_json.get('upload_url', None)
 
     @api_call
-    def update_info(self, data: dict, etag: Optional[str] = None) -> 'Item':
+    def update_info(self, *, data: dict, etag: Optional[str] = None, **kwargs: Any) -> 'Item':
         """
         Baseclass override.
         :param data:
@@ -120,10 +120,10 @@ def update_info(self, data: dict, etag: Optional[str] = None) -> 'Item':
         # pylint:disable=arguments-differ
         self.validate_item_id(self._object_id)
         headers = {'If-Match': etag} if etag is not None else None
-        return super().update_info(data, headers=headers)
+        return super().update_info(data=data, headers=headers, **kwargs)
 
     @api_call
-    def get(self, fields: Iterable[str] = None, etag: Optional[str] = None) -> 'Item':
+    def get(self, *, fields: Iterable[str] = None, etag: Optional[str] = None, **kwargs) -> 'Item':
         """
         Base class override.
 
@@ -138,11 +138,12 @@ def get(self, fields: Iterable[str] = None, etag: Optional[str] = None) -> 'Item
         # pylint:disable=arguments-differ,arguments-renamed
         self.validate_item_id(self._object_id)
         headers = {'If-None-Match': etag} if etag is not None else None
-        return super().get(fields=fields, headers=headers)
+        return super().get(fields=fields, headers=headers, **kwargs)
 
     @api_call
     def create_shared_link(
             self,
+            *,
             access: Optional[str] = None,
             etag: Optional[str] = None,
             unshared_at: Optional[str] = SDK_VALUE_NOT_SET,
@@ -198,6 +199,7 @@ def create_shared_link(
     @api_call
     def get_shared_link(
             self,
+            *,
             access: Optional[str] = None,
             etag: Optional[str] = None,
             unshared_at: Optional[str] = SDK_VALUE_NOT_SET,
@@ -248,7 +250,7 @@ def get_shared_link(
         )
 
     @api_call
-    def remove_shared_link(self, etag: Optional[str] = None, **kwargs: Any) -> bool:
+    def remove_shared_link(self, *, etag: Optional[str] = None, **kwargs: Any) -> bool:
         """
         Baseclass override.
 
@@ -264,7 +266,7 @@ def remove_shared_link(self, etag: Optional[str] = None, **kwargs: Any) -> bool:
         return super().remove_shared_link(etag=etag)
 
     @api_call
-    def delete(self, params: dict = None, etag: Optional[str] = None) -> bool:
+    def delete(self, *, params: dict = None, etag: Optional[str] = None, **kwargs) -> bool:
         """Delete the item.
 
         :param params:
@@ -278,7 +280,7 @@ def delete(self, params: dict = None, etag: Optional[str] = None) -> bool:
         # pylint:disable=arguments-differ,arguments-renamed
         self.validate_item_id(self._object_id)
         headers = {'If-Match': etag} if etag is not None else None
-        return super().delete(params, headers)
+        return super().delete(params=params, headers=headers, **kwargs)
 
     def metadata(self, scope: str = 'global', template: str = 'properties') -> Metadata:
         """

boxsdk/object/metadata.py

@@ -103,7 +103,6 @@ def __init__(self, session: 'Session', box_object: 'BaseObject', scope: str, tem
 
     def get_url(self, *args: Any) -> str:
         """ Base class override. """
-        # pylint:disable=arguments-differ
         return self._object.get_url('metadata', self._scope, self._template)
 
     @staticmethod