Fix: optional_data_string_conversion should detect String.init and leading-dot .init usages by nadeemnali · Pull Request #6372 · realm/SwiftLint

nadeemnali · 2025-12-09T21:23:57Z

Summary

The optional_data_string_conversion SwiftSyntax rule only detected direct calls like:

String(decoding: data, as: UTF8.self)

but it missed two common variants:

Type-qualified initializer: String.init(decoding: data, as: UTF8.self)
Leading dot initializer used with an explicit type annotation: let text: String = .init(decoding: data, as: UTF8.self)

This change updates the rule to detect both String.init(...) and leading-dot .init(...) when the contextual type is String. It also adds examples to the rule description and a unit test that verifies the rule's description examples.

Changes

Update OptionalDataStringConversionRule visitor to:
- keep existing detection for direct String(...) calls,
- detect String.init(...) (MemberAccessExpr with base String and member init),
- detect leading-dot .init(...) when assigned to a variable with an explicit String type annotation.
Add a test file which runs the rule's description examples.

Why

This fixes the bug reported where
let text: String = .init(decoding: data, as: UTF8.self) did not produce the expected lint violation.

Test plan

Unit test included: Tests/SwiftLintBuiltInRulesTests/OptionalDataStringConversionRuleTests.swift
The test verifies the rule's nonTriggering and triggering examples contained in the RuleDescription.

SwiftLintBot · 2025-12-09T21:31:47Z

	1 Warning
⚠️	This PR may need tests.

	19 Messages
📖	Building this branch resulted in a binary size of 26850.93 KiB vs 26850.74 KiB when built on `main` (0% larger).
📖	Linting Aerial with this PR took 0.18 s vs 0.2 s on `main` (10% faster).
📖	Linting Alamofire with this PR took 0.21 s vs 0.2 s on `main` (4% slower).
📖	Linting Brave with this PR took 0.81 s vs 0.83 s on `main` (2% faster).
📖	Linting DuckDuckGo with this PR took 3.03 s vs 3.02 s on `main` (0% slower).
📖	Linting Firefox with this PR took 1.47 s vs 1.5 s on `main` (2% faster).
📖	Linting Kickstarter with this PR took 0.89 s vs 0.89 s on `main` (0% slower).
📖	Linting Moya with this PR took 0.12 s vs 0.15 s on `main` (20% faster).
📖	Linting NetNewsWire with this PR took 0.31 s vs 0.32 s on `main` (3% faster).
📖	Linting Nimble with this PR took 0.16 s vs 0.16 s on `main` (0% slower).
📖	Linting PocketCasts with this PR took 0.88 s vs 0.87 s on `main` (1% slower).
📖	Linting Quick with this PR took 0.16 s vs 0.12 s on `main` (33% slower).
📖	Linting Realm with this PR took 0.46 s vs 0.43 s on `main` (6% slower).
📖	Linting Sourcery with this PR took 0.29 s vs 0.33 s on `main` (12% faster).
📖	Linting Swift with this PR took 0.45 s vs 0.48 s on `main` (6% faster).
📖	Linting SwiftLintPerformanceTests with this PR took 3.81 s vs 3.83 s on `main` (0% faster).
📖	Linting VLC with this PR took 0.23 s vs 0.26 s on `main` (11% faster).
📖	Linting Wire with this PR took 2.0 s vs 2.0 s on `main` (0% slower).
📖	Linting WordPress with this PR took 1.31 s vs 1.31 s on `main` (0% slower).

Generated by 🚫 Danger

SimplyDanny

Thanks for preparing the PR! Please consider my comments.

SimplyDanny · 2025-12-09T22:14:53Z

Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift

@@ -34,5 +37,53 @@ private extension OptionalDataStringConversionRule {
                violations.append(node.positionAfterSkippingLeadingTrivia)
            }
        }


We should rather implement visitPost(_ node: FunctionCallExprSyntax) instead. It can check all cases at the same time. If the called name is either String, String.init or .init and the arguments match, it might trigger.

The var x: String = .init(...) case is very special and I don't expect it to appear often. While String.init is safe, .init could appear everywhere (think of f(.init(...)). We might add an option for that (as I've already pointed out in #6359.

Tests/BuiltInRulesTests/OptionalDataStringConversionRuleTests.swift

SimplyDanny · 2025-12-09T22:16:25Z

Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift

