Rework File.spoiler to synchronise with the filename

Co-authored-by: Danny <Rapptz@users.noreply.github.com>

discord/file.py

@@ -23,11 +23,13 @@ DEALINGS IN THE SOFTWARE.
 """
 
 from __future__ import annotations
-from typing import Any, Dict, Optional, Union
+from typing import Any, Dict, Optional, Tuple, Union
 
 import os
 import io
 
+from .utils import MISSING
+
 # fmt: off
 __all__ = (
     'File',
@@ -35,6 +37,12 @@ __all__ = (
 # fmt: on
 
 
+def _strip_spoiler(filename: str) -> Tuple[str, bool]:
+    stripped = filename.lstrip('SPOILER_')
+    spoiler = stripped != filename
+    return stripped, spoiler
+
+
 class File:
     r"""A parameter object used for :meth:`abc.Messageable.send`
     for sending file objects.
@@ -58,26 +66,23 @@ class File:
 
             To pass binary data, consider usage of ``io.BytesIO``.
 
-    filename: Optional[:class:`str`]
-        The filename to display when uploading to Discord.
-        If this is not given then it defaults to ``fp.name`` or if ``fp`` is
-        a string then the ``filename`` will default to the string given.
     spoiler: :class:`bool`
-        Whether the attachment is a spoiler.
+        Whether the attachment is a spoiler. if left unspecified, the :attr:`~File.filename` is used
+        to determine if the file is a spoiler.
     description: Optional[:class:`str`]
         The file description to display, currently only supported for images.
 
         .. versionadded:: 2.0
     """
 
-    __slots__ = ('fp', 'filename', 'spoiler', 'description', '_original_pos', '_owner', '_closer')
+    __slots__ = ('fp', '_filename', 'spoiler', 'description', '_original_pos', '_owner', '_closer')
 
     def __init__(
         self,
         fp: Union[str, bytes, os.PathLike[Any], io.BufferedIOBase],
         filename: Optional[str] = None,
         *,
-        spoiler: bool = False,
+        spoiler: bool = MISSING,
         description: Optional[str] = None,
     ):
         if isinstance(fp, io.IOBase):
@@ -100,18 +105,29 @@ class File:
 
         if filename is None:
             if isinstance(fp, str):
-                _, self.filename = os.path.split(fp)
+                _, filename = os.path.split(fp)
             else:
-                self.filename = getattr(fp, 'name', None)
+                filename = getattr(fp, 'name', 'untitled')
-        else:
-            self.filename: Optional[str] = filename
 
-        if spoiler and self.filename is not None and not self.filename.startswith('SPOILER_'):
+        self._filename, filename_spoiler = _strip_spoiler(filename)  # type: ignore # the above getattr doesn't narrow the type
-            self.filename = 'SPOILER_' + self.filename
+        if spoiler is MISSING:
+            spoiler = filename_spoiler
 
-        self.spoiler: bool = spoiler or (self.filename is not None and self.filename.startswith('SPOILER_'))
+        self.spoiler: bool = spoiler
         self.description: Optional[str] = description
 
+    @property
+    def filename(self) -> str:
+        """:class:`str`: The filename to display when uploading to Discord.
+        If this is not given then it defaults to ``fp.name`` or if ``fp`` is
+        a string then the ``filename`` will default to the string given.
+        """
+        return 'SPOILER_' + self._filename if self.spoiler else self._filename
+
+    @filename.setter
+    def filename(self, value: str) -> None:
+        self._filename, self.spoiler = _strip_spoiler(value)
+
     def reset(self, *, seek: Union[int, bool] = True) -> None:
         # The `seek` parameter is needed because
         # the retry-loop is iterated over multiple times

docs/migrating.rst

@@ -1171,6 +1171,9 @@ The following changes have been made:
 
 - :meth:`Permissions.stage_moderator` now includes the :attr:`Permissions.manage_channels` permission and the :attr:`Permissions.request_to_speak` permission is no longer included.
 
+- :attr:`File.filename` will no longer be ``None``, in situations where previously this was the case the filename is set to `'untitled'`.
+
+
 .. _migrating_2_0_commands:
 
 Command Extension Changes

tests/test_files.py

@@ -0,0 +1,129 @@
+"""
+The MIT License (MIT)
+
+Copyright (c) 2015-present Rapptz
+
+Permission is hereby granted, free of charge, to any person obtaining a
+copy of this software and associated documentation files (the "Software"),
+to deal in the Software without restriction, including without limitation
+the rights to use, copy, modify, merge, publish, distribute, sublicense,
+and/or sell copies of the Software, and to permit persons to whom the
+Software is furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in
+all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+DEALINGS IN THE SOFTWARE.
+"""
+
+from __future__ import annotations
+
+from io import BytesIO
+
+import discord
+
+
+FILE = BytesIO()
+
+
+def test_file_with_no_name():
+    f = discord.File('.gitignore')
+    assert f.filename == '.gitignore'
+
+
+def test_io_with_no_name():
+    f = discord.File(FILE)
+    assert f.filename == 'untitled'
+
+
+def test_file_with_name():
+    f = discord.File('.gitignore', 'test')
+    assert f.filename == 'test'
+
+
+def test_io_with_name():
+    f = discord.File(FILE, 'test')
+    assert f.filename == 'test'
+
+
+def test_file_with_no_name_and_spoiler():
+    f = discord.File('.gitignore', spoiler=True)
+    assert f.filename == 'SPOILER_.gitignore'
+    assert f.spoiler == True
+
+
+def test_file_with_spoiler_name_and_implicit_spoiler():
+    f = discord.File('.gitignore', 'SPOILER_.gitignore')
+    assert f.filename == 'SPOILER_.gitignore'
+    assert f.spoiler == True
+
+
+def test_file_with_spoiler_name_and_spoiler():
+    f = discord.File('.gitignore', 'SPOILER_.gitignore', spoiler=True)
+    assert f.filename == 'SPOILER_.gitignore'
+    assert f.spoiler == True
+
+
+def test_file_with_spoiler_name_and_not_spoiler():
+    f = discord.File('.gitignore', 'SPOILER_.gitignore', spoiler=False)
+    assert f.filename == '.gitignore'
+    assert f.spoiler == False
+
+
+def test_file_with_name_and_double_spoiler_and_implicit_spoiler():
+    f = discord.File('.gitignore', 'SPOILER_SPOILER_.gitignore')
+    assert f.filename == 'SPOILER_.gitignore'
+    assert f.spoiler == True
+
+
+def test_file_with_name_and_double_spoiler_and_spoiler():
+    f = discord.File('.gitignore', 'SPOILER_SPOILER_.gitignore', spoiler=True)
+    assert f.filename == 'SPOILER_.gitignore'
+    assert f.spoiler == True
+
+
+def test_file_with_name_and_double_spoiler_and_not_spoiler():
+    f = discord.File('.gitignore', 'SPOILER_SPOILER_.gitignore', spoiler=False)
+    assert f.filename == '.gitignore'
+    assert f.spoiler == False
+
+
+def test_file_with_spoiler_with_overriding_name_not_spoiler():
+    f = discord.File('.gitignore', spoiler=True)
+    f.filename = '.gitignore'
+    assert f.filename == '.gitignore'
+    assert f.spoiler == False
+
+
+def test_file_with_spoiler_with_overriding_name_spoiler():
+    f = discord.File('.gitignore', spoiler=True)
+    f.filename = 'SPOILER_.gitignore'
+    assert f.filename == 'SPOILER_.gitignore'
+    assert f.spoiler == True
+
+
+def test_file_not_spoiler_with_overriding_name_not_spoiler():
+    f = discord.File('.gitignore')
+    f.filename = '.gitignore'
+    assert f.filename == '.gitignore'
+    assert f.spoiler == False
+
+
+def test_file_not_spoiler_with_overriding_name_spoiler():
+    f = discord.File('.gitignore')
+    f.filename = 'SPOILER_.gitignore'
+    assert f.filename == 'SPOILER_.gitignore'
+    assert f.spoiler == True
+
+
+def test_file_not_spoiler_with_overriding_name_double_spoiler():
+    f = discord.File('.gitignore')
+    f.filename = 'SPOILER_SPOILER_.gitignore'
+    assert f.filename == 'SPOILER_.gitignore'
+    assert f.spoiler == True