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.

Sign up for GitHub

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

Jump to bottom

Re-enable three swift-format rules. #2869

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Re-enable three swift-format rules. #2869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3390fcf
Enable `UseSynthesizedInitializer` and remove redundant initializers.
allevato Oct 2, 2024
d44de72
Enable `UseLetInEveryBoundCaseVariable` and update binding patterns.
allevato Oct 2, 2024
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
4 changes: 2 additions & 2 deletions .swift-format
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"AmbiguousTrailingClosureOverload": false,
"NoBlockComments": false,
"OrderedImports": true,
"UseLetInEveryBoundCaseVariable": false,
"UseSynthesizedInitializer": false
"UseLetInEveryBoundCaseVariable": true,
"UseSynthesizedInitializer": true
Copy link
Contributor

Choose a reason for hiding this comment

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

I’ve noticed that @ahoppen mentioned not having a strong opinion here, so I’d like to share my perspective. In my projects, when a case in a closure has multiple parameters, and I want to designate one parameter as a variable (var) and the others as constants (let), applying this rule makes sense since you can see what is mutable and what's not–don't know if this statment makes sense to u XD hope so.

Code example:

enum Foo {
  case bar(name: String: age: Int)
}

let foo = Foo.bar(name: "Tom", age: 22)

if case .bar(let name, var age) = foo {
  age += 1
  // do sth with increased age and not mutated name? 
}

Copy link
Member Author

Choose a reason for hiding this comment

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

This is one of the reasons I prefer UseLetInEveryBoundCaseVariable as well. Wrapping the whole pattern with let/var forces you into an all-or-nothing situation with the bindings. Once we have other kinds of bindings like borrowing, inout, etc., I think it'll be even more common for those to differ within a payload.

The other reason for this rule is that having the binding specifier immediately adjacent to the binding makes it easier to see what the intended outcome is, and avoids mistakes like this:

let age = 20
if case let .bar(name, age) = foo { ... }
// oops I wanted to bind `name` and match `age`, not create a
// new `age` binding

if case .bar(let name, age) = foo { ... }
// much clearer about which one is a new binding and which
// is an expression to be matched

Comment on lines +15 to +16
Copy link
Member

Choose a reason for hiding this comment

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

These are true by default, right? So we should be able to remove them from .swift-format to make it shorter.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's what I thought as well, but I just noticed that if I remove the line entirely and insert a violation, it isn't flagged.

Oof. I think this is a consequence of the decoding logic; if the rules dictionary is present in the JSON, its value is used verbatim. Only if it isn't present at all do we use the default dictionary. Then, when we check if a rule should be applied at a node, if the rule name isn't in the dictionary, it's always false.

We should fix this by always taking the default rule settings and then merging what we read from the JSON into that. That would ensure that the in-memory dictionary always contains all rules. That might reveal some other places in swift-syntax/swift-format where the rules aren't flagging violations, though.

