refactor(lint): add flake8-return rules #948

onerandomusername · 2023-02-23T21:35:04Z

Summary

Enable all RET errors on ruff.

As some of these error codes might not be desired, I seperated each of them into an individual commit (except for RET506, I fixed some of that in RET505 inadvertently).

Checklist

If code changes were made, then they have been tested
- I have updated the documentation to reflect the changes
- I have formatted the code properly by running pdm lint
- I have type-checked the code by running pdm run pyright
This PR fixes an issue
This PR adds something new (e.g. new method or parameters)
This PR is a breaking change (e.g. methods or parameters removed/renamed)
This PR is not a code change (e.g. documentation, README, ...)

some of these were inadvertently fixed in the RET505 commit

shiftinv

Seems alright overall, though I have to admit I'm not a fan of rewriting elif: return ... chains to if: when they all contain returns (RET505). Some examples:

They compile down to the same bytecode and all that, but make the code more difficult to read ("does the previous if always return or can it fall through?") and maintain (the links above and afonasev/flake8-return#128 outline this quite well).

Rewriting if + elif + else is mostly fine, but once you're dealing with multiple elifs, rewriting them all to ifs makes things worse and feels dangerous considering future changes, imho.

shiftinv · 2023-04-17T12:18:40Z

disnake/ui/view.py

@@ -192,7 +192,7 @@ async def __timeout_task_impl(self) -> None:
        while True:
            # Guard just in case someone changes the value of the timeout at runtime
            if self.timeout is None:
-                return
+                return None


This doesn't need an explicit None, all the return x() in this method are shortcuts for x(); return

okay, so i'll change the other two to

x() return

sounds good to me.

shiftinv · 2023-04-17T12:34:38Z

disnake/channel.py

            # the payload will always be the proper channel payload
            return self.__class__(state=self._state, guild=self.guild, data=payload)  # type: ignore

+        return None
+


nit: the spacing here seems weird, whitespace above the if would make more sense, i.e.

**kwargs, ) if payload is not None: # the payload will always be the proper channel payload return self.__class__(state=self._state, guild=self.guild, data=payload) # type: ignore return None

shiftinv · 2023-04-17T13:01:36Z

disnake/channel.py

    value = try_enum(ChannelType, channel_type)
    if value is ChannelType.text:
        return TextChannel, value
-    elif value is ChannelType.voice:
+    if value is ChannelType.voice:
        return VoiceChannel, value
-    elif value is ChannelType.category:
+    if value is ChannelType.category:
        return CategoryChannel, value
-    elif value is ChannelType.news:
+    if value is ChannelType.news:
        return TextChannel, value
-    elif value is ChannelType.stage_voice:
+    if value is ChannelType.stage_voice:
        return StageChannel, value
-    elif value is ChannelType.forum:
+    if value is ChannelType.forum:
        return ForumChannel, value
-    else:
-        return None, value
+    return None, value


These should be lookup tables - out of scope here though

shiftinv · 2023-04-17T13:20:24Z

disnake/ext/commands/common_bot_base.py

