Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Development: Add missing type hints #221

Merged
merged 11 commits into from
May 31, 2022

Conversation

DetachHead
Copy link
Contributor

@DetachHead DetachHead commented May 8, 2022

mypy was treating these as Any

@DetachHead DetachHead changed the title fix cli.action decorator removing the function type add/fix some types May 9, 2022
@priitlatt priitlatt self-requested a review May 11, 2022 12:37
Copy link
Contributor

@priitlatt priitlatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pulled latest changes from current tip of master into your fork and checked types with mypy. It reveals quite a few errors with the proposed changes. See the full mypy log below.

I'm afraid it needs some more work as of now.

Static code analysios results

Output of

MYPYPATH=stubs pipenv run mypy src/codemagic --pretty --show-error-context

is as follows:

src/codemagic/apple/resources/pre_release_version.py: note: In class "PreReleaseVersion":
src/codemagic/apple/resources/pre_release_version.py:16: error: Name "Optional"
is not defined
        relationships: Optional[Relationships] = None
                       ^
src/codemagic/apple/resources/pre_release_version.py:16: note: Did you forget to import it from "typing"? (Suggestion: "from typing import Optional")
src/codemagic/apple/resources/bundle_id_capability.py: note: In class "BundleIdCapability":
src/codemagic/apple/resources/bundle_id_capability.py:53: error: Incompatible
types in assignment (expression has type
"Optional[codemagic.apple.resources.bundle_id_capability.BundleIdCapability.Relationships]",
base class "Resource" defined the type as
"codemagic.apple.resources.resource.Resource.Relationships")
        relationships: Optional[Relationships] = None
                                                 ^
src/codemagic/apple/resources/bundle_id.py: note: In class "BundleId":
src/codemagic/apple/resources/bundle_id.py:16: error: Name "Optional" is not
defined
        relationships: Optional[Relationships] = None
                       ^
src/codemagic/apple/resources/bundle_id.py:16: note: Did you forget to import it from "typing"? (Suggestion: "from typing import Optional")
src/codemagic/apple/resources/build.py: note: In class "Build":
src/codemagic/apple/resources/build.py:30: error: Incompatible types in
assignment (expression has type
"Optional[codemagic.apple.resources.build.Build.Relationships]", base class
"Resource" defined the type as
"codemagic.apple.resources.resource.Resource.Relationships")
        relationships: Optional[Relationships] = None
                                                 ^
src/codemagic/apple/resources/beta_group.py: note: In class "BetaGroup":
src/codemagic/apple/resources/beta_group.py:17: error: Incompatible types in
assignment (expression has type
"Optional[codemagic.apple.resources.beta_group.BetaGroup.Relationships]", base
class "Resource" defined the type as
"codemagic.apple.resources.resource.Resource.Relationships")
        relationships: Optional[Relationships] = None
                                                 ^
src/codemagic/apple/resources/beta_build_localization.py: note: In class "BetaBuildLocalization":
src/codemagic/apple/resources/beta_build_localization.py:17: error:
Incompatible types in assignment (expression has type
"Optional[codemagic.apple.resources.beta_build_localization.BetaBuildLocalization.Relationships]",
base class "Resource" defined the type as
"codemagic.apple.resources.resource.Resource.Relationships")
        relationships: Optional[Relationships] = None
                                                 ^
src/codemagic/apple/resources/beta_app_review_submission.py: note: In class "BetaAppReviewSubmission":
src/codemagic/apple/resources/beta_app_review_submission.py:18: error:
Incompatible types in assignment (expression has type
"Optional[codemagic.apple.resources.beta_app_review_submission.BetaAppReviewSubmission.Relationships]",
base class "Resource" defined the type as
"codemagic.apple.resources.resource.Resource.Relationships")
        relationships: Optional[Relationships] = None
                                                 ^
src/codemagic/apple/resources/beta_app_review_detail.py: note: In class "BetaAppReviewDetail":
src/codemagic/apple/resources/beta_app_review_detail.py:16: error: Incompatible
types in assignment (expression has type
"Optional[codemagic.apple.resources.beta_app_review_detail.BetaAppReviewDetail.Relationships]",
base class "Resource" defined the type as
"codemagic.apple.resources.resource.Resource.Relationships")
        relationships: Optional[Relationships] = None
                                                 ^
