Skip to content

Conversation

@nadeemnali
Copy link

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
Copy link

SwiftLintBot commented Dec 9, 2025

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

Copy link
Collaborator

@SimplyDanny SimplyDanny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for preparing the PR! Please consider my comments.

expr.declName.baseName.description == "self" {
violations.append(node.positionAfterSkippingLeadingTrivia)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Example("String(bytes: data, encoding: .utf8)"),
Example("String(UTF8.self)"),
Example("String(a, b, c, UTF8.self)"),
Example("String(decoding: data, encoding: UTF8.self)"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing this example. Please add it.

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

### 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment on lines +8610 to +8611
* 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Example("String(bytes: data, encoding: .utf8)"),
Example("String(UTF8.self)"),
Example("String(a, b, c, UTF8.self)"),
Example("String(decoding: data, encoding: UTF8.self)"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing this example. Please add it.

guard node.arguments.map(\.label?.text) == ["decoding", "as"] else { return }

// Check that the `as:` argument is `UTF8.self`
func isUTF8Self(on call: FunctionCallExprSyntax) -> Bool {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be a guard.

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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants