Skip to content

Enable overridden_super_call SwiftLint rule and address violations#20935

Merged
mokagio merged 2 commits intotrunkfrom
mokagio/force-super-override
Jun 25, 2023
Merged

Enable overridden_super_call SwiftLint rule and address violations#20935
mokagio merged 2 commits intotrunkfrom
mokagio/force-super-override

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Jun 22, 2023

Recently, we discovered some issues possibly due to our code calling the wrong super life cycle method, e.g.

This PR enable as SwiftLint rule that ensures various overrides of UIKit methods call their respective superclass method. As an added bonus, the rule will pick up any inconsistent super call:

image

It's worth mentioning that calling super in those subclasses could be superfluous. At the same time, there's no guarantee UIKit will start doing something in those internal methods. Plus, enforcing calling super ensures that, were we to add a custom subclass in between the UIKit one, our custom code will be called. Finally, this ensures consistency across the codebase, which is almost always desirable.

mokagio added 2 commits June 22, 2023 20:13
@mokagio mokagio added the Tooling Build, Release, and Validation Tools label Jun 22, 2023
@mokagio mokagio added this to the 22.7 milestone Jun 22, 2023
- overridden_super_call

overridden_super_call:
severity: error
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After seeing what the feedback is on this PR, I might move the definition upstream in the shared config.

- Starscream (3.0.6)
- SVProgressHUD (2.2.5)
- SwiftLint (0.50.3)
- SwiftLint (0.52.2)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a "just in case" update

@wpmobilebot
Copy link
Contributor

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr20935-01053b9
Version22.6
Bundle IDcom.jetpack.alpha
Commit01053b9
App Center Buildjetpack-installable-builds #5103
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr20935-01053b9
Version22.6
Bundle IDorg.wordpress.alpha
Commit01053b9
App Center BuildWPiOS - One-Offs #6077
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@mokagio mokagio enabled auto-merge June 22, 2023 11:56
@mokagio mokagio modified the milestones: 22.7, 22.8 Jun 25, 2023
Copy link
Contributor

@crazytonyli crazytonyli left a comment

Choose a reason for hiding this comment

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

:shipit:

@mokagio mokagio merged commit 720d416 into trunk Jun 25, 2023
@mokagio mokagio deleted the mokagio/force-super-override branch June 25, 2023 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Tooling Build, Release, and Validation Tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants