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

Rule Request: Ignore CodingKey enums in Nesting #5641

Open
2 tasks done
braker1nine opened this issue Jun 24, 2024 · 2 comments · May be fixed by #5650
Open
2 tasks done

Rule Request: Ignore CodingKey enums in Nesting #5641

braker1nine opened this issue Jun 24, 2024 · 2 comments · May be fixed by #5650

Comments

@braker1nine
Copy link
Contributor

New Issue Checklist

New rule request

This is more of an edit to an existing rule than a new rule, so apologies if this is the wrong format... Add a boolean option to the Nesting rule. When this option is enabled, enum's conforming to CodingKey won't count as violations of nesting.

My main argument for this is that CodingKey's are less of a nested "type" and more of an annotation of the current type for Codable If you have types that have custom CodingKey types, you basically lose a

Triggering

struct Wrapper: Codable {
    struct Inner: Codable {
        struct WayInner: Codable {
        }
    }
}
struct Wrapper: Codable {
    enum CodingKeys: String, CodingKey {
        case id = "identifier"

        struct TechnicallyOkayButBad {
            struct Violation {}
         }
    }
}

Non Triggering

struct Wrapper: Codable {
    struct Inner: Codable {
        let id: Int
        
        enum CodingKeys: String, CodingKey {
            case id = "identifier
        }
    }
}

I think I would propose this be an opt-in setting on the nesting rule.

Looking at the criteria in the README

A rule that can have many false positives

I think this change would actually reduce false positives

A rule that is too slow

I don't think this would be an issue

A rule that is not general consensus or is only useful in some cases

I'm not sure if there's consensus here. And it's a very specific case...

@braker1nine
Copy link
Contributor Author

I've already spent some time on a rough version of implementation. So if this is something that feels worth adding, I'm happy to work in a PR for it

@SimplyDanny
Copy link
Collaborator

SimplyDanny commented Jun 29, 2024

Sounds like a reasonable exclusion. However, I recommend adding this only as an option to the rule. So the default behavior of the rule doesn't change.

If you have worked on a PR already, please go ahead and propose it. I'll be glad to have a look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants