Skip to content

Commit a5c868a

Browse files
committed
feat(go): general go tool support with golangci-lint filtering
Fixes two bugs in the original go tool golangci-lint implementation (commit 8f985a2) and extends support to all go tool invocations. Bug 1 — double 'run' in command args: rtk go tool golangci-lint run ./... was producing 'go tool golangci-lint run --output.json.path stdout run ./...' because run_go_tool_golangci_lint always prepended 'run' before appending all args (which already included 'run'). Fix: extract build_go_tool_golangci_args() helper that strips a leading 'run' before injecting the JSON flag. Both calling forms now work correctly: rtk go tool golangci-lint run ./... (explicit run) rtk go tool golangci-lint ./... (no run prefix) Bug 2 — rewrite rule too narrow: The registry pattern only matched go tool golangci-lint, so 'go tool staticcheck ./...' and any other managed tool was never rewritten to 'rtk go tool ...' by the Claude Code hook. Fix: drop the GoTool enum in favour of match_go_tool() returning the tool name as a String, matching any 'go tool <name>'. Unknown tools are routed through a new run_go_tool_passthrough() that executes transparently and records usage in the tracking DB so they appear in 'rtk gain --history'. The rewrite pattern is broadened to '^go\s+(test|build|vet|tool\s+\S+)'. Closes #744
1 parent 39583cf commit a5c868a

3 files changed

Lines changed: 213 additions & 41 deletions

File tree

src/cmds/go/go_cmd.rs

Lines changed: 160 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,12 @@ pub fn run_other(args: &[OsString], verbose: u8) -> Result<i32> {
127127
anyhow::bail!("go: no subcommand specified");
128128
}
129129

130-
// Intercept: `go tool <known>` invocations for filtered output
131-
if let Some((tool, tool_args)) = match_go_tool(args) {
132-
match tool {
133-
GoTool::GolangciLint => return run_go_tool_golangci_lint(tool_args, verbose),
134-
}
130+
// Intercept: `go tool <name>` — filter known tools, passthrough+track the rest
131+
if let Some((tool_name, tool_args)) = match_go_tool(args) {
132+
return match tool_name.as_str() {
133+
"golangci-lint" => run_go_tool_golangci_lint(tool_args, verbose),
134+
_ => run_go_tool_passthrough(&tool_name, tool_args, verbose),
135+
};
135136
}
136137

137138
let timer = tracking::TimedExecution::start();
@@ -203,31 +204,92 @@ fn has_golangci_format_flag(args: &[OsString]) -> bool {
203204
})
204205
}
205206

