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

Backtick variable/method names that conflict with Swift keywords #91

Merged
merged 4 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/scripts/compile-snapshots.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,7 @@ for snapshot in Tests/XCStringsToolTests/__Snapshots__/**/*.swift; do

if xcrun swiftc "$snapshot" -package-name "MyPackage" -o /dev/null ; then
echo " Success"
else
exit $?
fi
done
45 changes: 45 additions & 0 deletions Sources/StringGenerator/Extensions/String+BacktickIfNeeded.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import SwiftSyntax
import SwiftParser

extension String {
var backtickedVariableNameIfNeeded: String {
// In SwiftSyntax 600, use isValidIdentifier(for:)
// https://github.com/apple/swift-syntax/pull/2434
// Thanks @ahoppen!
_isValidVariableName ? self : "`\(self)`"
}

private var _isValidVariableName: Bool {
let name = self

var parser = Parser("var \(name)")
let decl = DeclSyntax.parse(from: &parser)

guard !decl.hasError && !decl.hasWarning else {
// There were syntax errors in the source code. So not valid.
return false
}

guard let variable = decl.as(VariableDeclSyntax.self) else {
return false
}

guard let identifier = variable.bindings.first?.pattern.as(IdentifierPatternSyntax.self)?.identifier else {
return false
}

guard case .identifier = identifier.tokenKind else {
// We parsed the name as a keyword, eg. `self`, so not a valid identifier.
return false
}

guard identifier.text.count == name.utf8.count else {
// The identifier doesn't cover all the characters in `name`, so we parsed
// some of these characters into trivia or another token.
// Thus, `name` is not a valid identifier.
return false
}

return true
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ extension SourceFile.LocalizedStringResourceExtension.StringsTableStruct {
!resource.arguments.isEmpty
}

var name: TokenSyntax {
var variableName: TokenSyntax {
.identifier(resource.identifier.backtickedVariableNameIfNeeded)
}

var nameForMemberAccess: TokenSyntax {
.identifier(resource.identifier)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ extension SourceFile.StringExtension.StringsTableStruct {
!resource.arguments.isEmpty
}

var name: TokenSyntax {
.identifier(resource.identifier)
var variableName: TokenSyntax {
.identifier(resource.identifier.backtickedVariableNameIfNeeded)
}

var type: TokenSyntax {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ struct LocalizedStringResourceStringsTableResourceFunctionSnippet: Snippet {
leadingTrivia: leadingTrivia,
attributes: attributes.map({ $0.with(\.trailingTrivia, .newline) }),
modifiers: modifiers,
name: accessor.name,
name: accessor.variableName,
signature: FunctionSignatureSyntax(
parameterClause: FunctionParameterClauseSyntax {
for argument in accessor.resource.arguments {
Expand Down Expand Up @@ -57,7 +57,7 @@ struct LocalizedStringResourceStringsTableResourceFunctionSnippet: Snippet {
LabeledExprSyntax(
label: accessor.sourceFile.tableVariableIdentifier,
expression: FunctionCallExprSyntax(
callee: MemberAccessExprSyntax(name: accessor.name)
callee: MemberAccessExprSyntax(name: accessor.nameForMemberAccess)
) {
for argument in accessor.resource.arguments {
LabeledExprSyntax(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ struct LocalizedStringResourceStringsTableResourceVariableSnippet: Snippet {
bindingSpecifier: .keyword(.var),
bindings: [
PatternBindingSyntax(
pattern: IdentifierPatternSyntax(identifier: accessor.name),
pattern: IdentifierPatternSyntax(identifier: accessor.variableName),
typeAnnotation: TypeAnnotationSyntax(type: accessor.type),
accessorBlock: AccessorBlockSyntax(
accessors: .getter(getter)
Expand Down Expand Up @@ -56,7 +56,7 @@ struct LocalizedStringResourceStringsTableResourceVariableSnippet: Snippet {
) {
LabeledExprSyntax(
label: accessor.sourceFile.tableVariableIdentifier,
expression: MemberAccessExprSyntax(name: accessor.name)
expression: MemberAccessExprSyntax(name: accessor.nameForMemberAccess)
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ struct StringStringsTableResourceFunctionSnippet: Snippet {
FunctionDeclSyntax(
leadingTrivia: leadingTrivia,
modifiers: modifiers,
name: accessor.name,
name: accessor.variableName,
signature: FunctionSignatureSyntax(
parameterClause: FunctionParameterClauseSyntax {
for argument in accessor.resource.arguments {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ struct StringStringsTableResourceVariableSnippet: Snippet {
bindingSpecifier: .keyword(.var),
bindings: [
PatternBindingSyntax(
pattern: IdentifierPatternSyntax(identifier: accessor.name),
pattern: IdentifierPatternSyntax(identifier: accessor.variableName),
typeAnnotation: TypeAnnotationSyntax(type: IdentifierTypeSyntax(name: accessor.type)),
accessorBlock: AccessorBlockSyntax(
accessors: .getter(getter)
Expand Down
12 changes: 12 additions & 0 deletions Tests/XCStringsToolTests/__Fixtures__/Localizable.xcstrings
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
{
"sourceLanguage" : "en",
"strings" : {
"continue" : {
"comment" : "A key that conflicts with a keyword in swift that isn't suitable for a variable/method and should be backticked.",
"extractionState" : "manual",
"localizations" : {
"en" : {
"stringUnit" : {
"state" : "translated",
"value" : "Continue"
}
}
}
},
"Empty" : {
"extractionState" : "manual"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ extension String {
///
/// ```swift
/// // Accessing the localized value directly
/// let value = String(localizable: .key)
/// value // "Default Value"
/// let value = String(localizable: .continue)
/// value // "Continue"
/// ```
///
/// Starting in iOS 16/macOS 13/tvOS 16/watchOS 9, `LocalizedStringResource` can also
/// be used:
///
/// ```swift
/// var resource = LocalizedStringResource(localizable: .key)
/// var resource = LocalizedStringResource(localizable: .continue)
/// resource.locale = Locale(identifier: "fr") // customise language
/// let value = String(localized: resource) // defer lookup
/// ```
Expand Down Expand Up @@ -107,6 +107,22 @@ extension String {
self.bundle = bundle
}

/// A key that conflicts with a keyword in swift that isn't suitable for a variable/method and should be backticked.
///
/// ### Source Localization
///
/// ```
/// Continue
/// ```
internal static var `continue`: Localizable {
Localizable(
key: "continue",
arguments: [],
table: "Localizable",
bundle: .current
)
}

/// This is a comment
///
/// ### Source Localization
Expand Down Expand Up @@ -236,15 +252,31 @@ extension LocalizedStringResource {
///
/// ```swift
/// // Accessing the localized value directly
/// let value = String(localized: .localizable.key)
/// value // "Default Value"
/// let value = String(localized: .localizable.continue)
/// value // "Continue"
///
/// // Working with SwiftUI
/// Text(.localizable.key)
/// Text(.localizable.continue)
/// ```
///
/// - Note: Using ``LocalizedStringResource.Localizable`` requires iOS 16/macOS 13 or later. See ``String.Localizable`` for a backwards compatible API.
internal struct Localizable {
/// A key that conflicts with a keyword in swift that isn't suitable for a variable/method and should be backticked.
///
/// ### Source Localization
///
/// ```
/// Continue
/// ```
@available (iOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
@available (macOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
@available (tvOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
@available (watchOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
@available (visionOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
internal var `continue`: LocalizedStringResource {
LocalizedStringResource(localizable: .continue)
}

/// This is a comment
///
/// ### Source Localization
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ extension String {
///
/// ```swift
/// // Accessing the localized value directly
/// let value = String(localizable: .key)
/// value // "Default Value"
/// let value = String(localizable: .continue)
/// value // "Continue"
/// ```
///
/// Starting in iOS 16/macOS 13/tvOS 16/watchOS 9, `LocalizedStringResource` can also
/// be used:
///
/// ```swift
/// var resource = LocalizedStringResource(localizable: .key)
/// var resource = LocalizedStringResource(localizable: .continue)
/// resource.locale = Locale(identifier: "fr") // customise language
/// let value = String(localized: resource) // defer lookup
/// ```
Expand Down Expand Up @@ -107,6 +107,22 @@ extension String {
self.bundle = bundle
}

/// A key that conflicts with a keyword in swift that isn't suitable for a variable/method and should be backticked.
///
/// ### Source Localization
///
/// ```
/// Continue
/// ```
package static var `continue`: Localizable {
Localizable(
key: "continue",
arguments: [],
table: "Localizable",
bundle: .current
)
}

/// This is a comment
///
/// ### Source Localization
Expand Down Expand Up @@ -236,15 +252,31 @@ extension LocalizedStringResource {
///
/// ```swift
/// // Accessing the localized value directly
/// let value = String(localized: .localizable.key)
/// value // "Default Value"
/// let value = String(localized: .localizable.continue)
/// value // "Continue"
///
/// // Working with SwiftUI
/// Text(.localizable.key)
/// Text(.localizable.continue)
/// ```
///
/// - Note: Using ``LocalizedStringResource.Localizable`` requires iOS 16/macOS 13 or later. See ``String.Localizable`` for a backwards compatible API.
package struct Localizable {
/// A key that conflicts with a keyword in swift that isn't suitable for a variable/method and should be backticked.
///
/// ### Source Localization
///
/// ```
/// Continue
/// ```
@available (iOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
@available (macOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
@available (tvOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
@available (watchOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
@available (visionOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
package var `continue`: LocalizedStringResource {
LocalizedStringResource(localizable: .continue)
}

/// This is a comment
///
/// ### Source Localization
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ extension String {
///
/// ```swift
/// // Accessing the localized value directly
/// let value = String(localizable: .key)
/// value // "Default Value"
/// let value = String(localizable: .continue)
/// value // "Continue"
/// ```
///
/// Starting in iOS 16/macOS 13/tvOS 16/watchOS 9, `LocalizedStringResource` can also
/// be used:
///
/// ```swift
/// var resource = LocalizedStringResource(localizable: .key)
/// var resource = LocalizedStringResource(localizable: .continue)
/// resource.locale = Locale(identifier: "fr") // customise language
/// let value = String(localized: resource) // defer lookup
/// ```
Expand Down Expand Up @@ -107,6 +107,22 @@ extension String {
self.bundle = bundle
}

/// A key that conflicts with a keyword in swift that isn't suitable for a variable/method and should be backticked.
///
/// ### Source Localization
///
/// ```
/// Continue
/// ```
public static var `continue`: Localizable {
Localizable(
key: "continue",
arguments: [],
table: "Localizable",
bundle: .current
)
}

/// This is a comment
///
/// ### Source Localization
Expand Down Expand Up @@ -236,15 +252,31 @@ extension LocalizedStringResource {
///
/// ```swift
/// // Accessing the localized value directly
/// let value = String(localized: .localizable.key)
/// value // "Default Value"
/// let value = String(localized: .localizable.continue)
/// value // "Continue"
///
/// // Working with SwiftUI
/// Text(.localizable.key)
/// Text(.localizable.continue)
/// ```
///
/// - Note: Using ``LocalizedStringResource.Localizable`` requires iOS 16/macOS 13 or later. See ``String.Localizable`` for a backwards compatible API.
public struct Localizable {
/// A key that conflicts with a keyword in swift that isn't suitable for a variable/method and should be backticked.
///
/// ### Source Localization
///
/// ```
/// Continue
/// ```
@available (iOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
@available (macOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
@available (tvOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
@available (watchOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
@available (visionOS, deprecated: 100000, message: "Use `String.Localizable.continue` instead. This property will be removed in the future.")
public var `continue`: LocalizedStringResource {
LocalizedStringResource(localizable: .continue)
}

/// This is a comment
///
/// ### Source Localization
Expand Down