Skip to content

Commit 42181e9

Browse files
beriberikixclaude
andcommitted
Remove code that only its tests used
A mechanism with tests but no production callers is not covered. It is a well-tested spare part, and its green tests actively disguise that nothing reaches it — the same blind spot that hid three wiring bugs earlier in this branch. Removing the eight such mechanisms in live code drops 42 tests: - URBTracker, a dead duplicate. UNLINK cancels through USBSubmitProcessor.cancelURB, which does its own tracking. - DeviceMonitor, 192 lines. Hotplug is handled directly by the deviceDiscovery callbacks ServerCoordinator installs. - The logDebug/logInfo/logWarning/logError/logCritical globals, an unused parallel API next to 739 uses of the logger methods. Their one test asserted nothing beyond "does not crash". - USBErrorMapping.mapUSBStatusToIOKit, the reverse of the direction the server actually needs. - The static USBErrorMapping.errorDescription(for:). - ServerConfig.resetToDefaults. - CompletionFormattingUtilities.formatDescription, and generateFallbackCompletion, which was also stale: it advertised attach and detach, removed from the CLI earlier. The instance errorDescription was kept. It shares a name with the deleted static but is LocalizedError conformance, reached through localizedDescription rather than a call site — a sweep keyed on call sites cannot see protocol dispatch, so hits need checking against conformances before deletion. 383 tests pass, lint clean, and the control transfer against a J-Link still returns its real descriptor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017rmyikdjWveP99ZUCDLY89
1 parent a458a96 commit 42181e9

13 files changed

Lines changed: 30 additions & 1147 deletions

Documentation/development/unwired-code-sweep.md

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,34 @@ failed. Fixed, with four tests over the mapping.
2828
`USBRequestHandler.init`, it was dead. Leaving it would have preserved the exact trap
2929
that caused the original bug: a setter that looks like the wiring point and is not.
3030

31+
## Removed: mechanisms with tests but no production callers
32+
33+
Deleting these removed 42 tests. That is the point — they were green tests over code
34+
the daemon never reached, and their passing actively disguised that nothing used it.
35+
36+
| Removed | Why it was safe |
37+
| --- | --- |
38+
| `URBTracker` and its ~30 tests | a dead duplicate; UNLINK cancels through `USBSubmitProcessor.cancelURB`, which tracks URBs in `activeURBs` |
39+
| `DeviceMonitor` (192 lines) and its tests | hotplug is handled directly by `deviceDiscovery.onDeviceConnected`/`onDeviceDisconnected` in `ServerCoordinator` |
40+
| `logDebug`/`logInfo`/`logWarning`/`logError`/`logCritical` globals | an unused parallel API; production uses the `logger.debug(…)` methods in 739 places. Their one test asserted nothing beyond "does not crash" |
41+
| `USBErrorMapping.mapUSBStatusToIOKit` | the reverse direction. The server maps IOKit → USB/IP via `mapIOKitError`, which is live |
42+
| `USBErrorMapping.errorDescription(for:)` (static) | no callers. The *instance* `errorDescription` is `LocalizedError` conformance and was kept |
43+
| `ServerConfig.resetToDefaults` | no callers |
44+
| `CompletionFormattingUtilities.formatDescription` | no callers |
45+
| `CompletionFormattingUtilities.generateFallbackCompletion` | no callers, and stale: it advertised `attach` and `detach`, removed from the CLI |
46+
47+
One near-miss worth recording: the sweep flagged `errorDescription`, but the instance
48+
property of that name is a `LocalizedError` requirement reached through
49+
`localizedDescription`. A sweep keyed on call sites cannot see protocol dispatch, so
50+
every hit needs checking against the conformance list before deletion. Only the static
51+
overload was genuinely unused.
52+
3153
## Confirmed unwired, not yet addressed
3254

