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

[organizeDeclarations] A property with didSet is treated as computed #1947

Open
anivaros opened this issue Dec 12, 2024 · 3 comments
Open

[organizeDeclarations] A property with didSet is treated as computed #1947

anivaros opened this issue Dec 12, 2024 · 3 comments
Labels
enhancement fixed in develop bug/feature resolved in the develop branch

Comments

@anivaros
Copy link

anivaros commented Dec 12, 2024

Properties with didSet are placed in the Computed Properties section.

// MARK: - Computed Properties

var foo: Bar = .baz {
  didSet {
    // Do some work
  }
}

But they must be in the section of regular properties.

@calda
Copy link
Collaborator

calda commented Dec 12, 2024

The category grouping here is "instance properties with a body", which includes computed properties and stored properties with didSet / willSet blocks. The problem here is that the mark comment text doesn't accurately describe the section:

case .instancePropertyWithBody:
    return "Computed Properties"

@oiuhr, any thoughts on what we should do here? Some options I can think of:

  1. Change the mark comment of instancePropertyWithBody from // MARK: - Computed properties to // MARK: - Properties with body
  2. Create a new computedInstanceProperty declaration type and remove instancePropertyWithBody from the default list of --organizationmode type. In that case these properties with didSet / willSet blocks would fall back to being part of the instanceProperty group.

@oiuhr
Copy link
Contributor

oiuhr commented Dec 14, 2024

either way looks fine to me 👍 i guess adding a new categories (reorganizing existing ones) is a thing tbd anyways

as a workaround op could also just provide a custom mark title for 'computedInstanceProperty'. option is poorly documented but it’s certainly working, maybe we should address this problem here as well

@calda
Copy link
Collaborator

calda commented Dec 23, 2024

Here's a fix: #1952

@calda calda added the fixed in develop bug/feature resolved in the develop branch label Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement fixed in develop bug/feature resolved in the develop branch
Projects
None yet
Development

No branches or pull requests

3 participants