Improve Code Quality (#2450)

* chore: refactor code quality issues

* Add comment for removing assert statements

* Remove deepsource config file

* Fix Coverage

Co-authored-by: Hinrich Mahler <hinrich.mahler@freenet.de>

docs/source/conf.py

@@ -13,7 +13,6 @@
 # serve to show the default.
 import sys
 import os
-import shlex
 # import telegram
 
 # If extensions (or modules to document with autodoc) are in another directory,

examples/echobot.py

@@ -34,7 +34,7 @@ def start(update: Update, _: CallbackContext) -> None:
     """Send a message when the command /start is issued."""
     user = update.effective_user
     update.message.reply_markdown_v2(
-        f'Hi {user.mention_markdown_v2()}\!',
+        fr'Hi {user.mention_markdown_v2()}\!',
         reply_markup=ForceReply(selective=True),
     )
 

setup.py

@@ -1,7 +1,5 @@
 #!/usr/bin/env python
 """The setup and build script for the python-telegram-bot library."""
-
-import codecs
 import os
 import subprocess
 import sys

telegram/base.py

@@ -76,7 +76,7 @@ class TelegramObject:
         return json.dumps(self.to_dict())
 
     def to_dict(self) -> JSONDict:
-        data = dict()
+        data = {}
 
         for key in iter(self.__dict__):
             if key == 'bot' or key.startswith('_'):

telegram/bot.py

@@ -3457,7 +3457,10 @@ class Bot(TelegramObject):
         data: JSONDict = {'shipping_query_id': shipping_query_id, 'ok': ok}
 
         if ok:
-            assert shipping_options
+            if not shipping_options:
+                # not using an assert statement directly here since they are removed in
+                # the optimized bytecode
+                raise AssertionError
             data['shipping_options'] = [option.to_dict() for option in shipping_options]
         if error_message is not None:
             data['error_message'] = error_message

telegram/ext/commandhandler.py

@@ -354,9 +354,9 @@ class PrefixHandler(CommandHandler):
         run_async: Union[bool, DefaultValue] = DEFAULT_FALSE,
     ):
 
