-
Notifications
You must be signed in to change notification settings - Fork 253
Fix type mismatch error in reflowMultilineStringLiterals
#979
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
Fix type mismatch error in reflowMultilineStringLiterals
#979
Conversation
|
Unfortunately, the current behavior is part of the swift-format release in the 6.1 toolchain (Xcode 16.3), so we can't just add a raw value to this enum without breaking how configurations created for that version of the tool would decode. What I think we can do is:
I think the easiest way to implement (2) would be create a second private copy of the enum without the That would avoid us having to bump the configuration version just for this oversight. |
|
@allevato Thanks for your soundness consideration on this issue. I've implement the legacy private enum based on your comment, and I've also added some document to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a couple small tests to Tests/SwiftFormatTests/API/ConfigurationTests.swift that verify that both formats decode correctly, just to make sure we don't accidentally regress this in the future?
| var isNever: Bool { | ||
| self == .never | ||
| } | ||
|
|
||
| var isAlways: Bool { | ||
| self == .always | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need these since nothing else in the formatter will access them.
Documentation/Configuration.md
Outdated
|
|
||
| > [!NOTE] | ||
| > This setting should be specified as a string value (e.g. "never") | ||
| > For backward compatibility with swift-format version 601.0.0, the configuration also accepts the legacy object format where the setting is specified as an object with a single key (e.g., { "never": {} }). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| > For backward compatibility with swift-format version 601.0.0, the configuration also accepts the legacy object format where the setting is specified as an object with a single key (e.g., { "never": {} }). | |
| > For backward compatibility with swift-format version 601.0.0, the configuration also accepts the legacy object format where the setting is specified as an object with a single key (e.g., `{ "never": {} }`). |
|
@allevato comments addressed, please check it again, thx! |
I think you missed this one 🙂 |
|
@allevato Sorry for missing the comment, tests are added, please check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! If you accept the suggestions here, I can kick off the tests.
| typealias MultilineStringReflowBehavior = Configuration.MultilineStringReflowBehavior | ||
|
|
||
| let testCases = [ | ||
| "never": MultilineStringReflowBehavior.never, | ||
| "always": MultilineStringReflowBehavior.always, | ||
| "onlyLinesOverLength": MultilineStringReflowBehavior.onlyLinesOverLength, | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| typealias MultilineStringReflowBehavior = Configuration.MultilineStringReflowBehavior | |
| let testCases = [ | |
| "never": MultilineStringReflowBehavior.never, | |
| "always": MultilineStringReflowBehavior.always, | |
| "onlyLinesOverLength": MultilineStringReflowBehavior.onlyLinesOverLength, | |
| ] | |
| let testCases: [String: Configuration.MultilineStringReflowBehavior] = [ | |
| "never": .never, | |
| "always": .always, | |
| "onlyLinesOverLength": .onlyLinesOverLength, | |
| ] |
| typealias MultilineStringReflowBehavior = Configuration.MultilineStringReflowBehavior | ||
|
|
||
| let testCases = [ | ||
| "{ \"never\": {} }": MultilineStringReflowBehavior.never, | ||
| "{ \"always\": {} }": MultilineStringReflowBehavior.always, | ||
| "{ \"onlyLinesOverLength\": {} }": MultilineStringReflowBehavior.onlyLinesOverLength, | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| typealias MultilineStringReflowBehavior = Configuration.MultilineStringReflowBehavior | |
| let testCases = [ | |
| "{ \"never\": {} }": MultilineStringReflowBehavior.never, | |
| "{ \"always\": {} }": MultilineStringReflowBehavior.always, | |
| "{ \"onlyLinesOverLength\": {} }": MultilineStringReflowBehavior.onlyLinesOverLength, | |
| ] | |
| let testCases: [String: Configuration.MultilineStringReflowBehavior] = [ | |
| "{ \"never\": {} }": .never, | |
| "{ \"always\": {} }": .always, | |
| "{ \"onlyLinesOverLength\": {} }": .onlyLinesOverLength, | |
| ] |
|
@allevato Addressed, thanks for your review! |
|
This looks like a bug in API digester: Adding a raw value to the enum certainly shouldn't have removed any of those declarations; the type is still @ahoppen What do you think? I'm inclined to say that @dongdong867 should just copy these into |
|
Let’s add them to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing this!
Problem description
Environment: swift-format with v601.0.0 or build-in support with Xcode 16.3
Issue:
When running
swift-format <path>command with a custom configuration that containsreflowMultilineStringLiteralswill receive the below errorHow to reproduce:
.swift-formatconfig file in any project with swift-format setupreflowMultilineStringLiteralsbased on the document.swift-format <path>command and the error should show in the consoleRoot cause
The
MultilineStringReflowBehaviorenum in/Sources/SwiftFormat/API/Configuration.swiftis used to parse the value ofreflowMultilineStringLiteralsfrom the config file, however, the origin enum raw type is missing which causes thedecodeIfPresent(_:forKey:)function throws anDecodingError.typeMismatcherror when decoding.The root cause can be verified by running the
swift-format dump-configurationcommand. The console shows that the default value of keyreflowMultilineStringLiteralsis structured as:but according to the documentation, the value of reflowMultilineStringLiterals should be a string like:
Solution
By adding a raw type
Stringto the enumMultilineStringReflowBehavior:This solves the current issue and makes the config file align with what is defined in the documentation.
{ "fileScopedDeclarationPrivacy" : { "accessLevel" : "private" }, "indentConditionalCompilationBlocks" : true, "indentSwitchCaseLabels" : false, "indentation" : { "spaces" : 2 }, "lineBreakAroundMultilineExpressionChainComponents" : false, "lineBreakBeforeControlFlowKeywords" : false, "lineBreakBeforeEachArgument" : false, "lineBreakBeforeEachGenericRequirement" : false, "lineBreakBetweenDeclarationAttributes" : false, "lineLength" : 100, "maximumBlankLines" : 1, "multiElementCollectionTrailingCommas" : true, "noAssignmentInExpressions" : { "allowedFunctions" : [ "XCTAssertNoThrow" ] }, "prioritizeKeepingFunctionOutputTogether" : false, "reflowMultilineStringLiterals" : "never", "respectsExistingLineBreaks" : true, "rules" : { "...ignored with unchanged" } "spacesAroundRangeFormationOperators" : false, "spacesBeforeEndOfLineComments" : 2, "tabWidth" : 8, "version" : 1 }