206-
/// Known `go tool` subcommands that RTK provides filtered output for.
207-
#[derive(Debug, Clone, Copy, PartialEq)]
208-
enum GoTool {
209-
GolangciLint,
207+
/// Match `go tool <name> [args...]` for ANY tool name.
208+
///
209+
/// Returns `(tool_name, remaining_args)` when the first two args are
210+
/// `["tool", "<name>"]`. Returns `None` if the subcommand is not `tool`
211+
/// or no tool name follows.
212+
fn match_go_tool(args: &[OsString]) -> Option<(String, &[OsString])> {
213+
if args.first()? != "tool" {
214+
return None;
215+
}
216+
let tool_name = args.get(1)?;
217+
if tool_name.is_empty() {
218+
return None;
219+
}
220+
Some((tool_name.to_string_lossy().into_owned(), &args[2..]))
210221
}
211222

212-
impl GoTool {
213-
fn from_name(name: &str) -> Option<Self> {
214-
match name {
215-
"golangci-lint" => Some(Self::GolangciLint),
216-
_ => None,
217-
}
223+
/// Run `go tool <name>` for any tool RTK doesn't know how to filter.
224+
///
225+
/// Executes the command transparently (no output filtering) but tracks it in
226+
/// the SQLite usage database so it appears in `rtk gain --history`.
227+
fn run_go_tool_passthrough(tool: &str, args: &[OsString], verbose: u8) -> Result<i32> {
228+
let timer = tracking::TimedExecution::start();
229+
230+
let mut cmd = resolved_command("go");
231+
cmd.arg("tool").arg(tool);
232+
for arg in args {
233+
cmd.arg(arg);
234+
}
235+
236+
if verbose > 0 {
237+
eprintln!("Running: go tool {} ...", tool);
218238
}
239+
240+
let output = cmd
241+
.output()
242+
.with_context(|| format!("Failed to run go tool {}", tool))?;
243+
244+
let stdout = String::from_utf8_lossy(&output.stdout);
245+
let stderr = String::from_utf8_lossy(&output.stderr);
246+
let raw = format!("{}\n{}", stdout, stderr);
247+
248+
print!("{}", stdout);
249+
eprint!("{}", stderr);
250+
251+
timer.track(
252+
&format!("go tool {}", tool),
253+
&format!("rtk go tool {}", tool),
254+
&raw,
255+
&raw, // No filtering — passthrough only
256+
);
257+
258+
Ok(exit_code_from_output(
259+
&output,
260+
&format!("go tool {}", tool),
261+
))
219262
}
220263

221-
/// If the first arg is `tool` identify if it is a tool we already handle.
222-
fn match_go_tool(args: &[OsString]) -> Option<(GoTool, &[OsString])> {
223-
if args.first().map(|a| a == "tool").unwrap_or(false) {
224-
if let Some(tool_arg) = args.get(1) {
225-
if let Some(tool) = GoTool::from_name(&tool_arg.to_string_lossy()) {
226-
return Some((tool, &args[2..]));
227-
}
264+
/// Build the golangci-lint arguments for `go tool golangci-lint`.
265+
///
266+
/// Strips a leading `"run"` from `args` (we always inject `"run"` ourselves),
267+
/// then prepends the JSON output flag unless the caller already specified one.
268+
/// This handles both forms:
269+
/// - `rtk go tool golangci-lint run ./...` (explicit "run" in args)
270+
/// - `rtk go tool golangci-lint ./...` (no "run" prefix)
271+
fn build_go_tool_golangci_args(args: &[OsString], version: u32) -> Vec<OsString> {
272+
// Strip a leading "run" — we always inject "run" ourselves to avoid duplication
273+
let run_args = if args.first().map(|a| a == "run").unwrap_or(false) {
274+
&args[1..]
275+
} else {
276+
args
277+
};
278+
279+
let mut result: Vec<OsString> = Vec::new();
280+
result.push("run".into());
281+
282+
if !has_golangci_format_flag(run_args) {
283+
if version >= 2 {
284+
result.push("--output.json.path".into());
285+
result.push("stdout".into());
286+
} else {
287+
result.push("--out-format=json".into());
228288
}
229289
}
230-
None
290+
291+
result.extend_from_slice(run_args);
292+
result
231293
}
232294

233295
/// Run `go tool golangci-lint` and filter its output via the golangci JSON filter.
@@ -240,19 +302,7 @@ fn run_go_tool_golangci_lint(args: &[OsString], verbose: u8) -> Result<i32> {
240302
let mut cmd = resolved_command("go");
241303
cmd.arg("tool").arg("golangci-lint");
242304

243-
let has_format = has_golangci_format_flag(args);
244-
245-
if !has_format {
246-
if version >= 2 {
247-
cmd.arg("run").arg("--output.json.path").arg("stdout");
248-
} else {
249-
cmd.arg("run").arg("--out-format=json");
250-
}
251-
} else {
252-
cmd.arg("run");
253-
}
254-
255-
for arg in args {
305+
for arg in build_go_tool_golangci_args(args, version) {
256306
cmd.arg(arg);
257307
}
258308

@@ -1025,21 +1075,41 @@ utils.go:15:5: unreachable code"#;
10251075
fn test_match_go_tool_golangci_lint() {
10261076
let args = os(&["tool", "golangci-lint", "run", "./..."]);
10271077
let (tool, rest) = match_go_tool(&args).expect("should match");
1028-
assert_eq!(tool, GoTool::GolangciLint);
1078+
assert_eq!(tool, "golangci-lint");
10291079
assert_eq!(rest.len(), 2); // ["run", "./..."]
10301080
}
10311081

10321082
#[test]
10331083
fn test_match_go_tool_bare() {
10341084
let args = os(&["tool", "golangci-lint"]);
10351085
let (tool, rest) = match_go_tool(&args).expect("should match");
1036-
assert_eq!(tool, GoTool::GolangciLint);
1086+
assert_eq!(tool, "golangci-lint");
1087+
assert!(rest.is_empty());
1088+
}
1089+
1090+
#[test]
1091+
fn test_match_go_tool_matches_any_known_tool() {
1092+
// Any `go tool <name>` should match — not just golangci-lint
1093+
let args = os(&["tool", "pprof", "cpu.prof"]);
1094+
let (tool, rest) = match_go_tool(&args).expect("pprof should match");
1095+
assert_eq!(tool, "pprof");
1096+
assert_eq!(rest.len(), 1);
1097+
1098+
let args = os(&["tool", "staticcheck", "./..."]);
1099+
let (tool, rest) = match_go_tool(&args).expect("staticcheck should match");
1100+
assert_eq!(tool, "staticcheck");
1101+
assert_eq!(rest.len(), 1);
1102+
1103+
// Also matches with no trailing args
1104+
let args = os(&["tool", "pprof"]);
1105+
let (tool, rest) = match_go_tool(&args).expect("bare pprof should match");
1106+
assert_eq!(tool, "pprof");
10371107
assert!(rest.is_empty());
10381108
}
10391109

10401110
#[test]
1041-
fn test_match_go_tool_rejects_unknown() {
1042-
assert!(match_go_tool(&os(&["tool", "pprof"])).is_none());
1111+
fn test_match_go_tool_requires_tool_name() {
1112+
// `go tool` with no name after it — nothing to dispatch to
10431113
assert!(match_go_tool(&os(&["tool"])).is_none());
10441114
assert!(match_go_tool(&os(&["test", "./..."])).is_none());
10451115
assert!(match_go_tool(&os(&[])).is_none());
@@ -1072,4 +1142,54 @@ utils.go:15:5: unreachable code"#;
10721142
assert!(!has_golangci_format_flag(&os(&[])));
10731143
assert!(!has_golangci_format_flag(&os(&["--fix"])));
10741144
}
1145+
1146+
// --- build_go_tool_golangci_args tests ---
1147+
1148+
#[test]
1149+
fn test_build_go_tool_args_explicit_run_not_doubled_v2() {
1150+
// Bug: `rtk go tool golangci-lint run ./...` was producing
1151+
// `go tool golangci-lint run --output.json.path stdout run ./...`
1152+
let args = os(&["run", "./..."]);
1153+
let result = build_go_tool_golangci_args(&args, 2);
1154+
assert_eq!(
1155+
result,
1156+
os(&["run", "--output.json.path", "stdout", "./..."]),
1157+
"leading 'run' must be stripped to avoid duplication"
1158+
);
1159+
}
1160+
1161+
#[test]
1162+
fn test_build_go_tool_args_no_run_prefix_v2() {
1163+
// `rtk go tool golangci-lint ./...` (bare, no explicit "run")
1164+
let args = os(&["./..."]);
1165+
let result = build_go_tool_golangci_args(&args, 2);
1166+
assert_eq!(result, os(&["run", "--output.json.path", "stdout", "./..."]));
1167+
}
1168+
1169+
#[test]
1170+
fn test_build_go_tool_args_explicit_run_not_doubled_v1() {
1171+
let args = os(&["run", "./..."]);
1172+
let result = build_go_tool_golangci_args(&args, 1);
1173+
assert_eq!(
1174+
result,
1175+
os(&["run", "--out-format=json", "./..."]),
1176+
"v1: leading 'run' must be stripped to avoid duplication"
1177+
);
1178+
}
1179+
1180+
#[test]
1181+
fn test_build_go_tool_args_preserves_explicit_format_flag_v1() {
1182+
// User already passed --out-format — don't inject a second one
1183+
let args = os(&["run", "--out-format=json", "./..."]);
1184+
let result = build_go_tool_golangci_args(&args, 1);
1185+
assert_eq!(result, os(&["run", "--out-format=json", "./..."]));
1186+
}
1187+
1188+
#[test]
1189+
fn test_build_go_tool_args_bare_invocation() {
1190+
// `rtk go tool golangci-lint` with no extra args
1191+
let args = os(&[]);
1192+
let result = build_go_tool_golangci_args(&args, 2);
1193+
assert_eq!(result, os(&["run", "--output.json.path", "stdout"]));
1194+
}
10751195
}

src/discover/registry.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2462,6 +2462,58 @@ mod tests {
24622462
);
24632463
}
24642464

2465+
#[test]
2466+
fn test_rewrite_go_tool_golangci_lint_run() {
2467+
assert_eq!(
2468+
rewrite_command_no_prefixes("go tool golangci-lint run ./...", &[]),
2469+
Some("rtk go tool golangci-lint run ./...".into())
2470+
);
2471+
}
2472+
2473+
#[test]
2474+
fn test_rewrite_go_tool_golangci_lint_bare() {
2475+
// `go tool golangci-lint` with no subcommand
2476+
assert_eq!(
2477+
rewrite_command_no_prefixes("go tool golangci-lint", &[]),
2478+
Some("rtk go tool golangci-lint".into())
2479+
);
2480+
}
2481+
2482+
#[test]
2483+
fn test_classify_go_tool_any_tool() {
2484+
// `go tool <x>` should be classified as supported for ANY tool name,
2485+
// not just golangci-lint. RTK passes unknown tools through transparently
2486+
// while still tracking usage.
2487+
assert!(matches!(
2488+
classify_command("go tool staticcheck ./..."),
2489+
Classification::Supported {
2490+
rtk_equivalent: "rtk go",
2491+
..
2492+
}
2493+
));
2494+
assert!(matches!(
2495+
classify_command("go tool pprof cpu.prof"),
2496+
Classification::Supported {
2497+
rtk_equivalent: "rtk go",
2498+
..
2499+
}
2500+
));
2501+
}
2502+
2503+
#[test]
2504+
fn test_rewrite_go_tool_any_tool() {
2505+
// Non-golangci-lint tools are rewritten to `rtk go tool <x>` so usage
2506+
// is tracked even when RTK cannot filter the output.
2507+
assert_eq!(
2508+
rewrite_command_no_prefixes("go tool staticcheck ./...", &[]),
2509+
Some("rtk go tool staticcheck ./...".into())
2510+
);
2511+
assert_eq!(
2512+
rewrite_command_no_prefixes("go tool pprof cpu.prof", &[]),
2513+
Some("rtk go tool pprof cpu.prof".into())
2514+
);
2515+
}
2516+
24652517
#[test]
24662518
fn test_rewrite_golangci_lint() {
24672519
assert_eq!(

src/discover/rules.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ pub const RULES: &[RtkRule] = &[
462462
subcmd_status: &[],
463463
},
464464
RtkRule {
465-
pattern: r"^go\s+(test|build|vet)",
465+
pattern: r"^go\s+(test|build|vet|tool\s+\S+)",
466466
rtk_cmd: "rtk go",
467467
rewrite_prefixes: &["go"],
468468
category: "Go",

0 commit comments

Comments
 (0)