@@ -154,18 +154,16 @@ async def is_owner(self, user: Union[disnake.User, disnake.Member]) -> bool:
        """
        if self.owner_id:
            return user.id == self.owner_id
-        elif self.owner_ids:
+        if self.owner_ids:
            return user.id in self.owner_ids


Suggested change

return user.id in self.owner_ids

return user.id in self.owner_ids

shiftinv · 2023-04-17T14:01:35Z

disnake/ext/commands/core.py

@@ -505,8 +504,7 @@ def _update_copy(self: CommandT, kwargs: Dict[str, Any]) -> CommandT:
            kw.update(self.__original_kwargs__)
            copy = self.__class__(self.callback, **kw)
            return self._ensure_assignment_on_copy(copy)
-        else:


should be early return, similar to base_core.py

shiftinv · 2023-04-17T14:38:07Z

disnake/shard.py

@@ -449,6 +449,7 @@ async def launch_shard(self, gateway: str, shard_id: int, *, initial: bool = Fal
        # keep reading the shard while others connect
        self.__shards[shard_id] = ret = Shard(ws, self, self.__queue.put_nowait)
        ret.launch()
+        return None


consider removing the return above instead?

shiftinv · 2023-04-17T14:48:18Z

examples/basic_voice.py

@@ -111,6 +112,7 @@ async def volume(self, ctx, volume: int):

        ctx.voice_client.source.volume = volume / 100
        await ctx.send(f"Changed volume to {volume}%")
+        return None


command callbacks shouldn't return anything.

shiftinv · 2023-04-17T14:48:50Z

examples/converters.py

@@ -40,6 +40,7 @@ async def userinfo_error(ctx: commands.Context, error: commands.CommandError):
    # so we handle this in this error handler:
    if isinstance(error, commands.UserNotFound):
        return await ctx.send("Couldn't find that user.")
+    return None


shiftinv · 2023-06-22T22:34:15Z

@onerandomusername would you mind updating and resolving conflicts?

onerandomusername added s: needs review Issue/PR is awaiting reviews t: refactor/typing/lint Refactors, typing changes and/or linting changes skip news labels Feb 23, 2023

onerandomusername changed the title ~~Lint/flake8 return~~ chore(lint): add flake8-return rules Feb 23, 2023

onerandomusername changed the title ~~chore(lint): add flake8-return rules~~ refactor(lint): add flake8-return rules Feb 24, 2023

onerandomusername marked this pull request as ready for review February 26, 2023 17:10

onerandomusername force-pushed the lint/flake8-return branch from e60e403 to 8dc64aa Compare March 1, 2023 18:36

onerandomusername force-pushed the lint/flake8-return branch 2 times, most recently from 3a17a35 to 5f2fbac Compare April 6, 2023 20:28

onerandomusername requested a review from shiftinv April 6, 2023 20:28

onerandomusername force-pushed the lint/flake8-return branch from 5f2fbac to 7b70e74 Compare April 8, 2023 07:21

enable some RET codes

18d64ba

onerandomusername force-pushed the lint/flake8-return branch from 7b70e74 to f00238b Compare April 16, 2023 22:33

onerandomusername added 7 commits April 16, 2023 18:49

fix all RET502 errors

39319e9

fix all RET503 errors

bcbac41

ignore all RET504 errors

bd922a4

fix all RET505 errors

c396fa5

fix all RET506 errors

11296e6

some of these were inadvertently fixed in the RET505 commit

fix all RET507 errors

d033bfa

fix all RET508 errors

73e5905

onerandomusername force-pushed the lint/flake8-return branch from f00238b to 73e5905 Compare April 16, 2023 22:49

shiftinv reviewed Apr 17, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(lint): add flake8-return rules #948

refactor(lint): add flake8-return rules #948

onerandomusername commented Feb 23, 2023

shiftinv left a comment •

edited

Loading

shiftinv Apr 17, 2023

onerandomusername Apr 28, 2023 •

edited

Loading

shiftinv Apr 28, 2023

shiftinv Apr 17, 2023

shiftinv Apr 17, 2023

shiftinv Apr 17, 2023

shiftinv Apr 17, 2023

shiftinv Apr 17, 2023

shiftinv Apr 17, 2023

shiftinv Apr 17, 2023

shiftinv commented Jun 22, 2023

	return user.id in self.owner_ids
	return user.id in self.owner_ids

refactor(lint): add flake8-return rules #948

Are you sure you want to change the base?

refactor(lint): add flake8-return rules #948

Conversation

onerandomusername commented Feb 23, 2023

Summary

Checklist

shiftinv left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onerandomusername Apr 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shiftinv commented Jun 22, 2023

shiftinv left a comment •

edited

Loading

onerandomusername Apr 28, 2023 •

edited

Loading