src/codemagic/apple/resources/beta_app_localization.py: note: In class "BetaAppLocalization":
src/codemagic/apple/resources/beta_app_localization.py:17: error: Incompatible
types in assignment (expression has type
"Optional[codemagic.apple.resources.beta_app_localization.BetaAppLocalization.Relationships]",
base class "Resource" defined the type as
"codemagic.apple.resources.resource.Resource.Relationships")
        relationships: Optional[Relationships] = None
                                                 ^
src/codemagic/apple/resources/app_store_version_localization.py: note: In class "AppStoreVersionLocalization":
src/codemagic/apple/resources/app_store_version_localization.py:17: error:
Incompatible types in assignment (expression has type
"Optional[codemagic.apple.resources.app_store_version_localization.AppStoreVersionLocalization.Relationships]",
base class "Resource" defined the type as
"codemagic.apple.resources.resource.Resource.Relationships")
        relationships: Optional[Relationships] = None
                                                 ^
src/codemagic/apple/resources/app_store_version.py: note: In class "AppStoreVersion":
src/codemagic/apple/resources/app_store_version.py:20: error: Incompatible
types in assignment (expression has type
"Optional[codemagic.apple.resources.app_store_version.AppStoreVersion.Relationships]",
base class "Resource" defined the type as
"codemagic.apple.resources.resource.Resource.Relationships")
        relationships: Optional[Relationships] = None
                                                 ^
src/codemagic/apple/resources/app.py: note: In class "App":
src/codemagic/apple/resources/app.py:18: error: Incompatible types in
assignment (expression has type
"Optional[codemagic.apple.resources.app.App.Relationships]", base class
"Resource" defined the type as
"codemagic.apple.resources.resource.Resource.Relationships")
        relationships: Optional[Relationships] = None
                                                 ^
src/codemagic/apple/resources/signing_certificate.py: note: In class "SigningCertificate":
src/codemagic/apple/resources/signing_certificate.py:23: error: Incompatible
types in assignment (expression has type
"Optional[codemagic.apple.resources.signing_certificate.SigningCertificate.Relationships]",
base class "Resource" defined the type as
"codemagic.apple.resources.resource.Resource.Relationships")
        relationships: Optional[Relationships] = None
                                                 ^
src/codemagic/apple/resources/profile.py: note: In class "Profile":
src/codemagic/apple/resources/profile.py:22: error: Incompatible types in
assignment (expression has type
"Optional[codemagic.apple.resources.profile.Profile.Relationships]", base class
"Resource" defined the type as
"codemagic.apple.resources.resource.Resource.Relationships")
        relationships: Optional[Relationships] = None
                                                 ^
src/codemagic/apple/app_store_connect/versioning/app_store_versions.py: note: In member "read_build" of class "AppStoreVersions":
src/codemagic/apple/app_store_connect/versioning/app_store_versions.py:104: error:
Item "None" of "Optional[Relationships]" has no attribute "build"
                url = app_store_version.relationships.build.links.related
                      ^
src/codemagic/apple/app_store_connect/versioning/app_store_versions.py:107: error:
Argument 1 to "get" of "Session" has incompatible type "Union[str, None, Any]";
expected "Union[str, bytes]"
            response = self.client.session.get(url).json()
                                               ^
src/codemagic/apple/app_store_connect/versioning/app_store_versions.py: note: In member "read_app_store_version_submission" of class "AppStoreVersions":
src/codemagic/apple/app_store_connect/versioning/app_store_versions.py:121: error:
Item "None" of "Optional[Relationships]" has no attribute
"appStoreVersionSubmission"
                url = app_store_version.relationships.appStoreVersionSubmi...
                      ^
src/codemagic/apple/app_store_connect/versioning/app_store_versions.py:124: error:
Argument 1 to "get" of "Session" has incompatible type "Union[str, None, Any]";
expected "Union[str, bytes]"
            response = self.client.session.get(url).json()
                                               ^
src/codemagic/apple/app_store_connect/versioning/app_store_versions.py: note: In member "list_app_store_version_localizations" of class "AppStoreVersions":
src/codemagic/apple/app_store_connect/versioning/app_store_versions.py:135: error:
Item "None" of "Optional[Relationships]" has no attribute
"appStoreVersionLocalizations"
                url = app_store_version.relationships.appStoreVersionLocal...
                      ^