3355
| Thing | State | Consequence |
3456
| --- | --- | --- |
3557
| `USBIPMessageValidator.validateUSBIPMessage` | zero callers, production *and* test | the buffer-size bound it contains is never applied |
3658
| `USBIPMessageValidator.validateSetupPacket` | zero callers | setup packets reach IOKit unvalidated |
37-
| `URBTracker` (`addPendingURB`, `removeCompletedURB`, `getPendingURB`, `getAllPendingSeqnums`, `clearAllPendingURBs`) | constructed once in `USBRequestHandler`, never called | a dead duplicate; the live tracking is `USBSubmitProcessor.activeURBs`. ~30 tests cover the unused copy |
38-
| `DeviceMonitor` | never constructed in production | `isActive`/`getKnownDevices` are tested against a class the daemon does not use |
3959
| `config.maxConnections` | only copied config→config by the `config` command | connection count is unbounded |
4060
| `config.connectionTimeout` | same | idle connections are never reaped |
4161
| `config.autoBindDevices` | same | setting it does nothing |
@@ -73,7 +93,11 @@ Most remaining zero-caller hits are inside the quarantined System Extension subs
7393
and are expected — that code is inert by design. The signal is confined to
7494
`USBIPDCore` outside `SystemExtension/`, `USBIPDCLI`, and `Common`.
7595

76-
The `TESTS-ONLY` category deserves the same suspicion as `DEAD`. A mechanism with
77-
tests but no production callers is not covered — it is a well-tested spare part, and
78-
its green tests actively obscure that nothing uses it. `URBTracker` is the clearest
79-
case in this repository.
96+
The `TESTS-ONLY` category deserved the same suspicion as `DEAD`, and is now empty for
97+
live code. A mechanism with tests but no production callers is not covered — it is a
98+
well-tested spare part whose green tests obscure that nothing uses it.
99+
100+
Re-running the sweep is the cheap way to keep it that way. What remains is confined to
101+
the quarantined System Extension subsystem, which is inert by design, and to the
102+
`config.*` and validator rows above, which are unwired behaviour rather than unused
103+
code and need decisions rather than deletions.

Sources/Common/Logger.swift

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -287,72 +287,3 @@ public final class Logger {
287287
return components.joined(separator: " ")
288288
}
289289
}
290-
291-
// MARK: - Global Convenience Functions
292-
293-
/// Global debug logging function
294-
public func logDebug(
295-
_ message: String,
296-
context: [String: Any] = [:],
297-
file: String = #file,
298-
function: String = #function,
299-
line: Int = #line
300-
) {
301-
Logger.shared.debug(message, context: context, file: file, function: function, line: line)
302-
}
303-
304-
/// Global info logging function
305-
public func logInfo(
306-
_ message: String,
307-
context: [String: Any] = [:],
308-
file: String = #file,
309-
function: String = #function,
310-
line: Int = #line
311-
) {
312-
Logger.shared.info(message, context: context, file: file, function: function, line: line)
313-
}
314-
315-
/// Global warning logging function
316-
public func logWarning(
317-
_ message: String,
318-
context: [String: Any] = [:],
319-
file: String = #file,
320-
function: String = #function,
321-
line: Int = #line
322-
) {
323-
Logger.shared.warning(message, context: context, file: file, function: function, line: line)
324-
}
325-
326-
/// Global error logging function
327-
public func logError(
328-
_ message: String,
329-
context: [String: Any] = [:],
330-
file: String = #file,
331-
function: String = #function,
332-
line: Int = #line
333-
) {
334-
Logger.shared.error(message, context: context, file: file, function: function, line: line)
335-
}
336-
337-
/// Global critical logging function
338-
public func logCritical(
339-
_ message: String,
340-
context: [String: Any] = [:],
341-
file: String = #file,
342-
function: String = #function,
343-
line: Int = #line
344-
) {
345-
Logger.shared.critical(message, context: context, file: file, function: function, line: line)
346-
}
347-
348-
/// Global error logging function for Error objects
349-
public func logError(
350-
_ error: Error,
351-
message: String? = nil,
352-
context: [String: Any] = [:],
353-
file: String = #file,
354-
function: String = #function,
355-
line: Int = #line
356-
) {
357-
Logger.shared.error(error, message: message, context: context, file: file, function: function, line: line)
358-
}

