Skip to content

fix: pass mypy --strict in the small packages and read "false" as False in text - #785

Open
Max Azatian (HardMax71) wants to merge 1 commit into
microsoft:mainfrom
HardMax71:fix/strict-typing-small-packages
Open

Max Azatian (HardMax71) wants to merge 1 commit into
microsoft:mainfrom
HardMax71:fix/strict-typing-small-packages

Conversation

@HardMax71

Copy link
Copy Markdown
Contributor

Overview

Second item of the checklist in #722. It brings kiota_authentication_azure, kiota_bundle and the json, form, text and multipart serialization packages to zero errors under mypy --strict, and sets strict = true in their [tool.mypy] sections, so the existing CI mypy step keeps them there.

Most of it is missing return annotations and bare dicts. Bundle's two errors were calls to the untyped __new__ of ParseNodeFactoryRegistry and SerializationWriterFactoryRegistry, so those two get annotated in kiota_abstractions too.

Typing TextParseNode turned up a real bug. get_bool_value returned bool(self._text), so any non-empty text came back True, including a text/plain body of false, which is what TextSerializationWriter writes for False. It now compares the stripped, lowercased text to "true", the way FormParseNode does.

Related Issue

Part of #722, which stays open for the remaining items.

Notes

Nothing here should break callers. The azure options parameter goes from dict to dict[str, Any], which **options already required at runtime, and the registry constructors still accept and ignore any arguments.

The old test_get_bool_value passed the bool False into the text: str parameter, which is why the is False checks were there. The test now uses strings. TextParseNode("") also stops leaving _text unset, so its getters return None instead of raising AttributeError. The factory rejects empty content before that point anyway.

In JsonParseNode.get_collection_of_primitive_values, the inner func is annotated -> Any. It dispatches through a dict of converters, and mypy can't tie each converter back to T, so this only spells out the type it already had.

Typing the registry constructors also clears the five untyped calls in api_client_builder and two in httpx_request_adapter. With mypy 2.4.0, mypy --strict across the eight packages goes from 203 to 179 errors.

Testing Instructions

  • mypy <package> passes from each of the six package folders, and it's a strict run now through the config. It passes with --python-version 3.10 and --python-version 3.14 too.
  • mypy kiota_abstractions from packages/abstractions and mypy validation from tests/validation still pass.
  • yapf -dr, pylint --disable=W and pytest pass for the six packages and abstractions.
  • Without the text_parse_node.py change, pytest tests/unit/test_text_parse_node.py in packages/serialization/text fails on test_get_bool_value[false-False].

…se in text

Fill the remaining annotation gaps in kiota_authentication_azure, kiota_bundle and the json, form, text and multipart serialization packages, and set strict = true in their [tool.mypy] sections so CI keeps them at zero. Bundle's two errors were calls to the untyped __new__ of ParseNodeFactoryRegistry and SerializationWriterFactoryRegistry, so those get annotated in kiota_abstractions too.

TextParseNode.get_bool_value returned bool(text), so any non-empty text read as True, including a text/plain body of "false". It now compares the stripped, lowercased text to "true", like the form parse node does. Part of microsoft#722.
@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant