squash: more work on core, fields, plugin, mixins

xblock/core.py

@@ -11,14 +11,16 @@
 import warnings
 import typing as t
 from collections import defaultdict
+from xml.etree import ElementTree as ET
 
 import pkg_resources
 from opaque_keys.edx.keys import LearningContextKey, UsageKey
 from web_fragments.fragment import Fragment
 
 import xblock.exceptions
 from xblock.exceptions import DisallowedFileError
-from xblock.fields import String, List, Scope
+from xblock.fields import String, List, Scope, ScopeIds
+from xblock.field_data import FieldData
 from xblock.internal import class_lazy
 import xblock.mixins
 from xblock.mixins import (
@@ -33,6 +35,10 @@
 from xblock.plugin import Plugin
 from xblock.validation import Validation
 
+
+if t.TYPE_CHECKING:
+    from xblock.runtime import Runtime
+
 # exposing XML_NAMESPACES as a member of core, in order to avoid importing mixins where
 # XML_NAMESPACES are needed (e.g. runtime.py).
 XML_NAMESPACES = xblock.mixins.XML_NAMESPACES
@@ -87,7 +93,7 @@ def get_i18n_js_namespace(cls) -> str | None:
         return cls.i18n_js_namespace
 
     @classmethod
-    def open_local_resource(cls, uri: str) -> t.BinaryIO:
+    def open_local_resource(cls, uri: str) -> t.IO[bytes]:
         """
         Open a local resource.
 
@@ -143,11 +149,11 @@ class XBlock(XmlSerializationMixin, HierarchyMixin, ScopedStorageMixin, RuntimeS
     tags = List(help="Tags for this block", scope=Scope.settings)
 
     @class_lazy
-    def _class_tags(cls: type[XBlock]) -> set[str]:  # pylint: disable=no-self-argument
+    def _class_tags(cls: type[XBlock]) -> set[str]:  # type: ignore[misc]
         """
         Collect the tags from all base classes.
         """
-        class_tags = set()
+        class_tags: set[str] = set()
 
         for base in cls.mro()[1:]:  # pylint: disable=no-member
             class_tags.update(getattr(base, '_class_tags', set()))
@@ -165,7 +171,9 @@ def dec(cls: type[XBlock]) -> type[XBlock]:
         return dec
 
     @classmethod
-    def load_tagged_classes(cls, tag, fail_silently=True) -> t.Iterable[type[XBlock]]:
+    def load_tagged_classes(
+        cls, tag: str, fail_silently: bool = True
+    ) -> t.Iterable[tuple[str, type[XBlock]]]:
         """
         Produce a sequence of all XBlock classes tagged with `tag`.
 
@@ -178,18 +186,17 @@ def load_tagged_classes(cls, tag, fail_silently=True) -> t.Iterable[type[XBlock]
         (e.g. on startup or first page load), and in what
         contexts. Hence, the flag.
         """
-        # Allow this method to access the `_class_tags`
-        # pylint: disable=W0212
         for name, class_ in cls.load_classes(fail_silently):
-            if tag in class_._class_tags:
-                yield name, class_
+            xblock_class: type[XBlock] = class_  # type: ignore
+            if tag in xblock_class._class_tags:  # pylint: disable=protected-access
+                yield name, xblock_class
 
     # pylint: disable=keyword-arg-before-vararg
     def __init__(
         self,
         runtime: Runtime,
         field_data: FieldData | None = None,
-        scope_ids: ScopeIds = UNSET,
+        scope_ids: ScopeIds | object = UNSET,
         *args,
         **kwargs
     ):
@@ -264,7 +271,7 @@ def ugettext(self, text) -> str:
         runtime_ugettext = runtime_service.ugettext
         return runtime_ugettext(text)
 
-    def add_xml_to_node(self, node: etree.Element) -> None:
+    def add_xml_to_node(self, node: ET.Element) -> None:
         """
         For exporting, set data on etree.Element `node`.
         """
@@ -273,18 +280,19 @@ def add_xml_to_node(self, node: etree.Element) -> None:
         self.add_children_to_node(node)
 
 
-XBlockAsideView: t.TypeAlias = t.Callable[[XBlockAside, XBlock, dict | None], Fragment]
+# An XBlockAside's view method takes itself, an XBlock, and optional context dict,
+# and returns a Fragment.
+AsideView = t.Callable[["XBlockAside", XBlock, t.Optional[dict]], Fragment]
 
 
 class XBlockAside(XmlSerializationMixin, ScopedStorageMixin, RuntimeServicesMixin, HandlersMixin, SharedBlockBase):
     """
     This mixin allows Xblock-like class to declare that it provides aside functionality.
     """
-
     entry_point: str = "xblock_asides.v1"
 
     @classmethod
-    def aside_for(cls, view_name: str) -> t.Callable[[XBlockAsideView], XBlockAsideView]:
+    def aside_for(cls, view_name: str) -> t.Callable[[AsideView], AsideView]:
         """
         A decorator to indicate a function is the aside view for the given view_name.
 
@@ -297,11 +305,11 @@ def student_aside(self, block, context=None):
 
         """
         # pylint: disable=protected-access
-        def _decorator(func: XBlockAsideView) -> XBlockAsideView:
+        def _decorator(func: AsideView) -> AsideView:
             if not hasattr(func, '_aside_for'):
-                func._aside_for = []
+                func._aside_for = []  # type: ignore
 
-            func._aside_for.append(view_name)  # pylint: disable=protected-access
+            func._aside_for.append(view_name)  # type: ignore
             return func
         return _decorator
 
@@ -321,14 +329,14 @@ def _combined_asides(cls) -> dict[str, str | None]:  # pylint: disable=no-self-a
         """
         # The method declares what views it decorates. We rely on `dir`
         # to handle subclasses and overrides.
-        combined_asides = defaultdict(None)
+        combined_asides: dict[str, str | None] = defaultdict(None)
         for _view_name, view_func in inspect.getmembers(cls, lambda attr: hasattr(attr, '_aside_for')):
             aside_for = getattr(view_func, '_aside_for', [])
             for view in aside_for:
                 combined_asides[view] = view_func.__name__
         return combined_asides
 
-    def aside_view_declaration(self, view_name: str) -> XBlockAsideView | None:
+    def aside_view_declaration(self, view_name: str) -> AsideView | None:
         """
         Find and return a function object if one is an aside_view for the given view_name
 

xblock/fields.py

@@ -57,14 +57,7 @@ class Sentinel:
     """
     Class for implementing sentinel objects (only equal to themselves).
     """
-    name: str = ""  # Name is 'optional' for ease of making sub-class instances.
-
-    def __post_init__(self):
-        """
-        Ensure `self.name` is set, either by constructor or subclass.
-        """
-        if not self.name:
-            raise ValueError("Coding error: sentinel must have a name!")
+    name: str
 
     def __repr__(self) -> str:
         return self.name
@@ -120,6 +113,11 @@ def scopes(cls) -> list[BlockScope]:
         """
         return list(cls)
 
+    @property
+    def attr_name(self) -> str:
+        return self.name.lower().replace('.', '_')
+
+
 
 class UserScope(Enum):
     """
@@ -154,6 +152,10 @@ def scopes(cls) -> list[UserScope]:
         """
         return list(cls)
 
+    @property
+    def attr_name(self) -> str:
+        return self.name.lower().replace('.', '_')
+
 
 class ScopeBase(t.NamedTuple):
     user: UserScope
@@ -224,9 +226,12 @@ def scopes(cls) -> list[ScopeBase]:
             if cls(user, block) not in named_scopes
         ]
 
-    def __new__(cls, user: UserScope, block, name: str | None = None) -> Scope:
+    def __new__(cls, user: UserScope, block, name: str | None = None) -> Scope:  # type: ignore
         """Create a new Scope, with an optional name."""
-        return cls(user, block, name or f'{user}_{block}')
+        # TODO: This is a pretty wacky way to set a default value for `name`.
+        #       We should try to refactor this so that Scope is just, like,
+        #       a dataclass with a __post_init__ hook that sets the default `name`.
+        return ScopeBase.__new__(cls, user, block, name or f'{user}_{block}')
 
     children: t.ClassVar = Sentinel('Scope.children')
     parent: t.ClassVar = Sentinel('Scope.parent')
@@ -262,7 +267,7 @@ class Unset(Sentinel):
     """
     Indicates that default value has not been provided.
     """
-    name = "fields.UNSET"
+    name: str = "fields.UNSET"
 
 
 @dataclass(frozen=True)
@@ -272,7 +277,7 @@ class UniqueId(Sentinel):
     definition to signal that the field should default to a unique string value
     calculated at runtime.
     """
-    name = "fields.UNIQUE_ID"
+    name: str = "fields.UNIQUE_ID"
 
 
 @dataclass(frozen=True)
@@ -281,7 +286,7 @@ class NoCacheValue(Sentinel):
     Placeholder ('nil') value to indicate when nothing has been stored
     in the cache ("None" may be a valid value in the cache, so we cannot use it).
     """
-    name = "fields.NO_CACHE_VALUE"
+    name: str = "fields.NO_CACHE_VALUE"
 
 
 @dataclass(frozen=True)
@@ -290,7 +295,7 @@ class ExplicitlySet(Sentinel):
     Placeholder value that indicates that a value is explicitly dirty,
     because it was explicitly set.
     """
-    name = "fields.EXPLICITLY_SET"
+    name: str = "fields.EXPLICITLY_SET"
 
 
 # For backwards API compatibility, define an instance of each Field-related sentinel.

xblock/mixins.py

@@ -2,6 +2,8 @@
 This module defines all of the Mixins that provide components of XBlock-family
 functionality, such as ScopeStorage, RuntimeServices, and Handlers.
 """
+from __future__ import annotations
+
 from collections import OrderedDict
 import copy
 import functools
@@ -14,7 +16,8 @@
 from webob import Response
 
 from xblock.exceptions import JsonHandlerError, KeyValueMultiSaveError, XBlockSaveError, FieldDataDeprecationWarning
-from xblock.fields import Field, Reference, Scope, ReferenceList
+from xblock.fields import Field, Reference, Scope, ScopeIds, ReferenceList
+from xblock.field_data import FieldData
 from xblock.internal import class_lazy, NamedAttributesMetaclass
 
 
@@ -188,7 +191,7 @@ def fields(cls):  # pylint: disable=no-self-argument
 
         return fields
 
-    def __init__(self, scope_ids, field_data=None, **kwargs):
+    def __init__(self, scope_ids: ScopeIds, field_data: FieldData | None = None, **kwargs):
         """
         Arguments:
             field_data (:class:`.FieldData`): Interface used by the XBlock