Sources/USBIPDCore/CLI/ShellCompletionFormatter.swift

Lines changed: 0 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -93,29 +93,6 @@ public enum CompletionFormattingUtilities {
9393
}
9494
}
9595

96-
/// Format a description string for shell completion
97-
/// - Parameters:
98-
/// - description: The description to format
99-
/// - shellType: Target shell type
100-
/// - Returns: Formatted description suitable for the shell
101-
public static func formatDescription(_ description: String, for shellType: String) -> String {
102-
let escaped = escapeForShell(description, shellType: shellType)
103-
104-
switch shellType {
105-
case "zsh":
106-
// Zsh supports rich descriptions in square brackets
107-
return "[\(escaped)]"
108-
case "fish":
109-
// Fish uses -d flag for descriptions
110-
return escaped
111-
case "bash":
112-
// Bash completion typically doesn't show descriptions
113-
return escaped
114-
default:
115-
return escaped
116-
}
117-
}
118-
11996
/// Generate a list of command names for completion
12097
/// - Parameter commands: Array of commands
12198
/// - Returns: Space-separated list of command names
@@ -230,57 +207,4 @@ public enum CompletionFormattingUtilities {
230207

231208
return issues
232209
}
233-
234-
/// Generate a fallback completion for unsupported scenarios
235-
/// - Parameters:
236-
/// - shellType: Target shell type
237-
/// - commandName: Name of the command
238-
/// - Returns: Basic fallback completion script
239-
public static func generateFallbackCompletion(for shellType: String, commandName: String) -> String {
240-
switch shellType {
241-
case "bash":
242-
return """
243-
# Fallback bash completion for \(commandName)
244-
complete -W "help list bind unbind attach detach daemon install-system-extension diagnose" \(commandName)
245-
"""
246-
247-
case "zsh":
248-
return """
249-
#compdef \(commandName)
250-
# Fallback zsh completion for \(commandName)
251-
_\(commandName)() {
252-
local commands=(
253-
'help:Display help information'
254-
'list:List available USB devices'
255-
'bind:Bind a USB device to USB/IP'
256-
'unbind:Unbind a USB device from USB/IP'
257-
'attach:Attach a remote USB device'
258-
'detach:Detach a remote USB device'
259-
'daemon:Start USB/IP daemon'
260-
'install-system-extension:Install and register the System Extension'
261-
'diagnose:Run comprehensive installation and system diagnostics'
262-
)
263-
_describe 'commands' commands
264-
}
265-
_\(commandName) "$@"
266-
"""
267-
268-
case "fish":
269-
return """
270-
# Fallback fish completion for \(commandName)
271-
complete -c \(commandName) -n '__fish_use_subcommand' -a 'help' -d 'Display help information'
272-
complete -c \(commandName) -n '__fish_use_subcommand' -a 'list' -d 'List available USB devices'
273-
complete -c \(commandName) -n '__fish_use_subcommand' -a 'bind' -d 'Bind a USB device to USB/IP'
274-
complete -c \(commandName) -n '__fish_use_subcommand' -a 'unbind' -d 'Unbind a USB device from USB/IP'
275-
complete -c \(commandName) -n '__fish_use_subcommand' -a 'attach' -d 'Attach a remote USB device'
276-
complete -c \(commandName) -n '__fish_use_subcommand' -a 'detach' -d 'Detach a remote USB device'
277-
complete -c \(commandName) -n '__fish_use_subcommand' -a 'daemon' -d 'Start USB/IP daemon'
278-
complete -c \(commandName) -n '__fish_use_subcommand' -a 'install-system-extension' -d 'Install and register the System Extension'
279-
complete -c \(commandName) -n '__fish_use_subcommand' -a 'diagnose' -d 'Run comprehensive installation and system diagnostics'
280-
"""
281-
282-
default:
283-
return "# Unsupported shell type: \(shellType)"
284-
}
285-
}
286210
}

0 commit comments

Comments
 (0)