From 9d4fc5c2cb5f04cd9d98033f4b6e9ce106001ac3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 18 Jul 2026 03:45:24 +0000 Subject: [PATCH 1/3] Initial plan From be3da0f8177e7427c319079be3216baf252a65f6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 18 Jul 2026 03:57:27 +0000 Subject: [PATCH 2/3] Initial plan: reduce largefunc findings in cmd/gh-aw and pkg/linters Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/agentic-auto-upgrade.yml | 2 +- .github/workflows/avenger.lock.yml | 5 ++--- .github/workflows/hourly-ci-cleaner.lock.yml | 5 ++--- .github/workflows/release.lock.yml | 4 ++-- .github/workflows/skillet.lock.yml | 5 ++--- 5 files changed, 9 insertions(+), 12 deletions(-) diff --git a/.github/workflows/agentic-auto-upgrade.yml b/.github/workflows/agentic-auto-upgrade.yml index 7035101ee35..e169cea32b6 100644 --- a/.github/workflows/agentic-auto-upgrade.yml +++ b/.github/workflows/agentic-auto-upgrade.yml @@ -34,7 +34,7 @@ name: Agentic Auto-Upgrade on: schedule: - - cron: "21 3 * * 5" # Weekly (auto-upgrade) + - cron: "11 4 * * 6" # Weekly (auto-upgrade) workflow_dispatch: permissions: diff --git a/.github/workflows/avenger.lock.yml b/.github/workflows/avenger.lock.yml index 092aada6f01..3d6eb6ec67f 100644 --- a/.github/workflows/avenger.lock.yml +++ b/.github/workflows/avenger.lock.yml @@ -1,5 +1,5 @@ # gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"6807590fb97b42c0858f3b0e068f5f2b823e6d3a5a873b1a1652f5658361eb44","body_hash":"e5fd04fe008ba327d939d9aee395ffb802836e30fd1f3772ca36984bdd1478f4","strict":true,"agent_id":"claude","agent_model":"claude-haiku-4.5","engine_versions":{"claude":"2.1.210"}} -# gh-aw-manifest: {"version":1,"secrets":["ANTHROPIC_API_KEY","COPILOT_GITHUB_TOKEN","GH_AW_CI_TRIGGER_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0","version":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-go","sha":"b7ad1dad31e06c5925ef5d2fc7ad053ef454303e","version":"v7.0.0"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.35","digest":"sha256:2202f63e8650b2b8b0d38033b44a05387b2b71ad3e690c4d23a34786f5462aed","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.35@sha256:2202f63e8650b2b8b0d38033b44a05387b2b71ad3e690c4d23a34786f5462aed"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.35","digest":"sha256:755b79d0dfda82bd6b43a208d68666721e504110c5d342a4eeb199802644ff04","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.35@sha256:755b79d0dfda82bd6b43a208d68666721e504110c5d342a4eeb199802644ff04"},{"image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.35","digest":"sha256:fe83cd274636efa9de3f456e2b078fae137328b9bb6ee4986ae510acaef0cec5","pinned_image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.35@sha256:fe83cd274636efa9de3f456e2b078fae137328b9bb6ee4986ae510acaef0cec5"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.35","digest":"sha256:f69282ec7b1326ba53891c399cf5b10475c0d3ccf4e1519b33d234a5427b57d3","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.35@sha256:f69282ec7b1326ba53891c399cf5b10475c0d3ccf4e1519b33d234a5427b57d3"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.1","digest":"sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.1@sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b","pinned_image":"ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},{"image":"ghcr.io/github/github-mcp-server:v1.6.0","digest":"sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3","pinned_image":"ghcr.io/github/github-mcp-server:v1.6.0@sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3"}]} +# gh-aw-manifest: {"version":1,"secrets":["ANTHROPIC_API_KEY","COPILOT_GITHUB_TOKEN","GH_AW_CI_TRIGGER_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0","version":"v7.0.0"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-go","sha":"b7ad1dad31e06c5925ef5d2fc7ad053ef454303e","version":"v7.0.0"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.35","digest":"sha256:2202f63e8650b2b8b0d38033b44a05387b2b71ad3e690c4d23a34786f5462aed","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.35@sha256:2202f63e8650b2b8b0d38033b44a05387b2b71ad3e690c4d23a34786f5462aed"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.35","digest":"sha256:755b79d0dfda82bd6b43a208d68666721e504110c5d342a4eeb199802644ff04","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.35@sha256:755b79d0dfda82bd6b43a208d68666721e504110c5d342a4eeb199802644ff04"},{"image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.35","digest":"sha256:fe83cd274636efa9de3f456e2b078fae137328b9bb6ee4986ae510acaef0cec5","pinned_image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.35@sha256:fe83cd274636efa9de3f456e2b078fae137328b9bb6ee4986ae510acaef0cec5"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.35","digest":"sha256:f69282ec7b1326ba53891c399cf5b10475c0d3ccf4e1519b33d234a5427b57d3","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.35@sha256:f69282ec7b1326ba53891c399cf5b10475c0d3ccf4e1519b33d234a5427b57d3"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.1","digest":"sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.1@sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b","pinned_image":"ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},{"image":"ghcr.io/github/github-mcp-server:v1.6.0","digest":"sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3","pinned_image":"ghcr.io/github/github-mcp-server:v1.6.0@sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3"}]} # This file was automatically generated by gh-aw. DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # # ___ _ _ @@ -44,7 +44,6 @@ # Custom actions used: # - actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 # - actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 -# - actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # 9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # - actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 # - actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 # - actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 @@ -1146,7 +1145,7 @@ jobs: GH_HOST="${GH_HOST#http://}" echo "GH_HOST=${GH_HOST}" >> "$GITHUB_ENV" - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # 9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: persist-credentials: false - name: Check last CI workflow run status on main branch diff --git a/.github/workflows/hourly-ci-cleaner.lock.yml b/.github/workflows/hourly-ci-cleaner.lock.yml index 7fbc3953765..4c96d7c4a51 100644 --- a/.github/workflows/hourly-ci-cleaner.lock.yml +++ b/.github/workflows/hourly-ci-cleaner.lock.yml @@ -1,5 +1,5 @@ # gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"8b86cc12904088cf1b7630e0faa28299f64081df184a028b6293cb4f8bfdd07d","body_hash":"8eb6f8fd1e8e3d63cfd268226d09333129d49634435ae9a3c88debb099521ed5","strict":true,"agent_id":"claude","engine_versions":{"claude":"2.1.210"}} -# gh-aw-manifest: {"version":1,"secrets":["ANTHROPIC_API_KEY","COPILOT_GITHUB_TOKEN","GH_AW_CI_TRIGGER_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0","version":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-go","sha":"b7ad1dad31e06c5925ef5d2fc7ad053ef454303e","version":"v7.0.0"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.35","digest":"sha256:2202f63e8650b2b8b0d38033b44a05387b2b71ad3e690c4d23a34786f5462aed","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.35@sha256:2202f63e8650b2b8b0d38033b44a05387b2b71ad3e690c4d23a34786f5462aed"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.35","digest":"sha256:755b79d0dfda82bd6b43a208d68666721e504110c5d342a4eeb199802644ff04","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.35@sha256:755b79d0dfda82bd6b43a208d68666721e504110c5d342a4eeb199802644ff04"},{"image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.35","digest":"sha256:fe83cd274636efa9de3f456e2b078fae137328b9bb6ee4986ae510acaef0cec5","pinned_image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.35@sha256:fe83cd274636efa9de3f456e2b078fae137328b9bb6ee4986ae510acaef0cec5"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.35","digest":"sha256:f69282ec7b1326ba53891c399cf5b10475c0d3ccf4e1519b33d234a5427b57d3","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.35@sha256:f69282ec7b1326ba53891c399cf5b10475c0d3ccf4e1519b33d234a5427b57d3"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.1","digest":"sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.1@sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b","pinned_image":"ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},{"image":"ghcr.io/github/github-mcp-server:v1.6.0","digest":"sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3","pinned_image":"ghcr.io/github/github-mcp-server:v1.6.0@sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3"}]} +# gh-aw-manifest: {"version":1,"secrets":["ANTHROPIC_API_KEY","COPILOT_GITHUB_TOKEN","GH_AW_CI_TRIGGER_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0","version":"v7.0.0"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-go","sha":"b7ad1dad31e06c5925ef5d2fc7ad053ef454303e","version":"v7.0.0"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.35","digest":"sha256:2202f63e8650b2b8b0d38033b44a05387b2b71ad3e690c4d23a34786f5462aed","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.35@sha256:2202f63e8650b2b8b0d38033b44a05387b2b71ad3e690c4d23a34786f5462aed"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.35","digest":"sha256:755b79d0dfda82bd6b43a208d68666721e504110c5d342a4eeb199802644ff04","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.35@sha256:755b79d0dfda82bd6b43a208d68666721e504110c5d342a4eeb199802644ff04"},{"image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.35","digest":"sha256:fe83cd274636efa9de3f456e2b078fae137328b9bb6ee4986ae510acaef0cec5","pinned_image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.35@sha256:fe83cd274636efa9de3f456e2b078fae137328b9bb6ee4986ae510acaef0cec5"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.35","digest":"sha256:f69282ec7b1326ba53891c399cf5b10475c0d3ccf4e1519b33d234a5427b57d3","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.35@sha256:f69282ec7b1326ba53891c399cf5b10475c0d3ccf4e1519b33d234a5427b57d3"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.1","digest":"sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.1@sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b","pinned_image":"ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},{"image":"ghcr.io/github/github-mcp-server:v1.6.0","digest":"sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3","pinned_image":"ghcr.io/github/github-mcp-server:v1.6.0@sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3"}]} # This file was automatically generated by gh-aw. DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # # ___ _ _ @@ -45,7 +45,6 @@ # Custom actions used: # - actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 # - actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 -# - actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # 9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # - actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 # - actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 # - actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 @@ -1153,7 +1152,7 @@ jobs: GH_HOST="${GH_HOST#http://}" echo "GH_HOST=${GH_HOST}" >> "$GITHUB_ENV" - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # 9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: persist-credentials: false - name: Check last CI workflow run status on main branch diff --git a/.github/workflows/release.lock.yml b/.github/workflows/release.lock.yml index 5e5b9a932d6..b670dfbb32e 100644 --- a/.github/workflows/release.lock.yml +++ b/.github/workflows/release.lock.yml @@ -47,7 +47,7 @@ # - actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 # - actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 # - actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 (source v9) -# - actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # b7ad1dad31e06c5925ef5d2fc7ad053ef454303e +# - actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 # - actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 # - anchore/sbom-action@e22c389904149dbc22b58101806040fa8d37a610 # v0.24.0 # - docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0 @@ -1949,7 +1949,7 @@ jobs: env: RELEASE_TAG: ${{ needs.config.outputs.release_tag }} - name: Setup Go - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # b7ad1dad31e06c5925ef5d2fc7ad053ef454303e + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: cache: false go-version-file: go.mod diff --git a/.github/workflows/skillet.lock.yml b/.github/workflows/skillet.lock.yml index 8d7320c75be..81a6ce1825b 100644 --- a/.github/workflows/skillet.lock.yml +++ b/.github/workflows/skillet.lock.yml @@ -1,5 +1,5 @@ # gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"2b7791e73899163b45a0881db45f5747495a6a7dbd2dc4e17f3f31af4ecf6898","body_hash":"ff6f6b1fbf4788ce998b5ddca9ed8f907259189ec1db98a5cd9768e4e0ed2f50","strict":true,"agent_id":"copilot","engine_versions":{"copilot":"1.0.70"}} -# gh-aw-manifest: {"version":1,"secrets":["COPILOT_GITHUB_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0","version":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"3a2844b7e9c422d3c10d287c895573f7108da1b3"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.35","digest":"sha256:2202f63e8650b2b8b0d38033b44a05387b2b71ad3e690c4d23a34786f5462aed","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.35@sha256:2202f63e8650b2b8b0d38033b44a05387b2b71ad3e690c4d23a34786f5462aed"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.35","digest":"sha256:755b79d0dfda82bd6b43a208d68666721e504110c5d342a4eeb199802644ff04","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.35@sha256:755b79d0dfda82bd6b43a208d68666721e504110c5d342a4eeb199802644ff04"},{"image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.35","digest":"sha256:fe83cd274636efa9de3f456e2b078fae137328b9bb6ee4986ae510acaef0cec5","pinned_image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.35@sha256:fe83cd274636efa9de3f456e2b078fae137328b9bb6ee4986ae510acaef0cec5"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.35","digest":"sha256:f69282ec7b1326ba53891c399cf5b10475c0d3ccf4e1519b33d234a5427b57d3","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.35@sha256:f69282ec7b1326ba53891c399cf5b10475c0d3ccf4e1519b33d234a5427b57d3"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.1","digest":"sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.1@sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b","pinned_image":"ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},{"image":"ghcr.io/github/github-mcp-server:v1.6.0","digest":"sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3","pinned_image":"ghcr.io/github/github-mcp-server:v1.6.0@sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3"}]} +# gh-aw-manifest: {"version":1,"secrets":["COPILOT_GITHUB_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0","version":"v7.0.0"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"3a2844b7e9c422d3c10d287c895573f7108da1b3"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.35","digest":"sha256:2202f63e8650b2b8b0d38033b44a05387b2b71ad3e690c4d23a34786f5462aed","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.35@sha256:2202f63e8650b2b8b0d38033b44a05387b2b71ad3e690c4d23a34786f5462aed"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.35","digest":"sha256:755b79d0dfda82bd6b43a208d68666721e504110c5d342a4eeb199802644ff04","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.35@sha256:755b79d0dfda82bd6b43a208d68666721e504110c5d342a4eeb199802644ff04"},{"image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.35","digest":"sha256:fe83cd274636efa9de3f456e2b078fae137328b9bb6ee4986ae510acaef0cec5","pinned_image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.27.35@sha256:fe83cd274636efa9de3f456e2b078fae137328b9bb6ee4986ae510acaef0cec5"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.35","digest":"sha256:f69282ec7b1326ba53891c399cf5b10475c0d3ccf4e1519b33d234a5427b57d3","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.35@sha256:f69282ec7b1326ba53891c399cf5b10475c0d3ccf4e1519b33d234a5427b57d3"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.1","digest":"sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.1@sha256:ad2a979c2cd8b50098e84938ca9c9c1580eb8e91526f101a90adfba7859b2c32"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b","pinned_image":"ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},{"image":"ghcr.io/github/github-mcp-server:v1.6.0","digest":"sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3","pinned_image":"ghcr.io/github/github-mcp-server:v1.6.0@sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3"}]} # This file was automatically generated by gh-aw. DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # # ___ _ _ @@ -45,7 +45,6 @@ # Custom actions used: # - actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 # - actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 -# - actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # 9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # - actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 # - actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 # - actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 @@ -1728,7 +1727,7 @@ jobs: GH_AW_INFO_AWF_VERSION: "v0.27.35" GH_AW_INFO_ENGINE_ID: "copilot" - name: Checkout skills directory - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # 9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: persist-credentials: false sparse-checkout: | From 0ba7325931eaa5dddaf30357204df96ec47776f5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 18 Jul 2026 04:25:57 +0000 Subject: [PATCH 3/3] Reduce largefunc findings in cmd/gh-aw and pkg/linters Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/agentic-auto-upgrade.yml | 2 +- cmd/gh-aw/main.go | 722 +++++++++--------- .../appendbytestring/appendbytestring.go | 112 +-- .../bytesbufferstring/bytesbufferstring.go | 145 ++-- .../bytescomparestring/bytescomparestring.go | 123 +-- pkg/linters/errorfwrapv/errorfwrapv.go | 86 ++- .../execcommandwithoutcontext.go | 93 +-- .../hardcodedfilepath/hardcodedfilepath.go | 85 ++- pkg/linters/httpnoctx/httpnoctx.go | 71 +- .../ioutildeprecated/ioutildeprecated.go | 21 +- pkg/linters/largefunc/largefunc.go | 82 +- pkg/linters/lenstringzero/lenstringzero.go | 92 +-- pkg/linters/mapclearloop/mapclearloop.go | 216 +++--- pkg/linters/mapdeletecheck/mapdeletecheck.go | 161 ++-- pkg/linters/nilctxpassed/nilctxpassed.go | 87 ++- pkg/linters/seenmapbool/seenmapbool.go | 58 +- pkg/linters/sprintferrdot/sprintferrdot.go | 84 +- pkg/linters/sprintfint/sprintfint.go | 94 +-- .../strconvparseignorederror.go | 88 ++- .../stringscountcontains.go | 82 +- .../stringsindexcontains.go | 96 +-- pkg/linters/timeafterleak/timeafterleak.go | 6 + .../writebytestring/writebytestring.go | 149 ++-- 23 files changed, 1467 insertions(+), 1288 deletions(-) diff --git a/.github/workflows/agentic-auto-upgrade.yml b/.github/workflows/agentic-auto-upgrade.yml index e169cea32b6..7035101ee35 100644 --- a/.github/workflows/agentic-auto-upgrade.yml +++ b/.github/workflows/agentic-auto-upgrade.yml @@ -34,7 +34,7 @@ name: Agentic Auto-Upgrade on: schedule: - - cron: "11 4 * * 6" # Weekly (auto-upgrade) + - cron: "21 3 * * 5" # Weekly (auto-upgrade) workflow_dispatch: permissions: diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index 308c5bedc55..30fe1600ac1 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -225,6 +225,177 @@ Any in-progress runs will be canceled before disabling. }, } +// compileRunFlags holds all flag values read for the compile command. +type compileRunFlags struct { + engineOverride string + actionMode string + actionTag string + actionsRepo string + ghAwRef string + validate bool + watch bool + dir string + workflowsDir string + noEmit bool + purge bool + strict bool + trial bool + logicalRepo string + dependabot bool + forceOverwrite bool + refreshStopTime bool + forceRefreshActionPins bool + allowActionRefs bool + zizmor bool + poutine bool + actionlint bool + runnerGuard bool + jsonOutput bool + showAllErrors bool + fix bool + stats bool + failFast bool + noCheckUpdate bool + scheduleSeed string + staged bool + approve bool + validateImages bool + disableModelsDevLookup bool + priorManifestFile string + ghes bool + verbose bool + useSamples bool +} + +// readCompileFlags reads all compile command flags into a compileRunFlags struct. +func readCompileFlags(cmd *cobra.Command) compileRunFlags { + var f compileRunFlags + f.engineOverride, _ = cmd.Flags().GetString("engine") + f.actionMode, _ = cmd.Flags().GetString("action-mode") + f.actionTag, _ = cmd.Flags().GetString("action-tag") + f.actionsRepo, _ = cmd.Flags().GetString("actions-repo") + f.ghAwRef, _ = cmd.Flags().GetString("gh-aw-ref") + f.validate, _ = cmd.Flags().GetBool("validate") + f.watch, _ = cmd.Flags().GetBool("watch") + f.dir, _ = cmd.Flags().GetString("dir") + f.workflowsDir, _ = cmd.Flags().GetString("workflows-dir") + f.noEmit, _ = cmd.Flags().GetBool("no-emit") + f.purge, _ = cmd.Flags().GetBool("purge") + f.strict, _ = cmd.Flags().GetBool("strict") + f.trial, _ = cmd.Flags().GetBool("trial") + f.logicalRepo, _ = cmd.Flags().GetString("logical-repo") + f.dependabot, _ = cmd.Flags().GetBool("dependabot") + f.forceOverwrite, _ = cmd.Flags().GetBool("force") + f.refreshStopTime, _ = cmd.Flags().GetBool("refresh-stop-time") + f.forceRefreshActionPins, _ = cmd.Flags().GetBool("force-refresh-action-pins") + f.allowActionRefs, _ = cmd.Flags().GetBool("allow-action-refs") + f.zizmor, _ = cmd.Flags().GetBool("zizmor") + f.poutine, _ = cmd.Flags().GetBool("poutine") + f.actionlint, _ = cmd.Flags().GetBool("actionlint") + f.runnerGuard, _ = cmd.Flags().GetBool("runner-guard") + f.jsonOutput, _ = cmd.Flags().GetBool("json") + f.showAllErrors, _ = cmd.Flags().GetBool("show-all") + f.fix, _ = cmd.Flags().GetBool("fix") + f.stats, _ = cmd.Flags().GetBool("stats") + f.failFast, _ = cmd.Flags().GetBool("fail-fast") + f.noCheckUpdate, _ = cmd.Flags().GetBool("no-check-update") + f.scheduleSeed, _ = cmd.Flags().GetString("schedule-seed") + f.staged, _ = cmd.Flags().GetBool("staged") + f.approve, _ = cmd.Flags().GetBool("approve") + f.validateImages, _ = cmd.Flags().GetBool("validate-images") + f.disableModelsDevLookup, _ = cmd.Flags().GetBool("no-models-dev-lookup") + f.priorManifestFile, _ = cmd.Flags().GetString("prior-manifest-file") + f.ghes, _ = cmd.Flags().GetBool("ghes") + f.verbose, _ = cmd.Flags().GetBool("verbose") + f.useSamples, _ = cmd.Flags().GetBool("use-samples") + return f +} + +// buildCompileConfig converts the compile run flags into a cli.CompileConfig. +func buildCompileConfig(args []string, f compileRunFlags) cli.CompileConfig { + workflowDir := f.dir + if f.workflowsDir != "" { + workflowDir = f.workflowsDir + } + return cli.CompileConfig{ + MarkdownFiles: args, + Verbose: f.verbose, + EngineOverride: f.engineOverride, + ActionMode: f.actionMode, + ActionTag: f.actionTag, + ActionsRepo: f.actionsRepo, + Validate: f.validate, + Watch: f.watch, + WorkflowDir: workflowDir, + SkipInstructions: false, + NoEmit: f.noEmit, + Purge: f.purge, + TrialMode: f.trial, + TrialLogicalRepoSlug: f.logicalRepo, + Strict: f.strict, + Dependabot: f.dependabot, + ForceOverwrite: f.forceOverwrite, + RefreshStopTime: f.refreshStopTime, + ForceRefreshActionPins: f.forceRefreshActionPins, + AllowActionRefs: f.allowActionRefs, + Zizmor: f.zizmor, + Poutine: f.poutine, + Actionlint: f.actionlint, + RunnerGuard: f.runnerGuard, + JSONOutput: f.jsonOutput, + ShowAllErrors: f.showAllErrors, + Stats: f.stats, + FailFast: f.failFast, + ScheduleSeed: f.scheduleSeed, + Staged: f.staged, + Approve: f.approve, + ValidateImages: f.validateImages, + DisableModelsDevLookup: f.disableModelsDevLookup, + PriorManifestFile: f.priorManifestFile, + GHESCompat: f.ghes, + UseSamples: f.useSamples, + } +} + +// runCompileCmd is the RunE handler for compileCmd. +func runCompileCmd(cmd *cobra.Command, args []string) error { + f := readCompileFlags(cmd) + + if f.ghAwRef != "" { + resolvedRef, resolveErr := workflow.ResolveGhAwRef(cmd.Context(), f.ghAwRef) + if resolveErr != nil { + return fmt.Errorf("--gh-aw-ref: %w", resolveErr) + } + f.actionMode = string(workflow.ActionModeRelease) + f.actionTag = resolvedRef + } + + if err := validateEngine(f.engineOverride); err != nil { + return err + } + + finishCompileUpdateCheck := cli.StartCompileUpdateCheck(cmd.Context(), f.noCheckUpdate, f.verbose) + defer finishCompileUpdateCheck() + + if f.fix { + fixConfig := cli.FixConfig{ + WorkflowIDs: args, + Write: true, + Verbose: f.verbose, + WorkflowDir: f.dir, + } + if err := cli.RunFix(fixConfig); err != nil { + return err + } + } + + config := buildCompileConfig(args, f) + if _, err := cli.CompileWorkflows(cmd.Context(), config); err != nil { + return err + } + return nil +} + var compileCmd = &cobra.Command{ Use: "compile [workflow]...", Short: "Compile agentic workflow Markdown files into GitHub Actions YAML", @@ -283,129 +454,7 @@ Unlike ` + "`gh aw upgrade`" + `, ` + "`gh aw compile`" + ` only applies codemod ` + string(constants.CLIExtensionPrefix) + ` compile --dependabot --force # Force overwrite existing dependabot.yml ` + string(constants.CLIExtensionPrefix) + ` compile --gh-aw-ref main # Pin workflows to the SHA of github/gh-aw main at compile time ` + string(constants.CLIExtensionPrefix) + ` compile --action-tag v1.2.3 # Pin workflows to a specific release tag`, - RunE: func(cmd *cobra.Command, args []string) error { - engineOverride, _ := cmd.Flags().GetString("engine") - actionMode, _ := cmd.Flags().GetString("action-mode") - actionTag, _ := cmd.Flags().GetString("action-tag") - actionsRepo, _ := cmd.Flags().GetString("actions-repo") - ghAwRef, _ := cmd.Flags().GetString("gh-aw-ref") - if ghAwRef != "" { - // --gh-aw-ref is a convenience alias: emit refs like - // `github/gh-aw/actions/setup@` so external e2e harnesses can - // test the compiled workflows against a specific gh-aw revision. - // Resolve branch/tag names to their commit SHA so the baked-in ref - // is immutable and not vulnerable to branch/tag mutation. - resolvedRef, resolveErr := workflow.ResolveGhAwRef(cmd.Context(), ghAwRef) - if resolveErr != nil { - return fmt.Errorf("--gh-aw-ref: %w", resolveErr) - } - actionMode = string(workflow.ActionModeRelease) - actionTag = resolvedRef - } - validate, _ := cmd.Flags().GetBool("validate") - watch, _ := cmd.Flags().GetBool("watch") - dir, _ := cmd.Flags().GetString("dir") - workflowsDir, _ := cmd.Flags().GetString("workflows-dir") - noEmit, _ := cmd.Flags().GetBool("no-emit") - purge, _ := cmd.Flags().GetBool("purge") - strict, _ := cmd.Flags().GetBool("strict") - trial, _ := cmd.Flags().GetBool("trial") - logicalRepo, _ := cmd.Flags().GetString("logical-repo") - dependabot, _ := cmd.Flags().GetBool("dependabot") - forceOverwrite, _ := cmd.Flags().GetBool("force") - refreshStopTime, _ := cmd.Flags().GetBool("refresh-stop-time") - forceRefreshActionPins, _ := cmd.Flags().GetBool("force-refresh-action-pins") - allowActionRefs, _ := cmd.Flags().GetBool("allow-action-refs") - zizmor, _ := cmd.Flags().GetBool("zizmor") - poutine, _ := cmd.Flags().GetBool("poutine") - actionlint, _ := cmd.Flags().GetBool("actionlint") - runnerGuard, _ := cmd.Flags().GetBool("runner-guard") - jsonOutput, _ := cmd.Flags().GetBool("json") - showAllErrors, _ := cmd.Flags().GetBool("show-all") - fix, _ := cmd.Flags().GetBool("fix") - stats, _ := cmd.Flags().GetBool("stats") - failFast, _ := cmd.Flags().GetBool("fail-fast") - noCheckUpdate, _ := cmd.Flags().GetBool("no-check-update") - scheduleSeed, _ := cmd.Flags().GetString("schedule-seed") - staged, _ := cmd.Flags().GetBool("staged") - approve, _ := cmd.Flags().GetBool("approve") - validateImages, _ := cmd.Flags().GetBool("validate-images") - disableModelsDevLookup, _ := cmd.Flags().GetBool("no-models-dev-lookup") - priorManifestFile, _ := cmd.Flags().GetString("prior-manifest-file") - ghes, _ := cmd.Flags().GetBool("ghes") - verbose, _ := cmd.Flags().GetBool("verbose") - useSamples, _ := cmd.Flags().GetBool("use-samples") - if err := validateEngine(engineOverride); err != nil { - return err - } - - finishCompileUpdateCheck := cli.StartCompileUpdateCheck(cmd.Context(), noCheckUpdate, verbose) - defer finishCompileUpdateCheck() - - // If --fix is specified, run fix --write first - if fix { - fixConfig := cli.FixConfig{ - WorkflowIDs: args, - Write: true, - Verbose: verbose, - WorkflowDir: dir, - } - if err := cli.RunFix(fixConfig); err != nil { - return err - } - } - - // Handle --workflows-dir deprecation (mutual exclusion is enforced by Cobra) - workflowDir := dir - if workflowsDir != "" { - workflowDir = workflowsDir - } - config := cli.CompileConfig{ - MarkdownFiles: args, - Verbose: verbose, - EngineOverride: engineOverride, - ActionMode: actionMode, - ActionTag: actionTag, - ActionsRepo: actionsRepo, - Validate: validate, - Watch: watch, - WorkflowDir: workflowDir, - SkipInstructions: false, // Deprecated field, kept for backward compatibility - NoEmit: noEmit, - Purge: purge, - TrialMode: trial, - TrialLogicalRepoSlug: logicalRepo, - Strict: strict, - Dependabot: dependabot, - ForceOverwrite: forceOverwrite, - RefreshStopTime: refreshStopTime, - ForceRefreshActionPins: forceRefreshActionPins, - AllowActionRefs: allowActionRefs, - Zizmor: zizmor, - Poutine: poutine, - Actionlint: actionlint, - RunnerGuard: runnerGuard, - JSONOutput: jsonOutput, - ShowAllErrors: showAllErrors, - Stats: stats, - FailFast: failFast, - ScheduleSeed: scheduleSeed, - Staged: staged, - Approve: approve, - ValidateImages: validateImages, - DisableModelsDevLookup: disableModelsDevLookup, - PriorManifestFile: priorManifestFile, - GHESCompat: ghes, - UseSamples: useSamples, - } - if _, err := cli.CompileWorkflows(cmd.Context(), config); err != nil { - // Return error as-is without additional formatting - // Errors from CompileWorkflows are already formatted with console.FormatError - // which provides IDE-parseable location information (file:line:column) - return err - } - return nil - }, + RunE: runCompileCmd, } var runCmd = &cobra.Command{ @@ -503,53 +552,58 @@ var versionCmd = &cobra.Command{ }, } -func init() { - // Add command groups to root command - rootCmd.AddGroup(&cobra.Group{ - ID: "setup", - Title: "Setup Commands:", - }) - rootCmd.AddGroup(&cobra.Group{ - ID: "development", - Title: "Development Commands:", - }) - rootCmd.AddGroup(&cobra.Group{ - ID: "execution", - Title: "Execution Commands:", - }) - rootCmd.AddGroup(&cobra.Group{ - ID: "analysis", - Title: "Analysis Commands:", - }) - rootCmd.AddGroup(&cobra.Group{ - ID: "utilities", - Title: "Utilities:", - }) - - // Add global verbose flag to root command - rootCmd.PersistentFlags().BoolVarP(&verboseFlag, "verbose", "v", false, "Enable verbose output showing detailed information") +// Package-level command vars for commands instantiated during init. +var ( + addCmd *cobra.Command + addWizardCmd *cobra.Command + updateCmd *cobra.Command + deployCmd *cobra.Command + trialCmd *cobra.Command + initCmd *cobra.Command + statusCmd *cobra.Command + listCmd *cobra.Command + mcpCmd *cobra.Command + logsCmd *cobra.Command + auditCmd *cobra.Command + viewCmd *cobra.Command + healthCmd *cobra.Command + outcomesCmd *cobra.Command + mcpServerCmd *cobra.Command + prCmd *cobra.Command + secretsCmd *cobra.Command + fixCmd *cobra.Command + upgradeCmd *cobra.Command + completionCmd *cobra.Command + hashCmd *cobra.Command + projectCmd *cobra.Command + doctorCmd *cobra.Command + checksCmd *cobra.Command + validateCmd *cobra.Command + lintCmd *cobra.Command + domainsCmd *cobra.Command + experimentsCmd *cobra.Command + forecastCmd *cobra.Command + envCmd *cobra.Command +) - // Add global banner flag to root command - rootCmd.PersistentFlags().BoolVar(&bannerFlag, "banner", false, "Display ASCII logo banner with purple GitHub color theme") +// registerCommandGroups adds the five display groups to rootCmd. +func registerCommandGroups() { + rootCmd.AddGroup(&cobra.Group{ID: "setup", Title: "Setup Commands:"}) + rootCmd.AddGroup(&cobra.Group{ID: "development", Title: "Development Commands:"}) + rootCmd.AddGroup(&cobra.Group{ID: "execution", Title: "Execution Commands:"}) + rootCmd.AddGroup(&cobra.Group{ID: "analysis", Title: "Analysis Commands:"}) + rootCmd.AddGroup(&cobra.Group{ID: "utilities", Title: "Utilities:"}) +} - // Set output to stderr for consistency with CLI logging guidelines +// configureRootCmdSettings sets global flags, output, silencing, version template, +// and corrects the help/version flag descriptions for the root command. +func configureRootCmdSettings() { + rootCmd.PersistentFlags().BoolVarP(&verboseFlag, "verbose", "v", false, "Enable verbose output showing detailed information") + rootCmd.PersistentFlags().BoolVar(&bannerFlag, "banner", false, "Display ASCII logo banner with purple GitHub color theme") rootCmd.SetOut(os.Stderr) - - // Silence usage output on errors - prevents cluttering terminal output with - // full usage text when application errors occur (e.g., compilation errors, - // network timeouts). Users can still run --help for usage information. rootCmd.SilenceUsage = true - - // Silence errors - since we're using RunE and returning errors, Cobra will - // print errors automatically. We handle error formatting ourselves in main(). rootCmd.SilenceErrors = true - - // Set version template to match the version subcommand format rootCmd.SetVersionTemplate(string(constants.CLIExtensionPrefix) + " version {{.Version}}\n") - - // Cobra generates flag descriptions using c.Name() which returns the first - // word of Use ("gh" from "gh aw"), producing "help for gh" and "version for - // gh". Explicitly initialize and override these flags so they display "gh aw". rootCmd.InitDefaultHelpFlag() if f := rootCmd.Flags().Lookup("help"); f != nil { f.Usage = "Show help for " + string(constants.CLIExtensionPrefix) @@ -558,88 +612,92 @@ func init() { if f := rootCmd.Flags().Lookup("version"); f != nil { f.Usage = "Print the current version" } +} - // Fix usage lines so subcommands show "gh aw " instead of "gh ". - // Cobra derives the root name from the first word of Use ("gh" from "gh aw"), - // so CommandPath() for subcommands omits "aw". We use SetUsageFunc to - // post-process the default output, replacing "gh " with "gh aw " in the - // two lines that reference the command path. - rootCmd.SetUsageFunc(func(cmd *cobra.Command) error { - fixPath := func(s string) string { - if s == "gh" { - return "gh aw" - } - if strings.HasPrefix(s, "gh ") && !strings.HasPrefix(s, "gh aw") { - return "gh aw " + s[3:] +// printSubcmdsByGroup prints the available subcommands in groups. +func printSubcmdsByGroup(cmd *cobra.Command, colFmt string) { + out := cmd.OutOrStderr() + for _, group := range cmd.Groups() { + fmt.Fprintf(out, "\n\n%s", group.Title) + for _, sub := range cmd.Commands() { + if sub.GroupID == group.ID && (sub.IsAvailableCommand() || sub.Name() == "help") { + fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) } - return s } - out := cmd.OutOrStderr() - fmt.Fprint(out, "Usage:") - if cmd.Runnable() { - fmt.Fprintf(out, "\n %s", fixPath(cmd.UseLine())) + } + if !cmd.AllChildCommandsHaveGroup() { + fmt.Fprint(out, "\n\nAdditional Commands:") + for _, sub := range cmd.Commands() { + if sub.GroupID == "" && (sub.IsAvailableCommand() || sub.Name() == "help") { + fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) + } } - if cmd.HasAvailableSubCommands() { - fmt.Fprintf(out, "\n %s [command]", fixPath(cmd.CommandPath())) + } +} + +// commandUsageFunc is the SetUsageFunc callback that rewrites "gh " → "gh aw " +// so that subcommand usage lines show the correct binary name. +func commandUsageFunc(cmd *cobra.Command) error { + fixPath := func(s string) string { + if s == "gh" { + return "gh aw" } - if len(cmd.Aliases) > 0 { - fmt.Fprintf(out, "\n\nAliases:\n %s", cmd.NameAndAliases()) + if strings.HasPrefix(s, "gh ") && !strings.HasPrefix(s, "gh aw") { + return "gh aw " + s[3:] } - if cmd.HasExample() { - fmt.Fprintf(out, "\n\nExamples:\n%s", cmd.Example) + return s + } + out := cmd.OutOrStderr() + fmt.Fprint(out, "Usage:") + if cmd.Runnable() { + fmt.Fprintf(out, "\n %s", fixPath(cmd.UseLine())) + } + if cmd.HasAvailableSubCommands() { + fmt.Fprintf(out, "\n %s [command]", fixPath(cmd.CommandPath())) + } + if len(cmd.Aliases) > 0 { + fmt.Fprintf(out, "\n\nAliases:\n %s", cmd.NameAndAliases()) + } + if cmd.HasExample() { + fmt.Fprintf(out, "\n\nExamples:\n%s", cmd.Example) + } + if cmd.HasAvailableSubCommands() { + cmds := cmd.Commands() + colWidth := 0 + for _, sub := range cmds { + if (sub.IsAvailableCommand() || sub.Name() == "help") && len(sub.Name()) > colWidth { + colWidth = len(sub.Name()) + } } - if cmd.HasAvailableSubCommands() { - cmds := cmd.Commands() - // Compute column width dynamically so long command names (e.g. hash-frontmatter) - // are aligned properly instead of overflowing a hard-coded width. - colWidth := 0 + colFmt := fmt.Sprintf("\n %%-%ds %%s", colWidth) + if len(cmd.Groups()) == 0 { + fmt.Fprint(out, "\n\nAvailable Commands:") for _, sub := range cmds { - if (sub.IsAvailableCommand() || sub.Name() == "help") && len(sub.Name()) > colWidth { - colWidth = len(sub.Name()) + if sub.IsAvailableCommand() || sub.Name() == "help" { + fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) } } - colFmt := fmt.Sprintf("\n %%-%ds %%s", colWidth) - if len(cmd.Groups()) == 0 { - fmt.Fprint(out, "\n\nAvailable Commands:") - for _, sub := range cmds { - if sub.IsAvailableCommand() || sub.Name() == "help" { - fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) - } - } - } else { - for _, group := range cmd.Groups() { - fmt.Fprintf(out, "\n\n%s", group.Title) - for _, sub := range cmds { - if sub.GroupID == group.ID && (sub.IsAvailableCommand() || sub.Name() == "help") { - fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) - } - } - } - if !cmd.AllChildCommandsHaveGroup() { - fmt.Fprint(out, "\n\nAdditional Commands:") - for _, sub := range cmds { - if sub.GroupID == "" && (sub.IsAvailableCommand() || sub.Name() == "help") { - fmt.Fprintf(out, colFmt, sub.Name(), sub.Short) - } - } - } - } - } - if cmd.HasAvailableLocalFlags() { - fmt.Fprintf(out, "\n\nFlags:\n%s", strings.TrimRight(cmd.LocalFlags().FlagUsages(), " \t\n")) - } - if cmd.HasAvailableInheritedFlags() { - fmt.Fprintf(out, "\n\nGlobal Flags:\n%s", strings.TrimRight(cmd.InheritedFlags().FlagUsages(), " \t\n")) - } - if cmd.HasAvailableSubCommands() { - fmt.Fprintf(out, "\n\nUse \"%s [command] --help\" for more information about a command.\n", fixPath(cmd.CommandPath())) } else { - fmt.Fprintln(out) + printSubcmdsByGroup(cmd, colFmt) } - return nil - }) + } + if cmd.HasAvailableLocalFlags() { + fmt.Fprintf(out, "\n\nFlags:\n%s", strings.TrimRight(cmd.LocalFlags().FlagUsages(), " \t\n")) + } + if cmd.HasAvailableInheritedFlags() { + fmt.Fprintf(out, "\n\nGlobal Flags:\n%s", strings.TrimRight(cmd.InheritedFlags().FlagUsages(), " \t\n")) + } + if cmd.HasAvailableSubCommands() { + fmt.Fprintf(out, "\n\nUse \"%s [command] --help\" for more information about a command.\n", fixPath(cmd.CommandPath())) + } else { + fmt.Fprintln(out) + } + return nil +} - // Create custom help command that supports "all" subcommand +// setCustomHelpCmd creates and installs the custom help command that supports +// "help all" for a full command reference printout. +func setCustomHelpCmd() { customHelpCmd := &cobra.Command{ Use: "help [command]", Short: "Help about any command", @@ -648,75 +706,80 @@ Simply type ` + string(constants.CLIExtensionPrefix) + ` help [path to command] Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all commands.`, RunE: func(c *cobra.Command, args []string) error { - // Check if the argument is "all" if len(args) == 1 && args[0] == "all" { - // Print header fmt.Fprintln(os.Stderr, console.FormatInfoMessage("GitHub Agentic Workflows CLI - Complete Command Reference")) fmt.Fprintln(os.Stderr, "") - - // Iterate through all commands and print their help for _, subCmd := range rootCmd.Commands() { - // Skip hidden commands and help itself if subCmd.Hidden || subCmd.Name() == "help" { continue } - - // Print command separator fmt.Fprintln(os.Stderr, console.FormatInfoMessage("═══════════════════════════════════════════════════════════════")) fmt.Fprintf(os.Stderr, "\n%s\n\n", console.FormatInfoMessage(fmt.Sprintf("Command: %s %s", string(constants.CLIExtensionPrefix), subCmd.Name()))) - - // Print the command's help _ = subCmd.Help() fmt.Fprintln(os.Stderr, "") } - - // Print footer fmt.Fprintln(os.Stderr, console.FormatInfoMessage("═══════════════════════════════════════════════════════════════")) fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, console.FormatInfoMessage("For more information, visit: https://github.github.com/gh-aw/")) return nil } - - // Otherwise, use the default help behavior cmd, _, e := rootCmd.Find(args) if cmd == nil || e != nil { return fmt.Errorf("unknown help topic [%#q]", args) - } else { - cmd.InitDefaultHelpFlag() // make possible 'help' flag to be shown - return cmd.Help() } + cmd.InitDefaultHelpFlag() + return cmd.Help() }, } - - // Replace the default help command rootCmd.SetHelpCommand(customHelpCmd) +} - // Create and setup add command - addCmd := cli.NewAddCommand(validateEngine) - - // Create and setup add-wizard command - addWizardCmd := cli.NewAddWizardCommand(validateEngine) - - // Create and setup update command - updateCmd := cli.NewUpdateCommand(validateEngine) - - // Create and setup deploy command - deployCmd := cli.NewDeployCommand(validateEngine) - - // Create and setup trial command - trialCmd := cli.NewTrialCommand(validateEngine) - - // Create and setup init command - initCmd := cli.NewInitCommand() +// instantiateAllCmds creates all commands that are constructed via cli.NewXxx helpers. +func instantiateAllCmds() { + addCmd = cli.NewAddCommand(validateEngine) + addWizardCmd = cli.NewAddWizardCommand(validateEngine) + updateCmd = cli.NewUpdateCommand(validateEngine) + deployCmd = cli.NewDeployCommand(validateEngine) + trialCmd = cli.NewTrialCommand(validateEngine) + initCmd = cli.NewInitCommand() cli.RegisterEngineFlagCompletion(initCmd) - // Add flags to new command + statusCmd = cli.NewStatusCommand() + listCmd = cli.NewListCommand() + mcpCmd = cli.NewMCPCommand() + logsCmd = cli.NewLogsCommand() + auditCmd = cli.NewAuditCommand() + viewCmd = cli.NewViewCommand() + healthCmd = cli.NewHealthCommand() + outcomesCmd = cli.NewOutcomesCommand() + mcpServerCmd = cli.NewMCPServerCommand() + prCmd = cli.NewPRCommand() + secretsCmd = cli.NewSecretsCommand() + fixCmd = cli.NewFixCommand() + upgradeCmd = cli.NewUpgradeCommand(validateEngine) + completionCmd = cli.NewCompletionCommand() + hashCmd = cli.NewHashCommand() + projectCmd = cli.NewProjectCommand() + doctorCmd = cli.NewDoctorCommand() + checksCmd = cli.NewChecksCommand() + validateCmd = cli.NewValidateCommand(validateEngine) + lintCmd = cli.NewLintCommand() + domainsCmd = cli.NewDomainsCommand() + experimentsCmd = cli.NewExperimentsCommand() + forecastCmd = cli.NewForecastCommand() + envCmd = cli.NewEnvCommand() +} + +// registerNewCmdFlags adds flags to newCmd. +func registerNewCmdFlags() { newCmd.Flags().BoolP("force", "f", false, "Overwrite existing files without confirmation") newCmd.Flags().BoolP("interactive", "i", false, "Launch interactive workflow creation wizard") newCmd.Flags().StringP("engine", "e", "", cli.EngineFlagOverrideUsage) cli.RegisterEngineFlagCompletion(newCmd) +} - // Add AI flag to compile and add commands +// registerCompileCmdFlags adds all flags, completions, and mutual-exclusion rules to compileCmd. +func registerCompileCmdFlags() { compileCmd.Flags().StringP("engine", "e", "", cli.EngineFlagOverrideUsage) compileCmd.Flags().String("action-mode", "", "How gh-aw action scripts are referenced in compiled workflows: 'dev' uses local paths (for developing gh-aw itself), 'release' emits SHA-pinned remote refs from github/gh-aw, 'action' uses the github/gh-aw-actions repository. Auto-detected from the binary build type if not specified") compileCmd.Flags().String("action-tag", "", "Pin compiled workflows to a specific version of gh-aw actions. Accepts a full commit SHA or a version tag (e.g. v1, v1.2.3). Sets --action-mode to 'release' unless --action-mode action is also specified. Cannot be combined with --gh-aw-ref; use --gh-aw-ref when you want to resolve a branch or tag name to its current SHA") @@ -757,37 +820,35 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all compileCmd.Flags().String("prior-manifest-file", "", "Path to a JSON file containing pre-cached gh-aw-manifests (map[lockFile]*GHAWManifest); used by the MCP server to supply a tamper-proof manifest baseline captured at startup") compileCmd.Flags().Bool("ghes", false, "Enable GitHub Enterprise Server (GHES) compatibility mode. Artifact actions continue using latest non-v3 pins (v3 is deprecated). Overrides the aw.json ghes field") if err := compileCmd.Flags().MarkHidden("prior-manifest-file"); err != nil { - // Non-fatal: flag is registered even if MarkHidden fails _ = err } compileCmd.MarkFlagsMutuallyExclusive("dir", "workflows-dir") - // --gh-aw-ref is a convenience alias for --action-mode release --action-tag ; - // combining it with either of those flags leads to one silently overwriting the other. compileCmd.MarkFlagsMutuallyExclusive("gh-aw-ref", "action-tag") compileCmd.MarkFlagsMutuallyExclusive("gh-aw-ref", "action-mode") - - // Register completions for compile command compileCmd.ValidArgsFunction = cli.CompleteWorkflowNames cli.RegisterEngineFlagCompletion(compileCmd) cli.RegisterDirFlagCompletion(compileCmd, "dir") - rootCmd.AddCommand(compileCmd) +} - // Add flags to remove command +// registerRemoveCmdFlags adds flags and completions for removeCmd. +func registerRemoveCmdFlags() { removeCmd.Flags().Bool("keep-orphans", false, "Skip removal of orphaned include files that are no longer referenced by any workflow") removeCmd.Flags().StringP("dir", "d", "", "Workflow directory (default: $GH_AW_WORKFLOWS_DIR or .github/workflows)") - // Register completions for remove command removeCmd.ValidArgsFunction = cli.CompleteWorkflowNames cli.RegisterDirFlagCompletion(removeCmd, "dir") +} - // Add flags to enable/disable commands +// registerToggleCmdFlags adds flags and completions for enableCmd and disableCmd. +func registerToggleCmdFlags() { enableCmd.Flags().StringP("repo", "r", "", "Target repository ([HOST/]owner/repo format). Defaults to current repository") disableCmd.Flags().StringP("repo", "r", "", "Target repository ([HOST/]owner/repo format). Defaults to current repository") - // Register completions for enable/disable commands enableCmd.ValidArgsFunction = cli.CompleteWorkflowNames disableCmd.ValidArgsFunction = cli.CompleteWorkflowNames +} - // Add flags to run command +// registerRunCmdFlags adds all flags and completions for runCmd. +func registerRunCmdFlags() { runCmd.Flags().Int("repeat", 0, "Number of additional times to run after the initial execution (e.g., --repeat 3 runs 4 times total)") runCmd.Flags().Bool("enable-if-needed", false, "Enable the workflow before running if needed, and restore state afterward") runCmd.Flags().StringP("engine", "e", "", cli.EngineFlagOverrideUsage) @@ -799,42 +860,21 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all runCmd.Flags().Bool("dry-run", false, "Preview workflow execution without triggering runs on GitHub Actions") runCmd.Flags().BoolP("json", "j", false, "Output results in JSON format") runCmd.Flags().Bool("approve", false, "Approve safe update manifest changes when --push triggers an automatic recompile step") - // Register completions for run command runCmd.ValidArgsFunction = cli.CompleteWorkflowNames cli.RegisterEngineFlagCompletion(runCmd) +} + +// registerFlagsForCmds sets up all flag registrations for each command. +func registerFlagsForCmds() { + registerNewCmdFlags() + registerCompileCmdFlags() + registerRemoveCmdFlags() + registerToggleCmdFlags() + registerRunCmdFlags() +} - // Create and setup status command - statusCmd := cli.NewStatusCommand() - - // Create and setup list command - listCmd := cli.NewListCommand() - - // Create commands that need group assignment - mcpCmd := cli.NewMCPCommand() - logsCmd := cli.NewLogsCommand() - auditCmd := cli.NewAuditCommand() - viewCmd := cli.NewViewCommand() - healthCmd := cli.NewHealthCommand() - outcomesCmd := cli.NewOutcomesCommand() - mcpServerCmd := cli.NewMCPServerCommand() - prCmd := cli.NewPRCommand() - secretsCmd := cli.NewSecretsCommand() - fixCmd := cli.NewFixCommand() - upgradeCmd := cli.NewUpgradeCommand(validateEngine) - completionCmd := cli.NewCompletionCommand() - hashCmd := cli.NewHashCommand() - projectCmd := cli.NewProjectCommand() - doctorCmd := cli.NewDoctorCommand() - checksCmd := cli.NewChecksCommand() - validateCmd := cli.NewValidateCommand(validateEngine) - lintCmd := cli.NewLintCommand() - domainsCmd := cli.NewDomainsCommand() - experimentsCmd := cli.NewExperimentsCommand() - forecastCmd := cli.NewForecastCommand() - envCmd := cli.NewEnvCommand() - - // Assign commands to groups - // Setup Commands +// assignGroupIDsToAllCmds sets the GroupID field on every command. +func assignGroupIDsToAllCmds() { initCmd.GroupID = "setup" newCmd.GroupID = "setup" addCmd.GroupID = "setup" @@ -847,7 +887,6 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all envCmd.GroupID = "setup" doctorCmd.GroupID = "setup" - // Development Commands compileCmd.GroupID = "development" validateCmd.GroupID = "development" lintCmd.GroupID = "development" @@ -855,13 +894,11 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all fixCmd.GroupID = "development" domainsCmd.GroupID = "development" - // Execution Commands runCmd.GroupID = "execution" enableCmd.GroupID = "execution" disableCmd.GroupID = "execution" trialCmd.GroupID = "execution" - // Analysis Commands logsCmd.GroupID = "analysis" auditCmd.GroupID = "analysis" viewCmd.GroupID = "analysis" @@ -873,16 +910,15 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all experimentsCmd.GroupID = "analysis" forecastCmd.GroupID = "analysis" - // Utilities mcpServerCmd.GroupID = "utilities" prCmd.GroupID = "utilities" completionCmd.GroupID = "utilities" hashCmd.GroupID = "utilities" projectCmd.GroupID = "utilities" +} - // version command is intentionally left without a group (common practice) - - // Add all commands to root +// addCmdsToRoot adds all commands to rootCmd. +func addCmdsToRoot() { rootCmd.AddCommand(addCmd) rootCmd.AddCommand(addWizardCmd) rootCmd.AddCommand(updateCmd) @@ -891,7 +927,6 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all rootCmd.AddCommand(trialCmd) rootCmd.AddCommand(newCmd) rootCmd.AddCommand(initCmd) - rootCmd.AddCommand(runCmd) rootCmd.AddCommand(removeCmd) rootCmd.AddCommand(statusCmd) @@ -920,27 +955,34 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all rootCmd.AddCommand(experimentsCmd) rootCmd.AddCommand(forecastCmd) rootCmd.AddCommand(envCmd) +} - // Fix help flag descriptions for all subcommands to be consistent with the - // root command ("Show help for gh aw" vs the Cobra default "help for [cmd]"). - var fixSubCmdHelpFlags func(cmd *cobra.Command) - fixSubCmdHelpFlags = func(cmd *cobra.Command) { - cmd.InitDefaultHelpFlag() - if f := cmd.Flags().Lookup("help"); f != nil { - cmdPath := cmd.CommandPath() - // CommandPath() uses Name() which returns the first word of Use - // ("gh" from "gh aw"), so subcommand paths look like "gh compile". - // Replace the leading "gh " prefix with "gh aw " to match the root - // command's display name. - if strings.HasPrefix(cmdPath, "gh ") && !strings.HasPrefix(cmdPath, "gh aw") { - cmdPath = "gh aw " + cmdPath[3:] - } - f.Usage = "Show help for " + cmdPath - } - for _, sub := range cmd.Commands() { - fixSubCmdHelpFlags(sub) +// fixSubCmdHelpFlags recursively updates the --help flag description for cmd +// and all its subcommands so it reads "Show help for gh aw " rather than +// the Cobra default "help for [cmd]". +func fixSubCmdHelpFlags(cmd *cobra.Command) { + cmd.InitDefaultHelpFlag() + if f := cmd.Flags().Lookup("help"); f != nil { + cmdPath := cmd.CommandPath() + if strings.HasPrefix(cmdPath, "gh ") && !strings.HasPrefix(cmdPath, "gh aw") { + cmdPath = "gh aw " + cmdPath[3:] } + f.Usage = "Show help for " + cmdPath + } + for _, sub := range cmd.Commands() { + fixSubCmdHelpFlags(sub) } +} + +func init() { + registerCommandGroups() + configureRootCmdSettings() + rootCmd.SetUsageFunc(commandUsageFunc) + setCustomHelpCmd() + instantiateAllCmds() + registerFlagsForCmds() + assignGroupIDsToAllCmds() + addCmdsToRoot() for _, sub := range rootCmd.Commands() { fixSubCmdHelpFlags(sub) } diff --git a/pkg/linters/appendbytestring/appendbytestring.go b/pkg/linters/appendbytestring/appendbytestring.go index bb960e2d93b..bffd7421d0b 100644 --- a/pkg/linters/appendbytestring/appendbytestring.go +++ b/pkg/linters/appendbytestring/appendbytestring.go @@ -43,65 +43,69 @@ func run(pass *analysis.Pass) (any, error) { } insp.Preorder(nodeFilter, func(n ast.Node) { - call, ok := n.(*ast.CallExpr) - if !ok { - return - } - - // Match append(b, x...) with exactly 2 arguments and an ellipsis. - ident, ok := call.Fun.(*ast.Ident) - if !ok || ident.Name != "append" { - return - } - if len(call.Args) != 2 || !call.Ellipsis.IsValid() { - return - } - - pos := pass.Fset.PositionFor(call.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "appendbytestring") { - return - } - - // The first argument must be []byte. - if !astutil.IsByteSlice(pass, call.Args[0]) { - return - } - - // The second argument must be a []byte(s) conversion where s is a string. - conv, ok := call.Args[1].(*ast.CallExpr) - if !ok { - return - } - if !astutil.IsByteSliceConversion(pass, conv) { - return - } - if len(conv.Args) != 1 { - return - } - strArg := conv.Args[0] - if !astutil.IsStringType(pass, strArg) { - return - } - - sText := astutil.NodeText(pass.Fset, strArg) - if sText == "" { - return - } - - pass.Report(analysis.Diagnostic{ - Pos: call.Pos(), - End: call.End(), - Message: fmt.Sprintf("append(b, []byte(%s)...) can be simplified to append(b, %s...); the []byte conversion is unnecessary", sText, sText), - SuggestedFixes: buildFix(pass, conv, strArg), - }) + checkAppendByteStringNode(pass, n, noLintIndex, generatedFiles) }) return nil, nil } +func checkAppendByteStringNode(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + call, ok := n.(*ast.CallExpr) + if !ok { + return + } + + // Match append(b, x...) with exactly 2 arguments and an ellipsis. + ident, ok := call.Fun.(*ast.Ident) + if !ok || ident.Name != "append" { + return + } + if len(call.Args) != 2 || !call.Ellipsis.IsValid() { + return + } + + pos := pass.Fset.PositionFor(call.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "appendbytestring") { + return + } + + // The first argument must be []byte. + if !astutil.IsByteSlice(pass, call.Args[0]) { + return + } + + // The second argument must be a []byte(s) conversion where s is a string. + conv, ok := call.Args[1].(*ast.CallExpr) + if !ok { + return + } + if !astutil.IsByteSliceConversion(pass, conv) { + return + } + if len(conv.Args) != 1 { + return + } + strArg := conv.Args[0] + if !astutil.IsStringType(pass, strArg) { + return + } + + sText := astutil.NodeText(pass.Fset, strArg) + if sText == "" { + return + } + + pass.Report(analysis.Diagnostic{ + Pos: call.Pos(), + End: call.End(), + Message: fmt.Sprintf("append(b, []byte(%s)...) can be simplified to append(b, %s...); the []byte conversion is unnecessary", sText, sText), + SuggestedFixes: buildFix(pass, conv, strArg), + }) +} + // buildFix returns a SuggestedFix rewriting append(b, []byte(s)...) to append(b, s...). func buildFix(pass *analysis.Pass, conv *ast.CallExpr, strArg ast.Expr) []analysis.SuggestedFix { sText := astutil.NodeText(pass.Fset, strArg) diff --git a/pkg/linters/bytesbufferstring/bytesbufferstring.go b/pkg/linters/bytesbufferstring/bytesbufferstring.go index f59288ef115..e980a649c14 100644 --- a/pkg/linters/bytesbufferstring/bytesbufferstring.go +++ b/pkg/linters/bytesbufferstring/bytesbufferstring.go @@ -44,80 +44,87 @@ func run(pass *analysis.Pass) (any, error) { } insp.Preorder(nodeFilter, func(n ast.Node) { - call, ok := n.(*ast.CallExpr) - if !ok { - return - } - - // Match string(...) type conversion. - typeInfo, ok := pass.TypesInfo.Types[call.Fun] - if !ok || !typeInfo.IsType() { - return - } - basic, ok := typeInfo.Type.(*types.Basic) - if !ok || basic.Kind() != types.String { - return - } - - if len(call.Args) != 1 { - return - } - - pos := pass.Fset.PositionFor(call.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "bytesbufferstring") { - return - } - - // The argument must be buf.Bytes() where buf is a bytes.Buffer value. - inner, ok := call.Args[0].(*ast.CallExpr) - if !ok { - return - } - sel, ok := inner.Fun.(*ast.SelectorExpr) - if !ok || sel.Sel.Name != "Bytes" { - return - } - if len(inner.Args) != 0 { - return - } - receiverType := pass.TypesInfo.TypeOf(sel.X) - if receiverType == nil { - return - } - // Only flag value receivers (bytes.Buffer), not pointer receivers (*bytes.Buffer). - // The rewrite string(buf.Bytes()) → buf.String() is not semantics-preserving when - // buf is a nil *bytes.Buffer: string(buf.Bytes()) panics, while buf.String() returns - // "". Restricting to value receivers avoids this semantic difference entirely. - if !isBytesBufferValue(receiverType) { - return - } - - receiverText := astutil.NodeText(pass.Fset, sel.X) - if receiverText == "" { - return - } - - pass.Report(analysis.Diagnostic{ - Pos: call.Pos(), - End: call.End(), - Message: fmt.Sprintf("string(%s.Bytes()) can be simplified to %s.String()", receiverText, receiverText), - SuggestedFixes: []analysis.SuggestedFix{{ - Message: fmt.Sprintf("Replace string(%s.Bytes()) with %s.String()", receiverText, receiverText), - TextEdits: []analysis.TextEdit{{ - Pos: call.Pos(), - End: call.End(), - NewText: []byte(receiverText + ".String()"), - }}, - }}, - }) + checkBytesBufferStringNode(pass, n, noLintIndex, generatedFiles) }) return nil, nil } +func checkBytesBufferStringNode(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + call, ok := n.(*ast.CallExpr) + if !ok { + return + } + + // Match string(...) type conversion. + typeInfo, ok := pass.TypesInfo.Types[call.Fun] + if !ok || !typeInfo.IsType() { + return + } + basic, ok := typeInfo.Type.(*types.Basic) + if !ok || basic.Kind() != types.String { + return + } + + if len(call.Args) != 1 { + return + } + + pos := pass.Fset.PositionFor(call.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "bytesbufferstring") { + return + } + + // The argument must be buf.Bytes() where buf is a bytes.Buffer value. + inner, ok := call.Args[0].(*ast.CallExpr) + if !ok { + return + } + sel, ok := inner.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Bytes" { + return + } + if len(inner.Args) != 0 { + return + } + receiverType := pass.TypesInfo.TypeOf(sel.X) + if receiverType == nil { + return + } + // Only flag value receivers (bytes.Buffer), not pointer receivers (*bytes.Buffer). + // The rewrite string(buf.Bytes()) → buf.String() is not semantics-preserving when + // buf is a nil *bytes.Buffer: string(buf.Bytes()) panics, while buf.String() returns + // "". Restricting to value receivers avoids this semantic difference entirely. + if !isBytesBufferValue(receiverType) { + return + } + + receiverText := astutil.NodeText(pass.Fset, sel.X) + if receiverText == "" { + return + } + reportBytesBufferStringDiag(pass, call, receiverText) +} + +func reportBytesBufferStringDiag(pass *analysis.Pass, call *ast.CallExpr, receiverText string) { + pass.Report(analysis.Diagnostic{ + Pos: call.Pos(), + End: call.End(), + Message: fmt.Sprintf("string(%s.Bytes()) can be simplified to %s.String()", receiverText, receiverText), + SuggestedFixes: []analysis.SuggestedFix{{ + Message: fmt.Sprintf("Replace string(%s.Bytes()) with %s.String()", receiverText, receiverText), + TextEdits: []analysis.TextEdit{{ + Pos: call.Pos(), + End: call.End(), + NewText: []byte(receiverText + ".String()"), + }}, + }}, + }) +} + // isBytesBufferValue reports whether t is exactly bytes.Buffer (value receiver, not pointer). // We intentionally exclude *bytes.Buffer: the rewrite string(buf.Bytes()) → buf.String() is not // semantics-preserving when buf is nil — the former panics while the latter returns "". diff --git a/pkg/linters/bytescomparestring/bytescomparestring.go b/pkg/linters/bytescomparestring/bytescomparestring.go index e34af17c94a..17626496a21 100644 --- a/pkg/linters/bytescomparestring/bytescomparestring.go +++ b/pkg/linters/bytescomparestring/bytescomparestring.go @@ -53,69 +53,73 @@ func run(pass *analysis.Pass) (any, error) { } insp.Preorder(nodeFilter, func(n ast.Node) { - bin, ok := n.(*ast.BinaryExpr) - if !ok { - return - } + checkBinaryExprForBytesComp(pass, n, noLintIndex, generatedFiles, seenImportFiles) + }) - // Only flag == and != operators. - if bin.Op != token.EQL && bin.Op != token.NEQ { - return - } + return nil, nil +} - pos := pass.Fset.PositionFor(bin.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "bytescomparestring") { - return - } +func checkBinaryExprForBytesComp(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex, seenImportFiles map[token.Pos]bool) { + bin, ok := n.(*ast.BinaryExpr) + if !ok { + return + } - // Both sides must be string(x) conversions where x is []byte. - lhsArg, ok := extractByteSliceStringConv(pass, bin.X) - if !ok { - return - } - rhsArg, ok := extractByteSliceStringConv(pass, bin.Y) - if !ok { - return - } + // Only flag == and != operators. + if bin.Op != token.EQL && bin.Op != token.NEQ { + return + } - lText := astutil.NodeText(pass.Fset, lhsArg) - rText := astutil.NodeText(pass.Fset, rhsArg) - if lText == "" || rText == "" { - return - } + pos := pass.Fset.PositionFor(bin.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "bytescomparestring") { + return + } - // Determine the local qualifier for "bytes" and whether the fix is safe. - qualifier, skipFix := bytesQualifier(pass, bin.Pos()) + // Both sides must be string(x) conversions where x is []byte. + lhsArg, ok := extractByteSliceStringConv(pass, bin.X) + if !ok { + return + } + rhsArg, ok := extractByteSliceStringConv(pass, bin.Y) + if !ok { + return + } - if bin.Op == token.EQL { - var fixes []analysis.SuggestedFix - if !skipFix { - fixes = buildFix(pass, bin, fmt.Sprintf("%s.Equal(%s, %s)", qualifier, lText, rText), seenImportFiles) - } - pass.Report(analysis.Diagnostic{ - Pos: bin.Pos(), - End: bin.End(), - Message: fmt.Sprintf("string(%s) == string(%s) is a []byte comparison written the long way; use bytes.Equal(%s, %s) for clearer intent", lText, rText, lText, rText), - SuggestedFixes: fixes, - }) - } else { - var fixes []analysis.SuggestedFix - if !skipFix { - fixes = buildFix(pass, bin, fmt.Sprintf("!%s.Equal(%s, %s)", qualifier, lText, rText), seenImportFiles) - } - pass.Report(analysis.Diagnostic{ - Pos: bin.Pos(), - End: bin.End(), - Message: fmt.Sprintf("string(%s) != string(%s) is a []byte comparison written the long way; use !bytes.Equal(%s, %s) for clearer intent", lText, rText, lText, rText), - SuggestedFixes: fixes, - }) - } - }) + lText := astutil.NodeText(pass.Fset, lhsArg) + rText := astutil.NodeText(pass.Fset, rhsArg) + if lText == "" || rText == "" { + return + } - return nil, nil + // Determine the local qualifier for "bytes" and whether the fix is safe. + qualifier, skipFix := bytesQualifier(pass, bin.Pos()) + + if bin.Op == token.EQL { + var fixes []analysis.SuggestedFix + if !skipFix { + fixes = buildFix(pass, bin, fmt.Sprintf("%s.Equal(%s, %s)", qualifier, lText, rText), seenImportFiles) + } + pass.Report(analysis.Diagnostic{ + Pos: bin.Pos(), + End: bin.End(), + Message: fmt.Sprintf("string(%s) == string(%s) is a []byte comparison written the long way; use bytes.Equal(%s, %s) for clearer intent", lText, rText, lText, rText), + SuggestedFixes: fixes, + }) + } else { + var fixes []analysis.SuggestedFix + if !skipFix { + fixes = buildFix(pass, bin, fmt.Sprintf("!%s.Equal(%s, %s)", qualifier, lText, rText), seenImportFiles) + } + pass.Report(analysis.Diagnostic{ + Pos: bin.Pos(), + End: bin.End(), + Message: fmt.Sprintf("string(%s) != string(%s) is a []byte comparison written the long way; use !bytes.Equal(%s, %s) for clearer intent", lText, rText, lText, rText), + SuggestedFixes: fixes, + }) + } } // bytesQualifier returns the local binding name for the "bytes" package in the @@ -200,9 +204,12 @@ func addBytesImportEdit(pass *analysis.Pass, pos token.Pos, seenImportFiles map[ } } - // Compute the edit to add and mark the file so subsequent violations in the - // same pass do not emit a duplicate overlapping TextEdit. + return insertBytesImportEdit(pass, file, seenImportFiles) +} +// insertBytesImportEdit adds an import edit for bytesPkg to the given file, +// choosing grouped, single-converted-to-grouped, or standalone import placement. +func insertBytesImportEdit(pass *analysis.Pass, file *ast.File, seenImportFiles map[token.Pos]bool) (analysis.TextEdit, bool) { // Find an existing grouped import declaration to add into. for _, decl := range file.Decls { genDecl, ok := decl.(*ast.GenDecl) diff --git a/pkg/linters/errorfwrapv/errorfwrapv.go b/pkg/linters/errorfwrapv/errorfwrapv.go index 1c427526314..681de6992d9 100644 --- a/pkg/linters/errorfwrapv/errorfwrapv.go +++ b/pkg/linters/errorfwrapv/errorfwrapv.go @@ -73,54 +73,58 @@ func run(pass *analysis.Pass) (any, error) { } insp.Preorder(nodeFilter, func(n ast.Node) { - call, ok := n.(*ast.CallExpr) - if !ok { - return - } + checkErrorfWrapVNode(pass, n, noLintIndex, generatedFiles) + }) - position := pass.Fset.PositionFor(call.Pos(), false) - if filecheck.ShouldSkipFilename(position.Filename, generatedFiles) { - return - } + return nil, nil +} - if !astutil.IsFmtErrorf(pass, call) { - return - } +func checkErrorfWrapVNode(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + call, ok := n.(*ast.CallExpr) + if !ok { + return + } - if len(call.Args) == 0 { - return - } + position := pass.Fset.PositionFor(call.Pos(), false) + if filecheck.ShouldSkipFilename(position.Filename, generatedFiles) { + return + } - lit, ok := call.Args[0].(*ast.BasicLit) - if !ok || lit.Kind != token.STRING { - return - } + if !astutil.IsFmtErrorf(pass, call) { + return + } - verbs := parseFormatVerbs(lit.Value) - for _, fv := range verbs { - if fv.verb != 'v' { - continue - } - callArgIdx := fv.argIdx + 1 - if callArgIdx >= len(call.Args) { - continue - } - tv, ok := pass.TypesInfo.Types[call.Args[callArgIdx]] - if !ok || tv.Type == nil { - continue - } - if !types.Implements(tv.Type, errorIface) { - continue - } - if nolint.HasDirectiveForLinter(position, noLintIndex, "errorfwrapv") { - return - } - pass.ReportRangef(call, "fmt.Errorf formats an error argument with %%v; use %%w to preserve the error chain") + if len(call.Args) == 0 { + return + } + + lit, ok := call.Args[0].(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + return + } + + verbs := parseFormatVerbs(lit.Value) + for _, fv := range verbs { + if fv.verb != 'v' { + continue + } + callArgIdx := fv.argIdx + 1 + if callArgIdx >= len(call.Args) { + continue + } + tv, ok := pass.TypesInfo.Types[call.Args[callArgIdx]] + if !ok || tv.Type == nil { + continue + } + if !types.Implements(tv.Type, errorIface) { + continue + } + if nolint.HasDirectiveForLinter(position, noLintIndex, "errorfwrapv") { return } - }) - - return nil, nil + pass.ReportRangef(call, "fmt.Errorf formats an error argument with %%v; use %%w to preserve the error chain") + return + } } func parseFormatVerbs(s string) []formatVerb { diff --git a/pkg/linters/execcommandwithoutcontext/execcommandwithoutcontext.go b/pkg/linters/execcommandwithoutcontext/execcommandwithoutcontext.go index 990011dd9eb..bb55daa3089 100644 --- a/pkg/linters/execcommandwithoutcontext/execcommandwithoutcontext.go +++ b/pkg/linters/execcommandwithoutcontext/execcommandwithoutcontext.go @@ -11,6 +11,7 @@ import ( "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" "github.com/github/gh-aw/pkg/linters/internal/astutil" "github.com/github/gh-aw/pkg/linters/internal/filecheck" @@ -45,59 +46,63 @@ func run(pass *analysis.Pass) (any, error) { if !ok { continue } - sel, ok := execCommandSelector(pass, call) - if !ok { - continue - } + checkExecCommandWithoutCtx(pass, cur, call, noLintIndex, generatedFiles) + } - pos := pass.Fset.PositionFor(call.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return nil, nil +} + +func checkExecCommandWithoutCtx(pass *analysis.Pass, cur inspector.Cursor, call *ast.CallExpr, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + sel, ok := execCommandSelector(pass, call) + if !ok { + return + } + + pos := pass.Fset.PositionFor(call.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "execcommandwithoutcontext") { + return + } + + for encl := range cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) { + funcNode := encl.Node() + funcType := astutil.EnclosingFuncType(funcNode) + if funcType == nil { continue } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "execcommandwithoutcontext") { + ctxParamName, hasCtx := astutil.ContextParamName(pass, funcType) + if !hasCtx { + if _, isFuncLit := funcNode.(*ast.FuncLit); isFuncLit && !astutil.IsGoOrDeferClosure(encl) { + break + } continue } - - for encl := range cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) { - funcNode := encl.Node() - funcType := astutil.EnclosingFuncType(funcNode) - if funcType == nil { - continue - } - ctxParamName, hasCtx := astutil.ContextParamName(pass, funcType) - if !hasCtx { - if _, isFuncLit := funcNode.(*ast.FuncLit); isFuncLit && !astutil.IsGoOrDeferClosure(encl) { - break - } - continue - } - pass.Report(analysis.Diagnostic{ - Pos: call.Pos(), - End: call.End(), - Message: fmt.Sprintf("use exec.CommandContext(%s, ...) instead of exec.Command to propagate context cancellation", ctxParamName), - SuggestedFixes: []analysis.SuggestedFix{ - { - Message: fmt.Sprintf("Replace exec.Command with exec.CommandContext(%s, ...)", ctxParamName), - TextEdits: []analysis.TextEdit{ - { - Pos: sel.Sel.Pos(), - End: sel.Sel.End(), - NewText: []byte("CommandContext"), - }, - { - Pos: call.Lparen + 1, - End: call.Lparen + 1, - NewText: []byte(ctxParamName + ", "), - }, + pass.Report(analysis.Diagnostic{ + Pos: call.Pos(), + End: call.End(), + Message: fmt.Sprintf("use exec.CommandContext(%s, ...) instead of exec.Command to propagate context cancellation", ctxParamName), + SuggestedFixes: []analysis.SuggestedFix{ + { + Message: fmt.Sprintf("Replace exec.Command with exec.CommandContext(%s, ...)", ctxParamName), + TextEdits: []analysis.TextEdit{ + { + Pos: sel.Sel.Pos(), + End: sel.Sel.End(), + NewText: []byte("CommandContext"), + }, + { + Pos: call.Lparen + 1, + End: call.Lparen + 1, + NewText: []byte(ctxParamName + ", "), }, }, }, - }) - break - } + }, + }) + break } - - return nil, nil } // execCommandSelector reports the selector expression for calls to diff --git a/pkg/linters/hardcodedfilepath/hardcodedfilepath.go b/pkg/linters/hardcodedfilepath/hardcodedfilepath.go index cd9add4e6c8..5884f4dc19d 100644 --- a/pkg/linters/hardcodedfilepath/hardcodedfilepath.go +++ b/pkg/linters/hardcodedfilepath/hardcodedfilepath.go @@ -253,54 +253,57 @@ func run(pass *analysis.Pass) (any, error) { if !ok || lit.Kind != token.STRING { continue } + checkHardcodedFilePathLit(pass, cur, lit, noLintIndex, generatedFiles, knownConsts) + } - pos := pass.Fset.PositionFor(lit.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - continue - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "hardcodedfilepath") { - continue - } + return nil, nil +} - raw := unquoteStringLit(lit.Value) - if !isPathLike(raw) { - continue - } - if hasFormatVerb(raw) { - continue - } +func checkHardcodedFilePathLit(pass *analysis.Pass, cur inspector.Cursor, lit *ast.BasicLit, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex, knownConsts map[string]constRef) { + pos := pass.Fset.PositionFor(lit.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "hardcodedfilepath") { + return + } - // Skip literals that are the value of a const declaration — those are - // the canonical definitions, not inline usages. - if isConstDeclValue(cur) { - continue - } + raw := unquoteStringLit(lit.Value) + if !isPathLike(raw) { + return + } + if hasFormatVerb(raw) { + return + } + + // Skip literals that are the value of a const declaration — those are + // the canonical definitions, not inline usages. + if isConstDeclValue(cur) { + return + } - // Detect whether the literal is a direct argument of a log/print call. - inLog := enclosingCallIsLogPrint(pass, cur) + // Detect whether the literal is a direct argument of a log/print call. + inLog := enclosingCallIsLogPrint(pass, cur) - if ref, found := knownConsts[raw]; found { - msg := fmt.Sprintf( - "hard-coded file path %q: use constant %s instead of inline string literal", - raw, ref, - ) - if inLog { - msg += " (path appears in log/print call — keeping consistent via constant is especially important)" - } - pass.ReportRangef(lit, "%s", msg) - } else { - msg := fmt.Sprintf( - "hard-coded file path %q: consider extracting as a named constant", - raw, - ) - if inLog { - msg += " (path appears in log/print call)" - } - pass.ReportRangef(lit, "%s", msg) + if ref, found := knownConsts[raw]; found { + msg := fmt.Sprintf( + "hard-coded file path %q: use constant %s instead of inline string literal", + raw, ref, + ) + if inLog { + msg += " (path appears in log/print call — keeping consistent via constant is especially important)" } + pass.ReportRangef(lit, "%s", msg) + } else { + msg := fmt.Sprintf( + "hard-coded file path %q: consider extracting as a named constant", + raw, + ) + if inLog { + msg += " (path appears in log/print call)" + } + pass.ReportRangef(lit, "%s", msg) } - - return nil, nil } // isConstDeclValue reports whether the cursor's node is the value expression diff --git a/pkg/linters/httpnoctx/httpnoctx.go b/pkg/linters/httpnoctx/httpnoctx.go index dcd087f7f47..5f0c6c457d0 100644 --- a/pkg/linters/httpnoctx/httpnoctx.go +++ b/pkg/linters/httpnoctx/httpnoctx.go @@ -57,52 +57,55 @@ func run(pass *analysis.Pass) (any, error) { if !ok { continue } + checkHTTPCall(pass, cursor, call, noLintIndex, generatedFiles) + } - pos := pass.Fset.PositionFor(call.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - continue - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "httpnoctx") { - continue - } + return nil, nil +} - sel, ok := call.Fun.(*ast.SelectorExpr) - if !ok { - continue - } - if contextFreeMethods[sel.Sel.Name] { - if isHTTPClientReceiver(pass, sel.X) { - pass.ReportRangef(call, - "(*http.Client).%s does not accept a context; use http.NewRequestWithContext + client.Do to propagate cancellation", - sel.Sel.Name, - ) - continue - } - - if isHTTPPackage(pass, sel.X) { - pass.ReportRangef(call, - "http.%s does not accept a context; use http.NewRequestWithContext + http.DefaultClient.Do to propagate cancellation", - sel.Sel.Name, - ) - continue - } - } +func checkHTTPCall(pass *analysis.Pass, cursor inspector.Cursor, call *ast.CallExpr, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + pos := pass.Fset.PositionFor(call.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "httpnoctx") { + return + } - if sel.Sel.Name == "NewRequest" && isHTTPPackage(pass, sel.X) && hasContextInEnclosingFunc(pass, cursor) { + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return + } + if contextFreeMethods[sel.Sel.Name] { + if isHTTPClientReceiver(pass, sel.X) { pass.ReportRangef(call, - "http.NewRequest does not propagate context; use http.NewRequestWithContext when context.Context is in scope", + "(*http.Client).%s does not accept a context; use http.NewRequestWithContext + client.Do to propagate cancellation", + sel.Sel.Name, ) - continue + return } - if sel.Sel.Name == "Do" && isHTTPDefaultClient(pass, sel.X) { + if isHTTPPackage(pass, sel.X) { pass.ReportRangef(call, - "http.DefaultClient.Do uses a timeout-less client; use a dedicated *http.Client with Timeout set", + "http.%s does not accept a context; use http.NewRequestWithContext + http.DefaultClient.Do to propagate cancellation", + sel.Sel.Name, ) + return } } - return nil, nil + if sel.Sel.Name == "NewRequest" && isHTTPPackage(pass, sel.X) && hasContextInEnclosingFunc(pass, cursor) { + pass.ReportRangef(call, + "http.NewRequest does not propagate context; use http.NewRequestWithContext when context.Context is in scope", + ) + return + } + + if sel.Sel.Name == "Do" && isHTTPDefaultClient(pass, sel.X) { + pass.ReportRangef(call, + "http.DefaultClient.Do uses a timeout-less client; use a dedicated *http.Client with Timeout set", + ) + } } // isHTTPClientReceiver reports whether expr has type *http.Client. diff --git a/pkg/linters/ioutildeprecated/ioutildeprecated.go b/pkg/linters/ioutildeprecated/ioutildeprecated.go index d9a48ab7d75..6a3061d7a21 100644 --- a/pkg/linters/ioutildeprecated/ioutildeprecated.go +++ b/pkg/linters/ioutildeprecated/ioutildeprecated.go @@ -9,6 +9,7 @@ import ( "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" "github.com/github/gh-aw/pkg/linters/internal/astutil" "github.com/github/gh-aw/pkg/linters/internal/filecheck" @@ -54,7 +55,16 @@ func run(pass *analysis.Pass) (any, error) { return nil, nil } - // Handle regular qualified imports: ioutil.ReadAll(...), ioutil.Discard, etc. + checkQualifiedIoutilUses(pass, root, noLintIndex, generatedFiles) + checkDotImportIoutilUses(pass, root, noLintIndex, generatedFiles) + + return nil, nil +} + +// checkQualifiedIoutilUses flags ioutil.Xxx(...) and ioutil.Discard-style +// qualified references, i.e. SelectorExpr nodes where the object's package +// is "io/ioutil". +func checkQualifiedIoutilUses(pass *analysis.Pass, root inspector.Cursor, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { for cur := range root.Preorder((*ast.SelectorExpr)(nil)) { sel, ok := cur.Node().(*ast.SelectorExpr) if !ok { @@ -87,10 +97,11 @@ func run(pass *analysis.Pass) (any, error) { pass.ReportRangef(sel, "ioutil.%s is deprecated; use %s instead", funcName, replacement) } } +} - // Handle dot imports: import . "io/ioutil" followed by bare ReadAll(r) or Discard. - // In this case the identifier is an *ast.Ident (not a SelectorExpr), and - // TypesInfo.Uses resolves it to an object whose package path is "io/ioutil". +// checkDotImportIoutilUses flags bare identifier references that resolve to +// functions/values from "io/ioutil" introduced via a dot-import. +func checkDotImportIoutilUses(pass *analysis.Pass, root inspector.Cursor, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { for cur := range root.Preorder((*ast.Ident)(nil)) { ident, ok := cur.Node().(*ast.Ident) if !ok { @@ -125,6 +136,4 @@ func run(pass *analysis.Pass) (any, error) { pass.ReportRangef(ident, "ioutil.%s is deprecated; use %s instead", name, replacement) } } - - return nil, nil } diff --git a/pkg/linters/largefunc/largefunc.go b/pkg/linters/largefunc/largefunc.go index bff3f3c7ba7..14fe9efbea8 100644 --- a/pkg/linters/largefunc/largefunc.go +++ b/pkg/linters/largefunc/largefunc.go @@ -58,47 +58,51 @@ func run(pass *analysis.Pass) (any, error) { } insp.Preorder(nodeFilter, func(n ast.Node) { - var body *ast.BlockStmt - var name string - var reportNode ast.Node - - switch fn := n.(type) { - case *ast.FuncDecl: - body = fn.Body - name = fn.Name.Name - reportNode = fn.Name - case *ast.FuncLit: - body = fn.Body - name = "func literal" - reportNode = body - } + checkFuncSizeNode(pass, n, noLintIndex, generatedFiles) + }) - if body == nil { - return - } + return nil, nil +} - position := pass.Fset.PositionFor(reportNode.Pos(), false) - if filecheck.ShouldSkipFilename(position.Filename, generatedFiles) { - return - } +func checkFuncSizeNode(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + var body *ast.BlockStmt + var name string + var reportNode ast.Node + + switch fn := n.(type) { + case *ast.FuncDecl: + body = fn.Body + name = fn.Name.Name + reportNode = fn.Name + case *ast.FuncLit: + body = fn.Body + name = "func literal" + reportNode = body + } - start := pass.Fset.Position(body.Lbrace) - end := pass.Fset.Position(body.Rbrace) - // Subtract 1 to exclude the closing brace line itself, counting only body lines. - lines := end.Line - start.Line - 1 - - if lines > maxLines { - if nolint.HasDirectiveForLinter(position, noLintIndex, "largefunc") { - return - } - pkgLog.Printf("flagging %s: %d lines exceeds limit %d", name, lines, maxLines) - pass.ReportRangef( - reportNode, - "%s is %d lines long (limit: %d); consider breaking it up", - name, lines, maxLines, - ) - } - }) + if body == nil { + return + } - return nil, nil + position := pass.Fset.PositionFor(reportNode.Pos(), false) + if filecheck.ShouldSkipFilename(position.Filename, generatedFiles) { + return + } + + start := pass.Fset.Position(body.Lbrace) + end := pass.Fset.Position(body.Rbrace) + // Subtract 1 to exclude the closing brace line itself, counting only body lines. + lines := end.Line - start.Line - 1 + + if lines > maxLines { + if nolint.HasDirectiveForLinter(position, noLintIndex, "largefunc") { + return + } + pkgLog.Printf("flagging %s: %d lines exceeds limit %d", name, lines, maxLines) + pass.ReportRangef( + reportNode, + "%s is %d lines long (limit: %d); consider breaking it up", + name, lines, maxLines, + ) + } } diff --git a/pkg/linters/lenstringzero/lenstringzero.go b/pkg/linters/lenstringzero/lenstringzero.go index 16ff2e04c75..d42580fcab1 100644 --- a/pkg/linters/lenstringzero/lenstringzero.go +++ b/pkg/linters/lenstringzero/lenstringzero.go @@ -45,56 +45,60 @@ func run(pass *analysis.Pass) (any, error) { nodeFilter := []ast.Node{(*ast.BinaryExpr)(nil)} insp.Preorder(nodeFilter, func(n ast.Node) { - expr, ok := n.(*ast.BinaryExpr) - if !ok { - return - } - switch expr.Op { - case token.EQL, token.NEQ, token.GTR, token.GEQ, token.LSS, token.LEQ: - default: - return - } + checkLenStringZeroNode(pass, n, noLintIndex, generatedFiles, lenStringAliases) + }) - pos := pass.Fset.PositionFor(expr.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "lenstringzero") { - return - } + return nil, nil +} - lenArg, isDirect, normalOp, lit, matched := matchLenLiteralExpr(pass, expr, lenStringAliases) - if !matched { - return - } +func checkLenStringZeroNode(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex, lenStringAliases map[types.Object]ast.Expr) { + expr, ok := n.(*ast.BinaryExpr) + if !ok { + return + } + switch expr.Op { + case token.EQL, token.NEQ, token.GTR, token.GEQ, token.LSS, token.LEQ: + default: + return + } - fixOp, cmpVerb, valid := resolveFixOp(normalOp, lit) - if !valid { - return - } + pos := pass.Fset.PositionFor(expr.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "lenstringzero") { + return + } - t := pass.TypesInfo.TypeOf(lenArg) - if t == nil { - return - } - basic, ok := t.Underlying().(*types.Basic) - if !ok || basic.Kind() != types.String { - return - } + lenArg, isDirect, normalOp, lit, matched := matchLenLiteralExpr(pass, expr, lenStringAliases) + if !matched { + return + } - var fixes []analysis.SuggestedFix - if isDirect { - fixes = buildLenStringFix(pass, expr, lenArg, fixOp) - } - pass.Report(analysis.Diagnostic{ - Pos: expr.Pos(), - End: expr.End(), - Message: fmt.Sprintf(`use s %s "" to check for %s string instead of len(s) %s %d`, fixOp, cmpVerb, normalOp, lit), - SuggestedFixes: fixes, - }) - }) + fixOp, cmpVerb, valid := resolveFixOp(normalOp, lit) + if !valid { + return + } - return nil, nil + t := pass.TypesInfo.TypeOf(lenArg) + if t == nil { + return + } + basic, ok := t.Underlying().(*types.Basic) + if !ok || basic.Kind() != types.String { + return + } + + var fixes []analysis.SuggestedFix + if isDirect { + fixes = buildLenStringFix(pass, expr, lenArg, fixOp) + } + pass.Report(analysis.Diagnostic{ + Pos: expr.Pos(), + End: expr.End(), + Message: fmt.Sprintf(`use s %s "" to check for %s string instead of len(s) %s %d`, fixOp, cmpVerb, normalOp, lit), + SuggestedFixes: fixes, + }) } // matchLenLiteralExpr tries to match len(s)/alias OP literal or literal OP len(s)/alias. diff --git a/pkg/linters/mapclearloop/mapclearloop.go b/pkg/linters/mapclearloop/mapclearloop.go index 6771d4b4afc..6d1cec3fc8a 100644 --- a/pkg/linters/mapclearloop/mapclearloop.go +++ b/pkg/linters/mapclearloop/mapclearloop.go @@ -42,115 +42,135 @@ func run(pass *analysis.Pass) (any, error) { nodeFilter := []ast.Node{(*ast.RangeStmt)(nil)} insp.Preorder(nodeFilter, func(n ast.Node) { - rangeStmt, ok := n.(*ast.RangeStmt) - if !ok { - return - } + checkMapClearLoopNode(pass, n, noLintIndex, generatedFiles) + }) - pos := pass.Fset.PositionFor(rangeStmt.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "mapclearloop") { - return - } + return nil, nil +} - // The range expression must be a map type. - mapType := pass.TypesInfo.TypeOf(rangeStmt.X) - if mapType == nil { - return - } - if _, ok := mapType.Underlying().(*types.Map); !ok { - return - } +// checkMapClearLoopNode inspects a single RangeStmt and reports if it can be +// replaced by a clear(m) call. +func checkMapClearLoopNode(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + rangeStmt, ok := n.(*ast.RangeStmt) + if !ok { + return + } - // The key variable must be present (not blank or absent). - keyIdent, ok := rangeStmt.Key.(*ast.Ident) - if !ok || keyIdent.Name == "_" { - return - } - keyObj := pass.TypesInfo.Defs[keyIdent] - if keyObj == nil { - keyObj = pass.TypesInfo.Uses[keyIdent] - } - if keyObj == nil { - return - } + pos := pass.Fset.PositionFor(rangeStmt.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "mapclearloop") { + return + } - // The value variable must be absent or blank. - if rangeStmt.Value != nil { - valueIdent, ok := rangeStmt.Value.(*ast.Ident) - if !ok || valueIdent.Name != "_" { - return - } - } + mText, ok := validateMapClearPattern(pass, rangeStmt) + if !ok { + return + } - // The body must contain exactly one statement: delete(m, k). - if len(rangeStmt.Body.List) != 1 { - return - } - exprStmt, ok := rangeStmt.Body.List[0].(*ast.ExprStmt) - if !ok { - return - } - callExpr, ok := exprStmt.X.(*ast.CallExpr) - if !ok { - return - } - delIdent, ok := callExpr.Fun.(*ast.Ident) - if !ok || delIdent.Name != "delete" { - return - } - delBuiltin, ok := pass.TypesInfo.Uses[delIdent].(*types.Builtin) - if !ok || delBuiltin.Name() != "delete" { - return - } - if len(callExpr.Args) != 2 { - return - } + diag := analysis.Diagnostic{ + Pos: rangeStmt.Pos(), + End: rangeStmt.End(), + Message: "range-delete loop over map can be replaced with clear(" + mText + ")", + } + if !hasOverlappingComment(pass.Files, rangeStmt.Pos(), rangeStmt.End()) { + diag.SuggestedFixes = []analysis.SuggestedFix{{ + Message: "Replace range-delete loop with clear", + TextEdits: []analysis.TextEdit{{ + Pos: rangeStmt.Pos(), + End: rangeStmt.End(), + NewText: []byte("clear(" + mText + ")"), + }}, + }} + } + pass.Report(diag) +} - // First arg to delete must be the same map as the range expression. - if !sameObject(pass, callExpr.Args[0], rangeStmt.X) { - return - } +// validateMapClearPattern reports whether rangeStmt is a range-over-map loop +// whose only body statement is delete(m, k), where k is the range key variable +// and m is the range expression. Returns the text of the map expression and +// whether the pattern matched. +func validateMapClearPattern(pass *analysis.Pass, rangeStmt *ast.RangeStmt) (mText string, ok bool) { + // The range expression must be a map type. + mapType := pass.TypesInfo.TypeOf(rangeStmt.X) + if mapType == nil { + return "", false + } + if _, ok := mapType.Underlying().(*types.Map); !ok { + return "", false + } - // Second arg to delete must be the key variable from the range. - delKeyIdent, ok := callExpr.Args[1].(*ast.Ident) - if !ok { - return - } - delKeyObj := pass.TypesInfo.Uses[delKeyIdent] - if delKeyObj == nil || delKeyObj != keyObj { - return - } + // The key variable must be present (not blank or absent). + keyIdent, ok := rangeStmt.Key.(*ast.Ident) + if !ok || keyIdent.Name == "_" { + return "", false + } + keyObj := pass.TypesInfo.Defs[keyIdent] + if keyObj == nil { + keyObj = pass.TypesInfo.Uses[keyIdent] + } + if keyObj == nil { + return "", false + } - mText := astutil.NodeText(pass.Fset, rangeStmt.X) - if mText == "" { - return - } - if !builtinVisibleAtPos(pass.Pkg, rangeStmt.Pos(), "clear") { - return + // The value variable must be absent or blank. + if rangeStmt.Value != nil { + valueIdent, ok := rangeStmt.Value.(*ast.Ident) + if !ok || valueIdent.Name != "_" { + return "", false } + } - diag := analysis.Diagnostic{ - Pos: rangeStmt.Pos(), - End: rangeStmt.End(), - Message: "range-delete loop over map can be replaced with clear(" + mText + ")", - } - if !hasOverlappingComment(pass.Files, rangeStmt.Pos(), rangeStmt.End()) { - diag.SuggestedFixes = []analysis.SuggestedFix{{ - Message: "Replace range-delete loop with clear", - TextEdits: []analysis.TextEdit{{ - Pos: rangeStmt.Pos(), - End: rangeStmt.End(), - NewText: []byte("clear(" + mText + ")"), - }}, - }} - } - pass.Report(diag) - }) + // The body must be exactly delete(m, k) with the range key. + if !validateDeleteBody(pass, rangeStmt, keyObj) { + return "", false + } - return nil, nil + mText = astutil.NodeText(pass.Fset, rangeStmt.X) + if mText == "" { + return "", false + } + if !builtinVisibleAtPos(pass.Pkg, rangeStmt.Pos(), "clear") { + return "", false + } + return mText, true +} + +// validateDeleteBody reports whether rangeStmt's body is exactly delete(m, k) +// where m is the range expression and k is keyObj. +func validateDeleteBody(pass *analysis.Pass, rangeStmt *ast.RangeStmt, keyObj types.Object) bool { + if len(rangeStmt.Body.List) != 1 { + return false + } + exprStmt, ok := rangeStmt.Body.List[0].(*ast.ExprStmt) + if !ok { + return false + } + callExpr, ok := exprStmt.X.(*ast.CallExpr) + if !ok { + return false + } + delIdent, ok := callExpr.Fun.(*ast.Ident) + if !ok || delIdent.Name != "delete" { + return false + } + delBuiltin, ok := pass.TypesInfo.Uses[delIdent].(*types.Builtin) + if !ok || delBuiltin.Name() != "delete" { + return false + } + if len(callExpr.Args) != 2 { + return false + } + if !sameObject(pass, callExpr.Args[0], rangeStmt.X) { + return false + } + delKeyIdent, ok := callExpr.Args[1].(*ast.Ident) + if !ok { + return false + } + delKeyObj := pass.TypesInfo.Uses[delKeyIdent] + return delKeyObj != nil && delKeyObj == keyObj } // builtinVisibleAtPos reports whether name resolves to a builtin object at pos. diff --git a/pkg/linters/mapdeletecheck/mapdeletecheck.go b/pkg/linters/mapdeletecheck/mapdeletecheck.go index f4a0994eee4..0fe2c5bbbbd 100644 --- a/pkg/linters/mapdeletecheck/mapdeletecheck.go +++ b/pkg/linters/mapdeletecheck/mapdeletecheck.go @@ -42,96 +42,99 @@ func run(pass *analysis.Pass) (any, error) { nodeFilter := []ast.Node{(*ast.IfStmt)(nil)} insp.Preorder(nodeFilter, func(n ast.Node) { - ifStmt, ok := n.(*ast.IfStmt) - if !ok { - return - } + checkMapDeleteCheckNode(pass, n, noLintIndex, generatedFiles) + }) - pos := pass.Fset.PositionFor(ifStmt.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "mapdeletecheck") { - return - } + return nil, nil +} - // Must have an init statement: _, ok := m[k] - // Must have a simple condition: ok (or the bool variable from the assignment) - // Must have a single-statement body: delete(m, k) - // Must have no else clause. +// checkMapDeleteCheckNode inspects a single IfStmt and reports if it is a +// redundant membership check before delete(m, k). +func checkMapDeleteCheckNode(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + ifStmt, ok := n.(*ast.IfStmt) + if !ok { + return + } - if ifStmt.Else != nil { - return - } + pos := pass.Fset.PositionFor(ifStmt.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "mapdeletecheck") { + return + } - mapExpr, keyExpr, okIdent := matchMapIndexAssign(pass, ifStmt.Init) - if okIdent == nil { - return - } + if ifStmt.Else != nil { + return + } - // The condition must be the ok identifier from the init. - condIdent, ok := ifStmt.Cond.(*ast.Ident) - if !ok { - return - } - // Make sure the condition refers to the same object as the init. - condObj := pass.TypesInfo.Uses[condIdent] - okObj := pass.TypesInfo.Defs[okIdent] - if condObj == nil || okObj == nil || condObj != okObj { - return - } + mapExpr, keyExpr, okIdent := matchMapIndexAssign(pass, ifStmt.Init) + if okIdent == nil { + return + } - // The body must be exactly one statement: delete(m, k) - if len(ifStmt.Body.List) != 1 { - return - } - exprStmt, ok := ifStmt.Body.List[0].(*ast.ExprStmt) - if !ok { - return - } - delCall, ok := exprStmt.X.(*ast.CallExpr) - if !ok { - return - } - delIdent, ok := delCall.Fun.(*ast.Ident) - if !ok || delIdent.Name != "delete" { - return - } - delBuiltin, ok := pass.TypesInfo.Uses[delIdent].(*types.Builtin) - if !ok || delBuiltin.Name() != "delete" { - return - } - if len(delCall.Args) != 2 { - return - } + // The condition must be the ok identifier from the init. + condIdent, ok := ifStmt.Cond.(*ast.Ident) + if !ok { + return + } + condObj := pass.TypesInfo.Uses[condIdent] + okObj := pass.TypesInfo.Defs[okIdent] + if condObj == nil || okObj == nil || condObj != okObj { + return + } - // delete(m, k) must use the same map and key as the index expression. - if !sameExpr(pass, delCall.Args[0], mapExpr) { - return - } - if !sameExpr(pass, delCall.Args[1], keyExpr) { - return - } + if !matchDeleteCallInIfBody(pass, ifStmt, mapExpr, keyExpr) { + return + } - mText := astutil.NodeText(pass.Fset, mapExpr) - kText := astutil.NodeText(pass.Fset, keyExpr) - - pass.Report(analysis.Diagnostic{ - Pos: ifStmt.Pos(), - End: ifStmt.End(), - Message: "redundant existence check before delete: delete(" + mText + ", " + kText + ") is already a no-op when the key is absent; remove the if statement", - SuggestedFixes: []analysis.SuggestedFix{{ - Message: "Replace if-check with plain delete", - TextEdits: []analysis.TextEdit{{ - Pos: ifStmt.Pos(), - End: ifStmt.End(), - NewText: []byte("delete(" + mText + ", " + kText + ")"), - }}, + mText := astutil.NodeText(pass.Fset, mapExpr) + kText := astutil.NodeText(pass.Fset, keyExpr) + + pass.Report(analysis.Diagnostic{ + Pos: ifStmt.Pos(), + End: ifStmt.End(), + Message: "redundant existence check before delete: delete(" + mText + ", " + kText + ") is already a no-op when the key is absent; remove the if statement", + SuggestedFixes: []analysis.SuggestedFix{{ + Message: "Replace if-check with plain delete", + TextEdits: []analysis.TextEdit{{ + Pos: ifStmt.Pos(), + End: ifStmt.End(), + NewText: []byte("delete(" + mText + ", " + kText + ")"), }}, - }) + }}, }) +} - return nil, nil +// matchDeleteCallInIfBody reports whether ifStmt's body is exactly delete(m, k) +// where m matches mapExpr and k matches keyExpr. +func matchDeleteCallInIfBody(pass *analysis.Pass, ifStmt *ast.IfStmt, mapExpr, keyExpr ast.Expr) bool { + if len(ifStmt.Body.List) != 1 { + return false + } + exprStmt, ok := ifStmt.Body.List[0].(*ast.ExprStmt) + if !ok { + return false + } + delCall, ok := exprStmt.X.(*ast.CallExpr) + if !ok { + return false + } + delIdent, ok := delCall.Fun.(*ast.Ident) + if !ok || delIdent.Name != "delete" { + return false + } + delBuiltin, ok := pass.TypesInfo.Uses[delIdent].(*types.Builtin) + if !ok || delBuiltin.Name() != "delete" { + return false + } + if len(delCall.Args) != 2 { + return false + } + if !sameExpr(pass, delCall.Args[0], mapExpr) { + return false + } + return sameExpr(pass, delCall.Args[1], keyExpr) } // matchMapIndexAssign returns map expression, key expression, and ok identifier diff --git a/pkg/linters/nilctxpassed/nilctxpassed.go b/pkg/linters/nilctxpassed/nilctxpassed.go index 2f15a6330f5..097e6bc7b26 100644 --- a/pkg/linters/nilctxpassed/nilctxpassed.go +++ b/pkg/linters/nilctxpassed/nilctxpassed.go @@ -42,60 +42,63 @@ func run(pass *analysis.Pass) (any, error) { if !ok { continue } + checkNilCtxPassedCall(pass, call, noLintIndex, generatedFiles) + } - callPos := pass.Fset.PositionFor(call.Pos(), false) - if filecheck.ShouldSkipFilename(callPos.Filename, generatedFiles) { - continue - } + return nil, nil +} - sig := calleeSignature(pass, call) - if sig == nil { - continue - } +func checkNilCtxPassedCall(pass *analysis.Pass, call *ast.CallExpr, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + callPos := pass.Fset.PositionFor(call.Pos(), false) + if filecheck.ShouldSkipFilename(callPos.Filename, generatedFiles) { + return + } - params := sig.Params() - for i, arg := range call.Args { - var paramType types.Type - if sig.Variadic() && params.Len() > 0 && i >= params.Len()-1 { - if call.Ellipsis.IsValid() && i == len(call.Args)-1 { - // Spread call passes the whole variadic slice (e.g. f(nil...)), - // not an individual variadic element. - continue - } - // Variadic: the last param is a slice; check its element type. - sliceType, ok := params.At(params.Len() - 1).Type().(*types.Slice) - if !ok { - continue - } - paramType = sliceType.Elem() - } else if i < params.Len() { - paramType = params.At(i).Type() - } else { - continue - } + sig := calleeSignature(pass, call) + if sig == nil { + return + } - if !isContextContext(paramType) { + params := sig.Params() + for i, arg := range call.Args { + var paramType types.Type + if sig.Variadic() && params.Len() > 0 && i >= params.Len()-1 { + if call.Ellipsis.IsValid() && i == len(call.Args)-1 { + // Spread call passes the whole variadic slice (e.g. f(nil...)), + // not an individual variadic element. continue } - - if !isBuiltinNil(pass, arg) { + // Variadic: the last param is a slice; check its element type. + sliceType, ok := params.At(params.Len() - 1).Type().(*types.Slice) + if !ok { continue } + paramType = sliceType.Elem() + } else if i < params.Len() { + paramType = params.At(i).Type() + } else { + continue + } - argPos := pass.Fset.PositionFor(arg.Pos(), false) - if nolint.HasDirectiveForLinter(argPos, noLintIndex, "nilctxpassed") { - continue - } + if !isContextContext(paramType) { + continue + } + + if !isBuiltinNil(pass, arg) { + continue + } - pass.Report(analysis.Diagnostic{ - Pos: arg.Pos(), - End: arg.End(), - Message: "nil passed as context.Context; use context.Background() or context.TODO() instead", - }) + argPos := pass.Fset.PositionFor(arg.Pos(), false) + if nolint.HasDirectiveForLinter(argPos, noLintIndex, "nilctxpassed") { + continue } - } - return nil, nil + pass.Report(analysis.Diagnostic{ + Pos: arg.Pos(), + End: arg.End(), + Message: "nil passed as context.Context; use context.Background() or context.TODO() instead", + }) + } } // isContextContext reports whether t is the context.Context interface type, diff --git a/pkg/linters/seenmapbool/seenmapbool.go b/pkg/linters/seenmapbool/seenmapbool.go index a2e41e6413a..94907b515b2 100644 --- a/pkg/linters/seenmapbool/seenmapbool.go +++ b/pkg/linters/seenmapbool/seenmapbool.go @@ -76,6 +76,37 @@ func inspectBody(pass *analysis.Pass, body *ast.BlockStmt, noLintIndex nolint.Di // First pass: collect declarations of map[string]bool locals. // Stop at nested FuncLit boundaries so each closure is handled by its own // Preorder visit — preventing duplicate diagnostics. + collectMapStringBoolDecls(pass, body, candidates) + + if len(candidates) == 0 { + return + } + + // Second pass: check that every write to these maps only assigns true. + // If any non-true assignment is found, remove the map from candidates. + nonSetMaps := make(map[types.Object]bool) + collectNonSetMapAssignments(pass, body, candidates, nonSetMaps) + + // Report remaining candidates that are pure sets. + for obj, declNode := range candidates { + if nonSetMaps[obj] { + continue + } + if nolint.HasDirectiveForLinter(pass.Fset.PositionFor(declNode.Pos(), false), noLintIndex, "seenmapbool") { + continue + } + pass.ReportRangef( + declNode, + "map[string]bool %q used as a set; use map[string]struct{} to avoid allocating a bool per entry", + obj.Name(), + ) + } +} + +// collectMapStringBoolDecls inspects body for map[string]bool local variable +// declarations and adds them to candidates. It stops at nested FuncLit +// boundaries to avoid duplicate diagnostics from inner closures. +func collectMapStringBoolDecls(pass *analysis.Pass, body *ast.BlockStmt, candidates map[types.Object]ast.Node) { ast.Inspect(body, func(n ast.Node) bool { if n == nil { return false @@ -132,15 +163,11 @@ func inspectBody(pass *analysis.Pass, body *ast.BlockStmt, noLintIndex nolint.Di } return true }) +} - if len(candidates) == 0 { - return - } - - // Second pass: check that every write to these maps only assigns true. - // If any non-true assignment is found, remove the map from candidates. - nonSetMaps := make(map[types.Object]bool) - +// collectNonSetMapAssignments walks body and marks any candidate map that +// receives an assignment of a value other than the literal true. +func collectNonSetMapAssignments(pass *analysis.Pass, body *ast.BlockStmt, candidates map[types.Object]ast.Node, nonSetMaps map[types.Object]bool) { ast.Inspect(body, func(n ast.Node) bool { assign, ok := n.(*ast.AssignStmt) if !ok { @@ -171,21 +198,6 @@ func inspectBody(pass *analysis.Pass, body *ast.BlockStmt, noLintIndex nolint.Di } return true }) - - // Report remaining candidates that are pure sets. - for obj, declNode := range candidates { - if nonSetMaps[obj] { - continue - } - if nolint.HasDirectiveForLinter(pass.Fset.PositionFor(declNode.Pos(), false), noLintIndex, "seenmapbool") { - continue - } - pass.ReportRangef( - declNode, - "map[string]bool %q used as a set; use map[string]struct{} to avoid allocating a bool per entry", - obj.Name(), - ) - } } // isMapStringBool returns true if t is map[string]bool. diff --git a/pkg/linters/sprintferrdot/sprintferrdot.go b/pkg/linters/sprintferrdot/sprintferrdot.go index 92fa8be5f57..b814962f2a8 100644 --- a/pkg/linters/sprintferrdot/sprintferrdot.go +++ b/pkg/linters/sprintferrdot/sprintferrdot.go @@ -62,53 +62,59 @@ func run(pass *analysis.Pass) (any, error) { } insp.Preorder(nodeFilter, func(n ast.Node) { - call, ok := n.(*ast.CallExpr) - if !ok { - return - } + checkSprintfErrDotCall(pass, n, noLintIndex, generatedFiles) + }) - pos := pass.Fset.PositionFor(call.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return - } + return nil, nil +} - formatArgIdx, variadicStart, ok := fmtFormatCallInfo(pass, call) - if !ok { - return - } - if formatArgIdx >= len(call.Args) || variadicStart > len(call.Args) { - return - } +// checkSprintfErrDotCall inspects a single CallExpr and reports if it uses .Error() +// redundantly on an error argument passed to an fmt format function. +func checkSprintfErrDotCall(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + call, ok := n.(*ast.CallExpr) + if !ok { + return + } - formatStr, ok := extractStringLit(call.Args[formatArgIdx]) - if !ok { - return - } + pos := pass.Fset.PositionFor(call.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } - verbs := parseSimpleFormatVerbs(formatStr) - if verbs == nil { - return - } + formatArgIdx, variadicStart, ok := fmtFormatCallInfo(pass, call) + if !ok { + return + } + if formatArgIdx >= len(call.Args) || variadicStart > len(call.Args) { + return + } - variadicArgs := call.Args[variadicStart:] - for i, arg := range variadicArgs { - if i >= len(verbs) { - break - } - if verbs[i] != 's' && verbs[i] != 'v' { + formatStr, ok := extractStringLit(call.Args[formatArgIdx]) + if !ok { + return + } + + verbs := parseSimpleFormatVerbs(formatStr) + if verbs == nil { + return + } + + variadicArgs := call.Args[variadicStart:] + for i, arg := range variadicArgs { + if i >= len(verbs) { + break + } + if verbs[i] != 's' && verbs[i] != 'v' { + continue + } + if isErrorDotCall(pass, arg) { + if nolint.HasDirectiveForLinter(pass.Fset.PositionFor(arg.Pos(), false), noLintIndex, "sprintferrdot") { continue } - if isErrorDotCall(pass, arg) { - if nolint.HasDirectiveForLinter(pass.Fset.PositionFor(arg.Pos(), false), noLintIndex, "sprintferrdot") { - continue - } - pass.Reportf(arg.Pos(), - "redundant .Error() call: pass the error value directly with %%%c", verbs[i]) - } + pass.Reportf(arg.Pos(), + "redundant .Error() call: pass the error value directly with %%%c", verbs[i]) } - }) - - return nil, nil + } } // fmtFormatCallInfo returns the format-string argument index and the diff --git a/pkg/linters/sprintfint/sprintfint.go b/pkg/linters/sprintfint/sprintfint.go index 99b53ce0f79..9e985393071 100644 --- a/pkg/linters/sprintfint/sprintfint.go +++ b/pkg/linters/sprintfint/sprintfint.go @@ -54,56 +54,62 @@ func run(pass *analysis.Pass) (any, error) { } insp.Preorder(nodeFilter, func(n ast.Node) { - call, ok := n.(*ast.CallExpr) - if !ok { - return - } + checkSprintfIntCall(pass, n, noLintIndex, generatedFiles, seenImportFiles) + }) - pos := pass.Fset.PositionFor(call.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "sprintfint") { - return - } + return nil, nil +} - // Match fmt.Sprintf(format, arg) with exactly two arguments. - sel, ok := call.Fun.(*ast.SelectorExpr) - if !ok || sel.Sel.Name != "Sprintf" { - return - } - if !astutil.IsPkgSelector(pass, sel, "fmt") { - return - } - if len(call.Args) != 2 { - return - } +// checkSprintfIntCall inspects a single CallExpr and reports if it is +// fmt.Sprintf("%d", x) where x is an int, suggesting strconv.Itoa(x) instead. +func checkSprintfIntCall(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex, seenImportFiles map[token.Pos]bool) { + call, ok := n.(*ast.CallExpr) + if !ok { + return + } - // The format argument must be the string literal "%d". - formatLit, ok := call.Args[0].(*ast.BasicLit) - if !ok || formatLit.Kind != token.STRING || formatLit.Value != `"%d"` { - return - } + pos := pass.Fset.PositionFor(call.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "sprintfint") { + return + } - // The value argument must have the exact type int (not int64, uint, etc.). - arg := call.Args[1] - argType := pass.TypesInfo.TypeOf(arg) - if argType == nil { - return - } - if argType != types.Typ[types.Int] { - return - } + // Match fmt.Sprintf(format, arg) with exactly two arguments. + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Sprintf" { + return + } + if !astutil.IsPkgSelector(pass, sel, "fmt") { + return + } + if len(call.Args) != 2 { + return + } - pass.Report(analysis.Diagnostic{ - Pos: call.Pos(), - End: call.End(), - Message: `use strconv.Itoa(x) instead of fmt.Sprintf("%d", x)`, - SuggestedFixes: buildItoaFix(pass, call, arg, seenImportFiles), - }) - }) + // The format argument must be the string literal "%d". + formatLit, ok := call.Args[0].(*ast.BasicLit) + if !ok || formatLit.Kind != token.STRING || formatLit.Value != `"%d"` { + return + } - return nil, nil + // The value argument must have the exact type int (not int64, uint, etc.). + arg := call.Args[1] + argType := pass.TypesInfo.TypeOf(arg) + if argType == nil { + return + } + if argType != types.Typ[types.Int] { + return + } + + pass.Report(analysis.Diagnostic{ + Pos: call.Pos(), + End: call.End(), + Message: `use strconv.Itoa(x) instead of fmt.Sprintf("%d", x)`, + SuggestedFixes: buildItoaFix(pass, call, arg, seenImportFiles), + }) } // buildItoaFix returns a SuggestedFix rewriting fmt.Sprintf("%d", x) → diff --git a/pkg/linters/strconvparseignorederror/strconvparseignorederror.go b/pkg/linters/strconvparseignorederror/strconvparseignorederror.go index 782091cb404..f7f74fd4306 100644 --- a/pkg/linters/strconvparseignorederror/strconvparseignorederror.go +++ b/pkg/linters/strconvparseignorederror/strconvparseignorederror.go @@ -52,48 +52,54 @@ func run(pass *analysis.Pass) (any, error) { } insp.Preorder(nodeFilter, func(n ast.Node) { - assign, ok := n.(*ast.AssignStmt) - if !ok { - return - } - // We need exactly 2 LHS targets where the second is _ - if len(assign.Lhs) != 2 || len(assign.Rhs) != 1 { - return - } - // Check second LHS is blank identifier - blank, ok := assign.Lhs[1].(*ast.Ident) - if !ok || blank.Name != "_" { - return - } - // Check RHS is a call to strconv.ParseXxx or strconv.Atoi - call, ok := assign.Rhs[0].(*ast.CallExpr) - if !ok { - return - } - sel, ok := call.Fun.(*ast.SelectorExpr) - if !ok { - return - } - if !strconvParseFuncs[sel.Sel.Name] { - return - } - // Verify the receiver is the strconv package - if ident, ok := sel.X.(*ast.Ident); ok { - obj := pass.TypesInfo.Uses[ident] - if pkgName, ok := obj.(*types.PkgName); ok { - if pkgName.Imported().Path() == "strconv" { - position := pass.Fset.PositionFor(call.Pos(), false) - if filecheck.ShouldSkipFilename(position.Filename, generatedFiles) { - return - } - if nolint.HasDirectiveForLinter(position, nolintIndex, "strconvparseignorederror") { - return - } - pass.ReportRangef(call, "error return from strconv.%s is discarded; parse failures produce zero values silently", sel.Sel.Name) - } - } - } + checkStrconvParseNode(pass, n, nolintIndex, generatedFiles) }) return nil, nil } + +// checkStrconvParseNode inspects a single AssignStmt and reports if it discards +// the error return value from a strconv parsing function. +func checkStrconvParseNode(pass *analysis.Pass, n ast.Node, nolintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + assign, ok := n.(*ast.AssignStmt) + if !ok { + return + } + // We need exactly 2 LHS targets where the second is _ + if len(assign.Lhs) != 2 || len(assign.Rhs) != 1 { + return + } + // Check second LHS is blank identifier + blank, ok := assign.Lhs[1].(*ast.Ident) + if !ok || blank.Name != "_" { + return + } + // Check RHS is a call to strconv.ParseXxx or strconv.Atoi + call, ok := assign.Rhs[0].(*ast.CallExpr) + if !ok { + return + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return + } + if !strconvParseFuncs[sel.Sel.Name] { + return + } + // Verify the receiver is the strconv package + if ident, ok := sel.X.(*ast.Ident); ok { + obj := pass.TypesInfo.Uses[ident] + if pkgName, ok := obj.(*types.PkgName); ok { + if pkgName.Imported().Path() == "strconv" { + position := pass.Fset.PositionFor(call.Pos(), false) + if filecheck.ShouldSkipFilename(position.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(position, nolintIndex, "strconvparseignorederror") { + return + } + pass.ReportRangef(call, "error return from strconv.%s is discarded; parse failures produce zero values silently", sel.Sel.Name) + } + } + } +} diff --git a/pkg/linters/stringscountcontains/stringscountcontains.go b/pkg/linters/stringscountcontains/stringscountcontains.go index 48011e733bd..fd4199cd2c3 100644 --- a/pkg/linters/stringscountcontains/stringscountcontains.go +++ b/pkg/linters/stringscountcontains/stringscountcontains.go @@ -43,51 +43,57 @@ func run(pass *analysis.Pass) (any, error) { nodeFilter := []ast.Node{(*ast.BinaryExpr)(nil)} insp.Preorder(nodeFilter, func(n ast.Node) { - expr, ok := n.(*ast.BinaryExpr) - if !ok { - return - } + checkStringsCountNode(pass, n, noLintIndex, generatedFiles) + }) - pos := pass.Fset.PositionFor(expr.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "stringscountcontains") { - return - } + return nil, nil +} - countCall, negated, matched := matchCountComparison(pass, expr) - if !matched { - return - } +// checkStringsCountNode inspects a single BinaryExpr and reports if it is a +// strings.Count comparison with 0 or 1 that can use strings.Contains instead. +func checkStringsCountNode(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + expr, ok := n.(*ast.BinaryExpr) + if !ok { + return + } - if len(countCall.Args) != 2 { - return - } + pos := pass.Fset.PositionFor(expr.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "stringscountcontains") { + return + } - sText := astutil.NodeText(pass.Fset, countCall.Args[0]) - subText := astutil.NodeText(pass.Fset, countCall.Args[1]) - pkgText := astutil.CallQualifierText(pass.Fset, countCall) - if sText == "" || subText == "" || pkgText == "" { - return - } + countCall, negated, matched := matchCountComparison(pass, expr) + if !matched { + return + } - var msg string - if negated { - msg = fmt.Sprintf("use !strings.Contains(%s, %s) instead of strings.Count comparison", sText, subText) - } else { - msg = fmt.Sprintf("use strings.Contains(%s, %s) instead of strings.Count comparison", sText, subText) - } + if len(countCall.Args) != 2 { + return + } - pass.Report(analysis.Diagnostic{ - Pos: expr.Pos(), - End: expr.End(), - Message: msg, - SuggestedFixes: astutil.BuildContainsFix(expr, pkgText, sText, subText, negated, "Replace strings.Count comparison with strings.Contains"), - }) - }) + sText := astutil.NodeText(pass.Fset, countCall.Args[0]) + subText := astutil.NodeText(pass.Fset, countCall.Args[1]) + pkgText := astutil.CallQualifierText(pass.Fset, countCall) + if sText == "" || subText == "" || pkgText == "" { + return + } - return nil, nil + var msg string + if negated { + msg = fmt.Sprintf("use !strings.Contains(%s, %s) instead of strings.Count comparison", sText, subText) + } else { + msg = fmt.Sprintf("use strings.Contains(%s, %s) instead of strings.Count comparison", sText, subText) + } + + pass.Report(analysis.Diagnostic{ + Pos: expr.Pos(), + End: expr.End(), + Message: msg, + SuggestedFixes: astutil.BuildContainsFix(expr, pkgText, sText, subText, negated, "Replace strings.Count comparison with strings.Contains"), + }) } // matchCountComparison reports whether expr is a strings.Count comparison with diff --git a/pkg/linters/stringsindexcontains/stringsindexcontains.go b/pkg/linters/stringsindexcontains/stringsindexcontains.go index c92f71e9870..c44985ed900 100644 --- a/pkg/linters/stringsindexcontains/stringsindexcontains.go +++ b/pkg/linters/stringsindexcontains/stringsindexcontains.go @@ -42,59 +42,65 @@ func run(pass *analysis.Pass) (any, error) { nodeFilter := []ast.Node{(*ast.BinaryExpr)(nil)} insp.Preorder(nodeFilter, func(n ast.Node) { - expr, ok := n.(*ast.BinaryExpr) - if !ok { - return - } + checkStringsIndexNode(pass, n, noLintIndex, generatedFiles) + }) - pos := pass.Fset.PositionFor(expr.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "stringsindexcontains") { - return - } + return nil, nil +} - // Match patterns: - // strings.Index(s, sub) != -1 → strings.Contains(s, sub) - // strings.Index(s, sub) >= 0 → strings.Contains(s, sub) - // strings.Index(s, sub) == -1 → !strings.Contains(s, sub) - // strings.Index(s, sub) < 0 → !strings.Contains(s, sub) - // (and yoda variants: -1 != strings.Index(...), etc.) +// checkStringsIndexNode inspects a single BinaryExpr and reports if it is a +// strings.Index comparison with -1 or 0 that can use strings.Contains instead. +func checkStringsIndexNode(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex) { + expr, ok := n.(*ast.BinaryExpr) + if !ok { + return + } - indexCall, negated, matched := matchIndexComparison(pass, expr) - if !matched { - return - } + pos := pass.Fset.PositionFor(expr.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "stringsindexcontains") { + return + } - if len(indexCall.Args) != 2 { - return - } + // Match patterns: + // strings.Index(s, sub) != -1 → strings.Contains(s, sub) + // strings.Index(s, sub) >= 0 → strings.Contains(s, sub) + // strings.Index(s, sub) == -1 → !strings.Contains(s, sub) + // strings.Index(s, sub) < 0 → !strings.Contains(s, sub) + // (and yoda variants: -1 != strings.Index(...), etc.) - sText := astutil.NodeText(pass.Fset, indexCall.Args[0]) - subText := astutil.NodeText(pass.Fset, indexCall.Args[1]) - pkgText := astutil.CallQualifierText(pass.Fset, indexCall) - if sText == "" || subText == "" || pkgText == "" { - return - } + indexCall, negated, matched := matchIndexComparison(pass, expr) + if !matched { + return + } - var msg string - if negated { - msg = "use !strings.Contains(" + sText + ", " + subText + ") instead of strings.Index comparison" - } else { - msg = "use strings.Contains(" + sText + ", " + subText + ") instead of strings.Index comparison" - } + if len(indexCall.Args) != 2 { + return + } - fix := astutil.BuildContainsFix(expr, pkgText, sText, subText, negated, "Replace strings.Index comparison with strings.Contains") - pass.Report(analysis.Diagnostic{ - Pos: expr.Pos(), - End: expr.End(), - Message: msg, - SuggestedFixes: fix, - }) - }) + sText := astutil.NodeText(pass.Fset, indexCall.Args[0]) + subText := astutil.NodeText(pass.Fset, indexCall.Args[1]) + pkgText := astutil.CallQualifierText(pass.Fset, indexCall) + if sText == "" || subText == "" || pkgText == "" { + return + } - return nil, nil + var msg string + if negated { + msg = "use !strings.Contains(" + sText + ", " + subText + ") instead of strings.Index comparison" + } else { + msg = "use strings.Contains(" + sText + ", " + subText + ") instead of strings.Index comparison" + } + + fix := astutil.BuildContainsFix(expr, pkgText, sText, subText, negated, "Replace strings.Index comparison with strings.Contains") + pass.Report(analysis.Diagnostic{ + Pos: expr.Pos(), + End: expr.End(), + Message: msg, + SuggestedFixes: fix, + }) } // matchIndexComparison reports whether expr is a strings.Index comparison with -1 or 0. diff --git a/pkg/linters/timeafterleak/timeafterleak.go b/pkg/linters/timeafterleak/timeafterleak.go index 33bcf1bb0de..c858ae8b7fa 100644 --- a/pkg/linters/timeafterleak/timeafterleak.go +++ b/pkg/linters/timeafterleak/timeafterleak.go @@ -148,6 +148,12 @@ func isInsideLoopSelectComm(cur inspector.Cursor) bool { // Walk up from the CommClause to find an enclosing for or range loop, // stopping at any function literal boundary. + return hasEnclosingLoop(clauseCur) +} + +// hasEnclosingLoop reports whether clauseCur has an enclosing for or range loop +// before any function literal boundary. +func hasEnclosingLoop(clauseCur inspector.Cursor) bool { for encl := range clauseCur.Enclosing( (*ast.ForStmt)(nil), (*ast.RangeStmt)(nil), diff --git a/pkg/linters/writebytestring/writebytestring.go b/pkg/linters/writebytestring/writebytestring.go index 4a47f363279..feaf13a57f5 100644 --- a/pkg/linters/writebytestring/writebytestring.go +++ b/pkg/linters/writebytestring/writebytestring.go @@ -71,79 +71,87 @@ func run(pass *analysis.Pass) (any, error) { } insp.Preorder(nodeFilter, func(n ast.Node) { - call, ok := n.(*ast.CallExpr) - if !ok { - return - } + checkWriteByteStringCall(pass, n, noLintIndex, generatedFiles, filesWithImportEdit) + }) - // Match .Write() - sel, ok := call.Fun.(*ast.SelectorExpr) - if !ok || sel.Sel.Name != "Write" { - return - } - if len(call.Args) != 1 { - return - } + return nil, nil +} - pos := pass.Fset.PositionFor(call.Pos(), false) - if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { - return - } - if nolint.HasDirectiveForLinter(pos, noLintIndex, "writebytestring") { - return - } +// checkWriteByteStringCall inspects a single CallExpr and reports if it is a +// w.Write([]byte(s)) call that can be replaced with io.WriteString(w, s). +func checkWriteByteStringCall(pass *analysis.Pass, n ast.Node, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex, filesWithImportEdit map[token.Pos]bool) { + call, ok := n.(*ast.CallExpr) + if !ok { + return + } - // The single argument must be a []byte(s) conversion where s is a string. - conv, ok := call.Args[0].(*ast.CallExpr) - if !ok { - return - } - if !astutil.IsByteSliceConversion(pass, conv) { - return - } - strArg := conv.Args[0] - if !astutil.IsStringType(pass, strArg) { - return - } + // Match .Write() + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Write" { + return + } + if len(call.Args) != 1 { + return + } - // The receiver must implement io.Writer. - if !implementsWriter(pass, sel.X) { - return - } + pos := pass.Fset.PositionFor(call.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "writebytestring") { + return + } - sText := astutil.NodeText(pass.Fset, strArg) - wText := astutil.NodeText(pass.Fset, sel.X) - if sText == "" || wText == "" { - return - } + // The single argument must be a []byte(s) conversion where s is a string. + conv, ok := call.Args[0].(*ast.CallExpr) + if !ok { + return + } + if !astutil.IsByteSliceConversion(pass, conv) { + return + } + strArg := conv.Args[0] + if !astutil.IsStringType(pass, strArg) { + return + } - // io.WriteString requires an exact predeclared string argument. If the - // argument is a named string type (e.g. type MyStr string), wrap it with - // string(...) so the emitted fix compiles. - sExpr := sText - if st := pass.TypesInfo.TypeOf(strArg); st != nil && !isExactString(st) { - sExpr = "string(" + sText + ")" - } + // The receiver must implement io.Writer. + if !implementsWriter(pass, sel.X) { + return + } - // When the receiver is an addressable value whose Write method lives on - // the pointer type (e.g. var buf bytes.Buffer), io.WriteString requires - // the pointer form so that the interface conversion compiles. - writerArg := wText - if t := pass.TypesInfo.TypeOf(sel.X); t != nil && - !types.Implements(t, writerIface) && - types.Implements(types.NewPointer(t), writerIface) { - writerArg = "&" + wText - } + sText, sExpr, wText, writerArg, argsOK := buildWriteStringArgs(pass, strArg, sel) + if !argsOK { + return + } - pass.Report(analysis.Diagnostic{ - Pos: call.Pos(), - End: call.End(), - Message: fmt.Sprintf("%s.Write([]byte(%s)) can be replaced with io.WriteString(%s, %s) to potentially avoid a []byte allocation if the writer implements io.StringWriter", wText, sText, writerArg, sExpr), - SuggestedFixes: buildFix(pass, call, writerArg, sExpr, filesWithImportEdit), - }) + pass.Report(analysis.Diagnostic{ + Pos: call.Pos(), + End: call.End(), + Message: fmt.Sprintf("%s.Write([]byte(%s)) can be replaced with io.WriteString(%s, %s) to potentially avoid a []byte allocation if the writer implements io.StringWriter", wText, sText, writerArg, sExpr), + SuggestedFixes: buildFix(pass, call, writerArg, sExpr, filesWithImportEdit), }) +} - return nil, nil +// buildWriteStringArgs computes the text representations and adjustments for +// the string and writer arguments needed to emit the io.WriteString replacement. +func buildWriteStringArgs(pass *analysis.Pass, strArg ast.Expr, sel *ast.SelectorExpr) (sText, sExpr, wText, writerArg string, ok bool) { + sText = astutil.NodeText(pass.Fset, strArg) + wText = astutil.NodeText(pass.Fset, sel.X) + if sText == "" || wText == "" { + return "", "", "", "", false + } + sExpr = sText + if st := pass.TypesInfo.TypeOf(strArg); st != nil && !isExactString(st) { + sExpr = "string(" + sText + ")" + } + writerArg = wText + if t := pass.TypesInfo.TypeOf(sel.X); t != nil && + !types.Implements(t, writerIface) && + types.Implements(types.NewPointer(t), writerIface) { + writerArg = "&" + wText + } + return sText, sExpr, wText, writerArg, true } // isExactString reports whether t is the predeclared string type, not a named @@ -242,21 +250,26 @@ func addIOImportEdit(pass *analysis.Pass, pos token.Pos, filesWithImportEdit map if file == nil { return analysis.TextEdit{}, false } - if filesWithImportEdit[file.Pos()] { return analysis.TextEdit{}, false } - markAndReturn := func(edit analysis.TextEdit) (analysis.TextEdit, bool) { - filesWithImportEdit[file.Pos()] = true - return edit, true - } + return insertIOImportEdit(pass, file, filesWithImportEdit) +} +// insertIOImportEdit inserts an "io" import into file. It assumes the caller +// has already verified that no edit has been emitted for this file in this pass. +func insertIOImportEdit(pass *analysis.Pass, file *ast.File, filesWithImportEdit map[token.Pos]bool) (analysis.TextEdit, bool) { for _, imp := range file.Imports { if imp.Path.Value == `"`+ioPkg+`"` { return analysis.TextEdit{}, false } } + markAndReturn := func(edit analysis.TextEdit) (analysis.TextEdit, bool) { + filesWithImportEdit[file.Pos()] = true + return edit, true + } + for _, decl := range file.Decls { genDecl, ok := decl.(*ast.GenDecl) if !ok || genDecl.Tok != token.IMPORT || !genDecl.Lparen.IsValid() {