@@ -17,7 +18,9 @@ struct OptionalDataStringConversionRule: Rule {
            Example("String(decoding: data, encoding: UTF8.self)"),


Please add more cases that shouldn't trigger, e.g. let text: Int = .init(decoding: data, as: UTF8.self). 🤷‍♂️

I'm missing this example. Please add it.

…ersion rule

SimplyDanny · 2025-12-17T18:53:40Z

CHANGELOG.md


+### Bug Fixes
+
+* optional_data_string_conversion: Detect `String.init(...)` and leading-dot `.init(...)` usages when converting `Data` to `String`. This fixes cases such as `String.init(decoding: data, as: UTF8.self)` and `let text: String = .init(decoding: data, as: UTF8.self)` which previously did not trigger the rule. (Fixes https://github.com/realm/SwiftLint/issues/6359)


Please add this entry to the current "Main" section at the top of the file.

SimplyDanny · 2025-12-17T18:56:25Z

CHANGELOG.md

+* optional_data_string_conversion: Detect `String.init(...)` and leading-dot `.init(...)` usages when converting `Data` to `String`. This fixes cases such as `String.init(decoding: data, as: UTF8.self)` and `let text: String = .init(decoding: data, as: UTF8.self)` which previously did not trigger the rule. (Fixes https://github.com/realm/SwiftLint/issues/6359)
+  [Nadeem Ali](https://github.com/nadeemnali)


Suggested change

* optional_data_string_conversion: Detect `String.init(...)` and leading-dot `.init(...)` usages when converting `Data` to `String`. This fixes cases such as `String.init(decoding: data, as: UTF8.self)` and `let text: String = .init(decoding: data, as: UTF8.self)` which previously did not trigger the rule. (Fixes https://github.com/realm/SwiftLint/issues/6359)

[Nadeem Ali](https://github.com/nadeemnali)

* Add detection of cases such as `String.init(decoding: data, as: UTF8.self)` and

`let text: String = .init(decoding: data, as: UTF8.self)` to `optional_data_string_conversion` rule.

[Nadeem Ali](https://github.com/nadeemnali)

[#6359](https://github.com/realm/SwiftLint/issues/6359)

SimplyDanny · 2025-12-17T18:56:47Z

Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift

@@ -17,7 +18,9 @@ struct OptionalDataStringConversionRule: Rule {
            Example("String(decoding: data, encoding: UTF8.self)"),


I'm missing this example. Please add it.

SimplyDanny · 2025-12-17T18:58:03Z

Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift

+            guard node.arguments.map(\.label?.text) == ["decoding", "as"] else { return }
+
+            // Check that the `as:` argument is `UTF8.self`
+            func isUTF8Self(on call: FunctionCallExprSyntax) -> Bool {


Why do we need this function? It's only called once, isn't it?

SimplyDanny · 2025-12-17T18:59:19Z

Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift

+            }
+
+            // Case 2 and 3: `.init` or `String.init`
+            if let member = called.as(MemberAccessExprSyntax.self), member.declName.baseName.text == "init" {


Can be a guard.

SimplyDanny · 2025-12-17T19:03:24Z

Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift

+                if member.base == nil {
+                    // Walk ancestors to find a VariableDecl or PatternBinding with a type annotation of `String`.
+                    var parent: Syntax? = node.parent
+                    while let ancestor = parent {


Better to not walk up the tree arbitrarily high. node.parent.parent gives you the binding to check for the String type.

SimplyDanny linked an issue Dec 9, 2025 that may be closed by this pull request

optional_data_string_conversion does not trigger when .init(decoding:as:) is called instead of String(decoding:as:) #6359

Open

2 tasks

SimplyDanny requested changes Dec 9, 2025

View reviewed changes

nadeemnali requested a review from SimplyDanny December 11, 2025 19:05

nadeemnali added 5 commits December 16, 2025 21:19

Detect String.init and leading-dot init in optional_data_string_conv…

cfd5820

…ersion rule

Update unit tests

ae666fe

Update linting issues

35729e9

Update to use visitPost(_ node: FunctionCallExprSyntax)

6dc0266

Updated non-triggering examples for better coverage

4cfbd90

nadeemnali force-pushed the feature/large-tuple-ignore-regex branch from 804ec4c to 4cfbd90 Compare December 16, 2025 21:19

SimplyDanny requested changes Dec 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: optional_data_string_conversion should detect String.init and leading-dot .init usages#6372

Fix: optional_data_string_conversion should detect String.init and leading-dot .init usages#6372
nadeemnali wants to merge 5 commits intorealm:mainfrom
nadeemnali:feature/large-tuple-ignore-regex

nadeemnali commented Dec 9, 2025

Uh oh!

SwiftLintBot commented Dec 9, 2025 •

edited

Loading

Uh oh!

SimplyDanny left a comment

Uh oh!

SimplyDanny Dec 9, 2025

Uh oh!

Uh oh!

SimplyDanny Dec 9, 2025

Uh oh!

SimplyDanny Dec 17, 2025

Uh oh!

SimplyDanny Dec 17, 2025

Uh oh!

SimplyDanny Dec 17, 2025

Uh oh!

SimplyDanny Dec 17, 2025

Uh oh!

SimplyDanny Dec 17, 2025

Uh oh!

SimplyDanny Dec 17, 2025

Uh oh!

SimplyDanny Dec 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -17,7 +18,9 @@ struct OptionalDataStringConversionRule: Rule {
		Example("String(decoding: data, encoding: UTF8.self)"),


		### Bug Fixes

		* optional_data_string_conversion: Detect `String.init(...)` and leading-dot `.init(...)` usages when converting `Data` to `String`. This fixes cases such as `String.init(decoding: data, as: UTF8.self)` and `let text: String = .init(decoding: data, as: UTF8.self)` which previously did not trigger the rule. (Fixes https://github.com/realm/SwiftLint/issues/6359)

		* optional_data_string_conversion: Detect `String.init(...)` and leading-dot `.init(...)` usages when converting `Data` to `String`. This fixes cases such as `String.init(decoding: data, as: UTF8.self)` and `let text: String = .init(decoding: data, as: UTF8.self)` which previously did not trigger the rule. (Fixes https://github.com/realm/SwiftLint/issues/6359)
		[Nadeem Ali](https://github.com/nadeemnali)

Conversation

nadeemnali commented Dec 9, 2025

Summary

Changes

Why

Test plan

Uh oh!

SwiftLintBot commented Dec 9, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SwiftLintBot commented Dec 9, 2025 •

edited

Loading