Fix invalid type annotations in generated SeleniumLibrary stub - #1997
Draft
simonyang08 wants to merge 1 commit into
Draft
Fix invalid type annotations in generated SeleniumLibrary stub#1997simonyang08 wants to merge 1 commit into
simonyang08 wants to merge 1 commit into
Conversation
The generator emitted invalid PEP 484 annotations in src/SeleniumLibrary/__init__.pyi: - Optional[Optional] (and Optional[Union[...]]) when an argument type already accepted None. get_type_string_from_type fell back to argument_type.__name__, which is 'Union' for typing.Union, typing.Optional and PEP 604 union types. keyword_line then wrapped that with Optional[...] whenever the default was None. - Bare 'Union' for any union-typed argument (e.g. locators), because every union form exposes __name__ == 'Union'. - Incompatible defaults such as 'modifier: Union = False' because the bare-Union type was paired with the runtime default. Rewrite get_type_string_from_type to walk typing.get_origin / get_args so typing.Union, typing.Optional and PEP 604 types.UnionType all expand to a valid Union[...] form, and teach keyword_line to skip the Optional[...] wrap when the rendered type already accepts None. Follow-up: the more accurate Union[...] rendering exposes two extra symbols (Secret in input_password/input_text, FirefoxProfile in open_browser) that the previous bare-Union stub never referenced. Add matching imports to the pyi boilerplate so the generated stub stays self-contained. Also fix the pre-existing find_elements(parent: WebElement = None) row in the boilerplate. Regenerated src/SeleniumLibrary/__init__.pyi with 'python3 gen_stub.py'. Validation: mypy --ignore-missing-imports on the regenerated stub in an isolated directory goes from 3 errors (3x [name-defined] for Secret / FirefoxProfile) on the previous patch to 0 errors. Full utest run remains green (258 passed, 2 skipped, unchanged). Closes robotframework#1919 (annotation validity portion; py.typed and stub deletion are separate decisions left to the maintainers). Signed-off-by: simonyang08 <ppt5928@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1919
Root cause
gen_stub.pyrendered everytyping.Union[...],typing.Optional[...], and PEP 604X | Yannotation as a bareUnion, because all three expose__name__ == "Union"and the generator trustedhasattr(t, "__name__").keyword_linealso wrappedOptional[...]around everyNone-defaulted parameter even when the type already includedNone. Together this produced the invalid annotations inSeleniumLibrary/__init__.pyi(Optional[Optional], bareUnion,modifier: Union = False,parent: WebElement = None) reported in the issue.Fix
get_type_string_from_typenow discriminates unions properly (typing.Union/Optionalviaget_origin, PEP 604 viatypes.UnionType), filtersNoneTypeout of the members, and recurses so parameters render asUnion[WebElement, str, list],Optional[str], etc.keyword_lineonly wrapsOptional[...]when the rendered type does not already includeNone.SecretandFirefoxProfile, which keyword annotations reference (matching the source imports inbrowsermanagement.py/formelement.py).src/SeleniumLibrary/__init__.pyiregenerated.__init__.pyiis a generated artifact — the hand-written change is confined togen_stub.py.Verification
mypy --ignore-missing-importson the regenerated stub in an isolated directory: 0 errors (the old committed stub reports 64, including 6+ incompatible-default hard errors and 107 bareUnionrenders).Optional[Optional], bareUnion,Union =defaults, or unimported names.utest/run.py: 258 passed, 2 skipped (unchanged from master).Notes for maintainers
drag_and_drop_across_framesandget_css_property_value, which existed in the source but were missing from the stale committed stub — no new keywords were added by this PR.py.typed(as discussed in the issue), and adding-> Nonereturn annotations in the generator (the remaining mypy[no-untyped-def]reports).DCO:
Signed-off-by: simonyang08 <ppt5928@gmail.com>.