Add `ignore_codingkeys` parameter to nesting #5650

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"
        }
    }
}

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

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

	17 Messages
📖	Linting Aerial with this PR took 0.91s vs 0.93s on main (2% faster)
📖	Linting Alamofire with this PR took 1.22s vs 1.23s on main (0% faster)
📖	Linting Brave with this PR took 7.31s vs 7.3s on main (0% slower)
📖	Linting DuckDuckGo with this PR took 4.09s vs 4.02s on main (1% slower)
📖	Linting Firefox with this PR took 10.36s vs 10.38s on main (0% faster)
📖	Linting Kickstarter with this PR took 9.21s vs 9.2s on main (0% slower)
📖	Linting Moya with this PR took 0.52s vs 0.52s on main (0% slower)
📖	Linting NetNewsWire with this PR took 2.5s vs 2.5s on main (0% slower)
📖	Linting Nimble with this PR took 0.75s vs 0.74s on main (1% slower)
📖	Linting PocketCasts with this PR took 7.77s vs 8.01s on main (2% faster)
📖	Linting Quick with this PR took 0.43s vs 0.42s on main (2% slower)
📖	Linting Realm with this PR took 4.64s vs 4.62s on main (0% slower)
📖	Linting Sourcery with this PR took 2.29s vs 2.31s on main (0% faster)
📖	Linting Swift with this PR took 4.44s vs 4.52s on main (1% faster)
📖	Linting VLC with this PR took 1.24s vs 1.22s on main (1% slower)
📖	Linting Wire with this PR took 17.12s vs 17.32s on main (1% faster)
📖	Linting WordPress with this PR took 12.76s vs 12.84s 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.

SimplyDanny · 2024-07-02T17:42:39Z

Source/SwiftLintCore/Extensions/SwiftSyntax+SwiftLint.swift

@@ -184,6 +184,25 @@ public extension EnumDeclSyntax {
            return rawValueTypes.contains(identifier)
        }
    }
+
+    /// True if this enum is a `CodingKey`


Suggested change

/// True if this enum is a `CodingKey`

/// True if this enum is a `CodingKey`. For that, it has to be named `CodingKeys` and must conform to the `CodingKey` protocol.

SimplyDanny · 2024-07-02T17:44:57Z

Source/SwiftLintCore/Extensions/SwiftSyntax+SwiftLint.swift

@@ -184,6 +184,25 @@ public extension EnumDeclSyntax {
            return rawValueTypes.contains(identifier)
        }
    }