src/codemagic/apple/app_store_connect/provisioning/profiles.py: note: In member "read_bundle_id" of class "Profiles":
src/codemagic/apple/app_store_connect/provisioning/profiles.py:113: error: Item
"None" of "Optional[Relationships]" has no attribute "bundleId"
                url = profile.relationships.bundleId.links.related
                      ^
src/codemagic/apple/app_store_connect/provisioning/profiles.py:116: error:
Argument 1 to "get" of "Session" has incompatible type "Union[str, None, Any]";
expected "Union[str, bytes]"
            response = self.client.session.get(url).json()
                                               ^
src/codemagic/apple/app_store_connect/provisioning/profiles.py: note: In member "get_bundle_id_resource_id" of class "Profiles":
src/codemagic/apple/app_store_connect/provisioning/profiles.py:124: error: Item
"None" of "Optional[Relationships]" has no attribute "bundleId"
                url = profile.relationships.bundleId.links.self
                      ^
src/codemagic/apple/app_store_connect/provisioning/profiles.py:127: error:
Argument 1 to "get" of "Session" has incompatible type "Union[str, None, Any]";
expected "Union[str, bytes]"
            response = self.client.session.get(url).json()
                                               ^
src/codemagic/apple/app_store_connect/provisioning/profiles.py: note: In member "list_certificates" of class "Profiles":
src/codemagic/apple/app_store_connect/provisioning/profiles.py:135: error: Item
"Relationships" of "Optional[Relationships]" has no attribute "profiles"
                url = profile.relationships.profiles.links.related
                      ^
src/codemagic/apple/app_store_connect/provisioning/profiles.py:135: error: Item
"None" of "Optional[Relationships]" has no attribute "profiles"
                url = profile.relationships.profiles.links.related
                      ^
src/codemagic/apple/app_store_connect/provisioning/profiles.py: note: In member "list_certificate_ids" of class "Profiles":
src/codemagic/apple/app_store_connect/provisioning/profiles.py:145: error: Item
"None" of "Optional[Relationships]" has no attribute "certificates"
                url = profile.relationships.certificates.links.self
                      ^
src/codemagic/apple/app_store_connect/provisioning/profiles.py: note: In member "list_devices" of class "Profiles":
src/codemagic/apple/app_store_connect/provisioning/profiles.py:155: error: Item
"Relationships" of "Optional[Relationships]" has no attribute "profiles"
                url = profile.relationships.profiles.links.related
                      ^
src/codemagic/apple/app_store_connect/provisioning/profiles.py:155: error: Item
"None" of "Optional[Relationships]" has no attribute "profiles"
                url = profile.relationships.profiles.links.related
                      ^
src/codemagic/apple/app_store_connect/provisioning/profiles.py: note: In member "list_device_ids" of class "Profiles":
src/codemagic/apple/app_store_connect/provisioning/profiles.py:165: error: Item
"Relationships" of "Optional[Relationships]" has no attribute "profiles"
                url = profile.relationships.profiles.links.self
                      ^
src/codemagic/apple/app_store_connect/provisioning/profiles.py:165: error: Item
"None" of "Optional[Relationships]" has no attribute "profiles"
                url = profile.relationships.profiles.links.self
                      ^
src/codemagic/apple/app_store_connect/builds/builds.py: note: In member "read_app" of class "Builds":
src/codemagic/apple/app_store_connect/builds/builds.py:77: error: Item "None"
of "Optional[Relationships]" has no attribute "app"
                url = build.relationships.app.links.related
                      ^
src/codemagic/apple/app_store_connect/builds/builds.py:80: error: Argument 1 to
"get" of "Session" has incompatible type "Union[str, None, Any]"; expected
"Union[str, bytes]"
            response = self.client.session.get(url).json()
                                               ^
src/codemagic/apple/app_store_connect/builds/builds.py: note: In member "read_app_store_version" of class "Builds":
src/codemagic/apple/app_store_connect/builds/builds.py:88: error: Item "None"
of "Optional[Relationships]" has no attribute "appStoreVersion"
                url = build.relationships.appStoreVersion.links.related
                      ^
src/codemagic/apple/app_store_connect/builds/builds.py:91: error: Argument 1 to
"get" of "Session" has incompatible type "Union[str, None, Any]"; expected
"Union[str, bytes]"
            response = self.client.session.get(url).json()
                                               ^
