Skip to content

Commit 213f657

Browse files
committed
fix(lint): eliminate silent failures in PathDataLengthRule and improve test coverage
- Emit warning diagnostic instead of silently returning empty array when URL parsing fails - Emit warning when Figma returns nil SVG URL for components (previously silently dropped) - Include icon names in batch failure diagnostics for better debuggability - Extract validateParsedSVG as internal method for unit testability - Add tests: components API failure, RTL variant filtering, variant deduplication, nil SVG URL handling, pathData critical error (happy path), short pathData (no error) - Update LintEngine rule count test from 8 to 9 - Add SVGKit dependency to ExFigTests for direct ParsedSVG construction - Document lint rule testability patterns in CLAUDE.md
1 parent 507d70a commit 213f657

4 files changed

Lines changed: 164 additions & 7 deletions

File tree

CLAUDE.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,8 @@ Uses `FigmaAPI.Client.request(SomeEndpoint(...))` directly (no convenience metho
341341
- Lint rules checking component names MUST use `comp.iconName` (not `comp.name`) — for variants, `name` is the variant value, not the icon name
342342
- Deduplicate variants by `containingComponentSet.nodeId` before grouping — multiple variants of one set are NOT duplicates
343343
- Adding error handling to `check()` increases cyclomatic complexity — extract per-entry logic into private methods
344+
- For unit-testable validation logic in lint rules, use `internal` (not `private`) methods — allows `@testable import` testing without network calls (e.g., `validateParsedSVG`)
345+
- `LintEngineTests.defaultEngineHasAllRules` checks exact set of rule IDs — must add new rule ID when registering in `LintEngine.default`
344346
- `NodesEndpoint` supports `geometry: .paths` parameter — returns `fillGeometry`/`strokeGeometry` with SVG path data on vector nodes. **Not suitable for pathData validation** — Figma's SVG export flattens masks/booleans into different paths than raw geometry
345347
- `PathDataLengthRule` checks ALL platform icon entries (iOS/Android/Flutter/Web), deduplicates by fileId+frame+page. Downloads SVGs via `ImageEndpoint` + `URLSession`, parses with `SVGParser`, validates with `PathDataValidator`. Only reports critical >32,767 byte errors (800-char threshold removed as too noisy). Groups by fileId, batches ImageEndpoint by 50, parallelizes SVG downloads (max 10 concurrent) and fileIds
346348

@@ -517,6 +519,7 @@ NooraUI.formatLink("url", useColors: true) // underlined primary
517519
| JSON output empty in quiet | `ui.info()` suppressed when `outputMode == .quiet` — machine-readable output (JSON) must use `TerminalOutputManager.shared.print()` directly |
518520
| `Bundle.module` in tests | SPM test targets without declared resources don't have `Bundle.module` — use `Bundle.main` or temp bundle |
519521
| SwiftFormat breaks `::` syntax | SwiftFormat 0.60.1+ required for Swift 6.3 module selectors (`FigmaAPI::Client`) |
522+
| `SVGKit` types in tests | Add `.product(name: "SVGKit", package: "swift-svgkit")` to ExFigTests dependencies in Package.swift for direct `ParsedSVG`/`SVGPath` construction |
520523
| `fillGeometry` wrong pathData | `fillGeometry` returns raw node geometry, NOT the SVG export result. Figma flattens masks/booleans into different paths. Use `ImageEndpoint` SVG download + `SVGParser` for accurate pathData validation |
521524
| `NodesEndpoint` no `fillGeometry` | Requires `geometry: .paths` param (swift-figma-api 0.4.0+). Fields are `nil` without it. Useful for geometry inspection but NOT for pathData length validation |
522525
| MCP SDK 0.12.0 breaking | `.text` has 3 associated values — pattern match as `.text(text, _, _)`; `GetPrompt.arguments` is `[String: String]?` now |

Package.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ let package = Package(
226226
dependencies: [
227227
"ExFigCLI",
228228
.product(name: "FigmaAPI", package: "swift-figma-api"),
229+
.product(name: "SVGKit", package: "swift-svgkit"),
229230
"ExFig-Flutter",
230231
"ExFig-Web",
231232
.product(name: "CustomDump", package: "swift-custom-dump"),

Sources/ExFigCLI/Lint/Rules/PathDataLengthRule.swift

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -125,21 +125,37 @@ struct PathDataLengthRule: LintRule {
125125
ImageEndpoint(fileId: fileId, nodeIds: nodeIds, params: SVGParams())
126126
)
127127
} catch {
128+
let batchNames = batch.prefix(5).map(\.iconName).joined(separator: ", ")
129+
let suffix = batch.count > 5 ? " and \(batch.count - 5) more" : ""
130+
let msg = "Cannot fetch SVG URLs for \(batch.count) icon(s) "
131+
+ "(\(batchNames)\(suffix)): \(error.localizedDescription)"
128132
diagnostics.append(diagnostic(
129133
severity: .warning,
130-
message: "Cannot fetch SVG URLs: \(error.localizedDescription)",
134+
message: msg,
131135
suggestion: "Check FIGMA_PERSONAL_TOKEN and file permissions"
132136
))
133137
continue
134138
}
135139

140+
var missingURLNames: [String] = []
136141
for comp in batch {
137142
if let urlOpt = imageURLs[comp.nodeId], let url = urlOpt {
138143
results.append(SVGItem(
139144
name: comp.iconName, nodeId: comp.nodeId, url: url
140145
))
146+
} else {
147+
missingURLNames.append(comp.iconName)
141148
}
142149
}
150+
if !missingURLNames.isEmpty {
151+
let names = missingURLNames.prefix(5).joined(separator: ", ")
152+
let suffix = missingURLNames.count > 5 ? " and \(missingURLNames.count - 5) more" : ""
153+
diagnostics.append(diagnostic(
154+
severity: .warning,
155+
message: "Figma returned no SVG URL for \(missingURLNames.count) icon(s): \(names)\(suffix)",
156+
suggestion: "These icons could not be rendered — check they are not empty components"
157+
))
158+
}
143159
}
144160

145161
return results
@@ -177,7 +193,15 @@ struct PathDataLengthRule: LintRule {
177193
}
178194

179195
private func validateSingleIcon(item: SVGItem) async -> [LintDiagnostic] {
180-
guard let url = URL(string: item.url) else { return [] }
196+
guard let url = URL(string: item.url) else {
197+
return [diagnostic(
198+
severity: .warning,
199+
message: "Invalid SVG URL for '\(item.name)' — cannot validate pathData",
200+
componentName: item.name,
201+
nodeId: item.nodeId,
202+
suggestion: "Re-run lint; if persistent, the Figma API may be returning malformed URLs"
203+
)]
204+
}
181205

182206
let svgData: Data
183207
do {
@@ -205,7 +229,13 @@ struct PathDataLengthRule: LintRule {
205229
)]
206230
}
207231

208-
let issues = PathDataValidator().validate(svg: svg, iconName: item.name)
232+
return validateParsedSVG(svg, name: item.name, nodeId: item.nodeId)
233+
}
234+
235+
/// Validates a parsed SVG and returns diagnostics for critical pathData issues.
236+
/// Internal for testability.
237+
func validateParsedSVG(_ svg: ParsedSVG, name: String, nodeId: String) -> [LintDiagnostic] {
238+
let issues = PathDataValidator().validate(svg: svg, iconName: name)
209239

210240
// Only report critical issues (>32,767 bytes) — the 800-char lint threshold
211241
// is too noisy for most icon sets (flags, illustrations regularly exceed it)
@@ -215,11 +245,11 @@ struct PathDataLengthRule: LintRule {
215245
severity: .error,
216246
message: """
217247
pathData exceeds 32,767 bytes (\(issue.result.byteLength) bytes) \
218-
in \(item.name)/\(issue.pathName). \
248+
in \(name)/\(issue.pathName). \
219249
This will cause STRING_TOO_LARGE error during Android build.
220250
""",
221-
componentName: item.name,
222-
nodeId: item.nodeId,
251+
componentName: name,
252+
nodeId: nodeId,
223253
suggestion: "Simplify the path in Figma or use raster format (PNG/WebP)"
224254
)
225255
}