+
+    /// True if this enum is a `CodingKey`
+    var isCodingKey: Bool {


The enum itself is not a coding key, but it defines the coding keys, doesn't it? So what about the name definesCodingKeys instead?

SimplyDanny · 2024-07-02T17:45:42Z

Source/SwiftLintCore/Extensions/SwiftSyntax+SwiftLint.swift

+            return false
+        }
+
+        guard self.name.text == "CodingKeys" else {


Suggested change

guard self.name.text == "CodingKeys" else {

guard name.text == "CodingKeys" else {

Can also be added to the previous guard.

SimplyDanny · 2024-07-02T17:47:45Z

Source/SwiftLintCore/Extensions/SwiftSyntax+SwiftLint.swift

+            guard let identifier = element.type.as(IdentifierTypeSyntax.self)?.name.text else {
+                return false
+            }
+
+            return identifier == "CodingKey"


Suggested change

guard let identifier = element.type.as(IdentifierTypeSyntax.self)?.name.text else {

return false

}

return identifier == "CodingKey"

element.type.as(IdentifierTypeSyntax.self)?.name.text == "CodingKey"

SimplyDanny · 2024-07-02T17:51:50Z

Source/SwiftLintBuiltInRules/Rules/Metrics/NestingRule.swift

@@ -64,7 +64,9 @@ private extension NestingRule {
        }

        override func visit(_ node: EnumDeclSyntax) -> SyntaxVisitorContinueKind {
-            validate(forFunction: false, triggeringToken: node.enumKeyword)
+            if !(configuration.ignoreCodingKeys && node.isCodingKey) {


Suggested change

if !(configuration.ignoreCodingKeys && node.isCodingKey) {

if !configuration.ignoreCodingKeys || !node.isCodingKey {

SimplyDanny · 2024-07-02T17:53:09Z

Tests/SwiftLintFrameworkTests/NestingRuleTests.swift

+            .init("""
+                struct Outer {
+                    struct Inner {
+                        enum Example: String, CodingKey {


Suggested change

enum Example: String, CodingKey {

↓enum Example: String, CodingKey {

SimplyDanny · 2024-07-02T17:56:52Z

Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/NestingConfiguration.swift

@@ -15,6 +15,8 @@ struct NestingConfiguration: RuleConfiguration {
    private(set) var alwaysAllowOneTypeInFunctions = false
    @ConfigurationElement(key: "ignore_typealiases_and_associatedtypes")
    private(set) var ignoreTypealiasesAndAssociatedtypes = false
+    @ConfigurationElement(key: "ignore_codingkeys")


Suggested change

@ConfigurationElement(key: "ignore_codingkeys")

@ConfigurationElement(key: "ignore_coding_keys")

SimplyDanny · 2024-07-02T18:00:42Z

CHANGELOG.md

@@ -16,6 +16,10 @@
  improvements done in the [SwiftSyntax](https://github.com/apple/swift-syntax)
  library.

+* Add `ignore_codingkeys` parameter to `nesting` rule.  


Please explain briefly what this option does.

Suggested change

* Add `ignore_codingkeys` parameter to `nesting` rule.

* Add `ignore_coding_keys` parameter to `nesting` rule.

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 added 2 commits July 1, 2024 19:41

Add an isCodingKey Bool getter to EnumDeclSyntax

a41834d

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

Add ignore_codingkeys parameter to nesting

317448e

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

Add entry to CHANGELOG

0be58c7

SimplyDanny requested changes Jul 2, 2024

View reviewed changes

SimplyDanny reviewed Jul 2, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `ignore_codingkeys` parameter to nesting #5650

Add `ignore_codingkeys` parameter to nesting #5650

braker1nine commented Jul 2, 2024

SwiftLintBot commented Jul 2, 2024 •

edited

Loading

SimplyDanny left a comment

SimplyDanny Jul 2, 2024

SimplyDanny Jul 2, 2024

SimplyDanny Jul 2, 2024

SimplyDanny Jul 2, 2024

SimplyDanny Jul 2, 2024

SimplyDanny Jul 2, 2024

SimplyDanny Jul 2, 2024

SimplyDanny Jul 2, 2024

braker1nine commented Jul 5, 2024

	/// True if this enum is a `CodingKey`
	/// True if this enum is a `CodingKey`. For that, it has to be named `CodingKeys` and must conform to the `CodingKey` protocol.

	guard self.name.text == "CodingKeys" else {
	guard name.text == "CodingKeys" else {

	if !(configuration.ignoreCodingKeys && node.isCodingKey) {
	if !configuration.ignoreCodingKeys \|\| !node.isCodingKey {

	enum Example: String, CodingKey {
	↓enum Example: String, CodingKey {

	@ConfigurationElement(key: "ignore_codingkeys")
	@ConfigurationElement(key: "ignore_coding_keys")

	* Add `ignore_codingkeys` parameter to `nesting` rule.
	* Add `ignore_coding_keys` parameter to `nesting` rule.

Add ignore_codingkeys parameter to nesting #5650

Are you sure you want to change the base?

Add ignore_codingkeys parameter to nesting #5650

Conversation

braker1nine commented Jul 2, 2024

SwiftLintBot commented Jul 2, 2024 • edited Loading

SimplyDanny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

braker1nine commented Jul 5, 2024

Add `ignore_codingkeys` parameter to nesting #5650

Add `ignore_codingkeys` parameter to nesting #5650

SwiftLintBot commented Jul 2, 2024 •

edited

Loading