optional_data_string_conversion throws error

[HHuckebein]

trafficstars

New Issue Checklist

• [ x] I've Updated SwiftLint to the latest version.
• [ x] I've searched for existing GitHub issues.

Bug Description

When using the code below, SwiftLint 0.57.0 reports an error like this Optional Data -> String Conversion Violation: Prefer failable String(data:encoding:) initializer when converting Data to String (optional_data_string_conversion)

but following the advice leads to Value of optional type 'String?' must be unwrapped to a value of type 'String'.

// This triggers a violation:
    var description: String {
        do {
            let data = try encode()
            return String(decoding: data, as: UTF8.self)
        } catch {
            return error.message.description
        }
    }

Mention the command or other SwiftLint integration method that caused the issue. Include stack traces or command output.

None

Environment

• SwiftLint version (run swiftlint version to be sure) 0.57.0
• Xcode version (run xcodebuild -version to be sure) Xcode 16.2 Build version 16B5100e
• Installation method used (Homebrew, CocoaPods, building from source, etc) Homebrew
• Configuration file:

disabled_rules:

opt_in_rules:
- collection_alignment
#- conditional_returns_on_newline
- convenience_type
- empty_count
- empty_string
- fallthrough
- fatal_error_message
- file_header
- file_name
- file_name_no_space
- first_where
- flatmap_over_map_reduce
- for_where
- force_unwrapping
- identical_operands
- implicit_return
- implicitly_unwrapped_optional
- is_disjoint
- joined_default_parameter
- legacy_multiple
- legacy_random
- last_where
- lower_acl_than_parent
- modifier_order
- multiline_parameters
- overridden_super_call
- prefixed_toplevel_constant
- prefer_zero_over_explicit_init
- private_action
- private_outlet
- static_operator
- strict_fileprivate
- toggle_bool
#- trailing_closure
- unavailable_function
- unowned_variable_capture
- unneeded_parentheses_in_closure_argument
- untyped_error_in_catch
- yoda_condition

analyzer_rules:
- unused_declaration
- unused_import
    
excluded: # paths to ignore during linting.
- Frameworks
- Products
- Recovered References
- Carthage
- packages/*/Package.swift
- Packages/*/Package.swift

- packages/*/Tests/
- Packages/*/Tests/

# Default rule configuration
cyclomatic_complexity:
    ignores_case_statements: true

empty_count:
    only_after_dot: true

function_parameter_count: 6

function_body_length: 80

identifier_name:
    allowed_symbols: _ # Is used for private property definitions
    max_length: 50

    excluded:
        - id
        - at
        - on
        - to
        - cc
        - i
        - ok
        - no
        - qa
        - ID
        - xl
        - d1
        - d2
        - tr
        - vc
        - up
        - by

large_tuple: # warn user when using 4 values in tuple, give error if there are 5
- 3

line_length: 200

nesting:
    type_level: 3

type_body_length: 270

type_name:
    max_length: 60
    excluded:
    - AX
    - LS

# Opt-in rule configuration
file_name:
 excluded:
 - "BundleConfig_Root.swift"
 - "BundleConfig_Root+Extension.swift"
 - "BundleConfig_Insurance.swift"
 - "BundleConfig_Insurance+Extension.swift"

file_header:
 required_pattern: |
                    \/\/
                    \/\/ Copyright \(c\) *******\. All rights reserved\.
                    \/\/

Are you using nested configurations? If so, paste their relative paths and respective contents.

[SimplyDanny]

That's the idea of the rule. It asks you to use the failable initializer to make you think about the error case. Your current code will just fail at runtime in case data cannot be converted.

[HHuckebein]

The object created by try encode() is non-optional and String(decoding: data, as: UTF8.self) is a non-failing initializer.

[SimplyDanny]

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

String(decoding: data, as: UTF8.self) may panic at runtime if data cannot be represented as UTF-8.

[davevdveen]

The observed behavior of String(decoding:as:) in Swift is as follows:

When decoding detects invalid bytes, it replaces them with the Unicode replacement character � (U+FFFD). As a result, it still returns a String, which may be acceptable depending on the use case.

let invalidBytes: [UInt8] = [0x48, 0x65, 0xFF, 0x6C, 0x6F] // H, e, (invalid), l, o
let str = String(decoding: invalidBytes, as: UTF8.self)
let str2 = String(data: Data(invalidBytes), encoding: .utf8)
let str3 = String(bytes: invalidBytes, encoding: .utf8)

print(str) // Output: He�lo
print(str2) // Output: nil
print(str3) // Output: nil

[davevdveen]

Related to https://github.com/realm/SwiftLint/issues/5785

[jshier]

String(decoding: data, as: UTF8.self) may panic at runtime if data cannot be represented as UTF-8.

This just isn't true, String(decoding:as:) will insert the replacement character (repair the string) as demonstrated, and as you can see in the code for the init itself. How did you come to be under such a misapprehension?

[SimplyDanny]

This just isn't true, String(decoding:as:) will insert the replacement character (repair the string) as demonstrated, and as you can see in the code for the init itself. How did you come to be under such a misapprehension?

Admittedly, that it would cause a panic was an exaggeration assuming a replacement character in a string is any better. I got slightly confused with all the back and forth around this rule.

Still, this issue presents an example of exactly the code this rule is supposed to trigger on. So I don't see any further action here. Please reach out if you think otherwise.

[davevdveen]

This just isn't true, String(decoding:as:) will insert the replacement character (repair the string) as demonstrated, and as you can see in the code for the init itself. How did you come to be under such a misapprehension?

Admittedly, that it would cause a panic was an exaggeration assuming a replacement character in a string is any better. I got slightly confused with all the back and forth around this rule.

Still, this issue presents an example of exactly the code this rule is supposed to trigger on. So I don't see any further action here. Please reach out if you think otherwise.

I don’t believe this rule should be a default one. The behaviour in question is not wrong by definition. It can be a valid and intentional choice depending on the context. Enforcing this rule by default assumes a strict interpretation that doesn't apply universally, and may unnecessarily flag legitimate use cases. It would be more appropriate for this rule to be opt-in, allowing projects that want stricter handling to enable it explicitly.

[SimplyDanny]

I don’t believe this rule should be a default one. The behaviour in question is not wrong by definition. It can be a valid and intentional choice depending on the context. Enforcing this rule by default assumes a strict interpretation that doesn't apply universally, and may unnecessarily flag legitimate use cases. It would be more appropriate for this rule to be opt-in, allowing projects that want stricter handling to enable it explicitly.

The same can be said about the force_try rule which has existed as a default rule since the beginning of SwiftLint. There is probably no rule that isn't opinionated and yet has someone at some point decided to enable a rule by default or add it as opt-it.

People who know well what they do, may ignore the rule entirely or locally. Others not aware of how data is converted now get a reason to think about it. So I don't see this rule causing any harm. Luckily, nothing is fixed and can be configured.