Add `ignore_codingkeys` parameter to nesting by braker1nine · Pull Request #5650 · realm/SwiftLint

braker1nine · 2024-07-02T00:01:44Z

Resolves #5641

This very explicitly targets enum's named CodingKeys which conform to CodingKey. To provide an exception for Codable users who have to provide custom keys.

OK ✅

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

Not OK ❌

struct Outer {
    struct Inner {
        let identifier: Int
        enum SomeKeys: String, CodingKey {
            case identifier = "id"
        }
    }
}

SwiftLintBot · 2024-07-02T00:24:58Z

	18 Messages
📖	Building this branch resulted in a binary size of 25246.57 KiB vs 25245.04 KiB when built on `main` (0% larger).
📖	Linting Aerial with this PR took 1.05 s vs 1.05 s on `main` (0% slower).
📖	Linting Alamofire with this PR took 1.41 s vs 1.41 s on `main` (0% slower).
📖	Linting Brave with this PR took 9.75 s vs 9.75 s on `main` (0% slower).
📖	Linting DuckDuckGo with this PR took 25.82 s vs 25.85 s on `main` (0% faster).
📖	Linting Firefox with this PR took 13.67 s vs 13.69 s on `main` (0% faster).
📖	Linting Kickstarter with this PR took 11.19 s vs 11.07 s on `main` (1% slower).
📖	Linting Moya with this PR took 0.56 s vs 0.56 s on `main` (0% slower).
📖	Linting NetNewsWire with this PR took 3.21 s vs 3.2 s on `main` (0% slower).
📖	Linting Nimble with this PR took 0.85 s vs 0.85 s on `main` (0% slower).
📖	Linting PocketCasts with this PR took 9.22 s vs 9.02 s on `main` (2% slower).
📖	Linting Quick with this PR took 0.49 s vs 0.49 s on `main` (0% slower).
📖	Linting Realm with this PR took 4.86 s vs 4.85 s on `main` (0% slower).
📖	Linting Sourcery with this PR took 2.47 s vs 2.47 s on `main` (0% slower).
📖	Linting Swift with this PR took 5.66 s vs 5.64 s on `main` (0% slower).
📖	Linting VLC with this PR took 1.48 s vs 1.48 s on `main` (0% slower).
📖	Linting Wire with this PR took 22.8 s vs 22.73 s on `main` (0% slower).
📖	Linting WordPress with this PR took 12.89 s vs 12.93 s on `main` (0% faster).

Generated by 🚫 Danger

SimplyDanny

Thank you for the contribution! Looks good in principle.

What about the following example?

struct Outer {
  enum CodingKeys: String, CodingKey {
    case id
    
    struct S {}
  }
}

I think, the rule should trigger on S.

You need to run IntegrationTests locally and fix all linting issues it reports and update the configuration set reference file.

Source/SwiftLintCore/Extensions/SwiftSyntax+SwiftLint.swift

Source/SwiftLintBuiltInRules/Rules/Metrics/NestingRule.swift

Tests/SwiftLintFrameworkTests/NestingRuleTests.swift

Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/NestingConfiguration.swift

CHANGELOG.md

braker1nine · 2024-07-05T01:58:31Z

FYI I'm out of town until next weekend. I'll jump back into this once I'm back

braker1nine · 2025-06-04T20:40:49Z

FYI I'm out of town until next weekend. I'll jump back into this once I'm back

It wasn't a year long vacation but here we are a year later 😓

What about the following example?

struct Outer {
  enum CodingKeys: String, CodingKey {
    case id
    
    struct S {}
  }
}

I think, the rule should trigger on S.

I've updated this so that this will now trigger the rule.

You need to run IntegrationTests locally and fix all linting issues it reports and update the configuration set reference file.

I think I've got this done? I moved the EnumDeclSyntax extension into its own file because the SwiftSyntax+SwiftLint file was too long and failing linting.

I don't fully understand what is failing now?

SimplyDanny

Glad you are back! 🥳 I have a few more comments ...

CHANGELOG.md

Source/SwiftLintCore/Extensions/SwiftSyntax+SwiftLint.swift

Source/SwiftLintBuiltInRules/Rules/Metrics/NestingRule.swift

SimplyDanny · 2025-06-30T20:03:34Z

Tests/BuiltInRulesTests/NestingRuleTests.swift

        verifyRule(description, ruleConfiguration: ["ignore_typealiases_and_associatedtypes": true])
    }
