Skip to content

Commit b72a66e

Browse files
obj-pclaude
andauthored
Fix bugs, reduce duplication, and improve robustness (#66)
* Fix bugs, reduce duplication, and improve robustness across codebase Bugs fixed: - Fix double IOSurfaceUnlock in SimulatorBridge when CIImage creation fails - Replace non-deterministic hashValue with stable FNV-1a hash in PreviewSession module naming - Fix exception safety in HostApp.loadPreview() — validate new loader before retiring old one Duplication reduced: - Extract MCP parameter helpers (extractString/Int/Double/Bool and optional variants) - Consolidate trait validation into PreviewTraits.validated() factory method - Define iOS target triple as PreviewPlatform.targetTriple constant - Replace magic tool name strings with ToolName enum in MCPServer Robustness improvements: - Use null-byte indexed placeholders in LiteralDiffer to prevent source collisions - Validate all paths in FileWatcher init, not just the first - Add LocalizedError conformance to DylibLoaderError and FileWatcherError Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review feedback: error messages, coercion, tests, and cleanup - Add ParamError.wrongType to distinguish missing vs wrong-type params - Handle double→int coercion in extractInt/extractOptionalInt - Fix iOS host app exception safety: set currentDylibHandle after dlsym - Nest TraitValidationError as PreviewTraits.ValidationError - Add tests for PreviewTraits.validated() (4 cases) - Add determinism tests for stableHash (known-value pin) - Remove unused CaseIterable on ToolName - Add null-byte rationale comment in LiteralDiffer - Fix inaccurate doc comment on parseTraits Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 631aa19 commit b72a66e

16 files changed

Lines changed: 300 additions & 207 deletions

Sources/PreviewsCLI/MCPServer.swift

Lines changed: 155 additions & 158 deletions
Large diffs are not rendered by default.

Sources/PreviewsCLI/RunCommand.swift

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,10 @@ struct RunCommand: ParsableCommand {
4545
throw ValidationError("File not found: \(file)")
4646
}
4747

48-
if let cs = colorScheme, !PreviewTraits.validColorSchemes.contains(cs) {
49-
throw ValidationError(
50-
"Invalid color scheme '\(cs)'. Must be 'light' or 'dark'.")
51-
}
52-
if let dts = dynamicTypeSize, !PreviewTraits.validDynamicTypeSizes.contains(dts) {
53-
throw ValidationError(
54-
"Invalid dynamic type size '\(dts)'. Valid values: \(PreviewTraits.validDynamicTypeSizes.sorted().joined(separator: ", "))"
55-
)
48+
do {
49+
_ = try PreviewTraits.validated(colorScheme: colorScheme, dynamicTypeSize: dynamicTypeSize)
50+
} catch {
51+
throw ValidationError(error.localizedDescription)
5652
}
5753

5854
switch platform {

Sources/PreviewsCLI/SnapshotCommand.swift

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,10 @@ struct SnapshotCommand: ParsableCommand {
4747
throw ValidationError("File not found: \(file)")
4848
}
4949

50-
if let cs = colorScheme, !PreviewTraits.validColorSchemes.contains(cs) {
51-
throw ValidationError(
52-
"Invalid color scheme '\(cs)'. Must be 'light' or 'dark'.")
53-
}
54-
if let dts = dynamicTypeSize, !PreviewTraits.validDynamicTypeSizes.contains(dts) {
55-
throw ValidationError(
56-
"Invalid dynamic type size '\(dts)'. Valid values: \(PreviewTraits.validDynamicTypeSizes.sorted().joined(separator: ", "))"
57-
)
50+
do {
51+
_ = try PreviewTraits.validated(colorScheme: colorScheme, dynamicTypeSize: dynamicTypeSize)
52+
} catch {
53+
throw ValidationError(error.localizedDescription)
5854
}
5955

6056
switch platform {

Sources/PreviewsCore/Compiler.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,10 @@ public actor Compiler {
5151
switch platform {
5252
case .macOS:
5353
self.sdkPath = try await Self.resolve("xcrun", "--show-sdk-path")
54-
self.targetTriple = "arm64-apple-macosx14.0"
5554
case .iOS:
5655
self.sdkPath = try await Self.resolve("xcrun", "--show-sdk-path", "--sdk", "iphonesimulator")
57-
self.targetTriple = "arm64-apple-ios17.0-simulator"
5856
}
57+
self.targetTriple = platform.targetTriple
5958
self.swiftcPath = try await Self.resolve("xcrun", "--find", "swiftc")
6059
self.codesignPath = try await Self.resolve("xcrun", "--find", "codesign")
6160
}

Sources/PreviewsCore/DylibLoader.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public final class DylibLoader: @unchecked Sendable {
2929
// Closing a dylib while its types are in use causes crashes.
3030
}
3131

32-
public enum DylibLoaderError: Error, CustomStringConvertible {
32+
public enum DylibLoaderError: Error, LocalizedError, CustomStringConvertible {
3333
case loadFailed(path: String, reason: String)
3434
case symbolNotFound(name: String, reason: String)
3535

@@ -41,4 +41,6 @@ public enum DylibLoaderError: Error, CustomStringConvertible {
4141
return "Symbol '\(name)' not found: \(reason)"
4242
}
4343
}
44+
45+
public var errorDescription: String? { description }
4446
}

Sources/PreviewsCore/FileWatcher.swift

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,13 @@ public final class FileWatcher: @unchecked Sendable {
2626
interval: TimeInterval = 0.5,
2727
callback: @escaping @Sendable () -> Void
2828
) throws {
29-
guard let first = paths.first, FileManager.default.fileExists(atPath: first) else {
30-
throw FileWatcherError.cannotOpen(path: paths.first ?? "<empty>")
29+
guard !paths.isEmpty else {
30+
throw FileWatcherError.cannotOpen(path: "<empty>")
31+
}
32+
for path in paths {
33+
guard FileManager.default.fileExists(atPath: path) else {
34+
throw FileWatcherError.cannotOpen(path: path)
35+
}
3136
}
3237

3338
self.filePaths = paths
@@ -77,7 +82,7 @@ public final class FileWatcher: @unchecked Sendable {
7782
}
7883
}
7984

80-
public enum FileWatcherError: Error, CustomStringConvertible {
85+
public enum FileWatcherError: Error, LocalizedError, CustomStringConvertible {
8186
case cannotOpen(path: String)
8287

8388
public var description: String {
@@ -86,4 +91,6 @@ public enum FileWatcherError: Error, CustomStringConvertible {
8691
return "Cannot watch file: \(path)"
8792
}
8893
}
94+
95+
public var errorDescription: String? { description }
8996
}

Sources/PreviewsCore/LiteralDiffer.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,10 @@ public enum LiteralDiffer {
4848

4949
private static func buildSkeleton(source: String, literals: [RawLiteralEntry]) -> String {
5050
var utf8 = Array(source.utf8)
51-
let placeholder = Array("__LIT__".utf8)
52-
// Replace from back to front
53-
for entry in literals.reversed() {
51+
// Replace from back to front with null-byte-delimited indexed placeholders.
52+
// Null bytes cannot appear in valid Swift source, so these won't collide.
53+
for (index, entry) in literals.enumerated().reversed() {
54+
let placeholder = Array("\0LIT_\(index)\0".utf8)
5455
utf8.replaceSubrange(entry.utf8Start..<entry.utf8End, with: placeholder)
5556
}
5657
return String(decoding: utf8, as: UTF8.self)

Sources/PreviewsCore/Platform.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,12 @@
22
public enum PreviewPlatform: String, Sendable {
33
case macOS
44
case iOS
5+
6+
/// The compiler target triple for this platform.
7+
public var targetTriple: String {
8+
switch self {
9+
case .macOS: return "arm64-apple-macosx14.0"
10+
case .iOS: return "arm64-apple-ios17.0-simulator"
11+
}
12+
}
513
}

Sources/PreviewsCore/PreviewSession.swift

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,19 @@ public actor PreviewSession {
173173

174174
private static func moduleName(for file: URL) -> String {
175175
let stem = file.deletingPathExtension().lastPathComponent
176-
let hash = String(abs(file.path.hashValue), radix: 16).prefix(6)
176+
let hash = String(stableHash(file.path), radix: 16).prefix(6)
177177
return "Preview_\(stem)_\(hash)"
178178
}
179+
180+
/// FNV-1a hash producing a stable, deterministic value across processes.
181+
static func stableHash(_ string: String) -> UInt64 {
182+
var hash: UInt64 = 0xcbf2_9ce4_8422_2325 // FNV offset basis
183+
for byte in string.utf8 {
184+
hash ^= UInt64(byte)
185+
hash &*= 0x0000_0100_0000_01B3 // FNV prime
186+
}
187+
return hash
188+
}
179189
}
180190

181191
public enum PreviewSessionError: Error, LocalizedError, CustomStringConvertible {

Sources/PreviewsCore/PreviewTraits.swift

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,33 @@ public struct PreviewTraits: Sendable, Equatable {
2929
"accessibility1", "accessibility2", "accessibility3",
3030
"accessibility4", "accessibility5",
3131
]
32+
33+
/// Validate optional trait values and return a PreviewTraits, or throw on invalid input.
34+
public static func validated(
35+
colorScheme: String?,
36+
dynamicTypeSize: String?
37+
) throws -> PreviewTraits {
38+
if let cs = colorScheme, !validColorSchemes.contains(cs) {
39+
throw ValidationError.invalidColorScheme(cs)
40+
}
41+
if let dts = dynamicTypeSize, !validDynamicTypeSizes.contains(dts) {
42+
throw ValidationError.invalidDynamicTypeSize(dts)
43+
}
44+
return PreviewTraits(colorScheme: colorScheme, dynamicTypeSize: dynamicTypeSize)
45+
}
46+
47+
public enum ValidationError: Error, LocalizedError {
48+
case invalidColorScheme(String)
49+
case invalidDynamicTypeSize(String)
50+
51+
public var errorDescription: String? {
52+
switch self {
53+
case .invalidColorScheme(let cs):
54+
return "Invalid color scheme '\(cs)'. Must be 'light' or 'dark'."
55+
case .invalidDynamicTypeSize(let dts):
56+
return
57+
"Invalid dynamic type size '\(dts)'. Valid values: \(PreviewTraits.validDynamicTypeSizes.sorted().joined(separator: ", "))"
58+
}
59+
}
60+
}
3261
}

0 commit comments

Comments
 (0)