src/codemagic/apple/app_store_connect/builds/builds.py: note: In member "read_pre_release_version" of class "Builds":
src/codemagic/apple/app_store_connect/builds/builds.py:101: error: Item "None"
of "Optional[Relationships]" has no attribute "preReleaseVersion"
                url = build.relationships.preReleaseVersion.links.related
                      ^
src/codemagic/apple/app_store_connect/builds/builds.py:104: error: Argument 1
to "get" of "Session" has incompatible type "Union[str, None, Any]"; expected
"Union[str, bytes]"
            response = self.client.session.get(url).json()
                                               ^
src/codemagic/apple/app_store_connect/apps/apps.py: note: In member "list_builds" of class "Apps":
src/codemagic/apple/app_store_connect/apps/apps.py:81: error: Item "None" of
"Optional[Relationships]" has no attribute "builds"
                url = app.relationships.builds.links.related
                      ^
src/codemagic/apple/app_store_connect/apps/apps.py: note: In member "list_pre_release_versions" of class "Apps":
src/codemagic/apple/app_store_connect/apps/apps.py:91: error: Item "None" of
"Optional[Relationships]" has no attribute "preReleaseVersions"
                url = app.relationships.preReleaseVersions.links.related
                      ^
src/codemagic/apple/app_store_connect/apps/apps.py: note: In member "list_app_store_versions" of class "Apps":
src/codemagic/apple/app_store_connect/apps/apps.py:106: error: Item "None" of
"Optional[Relationships]" has no attribute "appStoreVersions"
                url = app.relationships.appStoreVersions.links.related
                      ^
src/codemagic/apple/app_store_connect/apps/apps.py: note: In member "list_beta_app_localizations" of class "Apps":
src/codemagic/apple/app_store_connect/apps/apps.py:118: error: Item "None" of
"Optional[Relationships]" has no attribute "betaAppLocalizations"
                url = app.relationships.betaAppLocalizations.links.related
                      ^
src/codemagic/apple/app_store_connect/apps/apps.py:121: error: Argument 1 to
"get" of "Session" has incompatible type "Union[str, None, Any]"; expected
"Union[str, bytes]"
            response = self.client.session.get(url).json()
                                               ^
src/codemagic/apple/app_store_connect/apps/apps.py: note: In member "read_beta_app_review_detail" of class "Apps":
src/codemagic/apple/app_store_connect/apps/apps.py:129: error: Item "None" of
"Optional[Relationships]" has no attribute "betaAppReviewDetail"
                url = app.relationships.betaAppReviewDetail.links.related
                      ^
src/codemagic/apple/app_store_connect/apps/apps.py:132: error: Argument 1 to
"get" of "Session" has incompatible type "Union[str, None, Any]"; expected
"Union[str, bytes]"
            response = self.client.session.get(url).json()
                                               ^
src/codemagic/tools/universal_apk_generator.py: note: In member "generate" of class "UniversalApkGenerator":
src/codemagic/tools/universal_apk_generator.py:116: error: Argument 2 to
"build_universal_apks" of "AndroidAppBundle" has incompatible type
"**Dict[str, object]"; expected "Optional[Path]"
    ...dAppBundle().build_universal_apks(self.pattern, **signing_info_kwargs)
                                                         ^
src/codemagic/tools/universal_apk_generator.py:116: error: Argument 2 to
"build_universal_apks" of "AndroidAppBundle" has incompatible type
"**Dict[str, object]"; expected "Union[str, KeystorePassword, None]"
    ...dAppBundle().build_universal_apks(self.pattern, **signing_info_kwargs)
                                                         ^
src/codemagic/tools/universal_apk_generator.py:116: error: Argument 2 to
"build_universal_apks" of "AndroidAppBundle" has incompatible type
"**Dict[str, object]"; expected "Union[str, KeyAlias, None]"
    ...dAppBundle().build_universal_apks(self.pattern, **signing_info_kwargs)
                                                         ^
src/codemagic/tools/universal_apk_generator.py:116: error: Argument 2 to
"build_universal_apks" of "AndroidAppBundle" has incompatible type
"**Dict[str, object]"; expected "Union[str, KeyPassword, None]"
    ...dAppBundle().build_universal_apks(self.pattern, **signing_info_kwargs)
                                                         ^