+
+    func testNestingWithoutCodingKeys() {


You can move these examples directly into NestingRuleExamples. Any options deviating from the default will be added to the online documentation, so it's clear why an example triggers or why it doesn't.

I might need just a little bit more clarification what you mean by this? I was thinking because these are not using the default configuration they shouldn't be in the default examples.

But you're saying they should?

In the documentation that includes these examples later, we now also show the configuration in case it deviates from the default one. So having them as normal examples is even nicer as people also see how options influence the rule's behavior.

Ahhh okay I didn't realize that Example has a configuration option. That's why I was confused!

Okay I updated these and I removed the tests. Which I'm not 100% sure about... but I thought the testing covered all the default examples. Is that wrong?

The feature that configurations are added to the examples in the documentation is quite new and we won't convert existing tests. But we should prefer this way of documenting examples from now on.

Tests/BuiltInRulesTests/NestingRuleTests.swift

SimplyDanny · 2025-06-30T20:08:00Z

For the build, please rebase onto the current main. The results are outdated and there is conflict as well.

This only returns true when the name of the enum is CodingKeys and it conforms to CodingKey

This prevents nested CodingKeys (which may be necessary for functioning Codable conformance) from triggering nesting violations

Essentially this changes the logic so that coding keys do still COUNT towards the depth. But they don't cause violations themselves

…yntax)

CHANGELOG.md

SimplyDanny

Looks good now. Thanks for the updates, @braker1nine!

SimplyDanny requested changes Jul 2, 2024

View reviewed changes

SimplyDanny reviewed Jul 2, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

braker1nine force-pushed the ignore_coding_key branch from 0be58c7 to 2f71da4 Compare January 20, 2025 02:05

braker1nine force-pushed the ignore_coding_key branch from 2f71da4 to 0e488b2 Compare June 3, 2025 18:13

SimplyDanny requested changes Jun 30, 2025

View reviewed changes

braker1nine force-pushed the ignore_coding_key branch from a5e0ba1 to 16d7dd7 Compare July 1, 2025 17:51

braker1nine added 12 commits July 8, 2025 09:18

Add an definesCodingKeys Bool getter to EnumDeclSyntax

3684aa1

This only returns true when the name of the enum is CodingKeys and it conforms to CodingKey

Add ignore_coding_keys parameter to nesting

50adec9

This prevents nested CodingKeys (which may be necessary for functioning Codable conformance) from triggering nesting violations

Adjust the CodingKey exception logic

0c21b89

Essentially this changes the logic so that coding keys do still COUNT towards the depth. But they don't cause violations themselves

Add a bit more detail to the readme entry

b0fa2b4

Run rule registration

b4e2738

Fix changelog formatting

231be94

Disable file_length rule in SwiftSyntax extensions

b917236

Add a test for ignore_coding_keys = false

8f35ffe

Isolate the logic for ignoring coding keys to visit(_ node: EnumDeclS…

fe1b4b6

…yntax)

fixup! Disable file_length rule in SwiftSyntax extensions

2c3f7d6

fixup! Add a test for ignore_coding_keys = false

e748089

Move coding key examples into the nesting examples

44ffcd3

braker1nine force-pushed the ignore_coding_key branch from 16d7dd7 to 44ffcd3 Compare July 8, 2025 14:17

braker1nine added 2 commits July 8, 2025 10:28

Remove trailing commas

62e19b4

Remove vscode folder

ccaea19

SimplyDanny reviewed Jul 9, 2025

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Remove copy

e32f0e0

SimplyDanny approved these changes Jul 9, 2025

View reviewed changes

SimplyDanny enabled auto-merge (squash) July 9, 2025 18:43

SimplyDanny merged commit c97cf24 into realm:main Jul 9, 2025
20 checks passed

Conversation

braker1nine commented Jul 2, 2024

Uh oh!

SwiftLintBot commented Jul 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SimplyDanny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

braker1nine commented Jul 5, 2024

Uh oh!

braker1nine commented Jun 4, 2025

Uh oh!

SimplyDanny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SimplyDanny Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

braker1nine Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

SimplyDanny Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

braker1nine Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

SimplyDanny Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SimplyDanny commented Jun 30, 2025

Uh oh!

Uh oh!

SimplyDanny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SwiftLintBot commented Jul 2, 2024 •

edited

Loading