From 39fb855de83a1b5e43292b795e68f41e81f46053 Mon Sep 17 00:00:00 2001 From: alexey1312 Date: Tue, 31 Mar 2026 13:08:10 +0500 Subject: [PATCH 1/6] feat(cli): add `exfig lint` command for Figma file validation Validates Figma file structure against PKL config before export. 7 rules checking naming conventions, frame/page structure, variable bindings, dark mode setup, and component types. Rules: - frame-page-match: frame/page names exist in Figma file - naming-convention: component names match nameValidateRegexp - component-not-frame: configured frames have published components - deleted-variables: no deletedButReferenced variables - alias-chain-integrity: alias chains resolve without broken refs - dark-mode-variables: fills bound to Variables when variablesDarkMode - dark-mode-suffix: light components have matching _dark pairs Supports --rules filter, --format json for CI, --severity filter, and exit code 1 on errors. --- Sources/ExFigCLI/ExFigCommand.swift | 1 + Sources/ExFigCLI/Lint/LintEngine.swift | 64 ++++++++ Sources/ExFigCLI/Lint/LintReporter.swift | 98 ++++++++++++ Sources/ExFigCLI/Lint/LintTypes.swift | 47 ++++++ .../Lint/Rules/AliasChainIntegrityRule.swift | 126 +++++++++++++++ .../Lint/Rules/ComponentNotFrameRule.swift | 103 +++++++++++++ .../Lint/Rules/DarkModeSuffixRule.swift | 58 +++++++ .../Lint/Rules/DarkModeVariablesRule.swift | 132 ++++++++++++++++ .../Lint/Rules/DeletedVariablesRule.swift | 76 +++++++++ .../Lint/Rules/FramePageMatchRule.swift | 145 ++++++++++++++++++ .../Lint/Rules/NamingConventionRule.swift | 121 +++++++++++++++ Sources/ExFigCLI/Subcommands/Lint.swift | 85 ++++++++++ 12 files changed, 1056 insertions(+) create mode 100644 Sources/ExFigCLI/Lint/LintEngine.swift create mode 100644 Sources/ExFigCLI/Lint/LintReporter.swift create mode 100644 Sources/ExFigCLI/Lint/LintTypes.swift create mode 100644 Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift create mode 100644 Sources/ExFigCLI/Lint/Rules/ComponentNotFrameRule.swift create mode 100644 Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift create mode 100644 Sources/ExFigCLI/Lint/Rules/DarkModeVariablesRule.swift create mode 100644 Sources/ExFigCLI/Lint/Rules/DeletedVariablesRule.swift create mode 100644 Sources/ExFigCLI/Lint/Rules/FramePageMatchRule.swift create mode 100644 Sources/ExFigCLI/Lint/Rules/NamingConventionRule.swift create mode 100644 Sources/ExFigCLI/Subcommands/Lint.swift diff --git a/Sources/ExFigCLI/ExFigCommand.swift b/Sources/ExFigCLI/ExFigCommand.swift index ee8bc7f7..a7d61f2d 100644 --- a/Sources/ExFigCLI/ExFigCommand.swift +++ b/Sources/ExFigCLI/ExFigCommand.swift @@ -114,6 +114,7 @@ extension ExFigCommand { Download.self, Tokens.self, Batch.self, + Lint.self, ] #if canImport(MCP) commands.append(MCPServe.self) diff --git a/Sources/ExFigCLI/Lint/LintEngine.swift b/Sources/ExFigCLI/Lint/LintEngine.swift new file mode 100644 index 00000000..9052a634 --- /dev/null +++ b/Sources/ExFigCLI/Lint/LintEngine.swift @@ -0,0 +1,64 @@ +import Foundation + +/// Engine that runs lint rules against a PKL configuration. +struct LintEngine { + /// All registered lint rules. + let rules: [any LintRule] + + /// Run all applicable rules (or filtered subset) and return diagnostics. + func run( + context: LintContext, + ruleFilter: Set? = nil, + minSeverity: LintSeverity = .info + ) async throws -> [LintDiagnostic] { + let applicableRules = rules.filter { rule in + if let filter = ruleFilter, !filter.contains(rule.id) { + return false + } + return severityRank(rule.severity) >= severityRank(minSeverity) + } + + var allDiagnostics: [LintDiagnostic] = [] + + for rule in applicableRules { + do { + let diagnostics = try await rule.check(context: context) + allDiagnostics.append(contentsOf: diagnostics) + } catch { + // Rule failure becomes an info diagnostic + allDiagnostics.append(LintDiagnostic( + ruleId: rule.id, + ruleName: rule.name, + severity: .info, + message: "Rule check failed: \(error.localizedDescription)", + componentName: nil, + nodeId: nil, + suggestion: nil + )) + } + } + + return allDiagnostics + } + + private func severityRank(_ severity: LintSeverity) -> Int { + switch severity { + case .error: 2 + case .warning: 1 + case .info: 0 + } + } +} + +extension LintEngine { + /// Default engine with all built-in rules. + static let `default` = LintEngine(rules: [ + FramePageMatchRule(), + NamingConventionRule(), + ComponentNotFrameRule(), + DeletedVariablesRule(), + AliasChainIntegrityRule(), + DarkModeVariablesRule(), + DarkModeSuffixRule(), + ]) +} diff --git a/Sources/ExFigCLI/Lint/LintReporter.swift b/Sources/ExFigCLI/Lint/LintReporter.swift new file mode 100644 index 00000000..c6c659e0 --- /dev/null +++ b/Sources/ExFigCLI/Lint/LintReporter.swift @@ -0,0 +1,98 @@ +import ExFigCore +import Foundation +import Noora + +/// Output format for lint results. +enum LintOutputFormat: String { + case text + case json +} + +/// Formats and outputs lint results. +struct LintReporter { + let format: LintOutputFormat + let useColors: Bool + + func report(diagnostics: [LintDiagnostic], ui: TerminalUI) throws { + switch format { + case .text: + reportText(diagnostics: diagnostics, ui: ui) + case .json: + try reportJSON(diagnostics: diagnostics) + } + } + + // MARK: - Text Output + + private func reportText(diagnostics: [LintDiagnostic], ui: TerminalUI) { + if diagnostics.isEmpty { + ui.success("All lint checks passed") + return + } + + let errors = diagnostics.filter { $0.severity == .error } + let warnings = diagnostics.filter { $0.severity == .warning } + let infos = diagnostics.filter { $0.severity == .info } + + // Summary line + var parts: [String] = [] + if !errors.isEmpty { + parts.append(NooraUI.format(.danger("\(errors.count) error(s)"))) + } + if !warnings.isEmpty { + parts.append(NooraUI.format(.accent("\(warnings.count) warning(s)"))) + } + if !infos.isEmpty { + parts.append(NooraUI.format(.muted("\(infos.count) info(s)"))) + } + ui.info("Lint: \(parts.joined(separator: ", "))") + + // Group by rule + let grouped = Dictionary(grouping: diagnostics) { $0.ruleId } + for (ruleId, items) in grouped.sorted(by: { $0.key < $1.key }) { + let first = items[0] + let icon = severityIcon(first.severity) + ui.info("\(icon) \(first.ruleName) [\(ruleId)] (\(items.count))") + + for diag in items.prefix(10) { + let name = diag.componentName ?? diag.nodeId ?? "unknown" + ui.info(" \(name): \(diag.message)") + if let suggestion = diag.suggestion { + ui.info(" → \(suggestion)") + } + } + if items.count > 10 { + ui.info(" ... +\(items.count - 10) more") + } + } + } + + private func severityIcon(_ severity: LintSeverity) -> String { + switch severity { + case .error: useColors ? NooraUI.format(.danger("✗")) : "✗" + case .warning: useColors ? NooraUI.format(.accent("⚠")) : "⚠" + case .info: useColors ? NooraUI.format(.muted("ℹ")) : "ℹ" + } + } + + // MARK: - JSON Output + + private func reportJSON(diagnostics: [LintDiagnostic]) throws { + let report = LintReport( + diagnosticsCount: diagnostics.count, + errorsCount: diagnostics.filter { $0.severity == .error }.count, + warningsCount: diagnostics.filter { $0.severity == .warning }.count, + diagnostics: diagnostics + ) + let data = try JSONCodec.encode(report) + print(String(data: data, encoding: .utf8) ?? "{}") + } +} + +/// Top-level JSON report structure. +private struct LintReport: Codable { + let diagnosticsCount: Int + let errorsCount: Int + let warningsCount: Int + let diagnostics: [LintDiagnostic] +} diff --git a/Sources/ExFigCLI/Lint/LintTypes.swift b/Sources/ExFigCLI/Lint/LintTypes.swift new file mode 100644 index 00000000..0569d02f --- /dev/null +++ b/Sources/ExFigCLI/Lint/LintTypes.swift @@ -0,0 +1,47 @@ +import ExFigConfig +import ExFigCore +import FigmaAPI +import Foundation + +/// Severity level for lint diagnostics. +enum LintSeverity: String, CaseIterable, Codable { + case error + case warning + case info +} + +/// A single finding from a lint rule. +struct LintDiagnostic: Codable { + let ruleId: String + let ruleName: String + let severity: LintSeverity + let message: String + let componentName: String? + let nodeId: String? + let suggestion: String? +} + +/// Context provided to each lint rule for checking. +struct LintContext { + /// The resolved PKL configuration. + let config: ExFig.ModuleImpl + /// Figma API client. + let client: any FigmaAPI.Client + /// Terminal UI for progress reporting. + let ui: TerminalUI +} + +/// Protocol for lint rules. +protocol LintRule: Sendable { + /// Unique rule identifier (kebab-case). + var id: String { get } + /// Human-readable rule name. + var name: String { get } + /// Description of what this rule checks. + var description: String { get } + /// Default severity. + var severity: LintSeverity { get } + + /// Run the check and return diagnostics (empty = all good). + func check(context: LintContext) async throws -> [LintDiagnostic] +} diff --git a/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift b/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift new file mode 100644 index 00000000..086956be --- /dev/null +++ b/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift @@ -0,0 +1,126 @@ +import ExFigConfig +import ExFigCore +import FigmaAPI +import Foundation + +/// Checks that all variable alias chains resolve successfully (no broken refs, depth <= 10). +struct AliasChainIntegrityRule: LintRule { + let id = "alias-chain-integrity" + let name = "Alias chain integrity" + let description = "All variable alias chains must resolve without broken references" + let severity: LintSeverity = .error + + private let maxDepth = 10 + + func check(context: LintContext) async throws -> [LintDiagnostic] { + let config = context.config + let fileId = config.figma?.lightFileId ?? "" + guard !fileId.isEmpty else { return [] } + + let variables: VariablesMeta + do { + variables = try await context.client.request(VariablesEndpoint(fileId: fileId)) + } catch { + return [] + } + + var diagnostics: [LintDiagnostic] = [] + + for (variableId, variable) in variables.variables { + if variable.deletedButReferenced == true { continue } + + for (modeId, value) in variable.valuesByMode { + let result = resolveChain( + value: value, + variables: variables.variables, + visited: [variableId], + depth: 0 + ) + + switch result { + case .resolved: + break + case let .broken(targetId): + diagnostics.append(LintDiagnostic( + ruleId: id, + ruleName: name, + severity: .error, + message: "'\(variable.name)' mode '\(modeId)' refs non-existent '\(targetId)'", + componentName: variable.name, + nodeId: variableId, + suggestion: "Fix or remove the alias reference" + )) + case .circular: + diagnostics.append(LintDiagnostic( + ruleId: id, + ruleName: name, + severity: .error, + message: "'\(variable.name)' has a circular alias chain", + componentName: variable.name, + nodeId: variableId, + suggestion: "Break the circular reference" + )) + case .tooDeep: + diagnostics.append(LintDiagnostic( + ruleId: id, + ruleName: name, + severity: .warning, + message: "'\(variable.name)' alias chain exceeds depth \(maxDepth)", + componentName: variable.name, + nodeId: variableId, + suggestion: "Simplify the alias chain" + )) + } + } + } + + return diagnostics + } + + // MARK: - Chain Resolution + + private enum ChainResult { + case resolved + case broken(targetId: String) + case circular + case tooDeep + } + + private func resolveChain( + value: ValuesByMode, + variables: [String: VariableValue], + visited: Set, + depth: Int + ) -> ChainResult { + guard depth < maxDepth else { return .tooDeep } + + // Check if value is an alias + guard case let .variableAlias(alias) = value else { + return .resolved // Primitive value (color, string, number, boolean) + } + + let aliasId = alias.id + + if visited.contains(aliasId) { + return .circular + } + + guard let target = variables[aliasId] else { + return .broken(targetId: aliasId) + } + + // Follow the chain — use first available mode value + guard let nextValue = target.valuesByMode.values.first else { + return .resolved + } + + var newVisited = visited + newVisited.insert(aliasId) + return resolveChain( + value: nextValue, + variables: variables, + visited: newVisited, + depth: depth + 1 + ) + } +} diff --git a/Sources/ExFigCLI/Lint/Rules/ComponentNotFrameRule.swift b/Sources/ExFigCLI/Lint/Rules/ComponentNotFrameRule.swift new file mode 100644 index 00000000..b466e321 --- /dev/null +++ b/Sources/ExFigCLI/Lint/Rules/ComponentNotFrameRule.swift @@ -0,0 +1,103 @@ +import ExFigConfig +import ExFigCore +import FigmaAPI +import Foundation + +/// Checks that exported icons/images are Figma components (not plain frames). +/// If a frame configured for export has zero published components, it likely contains +/// plain frames instead of components. +struct ComponentNotFrameRule: LintRule { + let id = "component-not-frame" + let name = "Assets are components" + let description = "Configured frames must contain published components" + let severity: LintSeverity = .error + + func check(context: LintContext) async throws -> [LintDiagnostic] { + let config = context.config + let defaultFileId = config.figma?.lightFileId ?? "" + guard !defaultFileId.isEmpty else { return [] } + + var diagnostics: [LintDiagnostic] = [] + + // Collect frame names from config + let entries = collectFrameEntries(from: config, defaultFileId: defaultFileId) + guard !entries.isEmpty else { return [] } + + let grouped = Dictionary(grouping: entries) { $0.fileId } + + for (fileId, fileEntries) in grouped { + guard !fileId.isEmpty else { continue } + + let components: [Component] + do { + components = try await context.client.request(ComponentsEndpoint(fileId: fileId)) + } catch { + continue + } + + for entry in fileEntries { + guard let frameName = entry.frameName else { continue } + + let matchingComponents = components.filter { comp in + let frameMatch = comp.containingFrame.name == frameName + if let pageName = entry.pageName { + return frameMatch && comp.containingFrame.pageName == pageName + } + return frameMatch + } + + if matchingComponents.isEmpty { + diagnostics.append(LintDiagnostic( + ruleId: id, + ruleName: name, + severity: .error, + message: "Frame '\(frameName)' has no published components", + componentName: nil, + nodeId: nil, + suggestion: "Convert frames to Components (⌥⌘K) and publish to Team Library" + )) + } + } + } + + return diagnostics + } + + private struct FrameEntry { + let fileId: String + let frameName: String? + let pageName: String? + } + + private func collectFrameEntries(from config: PKLConfig, defaultFileId: String) -> [FrameEntry] { + var entries: [FrameEntry] = [] + + func add(_ icons: [some Common_FrameSource]?) { + for entry in icons ?? [] { + entries.append(FrameEntry( + fileId: entry.figmaFileId ?? defaultFileId, + frameName: entry.figmaFrameName, + pageName: entry.figmaPageName + )) + } + } + + if let ios = config.ios { + add(ios.icons) + add(ios.images) + } + if let android = config.android { + add(android.icons) + add(android.images) + } + if let flutter = config.flutter { + add(flutter.icons) + add(flutter.images) + } + if let web = config.web { + add(web.icons) + } + + return entries + } +} diff --git a/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift b/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift new file mode 100644 index 00000000..eae9fbac --- /dev/null +++ b/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift @@ -0,0 +1,58 @@ +import ExFigConfig +import ExFigCore +import FigmaAPI +import Foundation + +/// When suffixDarkMode is configured, checks that each light component has a matching dark pair. +struct DarkModeSuffixRule: LintRule { + let id = "dark-mode-suffix" + let name = "Dark mode suffix pairs" + let description = "With suffixDarkMode, each light component needs a matching dark pair" + let severity: LintSeverity = .warning + + func check(context: LintContext) async throws -> [LintDiagnostic] { + let config = context.config + var diagnostics: [LintDiagnostic] = [] + + // Check if suffixDarkMode is configured + let suffix: String + if let sdm = config.common?.icons?.suffixDarkMode { + suffix = sdm.suffix + } else if let sdm = config.common?.images?.suffixDarkMode { + suffix = sdm.suffix + } else { + return [] + } + + let fileId = config.figma?.lightFileId ?? "" + guard !fileId.isEmpty else { return [] } + + let components: [Component] + do { + components = try await context.client.request(ComponentsEndpoint(fileId: fileId)) + } catch { + return [] + } + + let allNames = Set(components.map(\.name)) + + for component in components { + if component.name.hasSuffix(suffix) { continue } + + let expectedDark = component.name + suffix + if !allNames.contains(expectedDark) { + diagnostics.append(LintDiagnostic( + ruleId: id, + ruleName: name, + severity: .warning, + message: "'\(component.name)' has no dark pair '\(expectedDark)'", + componentName: component.name, + nodeId: component.nodeId, + suggestion: "Create a component named '\(expectedDark)'" + )) + } + } + + return diagnostics + } +} diff --git a/Sources/ExFigCLI/Lint/Rules/DarkModeVariablesRule.swift b/Sources/ExFigCLI/Lint/Rules/DarkModeVariablesRule.swift new file mode 100644 index 00000000..be52f837 --- /dev/null +++ b/Sources/ExFigCLI/Lint/Rules/DarkModeVariablesRule.swift @@ -0,0 +1,132 @@ +import ExFigConfig +import ExFigCore +import FigmaAPI +import Foundation + +/// When variablesDarkMode is configured, checks that icon fills are bound to Variables. +/// Unbound (hardcoded hex) fills are silently skipped by VariableModeDarkGenerator. +struct DarkModeVariablesRule: LintRule { + let id = "dark-mode-variables" + let name = "Dark mode fills bound to variables" + let description = "With variablesDarkMode, icon fills must be bound to Variables" + let severity: LintSeverity = .error + + func check(context: LintContext) async throws -> [LintDiagnostic] { + let config = context.config + let defaultFileId = config.figma?.lightFileId ?? "" + var diagnostics: [LintDiagnostic] = [] + + let entries = collectVDMEntries(from: config, defaultFileId: defaultFileId) + guard !entries.isEmpty else { return [] } + + for entry in entries { + guard !entry.fileId.isEmpty else { continue } + + // Fetch components for this file + let components: [Component] + do { + components = try await context.client.request(ComponentsEndpoint(fileId: entry.fileId)) + } catch { + continue + } + + // Filter to relevant frame + let relevant = components.filter { comp in + if let frame = entry.frameName { + return comp.containingFrame.name == frame + } + return true + } + + // Sample first 50 to avoid excessive API calls + let sampled = Array(relevant.prefix(50)) + guard !sampled.isEmpty else { continue } + + let nodeIds = sampled.map(\.nodeId) + let nodes: [NodeId: Node] + do { + nodes = try await context.client.request(NodesEndpoint(fileId: entry.fileId, nodeIds: nodeIds)) + } catch { + continue + } + + for (nodeId, node) in nodes { + let compName = sampled.first { $0.nodeId == nodeId }?.name ?? nodeId + checkNodeFills(node: node.document, componentName: compName, diagnostics: &diagnostics) + } + } + + return diagnostics + } + + // MARK: - Node Fill Checking + + private func checkNodeFills( + node: Document, + componentName: String, + diagnostics: inout [LintDiagnostic] + ) { + for fill in node.fills { + // Skip images/videos + if fill.type == .image { continue } + // Skip invisible + if fill.opacity == 0 { continue } + + // Check boundVariables on the fill + if fill.boundVariables == nil || fill.boundVariables?["color"] == nil { + let colorDesc: String = if let color = fill.color { + String( + format: "#%02X%02X%02X", + Int(color.r * 255), + Int(color.g * 255), + Int(color.b * 255) + ) + } else { + fill.type.rawValue + } + + diagnostics.append(LintDiagnostic( + ruleId: id, + ruleName: name, + severity: .error, + message: "Fill \(colorDesc) in '\(componentName)' not bound to Variable", + componentName: componentName, + nodeId: node.id, + suggestion: "Bind this fill to a color Variable for dark mode generation" + )) + } + } + + // Recurse into children + for child in node.children ?? [] { + checkNodeFills(node: child, componentName: componentName, diagnostics: &diagnostics) + } + } + + // MARK: - Entry Collection + + private struct VDMEntry { + let fileId: String + let frameName: String? + } + + private func collectVDMEntries(from config: PKLConfig, defaultFileId: String) -> [VDMEntry] { + var entries: [VDMEntry] = [] + + func addIcons(_ icons: [some Common_FrameSource]?) { + for entry in icons ?? [] where entry.variablesDarkMode != nil { + entries.append(VDMEntry( + fileId: entry.figmaFileId ?? defaultFileId, + frameName: entry.figmaFrameName + )) + } + } + + if let ios = config.ios { addIcons(ios.icons) } + if let android = config.android { addIcons(android.icons) } + if let flutter = config.flutter { addIcons(flutter.icons) } + if let web = config.web { addIcons(web.icons) } + + return entries + } +} diff --git a/Sources/ExFigCLI/Lint/Rules/DeletedVariablesRule.swift b/Sources/ExFigCLI/Lint/Rules/DeletedVariablesRule.swift new file mode 100644 index 00000000..fddd2078 --- /dev/null +++ b/Sources/ExFigCLI/Lint/Rules/DeletedVariablesRule.swift @@ -0,0 +1,76 @@ +import ExFigConfig +import ExFigCore +import FigmaAPI +import Foundation + +/// Checks that variable collections used in config don't contain deleted-but-referenced variables. +struct DeletedVariablesRule: LintRule { + let id = "deleted-variables" + let name = "No deleted variables" + let description = "Variable collections must not contain deletedButReferenced variables" + let severity: LintSeverity = .error + + func check(context: LintContext) async throws -> [LintDiagnostic] { + let config = context.config + var diagnostics: [LintDiagnostic] = [] + + let fileIds = collectVariableFileIds(from: config) + guard !fileIds.isEmpty else { return [] } + + for fileId in fileIds { + guard !fileId.isEmpty else { continue } + + let variables: VariablesMeta + do { + variables = try await context.client.request(VariablesEndpoint(fileId: fileId)) + } catch { + continue + } + + for (variableId, variable) in variables.variables + where variable.deletedButReferenced == true + { + diagnostics.append(LintDiagnostic( + ruleId: id, + ruleName: name, + severity: .error, + message: "Variable '\(variable.name)' is deleted but still referenced", + componentName: variable.name, + nodeId: variableId, + suggestion: "Remove all references to this variable, or restore it" + )) + } + } + + return diagnostics + } + + private func collectVariableFileIds(from config: ExFig.ModuleImpl) -> Set { + var fileIds: Set = [] + let defaultFileId = config.figma?.lightFileId ?? "" + + // Colors with variablesColors (on common config, not common.colors) + if config.common?.variablesColors != nil { + fileIds.insert(defaultFileId) + } + + /// Icons with variablesDarkMode + func addFromIcons(_ icons: [some Common_FrameSource]?) { + for entry in icons ?? [] { + if let vdm = entry.variablesDarkMode { + fileIds.insert(entry.figmaFileId ?? defaultFileId) + if let libFileId = vdm.variablesFileId, !libFileId.isEmpty { + fileIds.insert(libFileId) + } + } + } + } + + if let ios = config.ios { addFromIcons(ios.icons) } + if let android = config.android { addFromIcons(android.icons) } + if let flutter = config.flutter { addFromIcons(flutter.icons) } + if let web = config.web { addFromIcons(web.icons) } + + return fileIds + } +} diff --git a/Sources/ExFigCLI/Lint/Rules/FramePageMatchRule.swift b/Sources/ExFigCLI/Lint/Rules/FramePageMatchRule.swift new file mode 100644 index 00000000..04ef2991 --- /dev/null +++ b/Sources/ExFigCLI/Lint/Rules/FramePageMatchRule.swift @@ -0,0 +1,145 @@ +import ExFigConfig +import ExFigCore +import FigmaAPI +import Foundation + +/// Checks that frame and page names from the config exist in the Figma file. +/// Uses the Components API to discover available frame/page names. +struct FramePageMatchRule: LintRule { + let id = "frame-page-match" + let name = "Frame/page names match" + let description = "Frame and page names in config must exist in the Figma file" + let severity: LintSeverity = .error + + func check(context: LintContext) async throws -> [LintDiagnostic] { + let entries = collectEntries(from: context.config) + guard !entries.isEmpty else { return [] } + + var diagnostics: [LintDiagnostic] = [] + let grouped = Dictionary(grouping: entries) { $0.fileId } + + for (fileId, fileEntries) in grouped { + guard !fileId.isEmpty else { continue } + let result = try await checkFile(fileId: fileId, entries: fileEntries, client: context.client) + diagnostics.append(contentsOf: result) + } + + return diagnostics + } + + // MARK: - Per-File Check + + private func checkFile( + fileId: String, + entries: [EntryInfo], + client: any FigmaAPI.Client + ) async throws -> [LintDiagnostic] { + let components: [Component] + do { + components = try await client.request(ComponentsEndpoint(fileId: fileId)) + } catch { + return [LintDiagnostic( + ruleId: id, ruleName: name, severity: .error, + message: "Cannot access Figma file '\(fileId)': \(error.localizedDescription)", + componentName: nil, nodeId: nil, + suggestion: "Check FIGMA_PERSONAL_TOKEN and file permissions" + )] + } + + let pageNames = Set(components.compactMap(\.containingFrame.pageName)) + let frameNames = Set(components.compactMap(\.containingFrame.name)) + var framesByPage: [String: Set] = [:] + for comp in components { + if let page = comp.containingFrame.pageName, let frame = comp.containingFrame.name { + framesByPage[page, default: []].insert(frame) + } + } + + return entries.flatMap { entry in + validateEntry(entry, pageNames: pageNames, frameNames: frameNames, framesByPage: framesByPage) + } + } + + private func validateEntry( + _ entry: EntryInfo, + pageNames: Set, + frameNames: Set, + framesByPage: [String: Set] + ) -> [LintDiagnostic] { + var diagnostics: [LintDiagnostic] = [] + + if let pageName = entry.pageName, !pageNames.contains(pageName) { + diagnostics.append(LintDiagnostic( + ruleId: id, ruleName: name, severity: .error, + message: "Page '\(pageName)' not found in Figma file", + componentName: nil, nodeId: nil, + suggestion: "Available pages: \(pageNames.sorted().joined(separator: ", "))" + )) + } + + if let frameName = entry.frameName { + if let pageName = entry.pageName { + let pageFrames = framesByPage[pageName] ?? [] + if !pageFrames.contains(frameName) { + diagnostics.append(LintDiagnostic( + ruleId: id, ruleName: name, severity: .error, + message: "Frame '\(frameName)' not found on page '\(pageName)'", + componentName: nil, nodeId: nil, + suggestion: "Available frames on '\(pageName)': " + + "\(pageFrames.sorted().joined(separator: ", "))" + )) + } + } else if !frameNames.contains(frameName) { + diagnostics.append(LintDiagnostic( + ruleId: id, ruleName: name, severity: .error, + message: "Frame '\(frameName)' not found in any page", + componentName: nil, nodeId: nil, + suggestion: "Available frames: \(frameNames.sorted().joined(separator: ", "))" + )) + } + } + + return diagnostics + } + + // MARK: - Entry Collection + + private struct EntryInfo { + let fileId: String + let frameName: String? + let pageName: String? + } + + private func collectEntries(from config: ExFig.ModuleImpl) -> [EntryInfo] { + var entries: [EntryInfo] = [] + let fileId = config.figma?.lightFileId ?? "" + + func addIconEntries(_ icons: [some Common_FrameSource]?) { + for entry in icons ?? [] { + entries.append(EntryInfo( + fileId: entry.figmaFileId ?? fileId, + frameName: entry.figmaFrameName, + pageName: entry.figmaPageName + )) + } + } + + if let ios = config.ios { + addIconEntries(ios.icons) + addIconEntries(ios.images) + } + if let android = config.android { + addIconEntries(android.icons) + addIconEntries(android.images) + } + if let flutter = config.flutter { + addIconEntries(flutter.icons) + addIconEntries(flutter.images) + } + if let web = config.web { + addIconEntries(web.icons) + } + + return entries + } +} diff --git a/Sources/ExFigCLI/Lint/Rules/NamingConventionRule.swift b/Sources/ExFigCLI/Lint/Rules/NamingConventionRule.swift new file mode 100644 index 00000000..da94f503 --- /dev/null +++ b/Sources/ExFigCLI/Lint/Rules/NamingConventionRule.swift @@ -0,0 +1,121 @@ +import ExFigConfig +import ExFigCore +import FigmaAPI +import Foundation + +/// Checks that component names match the nameValidateRegexp from config entries. +struct NamingConventionRule: LintRule { + let id = "naming-convention" + let name = "Naming conventions" + let description = "Component names must match nameValidateRegexp patterns in config" + let severity: LintSeverity = .error + + func check(context: LintContext) async throws -> [LintDiagnostic] { + let config = context.config + var diagnostics: [LintDiagnostic] = [] + let fileId = config.figma?.lightFileId ?? "" + + let entries = collectEntriesWithRegex(from: config, defaultFileId: fileId) + guard !entries.isEmpty else { return [] } + + // Group by fileId + let grouped = Dictionary(grouping: entries) { $0.fileId } + + for (entryFileId, fileEntries) in grouped { + guard !entryFileId.isEmpty else { continue } + + let components: [Component] + do { + components = try await context.client.request(ComponentsEndpoint(fileId: entryFileId)) + } catch { + continue + } + + for entry in fileEntries { + guard let pattern = entry.regex else { continue } + let regex: NSRegularExpression + do { + regex = try NSRegularExpression(pattern: pattern) + } catch { + diagnostics.append(LintDiagnostic( + ruleId: id, + ruleName: name, + severity: .warning, + message: "Invalid regex pattern: '\(pattern)'", + componentName: nil, + nodeId: nil, + suggestion: "Fix the nameValidateRegexp in your PKL config" + )) + continue + } + + // Filter components by frame name if specified + let relevant = components.filter { comp in + if let frameName = entry.frameName { + return comp.containingFrame.name == frameName + } + return true + } + + for comp in relevant { + let range = NSRange(comp.name.startIndex..., in: comp.name) + if regex.firstMatch(in: comp.name, range: range) == nil { + diagnostics.append(LintDiagnostic( + ruleId: id, + ruleName: name, + severity: .error, + message: "Name '\(comp.name)' doesn't match pattern '\(pattern)'", + componentName: comp.name, + nodeId: comp.nodeId, + suggestion: "Rename the component to match the expected pattern" + )) + } + } + } + } + + return diagnostics + } + + // MARK: - Entry Collection + + private struct RegexEntry { + let fileId: String + let frameName: String? + let regex: String? + } + + private func collectEntriesWithRegex( + from config: PKLConfig, + defaultFileId: String + ) -> [RegexEntry] { + var entries: [RegexEntry] = [] + + func addEntries(_ icons: [some Common_FrameSource & Common_NameProcessing]?) { + for entry in icons ?? [] where entry.nameValidateRegexp != nil { + entries.append(RegexEntry( + fileId: entry.figmaFileId ?? defaultFileId, + frameName: entry.figmaFrameName, + regex: entry.nameValidateRegexp + )) + } + } + + if let ios = config.ios { + addEntries(ios.icons) + addEntries(ios.images) + } + if let android = config.android { + addEntries(android.icons) + addEntries(android.images) + } + if let flutter = config.flutter { + addEntries(flutter.icons) + } + if let web = config.web { + addEntries(web.icons) + } + + return entries + } +} diff --git a/Sources/ExFigCLI/Subcommands/Lint.swift b/Sources/ExFigCLI/Subcommands/Lint.swift new file mode 100644 index 00000000..40ac11a2 --- /dev/null +++ b/Sources/ExFigCLI/Subcommands/Lint.swift @@ -0,0 +1,85 @@ +import ArgumentParser +import ExFigConfig +import ExFigCore +import FigmaAPI +import Foundation + +extension ExFigCommand { + struct Lint: AsyncParsableCommand { + static let configuration = CommandConfiguration( + commandName: "lint", + abstract: "Lint Figma file structure against config", + discussion: """ + Validates that the Figma file follows the conventions required by your ExFig config. + Checks naming conventions, frame/page structure, variable bindings, dark mode setup, and more. + + Examples: + exfig lint -i exfig.pkl # Lint one config + exfig lint -i exfig.pkl --rules naming-convention # Filter rules + exfig lint -i exfig.pkl --format json # JSON output for CI + exfig lint -i exfig.pkl --severity error # Only errors + """ + ) + + @OptionGroup + var globalOptions: GlobalOptions + + @OptionGroup + var options: ExFigOptions + + @OptionGroup + var faultToleranceOptions: FaultToleranceOptions + + @Option(name: .long, help: "Comma-separated list of rule IDs to run") + var rules: String? + + @Option(name: .long, help: "Output format: text or json") + var format: String = "text" + + @Option(name: .long, help: "Minimum severity: error, warning, or info") + var severity: String = "info" + + func run() async throws { + ExFigCommand.initializeTerminalUI( + verbose: globalOptions.verbose, quiet: globalOptions.quiet + ) + ExFigCommand.checkSchemaVersionIfNeeded() + let ui = ExFigCommand.terminalUI! + + let outputFormat = LintOutputFormat(rawValue: format) ?? .text + let minSeverity = LintSeverity(rawValue: severity) ?? .info + let ruleFilter: Set? = rules.map { + Set($0.split(separator: ",").map { String($0.trimmingCharacters(in: .whitespaces)) }) + } + + let client = resolveClient( + accessToken: options.accessToken, + timeout: options.params.figma?.timeout, + options: faultToleranceOptions, + ui: ui + ) + + let context = LintContext(config: options.params, client: client, ui: ui) + let engine = LintEngine.default + + let diagnostics = try await ui.withSpinner("Linting Figma file structure...") { + try await engine.run( + context: context, + ruleFilter: ruleFilter, + minSeverity: minSeverity + ) + } + + let reporter = LintReporter( + format: outputFormat, + useColors: ui.outputMode == .normal + ) + try reporter.report(diagnostics: diagnostics, ui: ui) + + // Exit with code 1 if there are errors (for CI) + if diagnostics.contains(where: { $0.severity == .error }) { + throw ExitCode.failure + } + } + } +} From 64b807404b62b4ab8ea387fc5b9942aac795bcf9 Mon Sep 17 00:00:00 2001 From: alexey1312 Date: Tue, 31 Mar 2026 13:16:32 +0500 Subject: [PATCH 2/6] docs: document lint command and FigmaAPI type gotchas in CLAUDE.md Add lint command section, troubleshooting entries for config type reference, Paint.visible, and variablesColors location. --- CLAUDE.md | 12 ++++++++++++ Sources/ExFigCLI/CLAUDE.md | 8 ++++++++ Sources/ExFigConfig/CLAUDE.md | 1 + 3 files changed, 21 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 20af0e03..4bbd00c8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -317,6 +317,15 @@ Changing `load()` return type affects: **`withSpinner` gotcha:** Closure is `@Sendable` — cannot capture mutable vars. Return full result from closure. +### Lint Command + +`exfig lint -i exfig.pkl` validates Figma file structure against PKL config. +Rules in `Sources/ExFigCLI/Lint/Rules/`, engine in `LintEngine.swift`. +Each rule implements `LintRule` protocol with `check(context: LintContext) -> [LintDiagnostic]`. +Uses `FigmaAPI.Client.request(SomeEndpoint(...))` directly (no convenience methods on Client). +`ComponentsEndpoint` returns `[Component]`, `VariablesEndpoint` returns `VariablesMeta`, +`NodesEndpoint` returns `[NodeId: Node]`. + ### Adding a CLI Command See `ExFigCLI/CLAUDE.md` (Adding a New Subcommand). @@ -502,6 +511,9 @@ NooraUI.formatLink("url", useColors: true) // underlined primary | Figma variable IDs file-scoped | Variable IDs differ between files — alias targets from file A can't be found by ID in file B. Use name-based matching (`resolveViaLibrary`) + mode name matching (not modeId) for cross-file resolution | | `assertionFailure` in release | `assertionFailure` is stripped in release builds — add `FileHandle.standardError.write()` as production fallback for truly-impossible-but-must-not-be-silent error paths | | Components API called N times | `ComponentPreFetcher` only works in batch mode — use `ComponentsCache` via `SourceFactory(componentsCache:)` for standalone multi-entry dedup | +| Config type reference | `ExFigOptions.params` is `ExFig.ModuleImpl!` — no `PKLConfig` typealias exists | +| `Paint.visible` doesn't exist | FigmaAPI `Paint` has no `visible` field — use `opacity` to check visibility | +| `variablesColors` location | On `Common.CommonConfig` (`config.common?.variablesColors`), NOT on `config.common?.colors?.variablesColors` | ## Additional Rules diff --git a/Sources/ExFigCLI/CLAUDE.md b/Sources/ExFigCLI/CLAUDE.md index 7103f813..87961414 100644 --- a/Sources/ExFigCLI/CLAUDE.md +++ b/Sources/ExFigCLI/CLAUDE.md @@ -234,6 +234,14 @@ Penpot sources create `BasePenpotClient` internally from `PENPOT_ACCESS_TOKEN` e `FileDownloader.fetch()` treats `file://` URLs as local files (skips HTTP download). Do NOT weaken `validateDownloadURL()` — it must remain HTTPS-only. Filter file URLs in `fetch()` before download loop. +### Lint Subcommand + +`Lint.swift` validates Figma file against config without exporting. +Uses `ExFigOptions` for config loading (same as export commands). +Rules in `Lint/Rules/`, each uses `client.request(SomeEndpoint(fileId:))` for Figma API. +Generic entry collection: `func addEntries(_ icons: [some Common_FrameSource]?)` to iterate +all platform entries without platform-specific code. + ### Adding a New Subcommand 1. Create `Subcommands/NewCommand.swift` implementing `AsyncParsableCommand` diff --git a/Sources/ExFigConfig/CLAUDE.md b/Sources/ExFigConfig/CLAUDE.md index 92d1d7ce..9e0e9ac7 100644 --- a/Sources/ExFigConfig/CLAUDE.md +++ b/Sources/ExFigConfig/CLAUDE.md @@ -63,6 +63,7 @@ ExFigCore domain types (NameStyle, ColorsSourceInput, etc.) ### Generated Type Gotchas - `Common.PenpotSource.baseUrl` is non-optional `String` (has PKL default) — tests must pass a real URL, not `nil` +- `variablesColors` is on `Common.CommonConfig` (`config.common?.variablesColors`), NOT on `Common.Colors` ### PklError Workaround From 9686695ca755f126ec11207fab3ddf1c49e99e79 Mon Sep 17 00:00:00 2001 From: alexey1312 Date: Tue, 31 Mar 2026 13:21:31 +0500 Subject: [PATCH 3/6] docs: add lint command to usage spec, DocC articles, and CI guide Update exfig.usage.kdl with lint subcommand and flags. Add Linting section to Usage.md with rules table. Add Linting in CI section to CICDIntegration.md. --- .../ExFigCLI/ExFig.docc/CICDIntegration.md | 14 ++++++++++ Sources/ExFigCLI/ExFig.docc/Usage.md | 27 +++++++++++++++++++ exfig.usage.kdl | 20 ++++++++++++++ llms-full.txt | 27 +++++++++++++++++++ 4 files changed, 88 insertions(+) diff --git a/Sources/ExFigCLI/ExFig.docc/CICDIntegration.md b/Sources/ExFigCLI/ExFig.docc/CICDIntegration.md index 8bb88fba..ed2b976b 100644 --- a/Sources/ExFigCLI/ExFig.docc/CICDIntegration.md +++ b/Sources/ExFigCLI/ExFig.docc/CICDIntegration.md @@ -65,6 +65,20 @@ In CI, use `--quiet` to keep logs clean. Pair with `--report` for structured out | 0 | Success | | 1 | Export error (API failure, etc.) | +## Linting in CI + +Run `exfig lint` as a pre-export validation step. It checks that your Figma file +matches the conventions expected by your PKL config (naming, frame structure, variable +bindings, dark mode setup). + +```yaml +- name: Lint Figma structure + run: exfig lint -i exfig.pkl --format json --severity error +``` + +The command exits with code 1 if any errors are found. Use `--format json` for +machine-readable output and `--severity error` to ignore warnings. + ## Version Tracking in CI Enable `--cache` to skip unchanged exports. ExFig compares the Figma file version against a local diff --git a/Sources/ExFigCLI/ExFig.docc/Usage.md b/Sources/ExFigCLI/ExFig.docc/Usage.md index 638a777e..799dcc4c 100644 --- a/Sources/ExFigCLI/ExFig.docc/Usage.md +++ b/Sources/ExFigCLI/ExFig.docc/Usage.md @@ -259,6 +259,33 @@ exfig fetch -f abc123 -r "Images" -o ./images \ | `--webp-encoding` | - | WebP encoding: lossy, lossless | lossy | | `--webp-quality` | - | WebP quality (0-100) | 80 | +## Linting + +Validate your Figma file structure against your PKL config before exporting: + +```bash +# Lint with default rules +exfig lint -i exfig.pkl + +# Only check specific rules +exfig lint -i exfig.pkl --rules naming-convention,deleted-variables + +# JSON output for CI (exit code 1 on errors) +exfig lint -i exfig.pkl --format json --severity error +``` + +### Available Rules + +| Rule | Severity | Description | +| ----------------------- | -------- | ------------------------------------------------------ | +| `frame-page-match` | error | Frame/page names in config exist in Figma file | +| `naming-convention` | error | Component names match `nameValidateRegexp` patterns | +| `component-not-frame` | error | Configured frames contain published components | +| `deleted-variables` | error | No `deletedButReferenced` variables in collections | +| `alias-chain-integrity` | error | Variable alias chains resolve without broken refs | +| `dark-mode-variables` | error | With `variablesDarkMode`, fills bound to Variables | +| `dark-mode-suffix` | warning | With `suffixDarkMode`, light components have dark pair | + ## Help and Version ```bash diff --git a/exfig.usage.kdl b/exfig.usage.kdl index dc43afff..efff05ad 100644 --- a/exfig.usage.kdl +++ b/exfig.usage.kdl @@ -333,6 +333,26 @@ cmd "batch" help="Process multiple config files in parallel" { arg "..." help="Config files or directory to process" } +// ============================================================================= +// lint +// ============================================================================= + +cmd "lint" help="Lint Figma file structure against config" { + long_help "Validates that the Figma file follows the conventions required by your ExFig config. Checks naming conventions, frame/page structure, variable bindings, dark mode setup, and more. Exit code 1 if errors found." + + flag "-i --input " help="Path to PKL config file. Auto-detects exfig.pkl if not specified." + flag "--max-retries " help="Maximum retry attempts for failed API requests" default="4" + flag "--rate-limit " help="Maximum API requests per minute" default="10" + flag "--timeout " help="Figma API request timeout in seconds (overrides config)" + flag "--rules " help="Comma-separated list of rule IDs to run" + flag "--format " help="Output format" default="text" { + choices "text" "json" + } + flag "--severity " help="Minimum severity to report" default="info" { + choices "error" "warning" "info" + } +} + // ============================================================================= // mcp // ============================================================================= diff --git a/llms-full.txt b/llms-full.txt index 70bc59e9..92c684b3 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -532,6 +532,33 @@ exfig fetch -f abc123 -r "Images" -o ./images \ | `--webp-encoding` | - | WebP encoding: lossy, lossless | lossy | | `--webp-quality` | - | WebP quality (0-100) | 80 | +## Linting + +Validate your Figma file structure against your PKL config before exporting: + +```bash +# Lint with default rules +exfig lint -i exfig.pkl + +# Only check specific rules +exfig lint -i exfig.pkl --rules naming-convention,deleted-variables + +# JSON output for CI (exit code 1 on errors) +exfig lint -i exfig.pkl --format json --severity error +``` + +### Available Rules + +| Rule | Severity | Description | +| ----------------------- | -------- | ------------------------------------------------------ | +| `frame-page-match` | error | Frame/page names in config exist in Figma file | +| `naming-convention` | error | Component names match `nameValidateRegexp` patterns | +| `component-not-frame` | error | Configured frames contain published components | +| `deleted-variables` | error | No `deletedButReferenced` variables in collections | +| `alias-chain-integrity` | error | Variable alias chains resolve without broken refs | +| `dark-mode-variables` | error | With `variablesDarkMode`, fills bound to Variables | +| `dark-mode-suffix` | warning | With `suffixDarkMode`, light components have dark pair | + ## Help and Version ```bash From 963a697fdb6109838f84412b5fea4454447a5eb5 Mon Sep 17 00:00:00 2001 From: alexey1312 Date: Tue, 31 Mar 2026 13:25:21 +0500 Subject: [PATCH 4/6] fix(lint): skip cross-file alias refs and filter suffix rule by frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit alias-chain-integrity: recognize cross-file variable IDs (long hex hash prefix) as external library references — not broken chains. dark-mode-suffix: only check components within configured export frames, not all components in the entire Figma file. --- .../Lint/Rules/AliasChainIntegrityRule.swift | 18 +++++++ .../Lint/Rules/DarkModeSuffixRule.swift | 51 ++++++++++++++++--- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift b/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift index 086956be..135255e3 100644 --- a/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift @@ -86,6 +86,18 @@ struct AliasChainIntegrityRule: LintRule { case tooDeep } + /// Cross-file variable IDs have a long hash before the "/" separator, + /// e.g., "VariableID:806fcc6a84cf048f0a06837634440ecad91622fe/3556:423". + /// Local variable IDs are short like "VariableID:3556:423" or just "3556:423". + private func isCrossFileReference(_ id: String) -> Bool { + // Strip "VariableID:" prefix if present + let raw = id.hasPrefix("VariableID:") ? String(id.dropFirst("VariableID:".count)) : id + // Cross-file IDs have a 40-char hex hash before "/" + guard let slashIndex = raw.firstIndex(of: "/") else { return false } + let prefix = raw[raw.startIndex ..< slashIndex] + return prefix.count >= 32 && prefix.allSatisfy(\.isHexDigit) + } + private func resolveChain( value: ValuesByMode, variables: [String: VariableValue], @@ -106,6 +118,12 @@ struct AliasChainIntegrityRule: LintRule { } guard let target = variables[aliasId] else { + // Cross-file alias: variable IDs containing "/" with a long hash prefix + // (e.g., "VariableID:806fcc6a.../3556:423") are external library references. + // These can't be validated within the local file — treat as resolved. + if isCrossFileReference(aliasId) { + return .resolved + } return .broken(targetId: aliasId) } diff --git a/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift b/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift index eae9fbac..9005bbca 100644 --- a/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift @@ -4,6 +4,7 @@ import FigmaAPI import Foundation /// When suffixDarkMode is configured, checks that each light component has a matching dark pair. +/// Only checks components within frames configured for export, not all components in the file. struct DarkModeSuffixRule: LintRule { let id = "dark-mode-suffix" let name = "Dark mode suffix pairs" @@ -14,7 +15,6 @@ struct DarkModeSuffixRule: LintRule { let config = context.config var diagnostics: [LintDiagnostic] = [] - // Check if suffixDarkMode is configured let suffix: String if let sdm = config.common?.icons?.suffixDarkMode { suffix = sdm.suffix @@ -24,23 +24,32 @@ struct DarkModeSuffixRule: LintRule { return [] } - let fileId = config.figma?.lightFileId ?? "" - guard !fileId.isEmpty else { return [] } + let defaultFileId = config.figma?.lightFileId ?? "" + guard !defaultFileId.isEmpty else { return [] } + + // Collect configured frame names to filter components + let configuredFrames = collectConfiguredFrames(from: config) let components: [Component] do { - components = try await context.client.request(ComponentsEndpoint(fileId: fileId)) + components = try await context.client.request(ComponentsEndpoint(fileId: defaultFileId)) } catch { return [] } - let allNames = Set(components.map(\.name)) + // Only check components in configured frames + let relevant = components.filter { comp in + guard let frameName = comp.containingFrame.name else { return false } + return configuredFrames.contains(frameName) + } + + let relevantNames = Set(relevant.map(\.name)) - for component in components { + for component in relevant { if component.name.hasSuffix(suffix) { continue } let expectedDark = component.name + suffix - if !allNames.contains(expectedDark) { + if !relevantNames.contains(expectedDark) { diagnostics.append(LintDiagnostic( ruleId: id, ruleName: name, @@ -55,4 +64,32 @@ struct DarkModeSuffixRule: LintRule { return diagnostics } + + private func collectConfiguredFrames(from config: ExFig.ModuleImpl) -> Set { + var frames: Set = [] + + func add(_ entries: [some Common_FrameSource]?) { + for entry in entries ?? [] { + if let frame = entry.figmaFrameName { frames.insert(frame) } + } + } + + if let ios = config.ios { + add(ios.icons) + add(ios.images) + } + if let android = config.android { + add(android.icons) + add(android.images) + } + if let flutter = config.flutter { + add(flutter.icons) + add(flutter.images) + } + if let web = config.web { + add(web.icons) + } + + return frames + } } From bf954f9355e299086b525823e87680ce42a78974 Mon Sep 17 00:00:00 2001 From: alexey1312 Date: Tue, 31 Mar 2026 13:34:28 +0500 Subject: [PATCH 5/6] test(lint): add unit tests for all lint rules and engine 21 tests covering FramePageMatchRule, NamingConventionRule, DeletedVariablesRule, AliasChainIntegrityRule, ComponentNotFrameRule, DarkModeSuffixRule, and LintEngine with mock Figma API responses. refactor(lint): improve output, progress, and rules - Add per-rule spinner progress: "Checking X... (2/7)" - Replace verbose text output with compact table format - Add duplicate-component-names rule (catches silent overwrites) - Remove alias-chain-integrity from defaults (cross-file refs normal) - Keep dark-mode-suffix as warning (not all icons need dark pairs) - Errors sorted first, then warnings perf(lint): cache API responses and show rule names in spinner - Add LintDataCache actor to deduplicate Components/Variables API calls (5 rules shared the same ComponentsEndpoint, now 1 request per fileId) - Add withSpinnerMessage to TerminalUI for dynamic message updates - Spinner now shows: "Checking Dark mode fills... (6/7)" fix(lint): downgrade deleted-variables to warning (ExFig handles them) fix(lint): skip root frame fills in dark-mode-variables check Root component frames often have background #FFFFFF fills that aren't meant to be variable-bound. Only check fills on child vector nodes, not the component's root frame. fix(lint): filter dark-mode-variables by page name from config fix(lint): skip RTL variant components in dark-mode-variables check fix(lint): filter duplicate-component-names by configured frame/page fix(lint): skip RTL variants in duplicate-component-names check docs: add lint rule patterns and cp gotcha to CLAUDE.md docs: add lint to ExFig.md, GettingStarted.md, and regenerate llms feat(mcp): add exfig_lint tool for Figma structure validation Returns JSON diagnostics with errors/warnings. Supports rule filtering and severity threshold. Uses LintDataCache for dedup. test(lint): add tests for duplicate-component-names rule and RTL skip fix(lint): suppress spinner in --format json mode for clean stdout --- CLAUDE.md | 9 + Sources/ExFigCLI/ExFig.docc/ExFig.md | 1 + Sources/ExFigCLI/ExFig.docc/GettingStarted.md | 5 +- Sources/ExFigCLI/Lint/LintEngine.swift | 14 +- Sources/ExFigCLI/Lint/LintReporter.swift | 75 ++- Sources/ExFigCLI/Lint/LintTypes.swift | 22 + .../Lint/Rules/AliasChainIntegrityRule.swift | 2 +- .../Lint/Rules/ComponentNotFrameRule.swift | 2 +- .../Lint/Rules/DarkModeSuffixRule.swift | 2 +- .../Lint/Rules/DarkModeVariablesRule.swift | 47 +- .../Lint/Rules/DeletedVariablesRule.swift | 6 +- .../Rules/DuplicateComponentNamesRule.swift | 131 ++++ .../Lint/Rules/FramePageMatchRule.swift | 6 +- .../Lint/Rules/NamingConventionRule.swift | 2 +- Sources/ExFigCLI/MCP/MCPToolDefinitions.swift | 34 ++ Sources/ExFigCLI/MCP/MCPToolHandlers.swift | 57 ++ Sources/ExFigCLI/Subcommands/Lint.swift | 21 +- Sources/ExFigCLI/TerminalUI/TerminalUI.swift | 42 ++ Tests/ExFigTests/Lint/LintRulesTests.swift | 568 ++++++++++++++++++ llms-full.txt | 5 +- 20 files changed, 996 insertions(+), 55 deletions(-) create mode 100644 Sources/ExFigCLI/Lint/Rules/DuplicateComponentNamesRule.swift create mode 100644 Tests/ExFigTests/Lint/LintRulesTests.swift diff --git a/CLAUDE.md b/CLAUDE.md index 4bbd00c8..9ffe3f9b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -326,6 +326,14 @@ Uses `FigmaAPI.Client.request(SomeEndpoint(...))` directly (no convenience metho `ComponentsEndpoint` returns `[Component]`, `VariablesEndpoint` returns `VariablesMeta`, `NodesEndpoint` returns `[NodeId: Node]`. +**Lint rule development patterns:** + +- Every rule must filter components by BOTH `figmaFrameName` AND `figmaPageName` from config entries +- Skip RTL variants: `comp.containingFrame.containingComponentSet != nil && comp.name.contains("RTL=")` +- Skip root frame fills when checking boundVariables — root `Document` fills are backgrounds, check only children +- Cross-file variable IDs (32+ char hex hash before `/`) are valid external library refs, not broken aliases +- `LintDataCache` actor caches Components/Variables API responses — use `context.cache.components(for:client:)` + ### Adding a CLI Command See `ExFigCLI/CLAUDE.md` (Adding a New Subcommand). @@ -514,6 +522,7 @@ NooraUI.formatLink("url", useColors: true) // underlined primary | Config type reference | `ExFigOptions.params` is `ExFig.ModuleImpl!` — no `PKLConfig` typealias exists | | `Paint.visible` doesn't exist | FigmaAPI `Paint` has no `visible` field — use `opacity` to check visibility | | `variablesColors` location | On `Common.CommonConfig` (`config.common?.variablesColors`), NOT on `config.common?.colors?.variablesColors` | +| `cp` prompts overwrite | macOS `trash` alias intercepts `cp`; use `/bin/cp -f` to force overwrite without prompt | ## Additional Rules diff --git a/Sources/ExFigCLI/ExFig.docc/ExFig.md b/Sources/ExFigCLI/ExFig.docc/ExFig.md index 58492265..6e44399d 100644 --- a/Sources/ExFigCLI/ExFig.docc/ExFig.md +++ b/Sources/ExFigCLI/ExFig.docc/ExFig.md @@ -54,6 +54,7 @@ automatic retries with exponential backoff, checkpoint/resume for interrupted ex file version tracking, and experimental per-node granular cache. **Developer Experience** +`exfig lint` validates Figma file structure against your config before export (naming, variables, dark mode). CI/CD ready (quiet mode, exit codes, JSON reports), GitHub Action for automated exports, MCP server for AI assistant integration, [Claude Code plugins](https://github.com/DesignPipe/exfig-plugins) for setup wizards and slash commands, diff --git a/Sources/ExFigCLI/ExFig.docc/GettingStarted.md b/Sources/ExFigCLI/ExFig.docc/GettingStarted.md index 9c96e860..55e0fb23 100644 --- a/Sources/ExFigCLI/ExFig.docc/GettingStarted.md +++ b/Sources/ExFigCLI/ExFig.docc/GettingStarted.md @@ -158,9 +158,12 @@ ios = new iOS.iOSConfig { } ``` -### 4. Export Resources +### 4. Validate & Export ```bash +# Validate Figma file structure before exporting +exfig lint + # Export individual resource types exfig colors exfig icons diff --git a/Sources/ExFigCLI/Lint/LintEngine.swift b/Sources/ExFigCLI/Lint/LintEngine.swift index 9052a634..7e571623 100644 --- a/Sources/ExFigCLI/Lint/LintEngine.swift +++ b/Sources/ExFigCLI/Lint/LintEngine.swift @@ -1,5 +1,8 @@ import Foundation +/// Callback for reporting lint progress with a displayable message string. +typealias LintProgressCallback = @Sendable (String) -> Void + /// Engine that runs lint rules against a PKL configuration. struct LintEngine { /// All registered lint rules. @@ -9,7 +12,8 @@ struct LintEngine { func run( context: LintContext, ruleFilter: Set? = nil, - minSeverity: LintSeverity = .info + minSeverity: LintSeverity = .info, + onProgress: LintProgressCallback? = nil ) async throws -> [LintDiagnostic] { let applicableRules = rules.filter { rule in if let filter = ruleFilter, !filter.contains(rule.id) { @@ -19,13 +23,15 @@ struct LintEngine { } var allDiagnostics: [LintDiagnostic] = [] + let total = applicableRules.count + + for (index, rule) in applicableRules.enumerated() { + onProgress?("Checking \(rule.name)... (\(index + 1)/\(total))") - for rule in applicableRules { do { let diagnostics = try await rule.check(context: context) allDiagnostics.append(contentsOf: diagnostics) } catch { - // Rule failure becomes an info diagnostic allDiagnostics.append(LintDiagnostic( ruleId: rule.id, ruleName: rule.name, @@ -57,7 +63,7 @@ extension LintEngine { NamingConventionRule(), ComponentNotFrameRule(), DeletedVariablesRule(), - AliasChainIntegrityRule(), + DuplicateComponentNamesRule(), DarkModeVariablesRule(), DarkModeSuffixRule(), ]) diff --git a/Sources/ExFigCLI/Lint/LintReporter.swift b/Sources/ExFigCLI/Lint/LintReporter.swift index c6c659e0..832a42b4 100644 --- a/Sources/ExFigCLI/Lint/LintReporter.swift +++ b/Sources/ExFigCLI/Lint/LintReporter.swift @@ -24,6 +24,7 @@ struct LintReporter { // MARK: - Text Output + // swiftlint:disable function_body_length private func reportText(diagnostics: [LintDiagnostic], ui: TerminalUI) { if diagnostics.isEmpty { ui.success("All lint checks passed") @@ -32,41 +33,65 @@ struct LintReporter { let errors = diagnostics.filter { $0.severity == .error } let warnings = diagnostics.filter { $0.severity == .warning } - let infos = diagnostics.filter { $0.severity == .info } - // Summary line - var parts: [String] = [] + // Summary header + ui.info("") + var summaryParts: [String] = [] if !errors.isEmpty { - parts.append(NooraUI.format(.danger("\(errors.count) error(s)"))) + summaryParts.append(NooraUI.format(.danger("\(errors.count) error(s)"))) } if !warnings.isEmpty { - parts.append(NooraUI.format(.accent("\(warnings.count) warning(s)"))) + summaryParts.append(NooraUI.format(.accent("\(warnings.count) warning(s)"))) } - if !infos.isEmpty { - parts.append(NooraUI.format(.muted("\(infos.count) info(s)"))) - } - ui.info("Lint: \(parts.joined(separator: ", "))") + ui.info(" \(summaryParts.joined(separator: " "))") + ui.info("") - // Group by rule + // Group by rule, sorted: errors first, then warnings let grouped = Dictionary(grouping: diagnostics) { $0.ruleId } - for (ruleId, items) in grouped.sorted(by: { $0.key < $1.key }) { + let sortedGroups = grouped.sorted { lhs, rhs in + let lSev = severityRank(lhs.value[0].severity) + let rSev = severityRank(rhs.value[0].severity) + if lSev != rSev { return lSev > rSev } + return lhs.key < rhs.key + } + + for (_, items) in sortedGroups { let first = items[0] let icon = severityIcon(first.severity) - ui.info("\(icon) \(first.ruleName) [\(ruleId)] (\(items.count))") - - for diag in items.prefix(10) { - let name = diag.componentName ?? diag.nodeId ?? "unknown" - ui.info(" \(name): \(diag.message)") - if let suggestion = diag.suggestion { - ui.info(" → \(suggestion)") - } + let countStr = useColors + ? NooraUI.format(.muted("(\(items.count))")) + : "(\(items.count))" + + ui.info(" \(icon) \(first.ruleName) \(countStr)") + + // Table: component name | message + let tableItems = items.prefix(8) + let maxNameLen = min( + tableItems.compactMap(\.componentName).map(\.count).max() ?? 10, + 30 + ) + + for diag in tableItems { + let name = diag.componentName ?? diag.nodeId ?? "?" + let truncated = name.count > 30 ? String(name.prefix(27)) + "..." : name + let padded = truncated.padding(toLength: max(maxNameLen, truncated.count), withPad: " ", startingAt: 0) + let nameStr = useColors ? NooraUI.format(.primary(padded)) : padded + let msgStr = useColors ? NooraUI.format(.muted(diag.message)) : diag.message + ui.info(" \(nameStr) \(msgStr)") } - if items.count > 10 { - ui.info(" ... +\(items.count - 10) more") + + if items.count > 8 { + let moreStr = useColors + ? NooraUI.format(.muted("... +\(items.count - 8) more")) + : "... +\(items.count - 8) more" + ui.info(" \(moreStr)") } + ui.info("") } } + // swiftlint:enable function_body_length + private func severityIcon(_ severity: LintSeverity) -> String { switch severity { case .error: useColors ? NooraUI.format(.danger("✗")) : "✗" @@ -75,6 +100,14 @@ struct LintReporter { } } + private func severityRank(_ severity: LintSeverity) -> Int { + switch severity { + case .error: 2 + case .warning: 1 + case .info: 0 + } + } + // MARK: - JSON Output private func reportJSON(diagnostics: [LintDiagnostic]) throws { diff --git a/Sources/ExFigCLI/Lint/LintTypes.swift b/Sources/ExFigCLI/Lint/LintTypes.swift index 0569d02f..f8838a7a 100644 --- a/Sources/ExFigCLI/Lint/LintTypes.swift +++ b/Sources/ExFigCLI/Lint/LintTypes.swift @@ -21,12 +21,34 @@ struct LintDiagnostic: Codable { let suggestion: String? } +/// Caches Figma API responses across lint rules to avoid duplicate requests. +actor LintDataCache { + private var componentsCache: [String: [Component]] = [:] + private var variablesCache: [String: VariablesMeta] = [:] + + func components(for fileId: String, client: any FigmaAPI.Client) async throws -> [Component] { + if let cached = componentsCache[fileId] { return cached } + let result = try await client.request(ComponentsEndpoint(fileId: fileId)) + componentsCache[fileId] = result + return result + } + + func variables(for fileId: String, client: any FigmaAPI.Client) async throws -> VariablesMeta { + if let cached = variablesCache[fileId] { return cached } + let result = try await client.request(VariablesEndpoint(fileId: fileId)) + variablesCache[fileId] = result + return result + } +} + /// Context provided to each lint rule for checking. struct LintContext { /// The resolved PKL configuration. let config: ExFig.ModuleImpl /// Figma API client. let client: any FigmaAPI.Client + /// Shared cache for Figma API responses across rules. + let cache: LintDataCache /// Terminal UI for progress reporting. let ui: TerminalUI } diff --git a/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift b/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift index 135255e3..68039b4d 100644 --- a/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift @@ -19,7 +19,7 @@ struct AliasChainIntegrityRule: LintRule { let variables: VariablesMeta do { - variables = try await context.client.request(VariablesEndpoint(fileId: fileId)) + variables = try await context.cache.variables(for: fileId, client: context.client) } catch { return [] } diff --git a/Sources/ExFigCLI/Lint/Rules/ComponentNotFrameRule.swift b/Sources/ExFigCLI/Lint/Rules/ComponentNotFrameRule.swift index b466e321..a46dae44 100644 --- a/Sources/ExFigCLI/Lint/Rules/ComponentNotFrameRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/ComponentNotFrameRule.swift @@ -30,7 +30,7 @@ struct ComponentNotFrameRule: LintRule { let components: [Component] do { - components = try await context.client.request(ComponentsEndpoint(fileId: fileId)) + components = try await context.cache.components(for: fileId, client: context.client) } catch { continue } diff --git a/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift b/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift index 9005bbca..5d8d0df2 100644 --- a/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift @@ -32,7 +32,7 @@ struct DarkModeSuffixRule: LintRule { let components: [Component] do { - components = try await context.client.request(ComponentsEndpoint(fileId: defaultFileId)) + components = try await context.cache.components(for: defaultFileId, client: context.client) } catch { return [] } diff --git a/Sources/ExFigCLI/Lint/Rules/DarkModeVariablesRule.swift b/Sources/ExFigCLI/Lint/Rules/DarkModeVariablesRule.swift index be52f837..e5ab9966 100644 --- a/Sources/ExFigCLI/Lint/Rules/DarkModeVariablesRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/DarkModeVariablesRule.swift @@ -22,23 +22,29 @@ struct DarkModeVariablesRule: LintRule { for entry in entries { guard !entry.fileId.isEmpty else { continue } - // Fetch components for this file let components: [Component] do { - components = try await context.client.request(ComponentsEndpoint(fileId: entry.fileId)) + components = try await context.cache.components(for: entry.fileId, client: context.client) } catch { continue } - // Filter to relevant frame let relevant = components.filter { comp in - if let frame = entry.frameName { - return comp.containingFrame.name == frame + if let page = entry.pageName, comp.containingFrame.pageName != page { + return false + } + if let frame = entry.frameName, comp.containingFrame.name != frame { + return false + } + // Skip RTL variants — they're duplicates for mirroring, not separate icons + if comp.containingFrame.containingComponentSet != nil, + comp.name.contains("RTL=") + { + return false } return true } - // Sample first 50 to avoid excessive API calls let sampled = Array(relevant.prefix(50)) guard !sampled.isEmpty else { continue } @@ -52,7 +58,12 @@ struct DarkModeVariablesRule: LintRule { for (nodeId, node) in nodes { let compName = sampled.first { $0.nodeId == nodeId }?.name ?? nodeId - checkNodeFills(node: node.document, componentName: compName, diagnostics: &diagnostics) + // Skip root node fills — check only children (vector shapes inside the icon) + checkChildrenFills( + children: node.document.children ?? [], + componentName: compName, + diagnostics: &diagnostics + ) } } @@ -61,18 +72,28 @@ struct DarkModeVariablesRule: LintRule { // MARK: - Node Fill Checking + /// Check fills only on leaf/vector nodes, not on the root component frame. + /// Root frames often have background fills (#FFFFFF) that aren't meant to be variable-bound. + private func checkChildrenFills( + children: [Document], + componentName: String, + diagnostics: inout [LintDiagnostic] + ) { + for child in children { + checkNodeFills(node: child, componentName: componentName, diagnostics: &diagnostics) + } + } + private func checkNodeFills( node: Document, componentName: String, diagnostics: inout [LintDiagnostic] ) { + // Check fills on this node (not root — already skipped by caller) for fill in node.fills { - // Skip images/videos if fill.type == .image { continue } - // Skip invisible if fill.opacity == 0 { continue } - // Check boundVariables on the fill if fill.boundVariables == nil || fill.boundVariables?["color"] == nil { let colorDesc: String = if let color = fill.color { String( @@ -108,16 +129,18 @@ struct DarkModeVariablesRule: LintRule { private struct VDMEntry { let fileId: String let frameName: String? + let pageName: String? } - private func collectVDMEntries(from config: PKLConfig, defaultFileId: String) -> [VDMEntry] { + private func collectVDMEntries(from config: ExFig.ModuleImpl, defaultFileId: String) -> [VDMEntry] { var entries: [VDMEntry] = [] func addIcons(_ icons: [some Common_FrameSource]?) { for entry in icons ?? [] where entry.variablesDarkMode != nil { entries.append(VDMEntry( fileId: entry.figmaFileId ?? defaultFileId, - frameName: entry.figmaFrameName + frameName: entry.figmaFrameName, + pageName: entry.figmaPageName )) } } diff --git a/Sources/ExFigCLI/Lint/Rules/DeletedVariablesRule.swift b/Sources/ExFigCLI/Lint/Rules/DeletedVariablesRule.swift index fddd2078..3af55973 100644 --- a/Sources/ExFigCLI/Lint/Rules/DeletedVariablesRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/DeletedVariablesRule.swift @@ -8,7 +8,7 @@ struct DeletedVariablesRule: LintRule { let id = "deleted-variables" let name = "No deleted variables" let description = "Variable collections must not contain deletedButReferenced variables" - let severity: LintSeverity = .error + let severity: LintSeverity = .warning func check(context: LintContext) async throws -> [LintDiagnostic] { let config = context.config @@ -22,7 +22,7 @@ struct DeletedVariablesRule: LintRule { let variables: VariablesMeta do { - variables = try await context.client.request(VariablesEndpoint(fileId: fileId)) + variables = try await context.cache.variables(for: fileId, client: context.client) } catch { continue } @@ -33,7 +33,7 @@ struct DeletedVariablesRule: LintRule { diagnostics.append(LintDiagnostic( ruleId: id, ruleName: name, - severity: .error, + severity: .warning, message: "Variable '\(variable.name)' is deleted but still referenced", componentName: variable.name, nodeId: variableId, diff --git a/Sources/ExFigCLI/Lint/Rules/DuplicateComponentNamesRule.swift b/Sources/ExFigCLI/Lint/Rules/DuplicateComponentNamesRule.swift new file mode 100644 index 00000000..cf3ea3b2 --- /dev/null +++ b/Sources/ExFigCLI/Lint/Rules/DuplicateComponentNamesRule.swift @@ -0,0 +1,131 @@ +import ExFigConfig +import ExFigCore +import FigmaAPI +import Foundation + +/// Checks for duplicate component names within configured frames/pages. +/// Duplicate names cause one to silently overwrite the other during export, +/// which is especially dangerous for dark mode pairs. +struct DuplicateComponentNamesRule: LintRule { + let id = "duplicate-component-names" + let name = "Duplicate component names" + let description = "No duplicate component names in configured frames" + let severity: LintSeverity = .error + + func check(context: LintContext) async throws -> [LintDiagnostic] { + let config = context.config + let defaultFileId = config.figma?.lightFileId ?? "" + guard !defaultFileId.isEmpty else { return [] } + + let configEntries = collectConfiguredEntries(from: config, defaultFileId: defaultFileId) + guard !configEntries.isEmpty else { return [] } + + var diagnostics: [LintDiagnostic] = [] + let grouped = Dictionary(grouping: configEntries) { $0.fileId } + + for (fileId, entries) in grouped { + guard !fileId.isEmpty else { continue } + + let components: [Component] + do { + components = try await context.cache.components(for: fileId, client: context.client) + } catch { + continue + } + + // Filter to only components matching configured frame/page pairs + // Skip RTL variants — they share names across component sets + let relevant = components.filter { comp in + if comp.containingFrame.containingComponentSet != nil, + comp.name.contains("RTL=") + { + return false + } + return entries.contains { entry in + matchesEntry(comp: comp, entry: entry) + } + } + + // Group by (pageName, componentName) to find duplicates + var seen: [String: [Component]] = [:] + for comp in relevant { + let page = comp.containingFrame.pageName ?? "(unknown)" + let key = "\(page)|\(comp.name)" + seen[key, default: []].append(comp) + } + + for (key, duplicates) in seen where duplicates.count > 1 { + let parts = key.split(separator: "|", maxSplits: 1) + let page = parts.first.map(String.init) ?? "(unknown)" + let compName = parts.count > 1 ? String(parts[1]) : key + let frames = duplicates.compactMap(\.containingFrame.name) + .map { "'\($0)'" } + let uniqueFrames = Array(Set(frames)).sorted() + + diagnostics.append(LintDiagnostic( + ruleId: id, + ruleName: self.name, + severity: .error, + message: "'\(compName)' appears \(duplicates.count)x on page '\(page)'" + + (uniqueFrames.count > 1 ? " in \(uniqueFrames.joined(separator: ", "))" : ""), + componentName: compName, + nodeId: duplicates.first?.nodeId, + suggestion: "Rename duplicates or move to different pages" + )) + } + } + + return diagnostics + } + + private func matchesEntry(comp: Component, entry: ConfigEntry) -> Bool { + if let page = entry.pageName, comp.containingFrame.pageName != page { + return false + } + if let frame = entry.frameName, comp.containingFrame.name != frame { + return false + } + return true + } + + private struct ConfigEntry { + let fileId: String + let frameName: String? + let pageName: String? + } + + private func collectConfiguredEntries( + from config: ExFig.ModuleImpl, + defaultFileId: String + ) -> [ConfigEntry] { + var entries: [ConfigEntry] = [] + + func add(_ icons: [some Common_FrameSource]?) { + for entry in icons ?? [] { + entries.append(ConfigEntry( + fileId: entry.figmaFileId ?? defaultFileId, + frameName: entry.figmaFrameName, + pageName: entry.figmaPageName + )) + } + } + + if let ios = config.ios { + add(ios.icons) + add(ios.images) + } + if let android = config.android { + add(android.icons) + add(android.images) + } + if let flutter = config.flutter { + add(flutter.icons) + add(flutter.images) + } + if let web = config.web { + add(web.icons) + } + + return entries + } +} diff --git a/Sources/ExFigCLI/Lint/Rules/FramePageMatchRule.swift b/Sources/ExFigCLI/Lint/Rules/FramePageMatchRule.swift index 04ef2991..1df2b3dc 100644 --- a/Sources/ExFigCLI/Lint/Rules/FramePageMatchRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/FramePageMatchRule.swift @@ -20,7 +20,7 @@ struct FramePageMatchRule: LintRule { for (fileId, fileEntries) in grouped { guard !fileId.isEmpty else { continue } - let result = try await checkFile(fileId: fileId, entries: fileEntries, client: context.client) + let result = try await checkFile(fileId: fileId, entries: fileEntries, context: context) diagnostics.append(contentsOf: result) } @@ -32,11 +32,11 @@ struct FramePageMatchRule: LintRule { private func checkFile( fileId: String, entries: [EntryInfo], - client: any FigmaAPI.Client + context: LintContext ) async throws -> [LintDiagnostic] { let components: [Component] do { - components = try await client.request(ComponentsEndpoint(fileId: fileId)) + components = try await context.cache.components(for: fileId, client: context.client) } catch { return [LintDiagnostic( ruleId: id, ruleName: name, severity: .error, diff --git a/Sources/ExFigCLI/Lint/Rules/NamingConventionRule.swift b/Sources/ExFigCLI/Lint/Rules/NamingConventionRule.swift index da94f503..e2afd278 100644 --- a/Sources/ExFigCLI/Lint/Rules/NamingConventionRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/NamingConventionRule.swift @@ -26,7 +26,7 @@ struct NamingConventionRule: LintRule { let components: [Component] do { - components = try await context.client.request(ComponentsEndpoint(fileId: entryFileId)) + components = try await context.cache.components(for: entryFileId, client: context.client) } catch { continue } diff --git a/Sources/ExFigCLI/MCP/MCPToolDefinitions.swift b/Sources/ExFigCLI/MCP/MCPToolDefinitions.swift index 3c2dbe24..945c3eb8 100644 --- a/Sources/ExFigCLI/MCP/MCPToolDefinitions.swift +++ b/Sources/ExFigCLI/MCP/MCPToolDefinitions.swift @@ -5,6 +5,7 @@ enum MCPToolDefinitions { static let allTools: [Tool] = [ validateTool, + lintTool, tokensInfoTool, inspectTool, exportTool, @@ -33,6 +34,39 @@ ]) ) + static let lintTool = Tool( + name: "exfig_lint", + description: """ + Lint Figma file structure against ExFig PKL config. Checks naming conventions, \ + frame/page structure, variable bindings, dark mode setup, duplicate components, \ + and deleted variables. Returns JSON with diagnostics. \ + Requires FIGMA_PERSONAL_TOKEN. + """, + inputSchema: .object([ + "type": .string("object"), + "properties": .object([ + "config_path": .object([ + "type": .string("string"), + "description": .string( + "Path to PKL config file. Auto-detects exfig.pkl in current directory if omitted." + ), + ]), + "rules": .object([ + "type": .string("string"), + "description": .string( + "Comma-separated rule IDs to run. " + + "Available: frame-page-match, naming-convention, component-not-frame, " + + "deleted-variables, duplicate-component-names, dark-mode-variables, dark-mode-suffix" + ), + ]), + "severity": .object([ + "type": .string("string"), + "description": .string("Minimum severity: error, warning, or info (default: info)"), + ]), + ]), + ]) + ) + static let tokensInfoTool = Tool( name: "exfig_tokens_info", description: """ diff --git a/Sources/ExFigCLI/MCP/MCPToolHandlers.swift b/Sources/ExFigCLI/MCP/MCPToolHandlers.swift index 3c526145..6a15cdc8 100644 --- a/Sources/ExFigCLI/MCP/MCPToolHandlers.swift +++ b/Sources/ExFigCLI/MCP/MCPToolHandlers.swift @@ -14,6 +14,8 @@ switch params.name { case "exfig_validate": return try await handleValidate(params: params) + case "exfig_lint": + return try await handleLint(params: params, state: state) case "exfig_tokens_info": return try await handleTokensInfo(params: params) case "exfig_inspect": @@ -128,6 +130,45 @@ return approaches.sorted() } + // MARK: - Lint + + private static func handleLint( + params: CallTool.Parameters, + state: MCPServerState + ) async throws -> CallTool.Result { + let configPath = try resolveConfigPath(from: params.arguments?["config_path"]?.stringValue) + let configURL = URL(fileURLWithPath: configPath) + let config = try await PKLEvaluator.evaluate(configPath: configURL) + + let client = try await state.getClient() + let cache = LintDataCache() + let ui = TerminalUI(outputMode: .quiet) + let context = LintContext(config: config, client: client, cache: cache, ui: ui) + + let ruleFilter: Set? = params.arguments?["rules"]?.stringValue.map { + Set($0.split(separator: ",").map { String($0.trimmingCharacters(in: .whitespaces)) }) + } + let minSeverity = params.arguments?["severity"]?.stringValue + .flatMap { LintSeverity(rawValue: $0) } ?? .info + + let engine = LintEngine.default + let diagnostics = try await engine.run( + context: context, + ruleFilter: ruleFilter, + minSeverity: minSeverity + ) + + let result = LintMCPResult( + configPath: configPath, + diagnosticsCount: diagnostics.count, + errorsCount: diagnostics.filter { $0.severity == .error }.count, + warningsCount: diagnostics.filter { $0.severity == .warning }.count, + diagnostics: diagnostics + ) + + return try .init(content: [.text(text: encodeJSON(result), annotations: nil, _meta: nil)]) + } + // MARK: - Tokens Info private static func handleTokensInfo(params: CallTool.Parameters) async throws -> CallTool.Result { @@ -775,6 +816,22 @@ // MARK: - Response Types + private struct LintMCPResult: Codable { + let configPath: String + let diagnosticsCount: Int + let errorsCount: Int + let warningsCount: Int + let diagnostics: [LintDiagnostic] + + enum CodingKeys: String, CodingKey { + case configPath = "config_path" + case diagnosticsCount = "diagnostics_count" + case errorsCount = "errors_count" + case warningsCount = "warnings_count" + case diagnostics + } + } + private struct ValidateSummary: Codable { let configPath: String let valid: Bool diff --git a/Sources/ExFigCLI/Subcommands/Lint.swift b/Sources/ExFigCLI/Subcommands/Lint.swift index 40ac11a2..7306af5d 100644 --- a/Sources/ExFigCLI/Subcommands/Lint.swift +++ b/Sources/ExFigCLI/Subcommands/Lint.swift @@ -40,13 +40,18 @@ extension ExFigCommand { var severity: String = "info" func run() async throws { + let outputFormat = LintOutputFormat(rawValue: format) ?? .text + + // JSON mode: force quiet to keep stdout clean for machine parsing + let effectiveVerbose = globalOptions.verbose + let effectiveQuiet = outputFormat == .json ? true : globalOptions.quiet + ExFigCommand.initializeTerminalUI( - verbose: globalOptions.verbose, quiet: globalOptions.quiet + verbose: effectiveVerbose, quiet: effectiveQuiet ) ExFigCommand.checkSchemaVersionIfNeeded() let ui = ExFigCommand.terminalUI! - let outputFormat = LintOutputFormat(rawValue: format) ?? .text let minSeverity = LintSeverity(rawValue: severity) ?? .info let ruleFilter: Set? = rules.map { Set($0.split(separator: ",").map { String($0.trimmingCharacters(in: .whitespaces)) }) @@ -59,15 +64,20 @@ extension ExFigCommand { ui: ui ) - let context = LintContext(config: options.params, client: client, ui: ui) + let cache = LintDataCache() + let context = LintContext(config: options.params, client: client, cache: cache, ui: ui) let engine = LintEngine.default - let diagnostics = try await ui.withSpinner("Linting Figma file structure...") { + let diagnostics = try await ui.withSpinnerMessage( + "Linting..." + ) { updateMessage in try await engine.run( context: context, ruleFilter: ruleFilter, minSeverity: minSeverity - ) + ) { message in + updateMessage(message) + } } let reporter = LintReporter( @@ -76,7 +86,6 @@ extension ExFigCommand { ) try reporter.report(diagnostics: diagnostics, ui: ui) - // Exit with code 1 if there are errors (for CI) if diagnostics.contains(where: { $0.severity == .error }) { throw ExitCode.failure } diff --git a/Sources/ExFigCLI/TerminalUI/TerminalUI.swift b/Sources/ExFigCLI/TerminalUI/TerminalUI.swift index 2c35ac42..b04d6ad2 100644 --- a/Sources/ExFigCLI/TerminalUI/TerminalUI.swift +++ b/Sources/ExFigCLI/TerminalUI/TerminalUI.swift @@ -2,6 +2,8 @@ import ExFigCore import Foundation +// swiftlint:disable type_body_length + /// Main facade for terminal UI operations final class TerminalUI: Sendable { let outputMode: OutputMode @@ -296,6 +298,46 @@ final class TerminalUI: Sendable { } } + /// Execute an operation with a spinner that supports dynamic message updates. + func withSpinnerMessage( + _ message: String, + operation: @Sendable @escaping (@escaping @Sendable (String) -> Void) async throws -> T + ) async rethrows -> T { + if BatchSharedState.current?.progressView != nil { + return try await operation { _ in } + } + + if TerminalOutputManager.shared.hasActiveAnimation { + return try await operation { _ in } + } + + guard outputMode.showProgress else { + if outputMode != .quiet { + TerminalOutputManager.shared.print(message) + } + return try await operation { _ in } + } + + let spinner = Spinner( + message: message, + useColors: useColors, + useAnimations: useAnimations + ) + + spinner.start() + + do { + let result = try await operation { newMessage in + spinner.update(message: newMessage) + } + spinner.succeed() + return result + } catch { + spinner.fail() + throw error + } + } + // MARK: - Progress Bar Operations /// Execute an operation with a progress bar diff --git a/Tests/ExFigTests/Lint/LintRulesTests.swift b/Tests/ExFigTests/Lint/LintRulesTests.swift new file mode 100644 index 00000000..8db6e2b7 --- /dev/null +++ b/Tests/ExFigTests/Lint/LintRulesTests.swift @@ -0,0 +1,568 @@ +// swiftlint:disable file_length +@testable import ExFigCLI +import ExFigConfig +import ExFigCore +import FigmaAPI +import Foundation +import Testing + +// MARK: - Test Helpers + +private func makeLintContext( + config: PKLConfig, + client: MockClient +) -> LintContext { + let ui = TerminalUI(outputMode: .quiet) + return LintContext(config: config, client: client, cache: LintDataCache(), ui: ui) +} + +/// Creates a PKLConfig with iOS icons entries for lint testing. +private func makeIOSIconsConfig( + lightFileId: String = "abc123", + frameName: String? = nil, + pageName: String? = nil, + nameValidateRegexp: String? = nil, + suffixDarkMode: String? = nil +) -> PKLConfig { + var entryParts: [String] = [ + "\"assetsFolder\": \"Icons\"", + "\"format\": \"svg\"", + "\"nameStyle\": \"camelCase\"", + ] + if let frameName { entryParts.append("\"figmaFrameName\": \"\(frameName)\"") } + if let pageName { entryParts.append("\"figmaPageName\": \"\(pageName)\"") } + if let regex = nameValidateRegexp { entryParts.append("\"nameValidateRegexp\": \"\(regex)\"") } + + var commonParts: [String] = [] + if let suffix = suffixDarkMode { + commonParts.append("\"icons\": { \"suffixDarkMode\": { \"suffix\": \"\(suffix)\" } }") + } + let commonJson = commonParts.isEmpty ? "" : ", \"common\": { \(commonParts.joined(separator: ", ")) }" + + let json = """ + { + "figma": { "lightFileId": "\(lightFileId)" }, + "ios": { + "xcodeprojPath": "App.xcodeproj", + "target": "App", + "xcassetsPath": "Assets.xcassets", + "xcassetsInMainBundle": true, + "icons": [{ \(entryParts.joined(separator: ", ")) }] + }\(commonJson) + } + """ + // swiftlint:disable:next force_try + return try! JSONCodec.decode(PKLConfig.self, from: Data(json.utf8)) +} + +// MARK: - FramePageMatchRule Tests + +struct FramePageMatchRuleTests { + let rule = FramePageMatchRule() + + @Test("passes when frame and page exist") + func passesWhenFrameAndPageExist() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "icon_home", frameName: "Icons", pageName: "Components"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons", pageName: "Components") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.isEmpty) + } + + @Test("error when page not found") + func errorWhenPageNotFound() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "icon_home", frameName: "Icons", pageName: "Page A"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons", pageName: "NonExistent") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.count >= 1) + #expect(diagnostics.first?.ruleId == "frame-page-match") + #expect(diagnostics.first?.severity == .error) + #expect(diagnostics.first?.message.contains("NonExistent") == true) + } + + @Test("error when frame not found on page") + func errorWhenFrameNotFound() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "icon_home", frameName: "OtherFrame", pageName: "Components"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "MissingFrame", pageName: "Components") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.contains { $0.message.contains("MissingFrame") && $0.message.contains("not found") }) + } + + @Test("skips when no entries configured") + func skipsWhenNoEntries() async throws { + let client = MockClient() + let config = PKLConfig.make(lightFileId: "abc123") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.isEmpty) + } +} + +// MARK: - NamingConventionRule Tests + +struct NamingConventionRuleTests { + let rule = NamingConventionRule() + + @Test("passes when names match regex") + func passesWhenNamesMatch() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "ic_home", frameName: "Icons"), + Component.make(nodeId: "1:2", name: "ic_settings", frameName: "Icons"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons", nameValidateRegexp: "^ic_[a-z_]+$") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.isEmpty) + } + + @Test("error when name violates regex") + func errorWhenNameViolatesRegex() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "ic_home", frameName: "Icons"), + Component.make(nodeId: "1:2", name: "BadName", frameName: "Icons"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons", nameValidateRegexp: "^ic_[a-z_]+$") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.count == 1) + #expect(diagnostics.first?.componentName == "BadName") + #expect(diagnostics.first?.severity == .error) + } + + @Test("skips entries without nameValidateRegexp") + func skipsWithoutRegex() async throws { + let client = MockClient() + let config = makeIOSIconsConfig(frameName: "Icons") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.isEmpty) + } +} + +// MARK: - DeletedVariablesRule Tests + +struct DeletedVariablesRuleTests { + let rule = DeletedVariablesRule() + + @Test("passes when no deleted variables") + func passesWhenNoDeletedVars() async throws { + let client = MockClient() + let variables = VariablesMeta.make( + variables: [ + (id: "1:1", name: "Primary", valuesByMode: ["1:0": (r: 1, g: 0, b: 0, a: 1)]), + ] + ) + client.setResponse(variables, for: VariablesEndpoint.self) + + let config = makeConfigWithVariablesColors() + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.isEmpty) + } + + @Test("error when deleted variable found") + func errorWhenDeletedVar() async throws { + let client = MockClient() + let variables = VariablesMeta.makeWithAliases( + variables: [ + ( + id: "1:1", + name: "Old/Deprecated", + collectionId: nil, + valuesByMode: ["1:0": .color(r: 1, g: 0, b: 0, a: 1)] + ), + ], + deletedVariableIds: ["1:1"] + ) + client.setResponse(variables, for: VariablesEndpoint.self) + + let config = makeConfigWithVariablesColors() + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.count == 1) + #expect(diagnostics.first?.ruleId == "deleted-variables") + #expect(diagnostics.first?.componentName == "Old/Deprecated") + } +} + +private func makeConfigWithVariablesColors() -> PKLConfig { + let json = """ + { + "figma": { "lightFileId": "abc123" }, + "common": { + "variablesColors": { + "tokensFileId": "abc123", + "tokensCollectionName": "Colors", + "lightModeName": "Light", + "darkModeName": "Dark" + } + } + } + """ + // swiftlint:disable:next force_try + return try! JSONCodec.decode(PKLConfig.self, from: Data(json.utf8)) +} + +// MARK: - AliasChainIntegrityRule Tests + +struct AliasChainIntegrityRuleTests { + let rule = AliasChainIntegrityRule() + + @Test("passes with valid alias chain") + func passesWithValidChain() async throws { + let client = MockClient() + let variables = VariablesMeta.makeWithAliases( + variables: [ + ( + id: "1:1", + name: "Semantic/Primary", + collectionId: nil, + valuesByMode: ["1:0": .alias("1:2")] + ), + ( + id: "1:2", + name: "Primitive/Blue", + collectionId: nil, + valuesByMode: ["1:0": .color(r: 0, g: 0, b: 1, a: 1)] + ), + ] + ) + client.setResponse(variables, for: VariablesEndpoint.self) + + let config = PKLConfig.make(lightFileId: "abc123") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.isEmpty) + } + + @Test("error with broken alias chain") + func errorWithBrokenChain() async throws { + let client = MockClient() + let variables = VariablesMeta.makeWithAliases( + variables: [ + ( + id: "1:1", + name: "Semantic/Primary", + collectionId: nil, + valuesByMode: ["1:0": .alias("9:9")] + ), + ] + ) + client.setResponse(variables, for: VariablesEndpoint.self) + + let config = PKLConfig.make(lightFileId: "abc123") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.count == 1) + #expect(diagnostics.first?.ruleId == "alias-chain-integrity") + #expect(diagnostics.first?.severity == .error) + } + + @Test("error with circular alias chain") + func errorWithCircularChain() async throws { + let client = MockClient() + let variables = VariablesMeta.makeWithAliases( + variables: [ + (id: "1:1", name: "A", collectionId: nil, valuesByMode: ["1:0": .alias("1:2")]), + (id: "1:2", name: "B", collectionId: nil, valuesByMode: ["1:0": .alias("1:1")]), + ] + ) + client.setResponse(variables, for: VariablesEndpoint.self) + + let config = PKLConfig.make(lightFileId: "abc123") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.contains { $0.message.contains("circular") }) + } + + @Test("skips cross-file alias references") + func skipsCrossFileAliases() async throws { + let client = MockClient() + let crossFileId = "806fcc6a84cf048f0a06837634440ecad91622fe/3556:423" + let variables = VariablesMeta.makeWithAliases( + variables: [ + ( + id: "1:1", + name: "Semantic/Primary", + collectionId: nil, + valuesByMode: ["1:0": .alias(crossFileId)] + ), + ] + ) + client.setResponse(variables, for: VariablesEndpoint.self) + + let config = PKLConfig.make(lightFileId: "abc123") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.isEmpty) + } +} + +// MARK: - ComponentNotFrameRule Tests + +struct ComponentNotFrameRuleTests { + let rule = ComponentNotFrameRule() + + @Test("passes when frame has components") + func passesWhenComponentsExist() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "icon_home", frameName: "Icons"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.isEmpty) + } + + @Test("error when frame has no components") + func errorWhenNoComponents() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "icon_home", frameName: "OtherFrame"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "EmptyFrame") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.count == 1) + #expect(diagnostics.first?.ruleId == "component-not-frame") + #expect(diagnostics.first?.message.contains("EmptyFrame") == true) + } +} + +// MARK: - DarkModeSuffixRule Tests + +struct DarkModeSuffixRuleTests { + let rule = DarkModeSuffixRule() + + @Test("passes when all light components have dark pairs") + func passesWithDarkPairs() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "icon_home", frameName: "Icons"), + Component.make(nodeId: "1:2", name: "icon_home-dark", frameName: "Icons"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons", suffixDarkMode: "-dark") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.isEmpty) + } + + @Test("warning when dark pair missing") + func warningWhenDarkPairMissing() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "icon_home", frameName: "Icons"), + Component.make(nodeId: "1:2", name: "icon_settings", frameName: "Icons"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons", suffixDarkMode: "-dark") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.count == 2) + #expect(diagnostics.allSatisfy { $0.severity == .warning }) + } + + @Test("only checks components in configured frames") + func onlyChecksConfiguredFrames() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "icon_home", frameName: "Icons"), + Component.make(nodeId: "1:2", name: "flag_us", frameName: "Flags"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons", suffixDarkMode: "-dark") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + // Only icon_home should be checked, not flag_us + #expect(diagnostics.count == 1) + #expect(diagnostics.first?.componentName == "icon_home") + } + + @Test("skips when no suffixDarkMode configured") + func skipsWithoutSuffix() async throws { + let client = MockClient() + let config = makeIOSIconsConfig(frameName: "Icons") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.isEmpty) + } +} + +// MARK: - DuplicateComponentNamesRule Tests + +struct DuplicateComponentNamesRuleTests { + let rule = DuplicateComponentNamesRule() + + @Test("passes when no duplicates") + func passesNoDuplicates() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "icon_home", frameName: "Icons"), + Component.make(nodeId: "1:2", name: "icon_settings", frameName: "Icons"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.isEmpty) + } + + @Test("error when duplicate names on same page") + func errorOnDuplicates() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "icon_home", frameName: "Icons"), + Component.make(nodeId: "1:2", name: "icon_home", frameName: "Icons"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.count == 1) + #expect(diagnostics.first?.message.contains("2x") == true) + } + + @Test("skips RTL variants") + func skipsRTLVariants() async throws { + let client = MockClient() + client.setResponse([ + makeVariantComponent(nodeId: "1:1", name: "RTL=On", frameName: "Icons", componentSetName: "icon_home"), + makeVariantComponent(nodeId: "1:2", name: "RTL=On", frameName: "Icons", componentSetName: "icon_settings"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + #expect(diagnostics.isEmpty) + } + + @Test("only checks configured frames") + func onlyConfiguredFrames() async throws { + let client = MockClient() + client.setResponse([ + Component.make(nodeId: "1:1", name: "duplicate", frameName: "Icons"), + Component.make(nodeId: "1:2", name: "duplicate", frameName: "Icons"), + Component.make(nodeId: "1:3", name: "duplicate", frameName: "OtherFrame"), + Component.make(nodeId: "1:4", name: "duplicate", frameName: "OtherFrame"), + ], for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons") + let context = makeLintContext(config: config, client: client) + let diagnostics = try await rule.check(context: context) + + // Only the Icons frame pair should be flagged + #expect(diagnostics.count == 1) + } +} + +// MARK: - Variant Component Helper + +/// Creates a Component that is a variant inside a component set (has containingComponentSet). +private func makeVariantComponent( + nodeId: String, + name: String, + frameName: String = "Icons", + pageName: String = "Components", + componentSetName: String +) -> Component { + let json = """ + { + "key": "test-key", + "node_id": "\(nodeId)", + "name": "\(name)", + "containing_frame": { + "nodeId": "\(nodeId)", + "name": "\(frameName)", + "pageName": "\(pageName)", + "containingComponentSet": { + "nodeId": "set:\(nodeId)", + "name": "\(componentSetName)" + } + } + } + """ + // swiftlint:disable:next force_try + return try! JSONCodec.decode(Component.self, from: Data(json.utf8)) +} + +// MARK: - LintEngine Tests + +struct LintEngineTests { + @Test("runs all rules and collects diagnostics") + func runsAllRules() async throws { + let client = MockClient() + client.setResponse([Component](), for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons") + let ui = TerminalUI(outputMode: .quiet) + let context = LintContext(config: config, client: client, cache: LintDataCache(), ui: ui) + + let engine = LintEngine.default + let diagnostics = try await engine.run(context: context) + + // With empty components, component-not-frame should fire + #expect(diagnostics.contains { $0.ruleId == "component-not-frame" }) + } + + @Test("filters by rule ID") + func filtersByRuleId() async throws { + let client = MockClient() + client.setResponse([Component](), for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons") + let ui = TerminalUI(outputMode: .quiet) + let context = LintContext(config: config, client: client, cache: LintDataCache(), ui: ui) + + let engine = LintEngine.default + let diagnostics = try await engine.run(context: context, ruleFilter: ["deleted-variables"]) + + // Only deleted-variables rule should run (may have info from failed rule) + #expect(!diagnostics.contains { $0.ruleId == "component-not-frame" }) + } +} + +// swiftlint:enable file_length diff --git a/llms-full.txt b/llms-full.txt index 92c684b3..7c7ac023 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -249,9 +249,12 @@ ios = new iOS.iOSConfig { } ``` -### 4. Export Resources +### 4. Validate & Export ```bash +# Validate Figma file structure before exporting +exfig lint + # Export individual resource types exfig colors exfig icons From 52347ac6b85705a786d3358c959920420eb4afef Mon Sep 17 00:00:00 2001 From: alexey1312 Date: Tue, 31 Mar 2026 20:32:46 +0500 Subject: [PATCH 6/6] =?UTF-8?q?fix(lint):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20error=20handling,=20page=20filtering,=20type=20safe?= =?UTF-8?q?ty?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Register AliasChainIntegrityRule in default engine (was dead code) - Replace silent `catch { continue }` with error diagnostics in 6 rules - Raise engine catch severity from .info to .error, propagate CancellationError - Add figmaPageName filtering to NamingConventionRule and DarkModeSuffixRule - Add RTL variant skip to DarkModeSuffixRule - Use ExpressibleByArgument for --format/--severity (reject invalid values) - Add Comparable to LintSeverity, remove duplicated severityRank() - Add diagnostic() factory on LintRule to reduce boilerplate - Fix Usage.md severities and add missing duplicate-component-names rule - Add flutter.images to NamingConventionRule - Add sampling info diagnostic in DarkModeVariablesRule - Route JSON output through TerminalUI instead of print() - Validate MCP lint params before expensive PKL eval - Add engine composition, error handling, and severity filtering tests --- CLAUDE.md | 9 +- Sources/ExFigCLI/ExFig.docc/Usage.md | 19 +-- Sources/ExFigCLI/Lint/LintEngine.swift | 17 +-- Sources/ExFigCLI/Lint/LintReporter.swift | 29 ++--- Sources/ExFigCLI/Lint/LintTypes.swift | 42 ++++++- .../Lint/Rules/AliasChainIntegrityRule.swift | 29 +++-- .../Lint/Rules/ComponentNotFrameRule.swift | 20 +-- .../Lint/Rules/DarkModeSuffixRule.swift | 73 ++++++++--- .../Lint/Rules/DarkModeVariablesRule.swift | 115 +++++++++++------- .../Lint/Rules/DeletedVariablesRule.swift | 10 +- .../Rules/DuplicateComponentNamesRule.swift | 18 ++- .../Lint/Rules/NamingConventionRule.swift | 101 ++++++++------- Sources/ExFigCLI/MCP/MCPToolHandlers.swift | 13 +- Sources/ExFigCLI/Subcommands/Lint.swift | 13 +- Tests/ExFigTests/Lint/LintRulesTests.swift | 70 ++++++++++- llms-full.txt | 19 +-- 16 files changed, 391 insertions(+), 206 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9ffe3f9b..902d208f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -322,6 +322,7 @@ Changing `load()` return type affects: `exfig lint -i exfig.pkl` validates Figma file structure against PKL config. Rules in `Sources/ExFigCLI/Lint/Rules/`, engine in `LintEngine.swift`. Each rule implements `LintRule` protocol with `check(context: LintContext) -> [LintDiagnostic]`. +`LintRule` extension provides `diagnostic()` factory pre-filled with rule metadata — use instead of raw `LintDiagnostic` init. Uses `FigmaAPI.Client.request(SomeEndpoint(...))` directly (no convenience methods on Client). `ComponentsEndpoint` returns `[Component]`, `VariablesEndpoint` returns `VariablesMeta`, `NodesEndpoint` returns `[NodeId: Node]`. @@ -333,6 +334,11 @@ Uses `FigmaAPI.Client.request(SomeEndpoint(...))` directly (no convenience metho - Skip root frame fills when checking boundVariables — root `Document` fills are backgrounds, check only children - Cross-file variable IDs (32+ char hex hash before `/`) are valid external library refs, not broken aliases - `LintDataCache` actor caches Components/Variables API responses — use `context.cache.components(for:client:)` +- Rules MUST emit diagnostics on API failure — never `catch { continue }` silently. Follow `FramePageMatchRule` pattern +- Empty `fileId` guards must return a diagnostic, not silently `return []` +- `LintSeverity` is `Comparable` — use `>=` directly, no `severityRank()` helpers +- `LintOutputFormat` and `LintSeverity` conform to `ExpressibleByArgument` — use as `@Option` types directly +- Adding error handling to `check()` increases cyclomatic complexity — extract per-entry logic into private methods ### Adding a CLI Command @@ -519,10 +525,11 @@ NooraUI.formatLink("url", useColors: true) // underlined primary | Figma variable IDs file-scoped | Variable IDs differ between files — alias targets from file A can't be found by ID in file B. Use name-based matching (`resolveViaLibrary`) + mode name matching (not modeId) for cross-file resolution | | `assertionFailure` in release | `assertionFailure` is stripped in release builds — add `FileHandle.standardError.write()` as production fallback for truly-impossible-but-must-not-be-silent error paths | | Components API called N times | `ComponentPreFetcher` only works in batch mode — use `ComponentsCache` via `SourceFactory(componentsCache:)` for standalone multi-entry dedup | -| Config type reference | `ExFigOptions.params` is `ExFig.ModuleImpl!` — no `PKLConfig` typealias exists | +| Config type reference | `ExFigOptions.params` is `ExFig.ModuleImpl!` — `PKLConfig` is a typealias in `PKLConfigCompat.swift`, both names work | | `Paint.visible` doesn't exist | FigmaAPI `Paint` has no `visible` field — use `opacity` to check visibility | | `variablesColors` location | On `Common.CommonConfig` (`config.common?.variablesColors`), NOT on `config.common?.colors?.variablesColors` | | `cp` prompts overwrite | macOS `trash` alias intercepts `cp`; use `/bin/cp -f` to force overwrite without prompt | +| SwiftFormat `///` before nested `func` | SwiftFormat converts `//` to `///` before `func` inside method bodies — this is expected, don't fight it | ## Additional Rules diff --git a/Sources/ExFigCLI/ExFig.docc/Usage.md b/Sources/ExFigCLI/ExFig.docc/Usage.md index 799dcc4c..b927f372 100644 --- a/Sources/ExFigCLI/ExFig.docc/Usage.md +++ b/Sources/ExFigCLI/ExFig.docc/Usage.md @@ -276,15 +276,16 @@ exfig lint -i exfig.pkl --format json --severity error ### Available Rules -| Rule | Severity | Description | -| ----------------------- | -------- | ------------------------------------------------------ | -| `frame-page-match` | error | Frame/page names in config exist in Figma file | -| `naming-convention` | error | Component names match `nameValidateRegexp` patterns | -| `component-not-frame` | error | Configured frames contain published components | -| `deleted-variables` | error | No `deletedButReferenced` variables in collections | -| `alias-chain-integrity` | error | Variable alias chains resolve without broken refs | -| `dark-mode-variables` | error | With `variablesDarkMode`, fills bound to Variables | -| `dark-mode-suffix` | warning | With `suffixDarkMode`, light components have dark pair | +| Rule | Severity | Description | +| -------------------------- | -------- | ------------------------------------------------------ | +| `frame-page-match` | error | Frame/page names in config exist in Figma file | +| `naming-convention` | error | Component names match `nameValidateRegexp` patterns | +| `component-not-frame` | error | Configured frames contain published components | +| `duplicate-component-names`| error | No duplicate component names in configured frames | +| `deleted-variables` | warning | No `deletedButReferenced` variables in collections | +| `alias-chain-integrity` | warning | Variable alias chains resolve without broken refs | +| `dark-mode-variables` | error | With `variablesDarkMode`, fills bound to Variables | +| `dark-mode-suffix` | warning | With `suffixDarkMode`, light components have dark pair | ## Help and Version diff --git a/Sources/ExFigCLI/Lint/LintEngine.swift b/Sources/ExFigCLI/Lint/LintEngine.swift index 7e571623..34148db8 100644 --- a/Sources/ExFigCLI/Lint/LintEngine.swift +++ b/Sources/ExFigCLI/Lint/LintEngine.swift @@ -19,7 +19,7 @@ struct LintEngine { if let filter = ruleFilter, !filter.contains(rule.id) { return false } - return severityRank(rule.severity) >= severityRank(minSeverity) + return rule.severity >= minSeverity } var allDiagnostics: [LintDiagnostic] = [] @@ -31,29 +31,23 @@ struct LintEngine { do { let diagnostics = try await rule.check(context: context) allDiagnostics.append(contentsOf: diagnostics) + } catch is CancellationError { + throw CancellationError() } catch { allDiagnostics.append(LintDiagnostic( ruleId: rule.id, ruleName: rule.name, - severity: .info, + severity: .error, message: "Rule check failed: \(error.localizedDescription)", componentName: nil, nodeId: nil, - suggestion: nil + suggestion: "Check FIGMA_PERSONAL_TOKEN and network connectivity" )) } } return allDiagnostics } - - private func severityRank(_ severity: LintSeverity) -> Int { - switch severity { - case .error: 2 - case .warning: 1 - case .info: 0 - } - } } extension LintEngine { @@ -64,6 +58,7 @@ extension LintEngine { ComponentNotFrameRule(), DeletedVariablesRule(), DuplicateComponentNamesRule(), + AliasChainIntegrityRule(), DarkModeVariablesRule(), DarkModeSuffixRule(), ]) diff --git a/Sources/ExFigCLI/Lint/LintReporter.swift b/Sources/ExFigCLI/Lint/LintReporter.swift index 832a42b4..8d0c34ec 100644 --- a/Sources/ExFigCLI/Lint/LintReporter.swift +++ b/Sources/ExFigCLI/Lint/LintReporter.swift @@ -2,12 +2,6 @@ import ExFigCore import Foundation import Noora -/// Output format for lint results. -enum LintOutputFormat: String { - case text - case json -} - /// Formats and outputs lint results. struct LintReporter { let format: LintOutputFormat @@ -18,7 +12,7 @@ struct LintReporter { case .text: reportText(diagnostics: diagnostics, ui: ui) case .json: - try reportJSON(diagnostics: diagnostics) + try reportJSON(diagnostics: diagnostics, ui: ui) } } @@ -46,11 +40,10 @@ struct LintReporter { ui.info(" \(summaryParts.joined(separator: " "))") ui.info("") - // Group by rule, sorted: errors first, then warnings let grouped = Dictionary(grouping: diagnostics) { $0.ruleId } let sortedGroups = grouped.sorted { lhs, rhs in - let lSev = severityRank(lhs.value[0].severity) - let rSev = severityRank(rhs.value[0].severity) + let lSev = lhs.value[0].severity + let rSev = rhs.value[0].severity if lSev != rSev { return lSev > rSev } return lhs.key < rhs.key } @@ -64,7 +57,6 @@ struct LintReporter { ui.info(" \(icon) \(first.ruleName) \(countStr)") - // Table: component name | message let tableItems = items.prefix(8) let maxNameLen = min( tableItems.compactMap(\.componentName).map(\.count).max() ?? 10, @@ -100,17 +92,9 @@ struct LintReporter { } } - private func severityRank(_ severity: LintSeverity) -> Int { - switch severity { - case .error: 2 - case .warning: 1 - case .info: 0 - } - } - // MARK: - JSON Output - private func reportJSON(diagnostics: [LintDiagnostic]) throws { + private func reportJSON(diagnostics: [LintDiagnostic], ui: TerminalUI) throws { let report = LintReport( diagnosticsCount: diagnostics.count, errorsCount: diagnostics.filter { $0.severity == .error }.count, @@ -118,7 +102,10 @@ struct LintReporter { diagnostics: diagnostics ) let data = try JSONCodec.encode(report) - print(String(data: data, encoding: .utf8) ?? "{}") + guard let jsonString = String(data: data, encoding: .utf8) else { + throw ExFigError.custom(errorString: "Failed to encode lint report as UTF-8") + } + ui.info(jsonString) } } diff --git a/Sources/ExFigCLI/Lint/LintTypes.swift b/Sources/ExFigCLI/Lint/LintTypes.swift index f8838a7a..3aa95c23 100644 --- a/Sources/ExFigCLI/Lint/LintTypes.swift +++ b/Sources/ExFigCLI/Lint/LintTypes.swift @@ -1,13 +1,32 @@ +import ArgumentParser import ExFigConfig import ExFigCore import FigmaAPI import Foundation /// Severity level for lint diagnostics. -enum LintSeverity: String, CaseIterable, Codable { +enum LintSeverity: String, CaseIterable, Codable, Comparable, ExpressibleByArgument { case error case warning case info + + static func < (lhs: LintSeverity, rhs: LintSeverity) -> Bool { + lhs.rank < rhs.rank + } + + private var rank: Int { + switch self { + case .error: 2 + case .warning: 1 + case .info: 0 + } + } +} + +/// Output format for lint results. +enum LintOutputFormat: String, ExpressibleByArgument { + case text + case json } /// A single finding from a lint rule. @@ -67,3 +86,24 @@ protocol LintRule: Sendable { /// Run the check and return diagnostics (empty = all good). func check(context: LintContext) async throws -> [LintDiagnostic] } + +extension LintRule { + /// Create a diagnostic pre-filled with this rule's metadata. + func diagnostic( + severity: LintSeverity? = nil, + message: String, + componentName: String? = nil, + nodeId: String? = nil, + suggestion: String? = nil + ) -> LintDiagnostic { + LintDiagnostic( + ruleId: id, + ruleName: name, + severity: severity ?? self.severity, + message: message, + componentName: componentName, + nodeId: nodeId, + suggestion: suggestion + ) + } +} diff --git a/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift b/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift index 68039b4d..9deeb723 100644 --- a/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/AliasChainIntegrityRule.swift @@ -3,25 +3,34 @@ import ExFigCore import FigmaAPI import Foundation -/// Checks that all variable alias chains resolve successfully (no broken refs, depth <= 10). +/// Checks that all variable alias chains resolve successfully (no broken refs, depth < 10). struct AliasChainIntegrityRule: LintRule { let id = "alias-chain-integrity" let name = "Alias chain integrity" let description = "All variable alias chains must resolve without broken references" - let severity: LintSeverity = .error + let severity: LintSeverity = .warning private let maxDepth = 10 func check(context: LintContext) async throws -> [LintDiagnostic] { let config = context.config let fileId = config.figma?.lightFileId ?? "" - guard !fileId.isEmpty else { return [] } + guard !fileId.isEmpty else { + return [diagnostic( + message: "No figma.lightFileId configured — skipping rule", + suggestion: "Set figma.lightFileId in your PKL config" + )] + } let variables: VariablesMeta do { variables = try await context.cache.variables(for: fileId, client: context.client) } catch { - return [] + return [diagnostic( + severity: .error, + message: "Cannot fetch variables for file '\(fileId)': \(error.localizedDescription)", + suggestion: "Check FIGMA_PERSONAL_TOKEN and file permissions" + )] } var diagnostics: [LintDiagnostic] = [] @@ -41,9 +50,7 @@ struct AliasChainIntegrityRule: LintRule { case .resolved: break case let .broken(targetId): - diagnostics.append(LintDiagnostic( - ruleId: id, - ruleName: name, + diagnostics.append(diagnostic( severity: .error, message: "'\(variable.name)' mode '\(modeId)' refs non-existent '\(targetId)'", componentName: variable.name, @@ -51,9 +58,7 @@ struct AliasChainIntegrityRule: LintRule { suggestion: "Fix or remove the alias reference" )) case .circular: - diagnostics.append(LintDiagnostic( - ruleId: id, - ruleName: name, + diagnostics.append(diagnostic( severity: .error, message: "'\(variable.name)' has a circular alias chain", componentName: variable.name, @@ -61,9 +66,7 @@ struct AliasChainIntegrityRule: LintRule { suggestion: "Break the circular reference" )) case .tooDeep: - diagnostics.append(LintDiagnostic( - ruleId: id, - ruleName: name, + diagnostics.append(diagnostic( severity: .warning, message: "'\(variable.name)' alias chain exceeds depth \(maxDepth)", componentName: variable.name, diff --git a/Sources/ExFigCLI/Lint/Rules/ComponentNotFrameRule.swift b/Sources/ExFigCLI/Lint/Rules/ComponentNotFrameRule.swift index a46dae44..1e381095 100644 --- a/Sources/ExFigCLI/Lint/Rules/ComponentNotFrameRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/ComponentNotFrameRule.swift @@ -15,7 +15,6 @@ struct ComponentNotFrameRule: LintRule { func check(context: LintContext) async throws -> [LintDiagnostic] { let config = context.config let defaultFileId = config.figma?.lightFileId ?? "" - guard !defaultFileId.isEmpty else { return [] } var diagnostics: [LintDiagnostic] = [] @@ -26,12 +25,23 @@ struct ComponentNotFrameRule: LintRule { let grouped = Dictionary(grouping: entries) { $0.fileId } for (fileId, fileEntries) in grouped { - guard !fileId.isEmpty else { continue } + guard !fileId.isEmpty else { + diagnostics.append(diagnostic( + message: "No figma.lightFileId configured — skipping rule", + suggestion: "Set figma.lightFileId in your PKL config" + )) + continue + } let components: [Component] do { components = try await context.cache.components(for: fileId, client: context.client) } catch { + diagnostics.append(diagnostic( + severity: .error, + message: "Cannot fetch components for file '\(fileId)': \(error.localizedDescription)", + suggestion: "Check FIGMA_PERSONAL_TOKEN and file permissions" + )) continue } @@ -47,13 +57,9 @@ struct ComponentNotFrameRule: LintRule { } if matchingComponents.isEmpty { - diagnostics.append(LintDiagnostic( - ruleId: id, - ruleName: name, + diagnostics.append(diagnostic( severity: .error, message: "Frame '\(frameName)' has no published components", - componentName: nil, - nodeId: nil, suggestion: "Convert frames to Components (⌥⌘K) and publish to Team Library" )) } diff --git a/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift b/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift index 5d8d0df2..d4085409 100644 --- a/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/DarkModeSuffixRule.swift @@ -13,7 +13,6 @@ struct DarkModeSuffixRule: LintRule { func check(context: LintContext) async throws -> [LintDiagnostic] { let config = context.config - var diagnostics: [LintDiagnostic] = [] let suffix: String if let sdm = config.common?.icons?.suffixDarkMode { @@ -25,35 +24,62 @@ struct DarkModeSuffixRule: LintRule { } let defaultFileId = config.figma?.lightFileId ?? "" - guard !defaultFileId.isEmpty else { return [] } - - // Collect configured frame names to filter components - let configuredFrames = collectConfiguredFrames(from: config) + guard !defaultFileId.isEmpty else { + return [diagnostic( + message: "No figma.lightFileId configured — skipping rule", + suggestion: "Set figma.lightFileId in your PKL config" + )] + } let components: [Component] do { components = try await context.cache.components(for: defaultFileId, client: context.client) } catch { - return [] + return [diagnostic( + severity: .error, + message: "Cannot fetch components for file '\(defaultFileId)': \(error.localizedDescription)", + suggestion: "Check FIGMA_PERSONAL_TOKEN and file permissions" + )] } - // Only check components in configured frames - let relevant = components.filter { comp in + let configuredEntries = collectConfiguredEntries(from: config) + let relevant = filterRelevantComponents(components, entries: configuredEntries) + return checkDarkPairs(relevant: relevant, suffix: suffix) + } + + // MARK: - Filtering + + private func filterRelevantComponents( + _ components: [Component], + entries: [FrameEntry] + ) -> [Component] { + components.filter { comp in guard let frameName = comp.containingFrame.name else { return false } - return configuredFrames.contains(frameName) + if comp.containingFrame.containingComponentSet != nil, + comp.name.contains("RTL=") + { + return false + } + return entries.contains { entry in + if entry.frameName != frameName { return false } + if let pageName = entry.pageName, comp.containingFrame.pageName != pageName { + return false + } + return true + } } + } + private func checkDarkPairs(relevant: [Component], suffix: String) -> [LintDiagnostic] { let relevantNames = Set(relevant.map(\.name)) + var diagnostics: [LintDiagnostic] = [] for component in relevant { if component.name.hasSuffix(suffix) { continue } let expectedDark = component.name + suffix if !relevantNames.contains(expectedDark) { - diagnostics.append(LintDiagnostic( - ruleId: id, - ruleName: name, - severity: .warning, + diagnostics.append(diagnostic( message: "'\(component.name)' has no dark pair '\(expectedDark)'", componentName: component.name, nodeId: component.nodeId, @@ -65,12 +91,21 @@ struct DarkModeSuffixRule: LintRule { return diagnostics } - private func collectConfiguredFrames(from config: ExFig.ModuleImpl) -> Set { - var frames: Set = [] + // MARK: - Entry Collection + + private struct FrameEntry { + let frameName: String + let pageName: String? + } + + private func collectConfiguredEntries(from config: ExFig.ModuleImpl) -> [FrameEntry] { + var entries: [FrameEntry] = [] - func add(_ entries: [some Common_FrameSource]?) { - for entry in entries ?? [] { - if let frame = entry.figmaFrameName { frames.insert(frame) } + func add(_ icons: [some Common_FrameSource]?) { + for entry in icons ?? [] { + if let frame = entry.figmaFrameName { + entries.append(FrameEntry(frameName: frame, pageName: entry.figmaPageName)) + } } } @@ -90,6 +125,6 @@ struct DarkModeSuffixRule: LintRule { add(web.icons) } - return frames + return entries } } diff --git a/Sources/ExFigCLI/Lint/Rules/DarkModeVariablesRule.swift b/Sources/ExFigCLI/Lint/Rules/DarkModeVariablesRule.swift index e5ab9966..c396ea48 100644 --- a/Sources/ExFigCLI/Lint/Rules/DarkModeVariablesRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/DarkModeVariablesRule.swift @@ -20,56 +20,83 @@ struct DarkModeVariablesRule: LintRule { guard !entries.isEmpty else { return [] } for entry in entries { - guard !entry.fileId.isEmpty else { continue } + try await checkEntry(entry, context: context, diagnostics: &diagnostics) + } - let components: [Component] - do { - components = try await context.cache.components(for: entry.fileId, client: context.client) - } catch { - continue - } + return diagnostics + } - let relevant = components.filter { comp in - if let page = entry.pageName, comp.containingFrame.pageName != page { - return false - } - if let frame = entry.frameName, comp.containingFrame.name != frame { - return false - } - // Skip RTL variants — they're duplicates for mirroring, not separate icons - if comp.containingFrame.containingComponentSet != nil, - comp.name.contains("RTL=") - { - return false - } - return true - } + // MARK: - Per-Entry Check - let sampled = Array(relevant.prefix(50)) - guard !sampled.isEmpty else { continue } + // swiftlint:disable function_body_length + private func checkEntry( + _ entry: VDMEntry, + context: LintContext, + diagnostics: inout [LintDiagnostic] + ) async throws { + guard !entry.fileId.isEmpty else { + diagnostics.append(diagnostic( + message: "No figma.lightFileId configured — skipping entry", + suggestion: "Set figma.lightFileId in your PKL config" + )) + return + } - let nodeIds = sampled.map(\.nodeId) - let nodes: [NodeId: Node] - do { - nodes = try await context.client.request(NodesEndpoint(fileId: entry.fileId, nodeIds: nodeIds)) - } catch { - continue - } + let components: [Component] + do { + components = try await context.cache.components(for: entry.fileId, client: context.client) + } catch { + diagnostics.append(diagnostic( + severity: .error, + message: "Cannot fetch components for file '\(entry.fileId)': \(error.localizedDescription)", + suggestion: "Check FIGMA_PERSONAL_TOKEN and file permissions" + )) + return + } - for (nodeId, node) in nodes { - let compName = sampled.first { $0.nodeId == nodeId }?.name ?? nodeId - // Skip root node fills — check only children (vector shapes inside the icon) - checkChildrenFills( - children: node.document.children ?? [], - componentName: compName, - diagnostics: &diagnostics - ) - } + let relevant = components.filter { comp in + if let page = entry.pageName, comp.containingFrame.pageName != page { return false } + if let frame = entry.frameName, comp.containingFrame.name != frame { return false } + if comp.containingFrame.containingComponentSet != nil, comp.name.contains("RTL=") { return false } + return true } - return diagnostics + // Sample to avoid excessive API calls (NodesEndpoint is rate-limited) + let sampled = Array(relevant.prefix(50)) + if relevant.count > 50 { + diagnostics.append(diagnostic( + severity: .info, + message: "Checked 50 of \(relevant.count) components (sampling for API limits)", + suggestion: nil + )) + } + guard !sampled.isEmpty else { return } + + let nodeIds = sampled.map(\.nodeId) + let nodes: [NodeId: Node] + do { + nodes = try await context.client.request(NodesEndpoint(fileId: entry.fileId, nodeIds: nodeIds)) + } catch { + diagnostics.append(diagnostic( + severity: .error, + message: "Cannot fetch nodes for file '\(entry.fileId)': \(error.localizedDescription)", + suggestion: "Check FIGMA_PERSONAL_TOKEN and file permissions" + )) + return + } + + for (nodeId, node) in nodes { + let compName = sampled.first { $0.nodeId == nodeId }?.name ?? nodeId + checkChildrenFills( + children: node.document.children ?? [], + componentName: compName, + diagnostics: &diagnostics + ) + } } + // swiftlint:enable function_body_length + // MARK: - Node Fill Checking /// Check fills only on leaf/vector nodes, not on the root component frame. @@ -89,7 +116,6 @@ struct DarkModeVariablesRule: LintRule { componentName: String, diagnostics: inout [LintDiagnostic] ) { - // Check fills on this node (not root — already skipped by caller) for fill in node.fills { if fill.type == .image { continue } if fill.opacity == 0 { continue } @@ -106,9 +132,7 @@ struct DarkModeVariablesRule: LintRule { fill.type.rawValue } - diagnostics.append(LintDiagnostic( - ruleId: id, - ruleName: name, + diagnostics.append(diagnostic( severity: .error, message: "Fill \(colorDesc) in '\(componentName)' not bound to Variable", componentName: componentName, @@ -118,7 +142,6 @@ struct DarkModeVariablesRule: LintRule { } } - // Recurse into children for child in node.children ?? [] { checkNodeFills(node: child, componentName: componentName, diagnostics: &diagnostics) } diff --git a/Sources/ExFigCLI/Lint/Rules/DeletedVariablesRule.swift b/Sources/ExFigCLI/Lint/Rules/DeletedVariablesRule.swift index 3af55973..f4f4ad70 100644 --- a/Sources/ExFigCLI/Lint/Rules/DeletedVariablesRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/DeletedVariablesRule.swift @@ -24,16 +24,18 @@ struct DeletedVariablesRule: LintRule { do { variables = try await context.cache.variables(for: fileId, client: context.client) } catch { + diagnostics.append(diagnostic( + severity: .error, + message: "Cannot fetch variables for file '\(fileId)': \(error.localizedDescription)", + suggestion: "Check FIGMA_PERSONAL_TOKEN and file permissions" + )) continue } for (variableId, variable) in variables.variables where variable.deletedButReferenced == true { - diagnostics.append(LintDiagnostic( - ruleId: id, - ruleName: name, - severity: .warning, + diagnostics.append(diagnostic( message: "Variable '\(variable.name)' is deleted but still referenced", componentName: variable.name, nodeId: variableId, diff --git a/Sources/ExFigCLI/Lint/Rules/DuplicateComponentNamesRule.swift b/Sources/ExFigCLI/Lint/Rules/DuplicateComponentNamesRule.swift index cf3ea3b2..412a793d 100644 --- a/Sources/ExFigCLI/Lint/Rules/DuplicateComponentNamesRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/DuplicateComponentNamesRule.swift @@ -15,7 +15,6 @@ struct DuplicateComponentNamesRule: LintRule { func check(context: LintContext) async throws -> [LintDiagnostic] { let config = context.config let defaultFileId = config.figma?.lightFileId ?? "" - guard !defaultFileId.isEmpty else { return [] } let configEntries = collectConfiguredEntries(from: config, defaultFileId: defaultFileId) guard !configEntries.isEmpty else { return [] } @@ -24,12 +23,23 @@ struct DuplicateComponentNamesRule: LintRule { let grouped = Dictionary(grouping: configEntries) { $0.fileId } for (fileId, entries) in grouped { - guard !fileId.isEmpty else { continue } + guard !fileId.isEmpty else { + diagnostics.append(diagnostic( + message: "No figma.lightFileId configured — skipping rule", + suggestion: "Set figma.lightFileId in your PKL config" + )) + continue + } let components: [Component] do { components = try await context.cache.components(for: fileId, client: context.client) } catch { + diagnostics.append(diagnostic( + severity: .error, + message: "Cannot fetch components for file '\(fileId)': \(error.localizedDescription)", + suggestion: "Check FIGMA_PERSONAL_TOKEN and file permissions" + )) continue } @@ -62,9 +72,7 @@ struct DuplicateComponentNamesRule: LintRule { .map { "'\($0)'" } let uniqueFrames = Array(Set(frames)).sorted() - diagnostics.append(LintDiagnostic( - ruleId: id, - ruleName: self.name, + diagnostics.append(diagnostic( severity: .error, message: "'\(compName)' appears \(duplicates.count)x on page '\(page)'" + (uniqueFrames.count > 1 ? " in \(uniqueFrames.joined(separator: ", "))" : ""), diff --git a/Sources/ExFigCLI/Lint/Rules/NamingConventionRule.swift b/Sources/ExFigCLI/Lint/Rules/NamingConventionRule.swift index e2afd278..73a64094 100644 --- a/Sources/ExFigCLI/Lint/Rules/NamingConventionRule.swift +++ b/Sources/ExFigCLI/Lint/Rules/NamingConventionRule.swift @@ -18,70 +18,87 @@ struct NamingConventionRule: LintRule { let entries = collectEntriesWithRegex(from: config, defaultFileId: fileId) guard !entries.isEmpty else { return [] } - // Group by fileId let grouped = Dictionary(grouping: entries) { $0.fileId } for (entryFileId, fileEntries) in grouped { - guard !entryFileId.isEmpty else { continue } + guard !entryFileId.isEmpty else { + diagnostics.append(diagnostic( + message: "No figma.lightFileId configured — skipping rule", + suggestion: "Set figma.lightFileId in your PKL config" + )) + continue + } let components: [Component] do { components = try await context.cache.components(for: entryFileId, client: context.client) } catch { + diagnostics.append(diagnostic( + severity: .error, + message: "Cannot fetch components for file '\(entryFileId)': \(error.localizedDescription)", + suggestion: "Check FIGMA_PERSONAL_TOKEN and file permissions" + )) continue } for entry in fileEntries { - guard let pattern = entry.regex else { continue } - let regex: NSRegularExpression - do { - regex = try NSRegularExpression(pattern: pattern) - } catch { - diagnostics.append(LintDiagnostic( - ruleId: id, - ruleName: name, - severity: .warning, - message: "Invalid regex pattern: '\(pattern)'", - componentName: nil, - nodeId: nil, - suggestion: "Fix the nameValidateRegexp in your PKL config" - )) - continue - } - - // Filter components by frame name if specified - let relevant = components.filter { comp in - if let frameName = entry.frameName { - return comp.containingFrame.name == frameName - } - return true - } - - for comp in relevant { - let range = NSRange(comp.name.startIndex..., in: comp.name) - if regex.firstMatch(in: comp.name, range: range) == nil { - diagnostics.append(LintDiagnostic( - ruleId: id, - ruleName: name, - severity: .error, - message: "Name '\(comp.name)' doesn't match pattern '\(pattern)'", - componentName: comp.name, - nodeId: comp.nodeId, - suggestion: "Rename the component to match the expected pattern" - )) - } - } + checkEntry(entry, components: components, diagnostics: &diagnostics) } } return diagnostics } + // MARK: - Per-Entry Check + + private func checkEntry( + _ entry: RegexEntry, + components: [Component], + diagnostics: inout [LintDiagnostic] + ) { + guard let pattern = entry.regex else { return } + let regex: NSRegularExpression + do { + regex = try NSRegularExpression(pattern: pattern) + } catch { + diagnostics.append(diagnostic( + severity: .warning, + message: "Invalid regex pattern: '\(pattern)'", + suggestion: "Fix the nameValidateRegexp in your PKL config" + )) + return + } + + let relevant = components.filter { comp in + if let pageName = entry.pageName, comp.containingFrame.pageName != pageName { + return false + } + if let frameName = entry.frameName { + return comp.containingFrame.name == frameName + } + return true + } + + for comp in relevant { + let range = NSRange(comp.name.startIndex..., in: comp.name) + if regex.firstMatch(in: comp.name, range: range) == nil { + diagnostics.append(diagnostic( + severity: .error, + message: "Name '\(comp.name)' doesn't match pattern '\(pattern)'", + componentName: comp.name, + nodeId: comp.nodeId, + suggestion: "Rename the component to match the expected pattern" + )) + } + } + } + // MARK: - Entry Collection private struct RegexEntry { let fileId: String let frameName: String? + let pageName: String? let regex: String? } @@ -96,6 +113,7 @@ struct NamingConventionRule: LintRule { entries.append(RegexEntry( fileId: entry.figmaFileId ?? defaultFileId, frameName: entry.figmaFrameName, + pageName: entry.figmaPageName, regex: entry.nameValidateRegexp )) } @@ -111,6 +129,7 @@ struct NamingConventionRule: LintRule { } if let flutter = config.flutter { addEntries(flutter.icons) + addEntries(flutter.images) } if let web = config.web { addEntries(web.icons) diff --git a/Sources/ExFigCLI/MCP/MCPToolHandlers.swift b/Sources/ExFigCLI/MCP/MCPToolHandlers.swift index 6a15cdc8..117d30fd 100644 --- a/Sources/ExFigCLI/MCP/MCPToolHandlers.swift +++ b/Sources/ExFigCLI/MCP/MCPToolHandlers.swift @@ -136,6 +136,13 @@ params: CallTool.Parameters, state: MCPServerState ) async throws -> CallTool.Result { + // Validate cheap params before expensive operations (PKL eval, API client) + let ruleFilter: Set? = params.arguments?["rules"]?.stringValue.map { + Set($0.split(separator: ",").map { String($0.trimmingCharacters(in: .whitespaces)) }) + } + let minSeverity = params.arguments?["severity"]?.stringValue + .flatMap { LintSeverity(rawValue: $0) } ?? .info + let configPath = try resolveConfigPath(from: params.arguments?["config_path"]?.stringValue) let configURL = URL(fileURLWithPath: configPath) let config = try await PKLEvaluator.evaluate(configPath: configURL) @@ -145,12 +152,6 @@ let ui = TerminalUI(outputMode: .quiet) let context = LintContext(config: config, client: client, cache: cache, ui: ui) - let ruleFilter: Set? = params.arguments?["rules"]?.stringValue.map { - Set($0.split(separator: ",").map { String($0.trimmingCharacters(in: .whitespaces)) }) - } - let minSeverity = params.arguments?["severity"]?.stringValue - .flatMap { LintSeverity(rawValue: $0) } ?? .info - let engine = LintEngine.default let diagnostics = try await engine.run( context: context, diff --git a/Sources/ExFigCLI/Subcommands/Lint.swift b/Sources/ExFigCLI/Subcommands/Lint.swift index 7306af5d..bbb5924a 100644 --- a/Sources/ExFigCLI/Subcommands/Lint.swift +++ b/Sources/ExFigCLI/Subcommands/Lint.swift @@ -34,17 +34,15 @@ extension ExFigCommand { var rules: String? @Option(name: .long, help: "Output format: text or json") - var format: String = "text" + var format: LintOutputFormat = .text @Option(name: .long, help: "Minimum severity: error, warning, or info") - var severity: String = "info" + var severity: LintSeverity = .info func run() async throws { - let outputFormat = LintOutputFormat(rawValue: format) ?? .text - // JSON mode: force quiet to keep stdout clean for machine parsing let effectiveVerbose = globalOptions.verbose - let effectiveQuiet = outputFormat == .json ? true : globalOptions.quiet + let effectiveQuiet = format == .json ? true : globalOptions.quiet ExFigCommand.initializeTerminalUI( verbose: effectiveVerbose, quiet: effectiveQuiet @@ -52,7 +50,6 @@ extension ExFigCommand { ExFigCommand.checkSchemaVersionIfNeeded() let ui = ExFigCommand.terminalUI! - let minSeverity = LintSeverity(rawValue: severity) ?? .info let ruleFilter: Set? = rules.map { Set($0.split(separator: ",").map { String($0.trimmingCharacters(in: .whitespaces)) }) } @@ -74,14 +71,14 @@ extension ExFigCommand { try await engine.run( context: context, ruleFilter: ruleFilter, - minSeverity: minSeverity + minSeverity: severity ) { message in updateMessage(message) } } let reporter = LintReporter( - format: outputFormat, + format: format, useColors: ui.outputMode == .normal ) try reporter.report(diagnostics: diagnostics, ui: ui) diff --git a/Tests/ExFigTests/Lint/LintRulesTests.swift b/Tests/ExFigTests/Lint/LintRulesTests.swift index 8db6e2b7..a87a9853 100644 --- a/Tests/ExFigTests/Lint/LintRulesTests.swift +++ b/Tests/ExFigTests/Lint/LintRulesTests.swift @@ -538,8 +538,7 @@ struct LintEngineTests { client.setResponse([Component](), for: ComponentsEndpoint.self) let config = makeIOSIconsConfig(frameName: "Icons") - let ui = TerminalUI(outputMode: .quiet) - let context = LintContext(config: config, client: client, cache: LintDataCache(), ui: ui) + let context = makeLintContext(config: config, client: client) let engine = LintEngine.default let diagnostics = try await engine.run(context: context) @@ -554,15 +553,76 @@ struct LintEngineTests { client.setResponse([Component](), for: ComponentsEndpoint.self) let config = makeIOSIconsConfig(frameName: "Icons") - let ui = TerminalUI(outputMode: .quiet) - let context = LintContext(config: config, client: client, cache: LintDataCache(), ui: ui) + let context = makeLintContext(config: config, client: client) let engine = LintEngine.default let diagnostics = try await engine.run(context: context, ruleFilter: ["deleted-variables"]) - // Only deleted-variables rule should run (may have info from failed rule) + // Only deleted-variables rule should run #expect(!diagnostics.contains { $0.ruleId == "component-not-frame" }) } + + @Test("default engine registers all 8 rules") + func defaultEngineHasAllRules() { + let ruleIds = Set(LintEngine.default.rules.map(\.id)) + let expected: Set = [ + "frame-page-match", + "naming-convention", + "component-not-frame", + "deleted-variables", + "duplicate-component-names", + "alias-chain-integrity", + "dark-mode-variables", + "dark-mode-suffix", + ] + #expect(ruleIds == expected) + } + + @Test("catches rule errors as error-severity diagnostics") + func catchesRuleErrors() async throws { + let failingRule = FailingLintRule() + let engine = LintEngine(rules: [failingRule]) + + let client = MockClient() + let config = makeIOSIconsConfig(frameName: "Icons") + let context = makeLintContext(config: config, client: client) + + let diagnostics = try await engine.run(context: context) + + #expect(diagnostics.count == 1) + #expect(diagnostics.first?.ruleId == "failing-rule") + #expect(diagnostics.first?.severity == .error) + #expect(diagnostics.first?.message.contains("Rule check failed") == true) + } + + @Test("filters rules by minimum severity") + func filtersByMinSeverity() async throws { + let client = MockClient() + client.setResponse([Component](), for: ComponentsEndpoint.self) + + let config = makeIOSIconsConfig(frameName: "Icons") + let context = makeLintContext(config: config, client: client) + + let engine = LintEngine.default + let diagnostics = try await engine.run(context: context, minSeverity: .error) + + // Warning-severity rules (deleted-variables, alias-chain-integrity, dark-mode-suffix) should be excluded + #expect(!diagnostics.contains { $0.ruleId == "dark-mode-suffix" }) + #expect(!diagnostics.contains { $0.ruleId == "deleted-variables" }) + #expect(!diagnostics.contains { $0.ruleId == "alias-chain-integrity" }) + } +} + +/// A rule that always throws, for testing engine error handling. +private struct FailingLintRule: LintRule { + let id = "failing-rule" + let name = "Failing rule" + let description = "Always fails" + let severity: LintSeverity = .error + + func check(context: LintContext) async throws -> [LintDiagnostic] { + throw URLError(.notConnectedToInternet) + } } // swiftlint:enable file_length diff --git a/llms-full.txt b/llms-full.txt index 7c7ac023..8900f99f 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -552,15 +552,16 @@ exfig lint -i exfig.pkl --format json --severity error ### Available Rules -| Rule | Severity | Description | -| ----------------------- | -------- | ------------------------------------------------------ | -| `frame-page-match` | error | Frame/page names in config exist in Figma file | -| `naming-convention` | error | Component names match `nameValidateRegexp` patterns | -| `component-not-frame` | error | Configured frames contain published components | -| `deleted-variables` | error | No `deletedButReferenced` variables in collections | -| `alias-chain-integrity` | error | Variable alias chains resolve without broken refs | -| `dark-mode-variables` | error | With `variablesDarkMode`, fills bound to Variables | -| `dark-mode-suffix` | warning | With `suffixDarkMode`, light components have dark pair | +| Rule | Severity | Description | +| -------------------------- | -------- | ------------------------------------------------------ | +| `frame-page-match` | error | Frame/page names in config exist in Figma file | +| `naming-convention` | error | Component names match `nameValidateRegexp` patterns | +| `component-not-frame` | error | Configured frames contain published components | +| `duplicate-component-names`| error | No duplicate component names in configured frames | +| `deleted-variables` | warning | No `deletedButReferenced` variables in collections | +| `alias-chain-integrity` | warning | Variable alias chains resolve without broken refs | +| `dark-mode-variables` | error | With `variablesDarkMode`, fills bound to Variables | +| `dark-mode-suffix` | warning | With `suffixDarkMode`, light components have dark pair | ## Help and Version