So my thinking is we merge this as-is ("OrderedImports" was already this way) and then fix that for the next release. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me. But yes, this seems like a swift-format bug to me to not merge the dictionaries that’s worth fixing. If you don’t have time to look at that now, could you file an issue for it?

}
}
2 changes: 1 addition & 1 deletion ...neration/Sources/generate-swift-syntax/templates/swiftparser/ParserTokenSpecSetFile.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ let parserTokenSpecSetFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {

for layoutNode in SYNTAX_NODES.compactMap(\.layoutNode) {
for child in layoutNode.children {
if case let .token(choices, _, _) = child.kind, choices.count > 1 {
if case .token(let choices, _, _) = child.kind, choices.count > 1 {
try! ExtensionDeclSyntax("extension \(layoutNode.kind.syntaxType)") {
try EnumDeclSyntax(
"""
Expand Down
2 changes: 1 addition & 1 deletion Examples/Sources/MacroExamples/Implementation/Accessor/EnvironmentValueMacro.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public struct EnvironmentValueMacro: AccessorMacro {
in context: some MacroExpansionContext
) throws -> [AccessorDeclSyntax] {
guard
case let .argumentList(arguments) = node.arguments,
case .argumentList(let arguments) = node.arguments,
let argument = arguments.first
else { return [] }

Expand Down
4 changes: 2 additions & 2 deletions Examples/Sources/MacroExamples/Implementation/ComplexMacros/OptionSetMacro.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ public struct OptionSetMacro {
) -> (StructDeclSyntax, EnumDeclSyntax, TypeSyntax)? {
// Determine the name of the options enum.
let optionsEnumName: String
if case let .argumentList(arguments) = attribute.arguments,
if case .argumentList(let arguments) = attribute.arguments,
let optionEnumNameArg = arguments.first(labeled: optionsEnumNameArgumentLabel)
{
// We have a options name; make sure it is a string literal.
guard let stringLiteral = optionEnumNameArg.expression.as(StringLiteralExprSyntax.self),
stringLiteral.segments.count == 1,
case let .stringSegment(optionsEnumNameString)? = stringLiteral.segments.first
case .stringSegment(let optionsEnumNameString)? = stringLiteral.segments.first
else {
if emitDiagnostics {
context.diagnose(
Expand Down
2 changes: 1 addition & 1 deletion Examples/Sources/MacroExamples/Implementation/Expression/WarningMacro.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public enum WarningMacro: ExpressionMacro {
let stringLiteral = firstElement.expression
.as(StringLiteralExprSyntax.self),
stringLiteral.segments.count == 1,
case let .stringSegment(messageString)? = stringLiteral.segments.first
case .stringSegment(let messageString)? = stringLiteral.segments.first
else {
throw CustomError.message("#myWarning macro requires a string literal")
}
Expand Down
4 changes: 2 additions & 2 deletions ...ples/Sources/MacroExamples/Implementation/MemberAttribute/WrapStoredPropertiesMacro.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ public struct WrapStoredPropertiesMacro: MemberAttributeMacro {
return []
}

guard case let .argumentList(arguments) = node.arguments,
guard case .argumentList(let arguments) = node.arguments,
let firstElement = arguments.first,
let stringLiteral = firstElement.expression
.as(StringLiteralExprSyntax.self),
stringLiteral.segments.count == 1,
case let .stringSegment(wrapperName)? = stringLiteral.segments.first
case .stringSegment(let wrapperName)? = stringLiteral.segments.first
else {
throw CustomError.message("macro requires a string literal containing the name of an attribute")
}
Expand Down
2 changes: 1 addition & 1 deletion Examples/Sources/MacroExamples/Implementation/Peer/AddAsyncMacro.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public struct AddAsyncMacro: PeerMacro {

// Drop the @addAsync attribute from the new declaration.
let newAttributeList = funcDecl.attributes.filter {
guard case let .attribute(attribute) = $0,
guard case .attribute(let attribute) = $0,
let attributeType = attribute.attributeName.as(IdentifierTypeSyntax.self),
let nodeType = node.attributeName.as(IdentifierTypeSyntax.self)
else {
Expand Down
2 changes: 1 addition & 1 deletion Examples/Sources/MacroExamples/Implementation/Peer/AddCompletionHandlerMacro.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public struct AddCompletionHandlerMacro: PeerMacro {

// Drop the @addCompletionHandler attribute from the new declaration.
let newAttributeList = funcDecl.attributes.filter {
guard case let .attribute(attribute) = $0,
guard case .attribute(let attribute) = $0,
let attributeType = attribute.attributeName.as(IdentifierTypeSyntax.self),
let nodeType = node.attributeName.as(IdentifierTypeSyntax.self)
else {
Expand Down
4 changes: 1 addition & 3 deletions Sources/SwiftCompilerPlugin/CompilerPlugin.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,7 @@ struct CompilerPluginError: Error, CustomStringConvertible {

struct MacroProviderAdapter<Plugin: CompilerPlugin>: PluginProvider {
let plugin: Plugin
init(plugin: Plugin) {
self.plugin = plugin
}

func resolveMacro(moduleName: String, typeName: String) throws -> Macro.Type {
try plugin.resolveMacro(moduleName: moduleName, typeName: typeName)
}
Expand Down
14 changes: 7 additions & 7 deletions Sources/SwiftCompilerPluginMessageHandling/JSON/CodingUtilities.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ internal enum _CodingPathNode {
switch self {
case .root:
return []
case let .node(key, parent):
case .node(let key, let parent):
return parent.path + [key]
case let .indexNode(index, parent):
case .indexNode(let index, let parent):
return parent.path + [_CodingKey(index: index)]
}
}
Expand Down Expand Up @@ -87,17 +87,17 @@ internal enum _CodingKey: CodingKey {

var stringValue: String {
switch self {
case let .string(str): return str
case let .int(int): return "\(int)"
case let .index(index): return "Index \(index)"
case .string(let str): return str
case .int(let int): return "\(int)"
case .index(let index): return "Index \(index)"
}
}

var intValue: Int? {
switch self {
case .string: return nil
case let .int(int): return int
case let .index(index): return index
case .int(let int): return int
case .index(let index): return index
}
}
}
6 changes: 3 additions & 3 deletions Sources/SwiftCompilerPluginMessageHandling/LRUCache.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,17 @@ public class LRUCache<Key: Hashable, Value> {

set {
switch (table[key], newValue) {
case let (nil, newValue?): // create.
case (nil, let newValue?): // create.
self.ensureCapacityForNewValue()
let node = _Node(key: key, value: newValue)
addToHead(node: node)
table[key] = node

case let (node?, newValue?): // update.
case (let node?, let newValue?): // update.
moveToHead(node: node)
node.value = newValue

case let (node?, nil): // delete.
case (let node?, nil): // delete.
remove(node: node)
table[key] = nil

Expand Down
10 changes: 0 additions & 10 deletions Sources/SwiftCompilerPluginMessageHandling/PluginMessages.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,19 +208,9 @@ public enum PluginMessage {
public struct Change: Codable {
public var range: PositionRange
public var newText: String

internal init(range: PositionRange, newText: String) {
self.range = range
self.newText = newText
}
}
public var message: String
public var changes: [Change]

internal init(message: String, changes: [Change]) {
self.message = message
self.changes = changes
}
}
public var message: String
public var severity: Severity
Expand Down
4 changes: 0 additions & 4 deletions Sources/SwiftParser/CharacterInfo.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ extension Character {
fileprivate struct Info: OptionSet {
var rawValue: UInt8

init(rawValue: UInt8) {
self.rawValue = rawValue
}

static let IDENT_START: Self = .init(rawValue: 0x01)
static let IDENT_CONT: Self = .init(rawValue: 0x02)
static let DECIMAL: Self = .init(rawValue: 0x04)
Expand Down
5 changes: 0 additions & 5 deletions Sources/SwiftParser/Declarations.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,6 @@ extension Parser {
struct DeclAttributes {
var attributes: RawAttributeListSyntax
var modifiers: RawDeclModifierListSyntax

init(attributes: RawAttributeListSyntax, modifiers: RawDeclModifierListSyntax) {
self.attributes = attributes
self.modifiers = modifiers
}
}

/// Parse a declaration.
Expand Down
12 changes: 2 additions & 10 deletions Sources/SwiftParser/Statements.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ extension Parser {
}

switch kind {
case let .optional(unexpectedBeforeBindingKeyword, bindingSpecifier, pattern):
case .optional(let unexpectedBeforeBindingKeyword, let bindingSpecifier, let pattern):
return .optionalBinding(
RawOptionalBindingConditionSyntax(
unexpectedBeforeBindingKeyword,
Expand All @@ -304,7 +304,7 @@ extension Parser {
arena: self.arena
)
)
case let .pattern(caseKeyword, pattern):
case .pattern(let caseKeyword, let pattern):
return .matchingPattern(
RawMatchingPatternConditionSyntax(
caseKeyword: caseKeyword,
Expand Down Expand Up @@ -816,14 +816,6 @@ extension Parser {
struct StatementLabel {
var label: RawTokenSyntax
var colon: RawTokenSyntax

init(
label: RawTokenSyntax,
colon: RawTokenSyntax
) {
self.label = label
self.colon = colon
}
}

/// Parse an optional label that defines a named control flow point.
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftRefactor/ConvertComputedPropertyToStored.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import SwiftSyntax
public struct ConvertComputedPropertyToStored: SyntaxRefactoringProvider {
public static func refactor(syntax: VariableDeclSyntax, in context: ()) -> VariableDeclSyntax? {
guard syntax.bindings.count == 1, let binding = syntax.bindings.first,
let accessorBlock = binding.accessorBlock, case let .getter(body) = accessorBlock.accessors, !body.isEmpty
let accessorBlock = binding.accessorBlock, case .getter(let body) = accessorBlock.accessors, !body.isEmpty
else {
return nil
}
Expand Down
6 changes: 3 additions & 3 deletions Sources/SwiftRefactor/ExpandEditorPlaceholder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ struct ExpandSingleEditorPlaceholder: EditRefactoringProvider {

let expanded: String
switch placeholder {
case let .basic(text):
case .basic(let text):
expanded = String(text)
case let .typed(text, type):
case .typed(let text, let type):
if let functionType = type.as(FunctionTypeSyntax.self) {
let basicFormat = BasicFormat(
indentationWidth: context.indentationWidth,
Expand Down Expand Up @@ -326,7 +326,7 @@ extension FunctionCallExprSyntax {
guard let expr = arg.expression.as(DeclReferenceExprSyntax.self),
expr.baseName.isEditorPlaceholder,
let data = EditorPlaceholderData(token: expr.baseName),
case let .typed(_, type) = data,
case .typed(_, let type) = data,
type.is(FunctionTypeSyntax.self)
else {
break
Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftSyntax/Documentation.docc/Resources/Formatter.step10.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ import SwiftSyntax
items[..<pivotPoint]
.sort { lhs, rhs in
guard
case let .import(lhsImport, _) = lhs,
case let .import(rhsImport, _) = rhs
case .import(let lhsImport, _) = lhs,
case .import(let rhsImport, _) = rhs
else {
fatalError("Partition must only contain import items!")
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftSyntax/Documentation.docc/Resources/Formatter.step11.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ import SwiftSyntax
items[..<pivotPoint]
.sort { lhs, rhs in
guard
case let .import(lhsImport, _) = lhs,
case let .import(rhsImport, _) = rhs
case .import(let lhsImport, _) = lhs,
case .import(let rhsImport, _) = rhs
else {
fatalError("Partition must only contain import items!")
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftSyntax/Documentation.docc/Resources/Formatter.step8.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ import SwiftSyntax
items[..<pivotPoint]
.sort { lhs, rhs in
guard
case let .import(lhsImport, _) = lhs,
case let .import(rhsImport, _) = rhs
case .import(let lhsImport, _) = lhs,
case .import(let rhsImport, _) = rhs
else {
fatalError("Partition must only contain import items!")
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftSyntax/Documentation.docc/Resources/Formatter.step9.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ import SwiftSyntax
items[..<pivotPoint]
.sort { lhs, rhs in
guard
case let .import(lhsImport, _) = lhs,
case let .import(rhsImport, _) = rhs
case .import(let lhsImport, _) = lhs,
case .import(let rhsImport, _) = rhs
else {
fatalError("Partition must only contain import items!")
}
Expand Down
28 changes: 14 additions & 14 deletions Sources/SwiftSyntax/SourceLocation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,36 +571,36 @@ fileprivate extension RawTriviaPiece {
) -> SourceLength {
var lineLength = prefix
switch self {
case let .spaces(count),
let .tabs(count),
let .verticalTabs(count),
let .formfeeds(count),
let .backslashes(count),
let .pounds(count):
case .spaces(let count),
.tabs(let count),
.verticalTabs(let count),
.formfeeds(let count),
.backslashes(let count),
.pounds(let count):
lineLength += SourceLength(utf8Length: count)
case let .newlines(count),
let .carriageReturns(count):
case .newlines(let count),
.carriageReturns(let count):
let newLineLength = SourceLength(utf8Length: 1)
body(lineLength + newLineLength)
for _ in 1..<count {
body(newLineLength)
}
lineLength = .zero
case let .carriageReturnLineFeeds(count):
case .carriageReturnLineFeeds(let count):
let carriageReturnLineLength = SourceLength(utf8Length: 2)
body(lineLength + carriageReturnLineLength)
for _ in 1..<count {
body(carriageReturnLineLength)
}
lineLength = .zero
case let .lineComment(text),
let .docLineComment(text):
case .lineComment(let text),
.docLineComment(let text):
// Line comments are not supposed to contain newlines.
precondition(!text.containsSwiftNewline(), "line comment created that contained a new-line character")
lineLength += SourceLength(utf8Length: text.count)
case let .blockComment(text),
let .docBlockComment(text),
let .unexpectedText(text):
case .blockComment(let text),
.docBlockComment(let text),
.unexpectedText(let text):
lineLength = text.forEachLineLength(prefix: lineLength, body: body)
}
return lineLength
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftSyntaxMacroExpansion/MacroReplacement.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ extension MacroDeclSyntax {
) -> ExprSyntax {
// Dig out the argument list.
let argumentList: LabeledExprListSyntax?
if case let .argumentList(argList) = node.arguments {
if case .argumentList(let argList) = node.arguments {
argumentList = argList
} else {
argumentList = nil
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ extension MacroApplication {
}

return attributedNode.attributes.compactMap {
guard case let .attribute(attribute) = $0,
guard case .attribute(let attribute) = $0,
let attributeName = attribute.attributeName.as(IdentifierTypeSyntax.self)?.name.text,
let macroSpec = macroSystem.lookup(attributeName)
else {
Expand Down
4 changes: 2 additions & 2 deletions Sources/_SwiftSyntaxTestSupport/Syntax+Assertions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@ public enum SubtreeError: Error, CustomStringConvertible {

public var description: String {
switch self {
case let .invalidMarker(name):
case .invalidMarker(let name):
return "Could not find marker with name '\(name)'"
case let .invalidSubtree(tree, afterUTF8Offset, type):
case .invalidSubtree(let tree, let afterUTF8Offset, let type):
return
"Could not find subtree after UTF8 offset \(afterUTF8Offset) with type \(type) in:\n\(tree.debugDescription)"
}
Expand Down
Loading