src/codemagic/tools/_app_store_connect/action_groups/beta_app_review_submissions_action_group.py: note: In class "BetaAppReviewSubmissionsActionGroup":
src/codemagic/tools/_app_store_connect/action_groups/beta_app_review_submissions_action_group.py:21: error:
Return type "AppStoreVersionSubmission" of "create_beta_app_review_submission"
incompatible with return type "BetaAppReviewSubmission" in supertype
"AbstractBaseAction"
                    BuildArgument.BUILD_ID_RESOURCE_ID,
        ^
Found 48 errors in 20 files (checked 171 source files)

@DetachHead
Copy link
Contributor Author

@priitlatt thanks, will try to fix those now

is there a reason the CI isn't running? i'm having trouble setting up the dependencies locally so i'd like to be able to rely on the CI checks

@DetachHead DetachHead requested a review from priitlatt May 14, 2022 08:08
@priitlatt
Copy link
Contributor

@DetachHead I've made some changes to the test workflow. Please get your fork up to date with the upstream and then you can enable Checks workflow which will be executed on push events.

@DetachHead
Copy link
Contributor Author

@priitlatt done, though it looks like you have to approve the run since i'm a first-time contributor

image

@DetachHead DetachHead force-pushed the master branch 2 times, most recently from 1e8fd59 to 669313b Compare May 26, 2022 09:54
Copy link
Contributor

@priitlatt priitlatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of notes.

  1. Please revise attributes and relationships declarations in src/codemagic/apple/resources. Latest changes from upstream introduced some new resources there which are now lacking those. But there are some older resource definitions such as AppStoreVersionSubmission that do not define them either. I suppose we should persist a uniform design for these.
  2. submit_to_app_store method in src/codemagic/tools/_app_store_connect/abstract_base_action.py has invalid return type.

src/codemagic/cli/cli_app.py Outdated Show resolved Hide resolved
src/codemagic/cli/cli_app.py Outdated Show resolved Hide resolved
src/codemagic/tools/universal_apk_generator.py Outdated Show resolved Hide resolved
@priitlatt priitlatt changed the title add/fix some types Development: Add missing type hints May 26, 2022
@DetachHead DetachHead force-pushed the master branch 4 times, most recently from 70bba5b to 114b461 Compare May 28, 2022 04:46
@DetachHead
Copy link
Contributor Author

  1. Please revise attributes and relationships declarations in src/codemagic/apple/resources. Latest changes from upstream introduced some new resources there which are now lacking those. But there are some older resource definitions such as AppStoreVersionSubmission that do not define them either. I suppose we should persist a uniform design for these.

i've made attributes and relationships abstract properties on Resource, so now mypy will enforce that all subclasses define them

  1. submit_to_app_store method in src/codemagic/tools/_app_store_connect/abstract_base_action.py has invalid return type.

fixed, i guess it was always wrong but the error was hidden since the decorator was removing the types.

@DetachHead DetachHead force-pushed the master branch 4 times, most recently from b60b163 to bfcf82d Compare May 29, 2022 00:31
@DetachHead DetachHead requested a review from priitlatt May 29, 2022 00:34
Copy link
Contributor

@priitlatt priitlatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of minor nitpicks are still remaining, but overall looks very good!

src/codemagic/apple/resources/resource.py Outdated Show resolved Hide resolved
src/codemagic/apple/resources/resource.py Outdated Show resolved Hide resolved
tests/apple/resources/mocks/certificate.json Outdated Show resolved Hide resolved
tests/apple/resources/mocks/device.json Outdated Show resolved Hide resolved
@DetachHead DetachHead force-pushed the master branch 4 times, most recently from ab2643a to b06f426 Compare May 31, 2022 09:50
@DetachHead DetachHead requested a review from priitlatt May 31, 2022 11:03
Copy link
Contributor

@priitlatt priitlatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good now! 🚀 Thank you very much for the contribution and effort.

@priitlatt priitlatt merged commit 5058be6 into codemagic-ci-cd:master May 31, 2022
priitlatt added a commit that referenced this pull request May 31, 2022
@priitlatt
Copy link
Contributor

@DetachHead FYI, this is now released as part of version 0.27.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants