Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverse Data -> String conversion rule #5601

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@

#### Enhancements

* None.
* Add new `optional_data_string_conversion` rule to enforce
failable conversions of `Data` -> UTF-8 `String`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please append two spaces to the end of the description. They are required to enforce a line break in the rendered Markdown.

[Sam Rayner](https://github.com/samrayner)
[#5263](https://github.com/realm/SwiftLint/issues/5263)

#### Bug Fixes

Expand All @@ -24,6 +27,11 @@
[SimplyDanny](https://github.com/SimplyDanny)
[#5153](https://github.com/realm/SwiftLint/issues/5153)

* Revert `optional_data_string_conversion` enforcing
non-failable conversions of `Data` -> UTF-8 `String`.
Comment on lines +30 to +31
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
* Revert `optional_data_string_conversion` enforcing
non-failable conversions of `Data` -> UTF-8 `String`.
* Revert the part of the `non_optional_string_data_conversion`
rule that enforces non-failable conversions of `Data` -> UTF-8
`String`. This is due to the fact that the data to be converted
can be arbitrary and especially doesn't need to represent a valid
UTF-8-encoded string.

Please append two spaces to the end of the description. They are required to enforce a line break in the rendered Markdown.

Also, this should go into the "Breaking" section.

[Sam Rayner](https://github.com/samrayner)
[#5263](https://github.com/realm/SwiftLint/issues/5263)

## 0.55.1: Universal Washing Powder

#### Breaking
Expand Down
1 change: 1 addition & 0 deletions Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ public let builtInRules: [any Rule.Type] = [
OpeningBraceRule.self,
OperatorFunctionWhitespaceRule.self,
OperatorUsageWhitespaceRule.self,
OptionalDataStringConversionRule.self,
OptionalEnumCaseMatchingRule.self,
OrphanedDocCommentRule.self,
OverriddenSuperCallRule.self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@ struct NonOptionalStringDataConversionRule: Rule {
var configuration = SeverityConfiguration<Self>(.warning)
static let description = RuleDescription(
identifier: "non_optional_string_data_conversion",
name: "Non-Optional String <-> Data Conversion",
description: "Prefer using UTF-8 encoded strings when converting between `String` and `Data`",
name: "Non-Optional String -> Data Conversion",
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
name: "Non-Optional String -> Data Conversion",
name: "Non-optional String -> Data Conversion",

description: "Prefer using non-optional Data(_:) when converting a UTF-8 String to Data",
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
description: "Prefer using non-optional Data(_:) when converting a UTF-8 String to Data",
description: "Prefer non-optional `Data(_:)` initializer when converting `String` to `Data`",

kind: .lint,
nonTriggeringExamples: [
Example("Data(\"foo\".utf8)"),
Example("String(decoding: data, as: UTF8.self)")
],
triggeringExamples: [
Example("\"foo\".data(using: .utf8)"),
Example("String(data: data, encoding: .utf8)")
]
)
}
Expand All @@ -31,15 +29,6 @@ private extension NonOptionalStringDataConversionRule {
violations.append(node.positionAfterSkippingLeadingTrivia)
}
}

override func visitPost(_ node: DeclReferenceExprSyntax) {
if node.baseName.text == "String",
let parent = node.parent?.as(FunctionCallExprSyntax.self),
parent.arguments.map({ $0.label?.text }) == ["data", "encoding"],
parent.arguments.last?.expression.as(MemberAccessExprSyntax.self)?.isUTF8 == true {
violations.append(node.positionAfterSkippingLeadingTrivia)
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import SwiftSyntax

@SwiftSyntaxRule
struct OptionalDataStringConversionRule: Rule {
var configuration = SeverityConfiguration<Self>(.warning)
static let description = RuleDescription(
identifier: "optional_data_string_conversion",
name: "Optional Data -> String Conversion",
description: "Prefer using failable String(data:encoding:) when converting from `Data` to a UTF-8 `String`",
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
description: "Prefer using failable String(data:encoding:) when converting from `Data` to a UTF-8 `String`",
description: "Prefer failable `String(data:encoding:)` initializer when converting `Data` to `String`",

kind: .lint,
nonTriggeringExamples: [
Example("String(data: data, encoding: .utf8)")
],
triggeringExamples: [
Example("String(decoding: data, as: UTF8.self)")
]
)
}

private extension OptionalDataStringConversionRule {
final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
override func visitPost(_ node: DeclReferenceExprSyntax) {
if node.baseName.text == "String",
let parent = node.parent?.as(FunctionCallExprSyntax.self),
parent.arguments.map({ $0.label?.text }) == ["decoding", "as"],
parent.arguments.last?.expression.as(IdentifierTypeSyntax.self)?.name == "UTF8.self" {
samrayner marked this conversation as resolved.
Show resolved Hide resolved
violations.append(node.positionAfterSkippingLeadingTrivia)
}
}
}
}
2 changes: 1 addition & 1 deletion Source/SwiftLintCore/Extensions/Configuration+Remote.swift
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ internal extension Configuration.FileGraph.FilePath {
guard
taskResult.2 == nil, // No error
(taskResult.1 as? HTTPURLResponse)?.statusCode == 200,
let configStr = (taskResult.0.flatMap { String(decoding: $0, as: UTF8.self) })
let configStr = (taskResult.0.flatMap { String(data: $0, encoding: .utf8) })
else {
return try handleWrongData(
urlString: urlString,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ public struct RegexConfiguration<Parent: Rule>: SeverityBasedRuleConfiguration,
.map({ $0.rawValue }).sorted(by: <).joined(separator: ","),
severity.rawValue
]
if let jsonData = try? JSONSerialization.data(withJSONObject: jsonObject) {
return String(decoding: jsonData, as: UTF8.self)
if let jsonData = try? JSONSerialization.data(withJSONObject: jsonObject),
let jsonString = String(data: jsonData, encoding: .utf8) {
return jsonString
}
queuedFatalError("Could not serialize regex configuration for cache")
}
Expand Down
6 changes: 4 additions & 2 deletions Source/swiftlint/Extensions/Configuration+CommandLine.swift
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,10 @@ extension Configuration {
fileprivate func getFiles(with visitor: LintableFilesVisitor) async throws -> [SwiftLintFile] {
if visitor.useSTDIN {
let stdinData = FileHandle.standardInput.readDataToEndOfFile()
let stdinString = String(decoding: stdinData, as: UTF8.self)
return [SwiftLintFile(contents: stdinString)]
if let stdinString = String(data: stdinData, encoding: .utf8) {
return [SwiftLintFile(contents: stdinString)]
}
throw SwiftLintError.usageError(description: "stdin isn't a UTF8-encoded string")
}
if visitor.useScriptInputFiles {
let files = try scriptInputFiles()
Expand Down
4 changes: 2 additions & 2 deletions Source/swiftlint/Helpers/LintableFilesVisitor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ struct LintableFilesVisitor {
}

private static func loadLogCompilerInvocations(_ path: String) -> [[String]]? {
if let data = FileManager.default.contents(atPath: path) {
let logContents = String(decoding: data, as: UTF8.self)
if let data = FileManager.default.contents(atPath: path),
let logContents = String(data: data, encoding: .utf8) {
if logContents.isEmpty {
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion Source/swiftlint/Helpers/SwiftPMCompilationDB.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ struct SwiftPMCompilationDB: Codable {
let pathToReplace = Array(nodes.nodes.keys.filter({ node in
node.hasSuffix(suffix)
}))[0].dropLast(suffix.count - 1)
let stringFileContents = String(decoding: yaml, as: UTF8.self)
let stringFileContents = String(data: yaml, encoding: .utf8)!
.replacingOccurrences(of: pathToReplace, with: "")
compilationDB = try decoder.decode(Self.self, from: stringFileContents)
} else {
Expand Down
6 changes: 6 additions & 0 deletions Tests/GeneratedTests/GeneratedTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,12 @@ final class OperatorUsageWhitespaceRuleGeneratedTests: SwiftLintTestCase {
}
}

final class OptionalDataStringConversionRuleGeneratedTests: SwiftLintTestCase {
func testWithDefaultConfiguration() {
verifyRule(OptionalDataStringConversionRule.description)
}
}

final class OptionalEnumCaseMatchingRuleGeneratedTests: SwiftLintTestCase {
func testWithDefaultConfiguration() {
verifyRule(OptionalEnumCaseMatchingRule.description)
Expand Down
4 changes: 2 additions & 2 deletions Tests/IntegrationTests/IntegrationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@ private func execute(_ args: [String],
queue.async(group: group) { stderrData = stderrPipe.fileHandleForReading.readDataToEndOfFile() }
process.waitUntilExit()
group.wait()
let stdout = stdoutData.map { String(decoding: $0, as: UTF8.self) } ?? ""
let stderr = stderrData.map { String(decoding: $0, as: UTF8.self) } ?? ""
let stdout = stdoutData.flatMap { String(data: $0, encoding: .utf8) } ?? ""
let stderr = stderrData.flatMap { String(data: $0, encoding: .utf8) } ?? ""
return (process.terminationStatus, stdout, stderr)
}

Expand Down