Skip to content

feat: declare the attribute contracts the cancel and test mixins rely on - #68

Merged
cigamit merged 1 commit into
ctrliq:mainfrom
blaipr:feat/type-the-cancel-and-test-mixins
Sep 13, 2026
Merged

cigamit merged 1 commit into
ctrliq:mainfrom
blaipr:feat/type-the-cancel-and-test-mixins

Conversation

@blaipr

@blaipr blaipr commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

HasCancel and HasTest read self.page and self.resource, which the CustomAction they are mixed into supplies. AssociationMixin, a few classes down the same file, has carried the annotations describing exactly that since #50 and #51. These two arrived in #62 and #65 without them, so the type checker reported seven unresolved attributes for a contract the file already knew how to state.

    # Supplied by the CustomAction this is mixed into. Annotations rather than
    # assignments: they describe the contract without creating class attributes
    # that would shadow the real ones.
    page: 'api.pages.Page'
    resource: str

Annotations rather than assignments, for the reason the comment on AssociationMixin already gives: an assignment would create a class attribute that shadows the real one at runtime.

Verification

  • ty goes from 35 diagnostics to 28, which is the seven these two classes produced and nothing else. The one finding left in this file, the Launchable.add_arguments override on line 260, predates feat: add the cancel action to the CLI #62 and feat: add the test action to the CLI #65 and is not touched here.
  • Neither page nor resource appears in vars() on either class, and both appear in __annotations__. That is the property the comment claims, checked rather than assumed.
  • The action registry still resolves all six cancel entries and all three test entries.
  • ruff format --check, ruff check and the unit suite (366 passing) are unchanged.

Scope

This is the uncontroversial part of the follow-up #42 set up. The remaining 28 diagnostics are genuine annotation gaps in older code, each needing a small decision of its own: a TypedDict for the heterogeneous inventory structure in ascender/inventory.py, the self.__class__(...) constructor calls in pages/page.py, and the override signatures in pages/notifications.py and cli/custom.py:260. None of them is a runtime bug, checked individually. They are not in this pull request.

HasCancel and HasTest read self.page and self.resource, which the CustomAction
they are mixed into supplies. AssociationMixin in the same file has carried the
annotations describing that since ctrliq#50, and these two arrived in ctrliq#62 and ctrliq#65
without them, so the type checker reported seven unresolved attributes for a
contract the file already knew how to state.

Annotations rather than assignments, for the reason the comment on
AssociationMixin gives: an assignment would create a class attribute that
shadows the real one at runtime. Verified that neither name appears in vars() on
either class, and that both land in __annotations__.

ty goes from 35 diagnostics to 28. The one remaining in this file, the
Launchable.add_arguments override on line 260, predates all of this.
@ciq-it-service-account

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@cigamit cigamit self-assigned this Sep 13, 2026
@cigamit cigamit added the enhancement New feature or request label Sep 13, 2026
@cigamit
cigamit merged commit c753a6a into ctrliq:main Sep 13, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Development

Successfully merging this pull request may close these issues.

3 participants