realm / SwiftLint

A tool to enforce Swift style and conventions.
https://realm.github.io/SwiftLint
MIT License
18.52k stars 2.2k forks source link

Unneeded Synthesized Initializer removes needed initializers with property wrappers #5153

Open NachoSoto opened 12 months ago

NachoSoto commented 12 months ago

New Issue Checklist

Describe the bug

This new rule correct actually needed initializers. Example:

private struct Data {
    @PropertyWrapper<E> var e: E

    init(e: E) {
        self.e = e
    }
}

SwiftLint removes that constructor, but it is required to be able to initialize it with a value of type E instead of @PropertyWrapper<E>

Environment

aclima93 commented 2 months ago

To add to this, it also removes initializers that just apply property wrappers to the arguments (it does not seem to trigger the rule in the linting digest?)

struct Title {
        let text: String
        let color: Color
        let font: Font

        init(
            @Localised text: String, // 👈 some property wrapper that alters the input
            color: Color,
            font: Font
        ) {
            self.text = text
            self.color = color
            self.font = font
        }
    }

weirder still, it will produce a "Superfluous Disable Command Violation" warning if disabled

        // swiftlint:disable:next unneeded_synthesized_initializer // 👈 stops the removal but produces a superfluous warning
        init(
            @Localised text: String,

swiftlint version: 0.55.1

SimplyDanny commented 2 months ago

To add to this, it also removes initializers that just apply property wrappers to the arguments (it does not seem to trigger the rule in the linting digest?)

struct Title {
        let text: String
        let color: Color
        let font: Font

        init(
            @Localised text: String, // 👈 some property wrapper that alters the input
            color: Color,
            font: Font
        ) {
            self.text = text
            self.color = color
            self.font = font
        }
    }

This is addressed by #5594.

weirder still, it will produce a "Superfluous Disable Command Violation" warning if disabled

        // swiftlint:disable:next unneeded_synthesized_initializer // 👈 stops the removal but produces a superfluous warning
        init(
            @Localised text: String,

I'm unable to reproduce this. The disable command works as expected in my little test setup.

SimplyDanny commented 2 months ago

@NachoSoto: Taking a look at your initially reported example, I wonder why e would be initialized as @PropertyWrapper<E> without the explicit initializer. In a little example, the compiler seems to be happy without the initializer. Could you provide a complete example that shows the issue?

SimplyDanny commented 3 weeks ago

@NachoSoto, @aclima93: Are these still issues for you?

aclima93 commented 3 weeks ago

... it also removes initializers that just apply property wrappers to the arguments

This is still happening,

struct Title {
    let text: String

    init(@Localised text: String) {
        self.text = text
    }
}

gets re-written as

struct Title {
    let text: String
}

unless I disable it with unneeded_synthesized_initializer

    // swiftlint:disable:next unneeded_synthesized_initializer
    init(@Localised text: String) {

weirder still, it will produce a "Superfluous Disable Command Violation" warning if disabled

I can no longer reproduce this part.

installed via homebrew and executed as /opt/homebrew/bin/swiftlint --fix && /opt/homebrew/bin/swiftlint

image
SimplyDanny commented 3 weeks ago

... it also removes initializers that just apply property wrappers to the arguments

This is still happening,

struct Title {
    let text: String

    init(@Localised text: String) {
        self.text = text
    }
}

gets re-written as

struct Title {
    let text: String
}

unless I disable it with unneeded_synthesized_initializer

    // swiftlint:disable:next unneeded_synthesized_initializer
    init(@Localised text: String) {

Yeah, this got fixed after the 0.55.1 release only.