Tests/ExFigTests/Lint/LintRulesTests.swift

Lines changed: 124 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import ExFigConfig
44
import ExFigCore
55
import FigmaAPI
66
import Foundation
7+
import SVGKit
78
import Testing
89

910
// MARK: - Test Helpers
@@ -629,7 +630,7 @@ struct LintEngineTests {
629630
#expect(!diagnostics.contains { $0.ruleId == "component-not-frame" })
630631
}
631632

632-
@Test("default engine registers all 8 rules")
633+
@Test("default engine registers all 9 rules")
633634
func defaultEngineHasAllRules() {
634635
let ruleIds = Set(LintEngine.default.rules.map(\.id))
635636
let expected: Set = [
@@ -641,6 +642,7 @@ struct LintEngineTests {
641642
"alias-chain-integrity",
642643
"dark-mode-variables",
643644
"dark-mode-suffix",
645+
"path-data-length",
644646
]
645647
#expect(ruleIds == expected)
646648
}
@@ -807,6 +809,127 @@ struct PathDataLengthRuleTests {
807809
#expect(warnings.count == 1)
808810
#expect(warnings.first?.componentName == "ic_a")
809811
}
812+
813+
@Test("emits error when components API fails")
814+
func emitsErrorWhenComponentsFail() async throws {
815+
let client = MockClient()
816+
client.setError(URLError(.notConnectedToInternet), for: ComponentsEndpoint.self)
817+
818+
let config = makeAndroidIconsConfig(frameName: "Icons")
819+
let context = makeLintContext(config: config, client: client)
820+
let diagnostics = try await rule.check(context: context)
821+
822+
#expect(diagnostics.count == 1)
823+
#expect(diagnostics.first?.severity == .error)
824+
#expect(diagnostics.first?.message.contains("Cannot fetch components") == true)
825+
}
826+
827+
@Test("skips RTL variants")
828+
func skipsRTLVariants() async throws {
829+
let client = MockClient()
830+
client.setResponse([
831+
Component.make(nodeId: "1:1", name: "ic_home", frameName: "Icons", pageName: "Page"),
832+
makeVariantComponent(
833+
nodeId: "1:2", name: "RTL=On", frameName: "Icons", pageName: "Page",
834+
componentSetName: "ic_home"
835+
),
836+
], for: ComponentsEndpoint.self)
837+
838+
let imageURLs: [NodeId: ImagePath?] = ["1:1": "https://invalid.test/home.svg"]
839+
client.setResponse(imageURLs, for: ImageEndpoint.self)
840+
841+
let config = makeAndroidIconsConfig(frameName: "Icons", pageName: "Page")
842+
let context = makeLintContext(config: config, client: client)
843+
let diagnostics = try await rule.check(context: context)
844+
845+
// Only ic_home checked, RTL variant skipped — one warning from download failure
846+
let warnings = diagnostics.filter { $0.severity == .warning }
847+
#expect(warnings.count == 1)
848+
#expect(warnings.first?.componentName == "ic_home")
849+
}
850+
851+
@Test("deduplicates variants by component set")
852+
func deduplicatesVariants() async throws {
853+
let client = MockClient()
854+
client.setResponse([
855+
makeVariantComponent(
856+
nodeId: "1:1", name: "Style=Default", frameName: "Icons", pageName: "Page",
857+
componentSetName: "ic_star", componentSetNodeId: "set:1"
858+
),
859+
makeVariantComponent(
860+
nodeId: "1:2", name: "Style=Filled", frameName: "Icons", pageName: "Page",
861+
componentSetName: "ic_star", componentSetNodeId: "set:1"
862+
),
863+
], for: ComponentsEndpoint.self)
864+
865+
// Only one SVG URL — only first variant should be checked
866+
let imageURLs: [NodeId: ImagePath?] = ["1:1": "https://invalid.test/star.svg"]
867+
client.setResponse(imageURLs, for: ImageEndpoint.self)
868+
869+
let config = makeAndroidIconsConfig(frameName: "Icons", pageName: "Page")
870+
let context = makeLintContext(config: config, client: client)
871+
let diagnostics = try await rule.check(context: context)
872+
873+
// One warning from download failure — proves only one variant was checked
874+
let warnings = diagnostics.filter { $0.severity == .warning }
875+
#expect(warnings.count == 1)
876+
}
877+
878+
@Test("warns when Figma returns nil SVG URL")
879+
func warnsWhenNilSVGURL() async throws {
880+
let client = MockClient()
881+
client.setResponse([
882+
Component.make(nodeId: "1:1", name: "ic_empty", frameName: "Icons", pageName: "Page"),
883+
], for: ComponentsEndpoint.self)
884+
885+
// ImageEndpoint returns nil URL for the component
886+
let imageURLs: [NodeId: ImagePath?] = ["1:1": nil]
887+
client.setResponse(imageURLs, for: ImageEndpoint.self)
888+
889+
let config = makeAndroidIconsConfig(frameName: "Icons", pageName: "Page")
890+
let context = makeLintContext(config: config, client: client)
891+
let diagnostics = try await rule.check(context: context)
892+
893+
#expect(diagnostics.contains { $0.message.contains("no SVG URL") })
894+
}
895+
896+
@Test("validates pathData and reports critical errors")
897+
func reportsPathDataCriticalError() {
898+
// Generate a path that exceeds 32,767 bytes
899+
let longPath = String(repeating: "M0 0L1 1", count: 5000)
900+
let svg = ParsedSVG(
901+
width: 24, height: 24, viewportWidth: 24, viewportHeight: 24,
902+
paths: [SVGPath(
903+
pathData: longPath, commands: [], fill: nil, fillType: .none,
904+
stroke: nil, strokeWidth: nil, strokeLineCap: nil, strokeLineJoin: nil,
905+
strokeDashArray: nil, strokeDashOffset: nil, fillRule: nil, opacity: nil,
906+
fillOpacity: nil
907+
)]
908+
)
909+
910+
let diagnostics = rule.validateParsedSVG(svg, name: "ic_complex", nodeId: "1:1")
911+
912+
#expect(diagnostics.count == 1)
913+
#expect(diagnostics.first?.severity == .error)
914+
#expect(diagnostics.first?.message.contains("32,767 bytes") == true)
915+
#expect(diagnostics.first?.componentName == "ic_complex")
916+
}
917+
918+
@Test("no error for short pathData")
919+
func noErrorForShortPathData() {
920+
let svg = ParsedSVG(
921+
width: 24, height: 24, viewportWidth: 24, viewportHeight: 24,
922+
paths: [SVGPath(
923+
pathData: "M12 2L22 12L12 22L2 12Z", commands: [], fill: nil, fillType: .none,
924+
stroke: nil, strokeWidth: nil, strokeLineCap: nil, strokeLineJoin: nil,
925+
strokeDashArray: nil, strokeDashOffset: nil, fillRule: nil, opacity: nil,
926+
fillOpacity: nil
927+
)]
928+
)
929+
930+
let diagnostics = rule.validateParsedSVG(svg, name: "ic_simple", nodeId: "1:1")
931+
#expect(diagnostics.isEmpty)
932+
}
810933
}
811934

812935
// swiftlint:enable file_length

0 commit comments

Comments
 (0)