Add detection of leading-dot when the contextual type is String to optional_data_string_conversion` rule#6372
Conversation
Generated by 🚫 Danger |
SimplyDanny
left a comment
There was a problem hiding this comment.
Thanks for preparing the PR! Please consider my comments.
Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift
Outdated
Show resolved
Hide resolved
Tests/BuiltInRulesTests/OptionalDataStringConversionRuleTests.swift
Outdated
Show resolved
Hide resolved
Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift
Show resolved
Hide resolved
804ec4c to
4cfbd90
Compare
Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift
Show resolved
Hide resolved
Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift
Outdated
Show resolved
Hide resolved
Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift
Outdated
Show resolved
Hide resolved
Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift
Outdated
Show resolved
Hide resolved
|
Hey @nadeemnali, is anything blocking you from moving this forward? |
Hey @SimplyDanny Apologies, I was busy for past weeks, will update the comments ASAP |
|
@SimplyDanny Can you please help me with the danger issue in pipeline, can't seem to find any logs for the failure |
Danger doesn't like merge commits. Please rebase your branch onto main instead. |
5de3ae6 to
a7dd1f5
Compare
String.init(decoding: data, as: UTF8.self) and let text: String = .init(decoding: data, as: UTF8.self) to optional_data_string_conversion rule
String.init(decoding: data, as: UTF8.self) and let text: String = .init(decoding: data, as: UTF8.self) to optional_data_string_conversion rule to optional_data_string_conversion` rule
bd3286e to
13b6617
Compare
SimplyDanny
left a comment
There was a problem hiding this comment.
Thanks for addressing my remarks! There is one more thing about allowing .init(...) even without an explicit type optionally.
Source/SwiftLintBuiltInRules/Rules/Lint/OptionalDataStringConversionRule.swift
Show resolved
Hide resolved
5160e5b to
7b74331
Compare
| name: "Optional Data -> String Conversion", | ||
| description: "Prefer failable `String(bytes:encoding:)` initializer when converting `Data` to `String`", | ||
| description: """ | ||
| Prefer failable `String(bytes:encoding:)` initializer when converting `Data` to `String`. \ |
There was a problem hiding this comment.
I'd rather keep it as is. Mentioning configuration parameters here increases the maintenance effort.
| private(set) var severityConfiguration = SeverityConfiguration<Parent>.warning | ||
|
|
||
| // When true, also flag leading-dot `.init(decoding:as:)` without explicit `String` type annotation. | ||
| // Default is false to preserve conservative behavior. |
There was a problem hiding this comment.
Skip that last sentence. Should be clear even without it.
| Example("String(decoding: data, as: UTF8.self)") | ||
| Example("String(decoding: data, as: UTF8.self)"), | ||
| Example("String.init(decoding: data, as: UTF8.self)"), | ||
| Example("let text: String = .init(decoding: data, as: UTF8.self)"), |
There was a problem hiding this comment.
Tests (triggering and non-triggering) are missing for the new option. Just add them here with a configuration.
…ting the arbitrary tree walking.
6098bab to
6faf8d2
Compare
Summary
The optional_data_string_conversion SwiftSyntax rule only detected direct calls like:
but it missed two common variants:
String.init(decoding: data, as: UTF8.self)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 isString. It also adds examples to the rule description and a unit test that verifies the rule's description examples.Changes
String(...)calls,String.init(...)(MemberAccessExpr with baseStringand memberinit),.init(...)when assigned to a variable with an explicitStringtype annotation.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