-        self._prefix: List[str] = list()
-        self._command: List[str] = list()
-        self._commands: List[str] = list()
+        self._prefix: List[str] = []
+        self._command: List[str] = []
+        self._commands: List[str] = []
 
         super().__init__(
             'nocommand',

telegram/ext/conversationhandler.py

@@ -241,9 +241,9 @@ class ConversationHandler(Handler[Update]):
         Set by dispatcher"""
         self._map_to_parent = map_to_parent
 
-        self.timeout_jobs: Dict[Tuple[int, ...], 'Job'] = dict()
+        self.timeout_jobs: Dict[Tuple[int, ...], 'Job'] = {}
         self._timeout_jobs_lock = Lock()
-        self._conversations: ConversationDict = dict()
+        self._conversations: ConversationDict = {}
         self._conversations_lock = Lock()
 
         self.logger = logging.getLogger(__name__)
@@ -257,7 +257,7 @@ class ConversationHandler(Handler[Update]):
                 "since message IDs are not globally unique."
             )
 
-        all_handlers: List[Handler] = list()
+        all_handlers: List[Handler] = []
         all_handlers.extend(entry_points)
         all_handlers.extend(fallbacks)
 
@@ -407,7 +407,7 @@ class ConversationHandler(Handler[Update]):
         chat = update.effective_chat
         user = update.effective_user
 
-        key = list()
+        key = []
 
         if self.per_chat:
             key.append(chat.id)  # type: ignore[union-attr]

telegram/ext/dispatcher.py

@@ -511,7 +511,7 @@ class Dispatcher:
             handler.conversations = self.persistence.get_conversations(handler.name)
 
         if group not in self.handlers:
-            self.handlers[group] = list()
+            self.handlers[group] = []
             self.groups.append(group)
             self.groups = sorted(self.groups)
 

telegram/ext/handler.py

@@ -202,7 +202,7 @@ class Handler(Generic[UT], ABC):
             check_result: The result from check_update
 
         """
-        optional_args: Dict[str, object] = dict()
+        optional_args: Dict[str, object] = {}
 
         if self.pass_update_queue:
             optional_args['update_queue'] = dispatcher.update_queue

telegram/ext/picklepersistence.py

@@ -106,7 +106,7 @@ class PicklePersistence(BasePersistence):
                 self.bot_data = data.get('bot_data', {})
                 self.conversations = data['conversations']
         except OSError:
-            self.conversations = dict()
+            self.conversations = {}
             self.user_data = defaultdict(dict)
             self.chat_data = defaultdict(dict)
             self.bot_data = {}
@@ -233,7 +233,7 @@ class PicklePersistence(BasePersistence):
             new_state (:obj:`tuple` | :obj:`any`): The new state for the given key.
         """
         if not self.conversations:
-            self.conversations = dict()
+            self.conversations = {}
         if self.conversations.setdefault(name, {}).get(key) == new_state:
             return
         self.conversations[name][key] = new_state

telegram/games/game.py

@@ -83,7 +83,7 @@ class Game(TelegramObject):
         self.photo = photo
         # Optionals
         self.text = text
-        self.text_entities = text_entities or list()
+        self.text_entities = text_entities or []
         self.animation = animation
 
         self._id_attrs = (self.title, self.description, self.photo)

telegram/message.py

@@ -438,12 +438,12 @@ class Message(TelegramObject):
         self.reply_to_message = reply_to_message
         self.edit_date = edit_date
         self.text = text
-        self.entities = entities or list()
-        self.caption_entities = caption_entities or list()
+        self.entities = entities or []
+        self.caption_entities = caption_entities or []
         self.audio = audio
         self.game = game
         self.document = document
-        self.photo = photo or list()
+        self.photo = photo or []
         self.sticker = sticker
         self.video = video
         self.voice = voice
@@ -452,10 +452,10 @@ class Message(TelegramObject):
         self.contact = contact
         self.location = location
         self.venue = venue
-        self.new_chat_members = new_chat_members or list()
+        self.new_chat_members = new_chat_members or []
         self.left_chat_member = left_chat_member
         self.new_chat_title = new_chat_title
-        self.new_chat_photo = new_chat_photo or list()
+        self.new_chat_photo = new_chat_photo or []
         self.delete_chat_photo = bool(delete_chat_photo)
         self.group_chat_created = bool(group_chat_created)
         self.supergroup_chat_created = bool(supergroup_chat_created)

telegram/utils/request.py

@@ -119,7 +119,7 @@ class Request:
         read_timeout: float = 5.0,
     ):
         if urllib3_proxy_kwargs is None:
-            urllib3_proxy_kwargs = dict()
+            urllib3_proxy_kwargs = {}
 
         self._connect_timeout = connect_timeout
 

tests/test_bot.py

@@ -1442,6 +1442,9 @@ class TestBot:
         with pytest.raises(TelegramError, match='should not be empty and there should not be'):
             bot.answer_shipping_query(1, True)
 
+        with pytest.raises(AssertionError):
+            bot.answer_shipping_query(1, True, shipping_options=[])
+
     # TODO: Needs improvement. Need incoming pre checkout queries to test
     def test_answer_pre_checkout_query_ok(self, monkeypatch, bot):
         # For now just test that our internals pass the correct data

tests/test_chat.py

@@ -213,7 +213,7 @@ class TestChat:
         def make_assertion(*_, **kwargs):
             chat_id = kwargs['chat_id'] == chat.id
             user_id = kwargs['user_id'] == 42
-            o_i_b = kwargs.get('only_if_banned', None) == only_if_banned
+            o_i_b = kwargs.get('only_if_banned') == only_if_banned
             return chat_id and user_id and o_i_b
 
         assert check_shortcut_signature(Chat.unban_member, Bot.unban_chat_member, ['chat_id'], [])
@@ -228,7 +228,7 @@ class TestChat:
         def make_assertion(*_, **kwargs):
             chat_id = kwargs['chat_id'] == chat.id
             user_id = kwargs['user_id'] == 42
-            o_i_b = kwargs.get('is_anonymous', None) == is_anonymous
+            o_i_b = kwargs.get('is_anonymous') == is_anonymous
             return chat_id and user_id and o_i_b
 
         assert check_shortcut_signature(
@@ -246,7 +246,7 @@ class TestChat:
         def make_assertion(*_, **kwargs):
             chat_id = kwargs['chat_id'] == chat.id
             user_id = kwargs['user_id'] == 42
-            o_i_b = kwargs.get('permissions', None) == permissions
+            o_i_b = kwargs.get('permissions') == permissions
             return chat_id and user_id and o_i_b
 
         assert check_shortcut_signature(

tests/test_commandhandler.py

@@ -79,7 +79,7 @@ class BaseTest:
 
     def make_callback_for(self, pass_keyword):
         def callback(bot, update, **kwargs):
-            self.test_flag = kwargs.get(keyword, None) is not None
+            self.test_flag = kwargs.get(keyword) is not None
 
         keyword = pass_keyword[5:]
         return callback

tests/test_conversationhandler.py

@@ -99,7 +99,7 @@ class TestConversationHandler:
     def reset(self):
         self.raise_dp_handler_stop = False
         self.test_flag = False
-        self.current_state = dict()
+        self.current_state = {}
         self.entry_points = [CommandHandler('start', self.start)]
         self.states = {
             self.THIRSTY: [CommandHandler('brew', self.brew), CommandHandler('wait', self.start)],

tests/test_dispatcher.py

@@ -569,7 +569,7 @@ class TestDispatcher:
                 self.store_bot_data = True
 
             def get_bot_data(self):
-                return dict()
+                return {}
 
             def update_bot_data(self, data):
                 raise Exception

tests/test_error.py

@@ -131,7 +131,7 @@ class TestErrors:
         """
 
         def make_assertion(cls):
-            assert {sc for sc in cls.__subclasses__()} == covered_subclasses[cls]
+            assert set(cls.__subclasses__()) == covered_subclasses[cls]
             for subcls in cls.__subclasses__():
                 make_assertion(subcls)
 

tests/test_persistence.py

@@ -960,10 +960,16 @@ class TestPickelPersistence:
         assert not pickle_persistence.conversations['name1'] == conversation1
         pickle_persistence.update_conversation('name1', (123, 123), 5)
         assert pickle_persistence.conversations['name1'] == conversation1
+        assert pickle_persistence.get_conversations('name1') == conversation1
         with open('pickletest_conversations', 'rb') as f:
             conversations_test = defaultdict(dict, pickle.load(f))
         assert conversations_test['name1'] == conversation1
 
+        pickle_persistence.conversations = None
+        pickle_persistence.update_conversation('name1', (123, 123), 5)
+        assert pickle_persistence.conversations['name1'] == {(123, 123): 5}
+        assert pickle_persistence.get_conversations('name1') == {(123, 123): 5}
+
     def test_updating_single_file(self, pickle_persistence, good_pickle_files):
         pickle_persistence.single_file = True
 
@@ -999,10 +1005,16 @@ class TestPickelPersistence:
         assert not pickle_persistence.conversations['name1'] == conversation1
         pickle_persistence.update_conversation('name1', (123, 123), 5)
         assert pickle_persistence.conversations['name1'] == conversation1
+        assert pickle_persistence.get_conversations('name1') == conversation1
         with open('pickletest', 'rb') as f:
             conversations_test = defaultdict(dict, pickle.load(f)['conversations'])
         assert conversations_test['name1'] == conversation1
 
+        pickle_persistence.conversations = None
+        pickle_persistence.update_conversation('name1', (123, 123), 5)
+        assert pickle_persistence.conversations['name1'] == {(123, 123): 5}
+        assert pickle_persistence.get_conversations('name1') == {(123, 123): 5}
+
     def test_save_on_flush_multi_files(self, pickle_persistence, good_pickle_files):
         # Should run without error
         pickle_persistence.flush()