Skip to content

Commit c548038

Browse files
authored
Add deadcode CI check and remove unreachable functions (#4974)
## Why The CLI is not meant as a library and as such any function not reachable from `main()` is dead code. The existing `unused` linter skips exported functions by default (it assumes external consumers might use them), leaving a gap. `deadcode` from `golang.org/x/tools` does whole-program reachability analysis and catches these. ## Changes Added `deadcode` as a CI check via `make checks`. A Python wrapper script (`tools/check_deadcode.py`) runs `deadcode -test ./...` and supports two suppression mechanisms: 1. **Directory exclusions** for directories where everything is a false positive (e.g. `libs/gorules/`, which contains lint rule definitions loaded by golangci-lint's ruleguard engine, not through Go's call graph). 2. **Inline comments** (`//deadcode:allow <reason>`) above a function, matching the `//nolint:` pattern. The wrapper walks backward from the reported func line, stopping at a blank line, so the allow comment is found whether it sits immediately above the function or above a doc-comment block. The wrapper is needed because raw `deadcode` has no suppression mechanism. It reports every unreachable function with no way to exclude known false positives (code loaded via reflection, plugin systems, or code generators). Without the wrapper, the only options would be to either accept noisy output that developers learn to ignore, or not run the check at all. The wrapper keeps the check strict (zero tolerance, CI fails on any finding) while giving developers two escape hatches for legitimate exceptions. Both mechanisms are documented in the script itself. Removed 40 dead functions across 23 files found in the initial run. Preserved `DisabledTestNoDuplicatedAnnotations` with `//deadcode:allow` since the `Disabled` prefix was a deliberate "park for later" marker from the original author. ## Test plan - [x] `make deadcode` passes clean ("No dead code found.") - [x] `make checks` passes (includes deadcode) - [x] `make lintfull` passes (0 issues, no unused imports) - [x] `go build ./...` passes - [x] Unit tests pass for all affected packages This pull request was AI-assisted by Isaac.
1 parent a69b39f commit c548038

27 files changed

Lines changed: 127 additions & 408 deletions

File tree

Makefile

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,13 @@ wsfix:
6565
links:
6666
./tools/update_github_links.py
6767

68+
.PHONY: deadcode
69+
deadcode:
70+
./tools/check_deadcode.py
71+
6872
# Checks other than 'fmt' and 'lint'; these are fast, so can be run first
6973
.PHONY: checks
70-
checks: tidy ws links
74+
checks: tidy ws links deadcode
7175

7276

7377
.PHONY: install-pythons

bundle/internal/schema/main_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ func getAnnotations(path string) (annotation.File, error) {
125125
return data, err
126126
}
127127

128+
//deadcode:allow disabled pending annotation system overhaul; preserved intentionally
128129
func DisabledTestNoDuplicatedAnnotations(t *testing.T) {
129130
// Check for duplicated annotations in annotation files
130131
files := []string{

bundle/libraries/upload.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,6 @@ func Upload(libs map[string][]LocationToUpdate) bundle.Mutator {
3030
}
3131
}
3232

33-
func UploadWithClient(libs map[string][]LocationToUpdate, client filer.Filer) bundle.Mutator {
34-
return &upload{
35-
libs: libs,
36-
client: client,
37-
}
38-
}
39-
4033
type upload struct {
4134
client filer.Filer
4235
libs map[string][]LocationToUpdate

cmd/bundle/utils/utils.go

Lines changed: 0 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,6 @@ import (
44
"context"
55

66
"github.com/databricks/cli/bundle"
7-
bundleenv "github.com/databricks/cli/bundle/env"
8-
"github.com/databricks/cli/bundle/phases"
9-
"github.com/databricks/cli/libs/diag"
10-
"github.com/databricks/cli/libs/env"
117
"github.com/databricks/cli/libs/logdiag"
128
"github.com/spf13/cobra"
139
)
@@ -20,85 +16,3 @@ func configureVariables(cmd *cobra.Command, b *bundle.Bundle, variables []string
2016
}
2117
})
2218
}
23-
24-
// getTargetFromCmd returns the target name from command flags or environment.
25-
func getTargetFromCmd(cmd *cobra.Command) string {
26-
// Check command line flag first
27-
if flag := cmd.Flag("target"); flag != nil {
28-
if value := flag.Value.String(); value != "" {
29-
return value
30-
}
31-
}
32-
33-
// Check deprecated environment flag
34-
if flag := cmd.Flag("environment"); flag != nil {
35-
if value := flag.Value.String(); value != "" {
36-
return value
37-
}
38-
}
39-
40-
// Fall back to environment variable
41-
target, _ := bundleenv.Target(cmd.Context())
42-
return target
43-
}
44-
45-
// ReloadBundle reloads the bundle configuration without modifying the command context.
46-
// This is useful when you need to refresh the bundle configuration after changes
47-
// without side effects like setting values on the context.
48-
func ReloadBundle(cmd *cobra.Command) *bundle.Bundle {
49-
ctx := cmd.Context()
50-
51-
// Load the bundle configuration fresh from the filesystem
52-
b := bundle.MustLoad(ctx)
53-
if b == nil || logdiag.HasError(ctx) {
54-
return b
55-
}
56-
57-
// Load the target configuration
58-
if target := getTargetFromCmd(cmd); target == "" {
59-
phases.LoadDefaultTarget(ctx, b)
60-
} else {
61-
phases.LoadNamedTarget(ctx, b, target)
62-
}
63-
64-
if logdiag.HasError(ctx) {
65-
return b
66-
}
67-
68-
// Configure the workspace profile if provided
69-
configureProfile(cmd, b)
70-
71-
// Configure variables if provided
72-
variables, err := cmd.Flags().GetStringSlice("var")
73-
if err != nil {
74-
logdiag.LogDiag(ctx, diag.FromErr(err)[0])
75-
return b
76-
}
77-
configureVariables(cmd, b, variables)
78-
return b
79-
}
80-
81-
// configureProfile applies the profile flag to the bundle.
82-
func configureProfile(cmd *cobra.Command, b *bundle.Bundle) {
83-
profile := getProfileFromCmd(cmd)
84-
if profile == "" {
85-
return
86-
}
87-
88-
bundle.ApplyFuncContext(cmd.Context(), b, func(ctx context.Context, b *bundle.Bundle) {
89-
b.Config.Workspace.Profile = profile
90-
})
91-
}
92-
93-
// getProfileFromCmd returns the profile from command flags or environment.
94-
func getProfileFromCmd(cmd *cobra.Command) string {
95-
// Check command line flag first
96-
if flag := cmd.Flag("profile"); flag != nil {
97-
if value := flag.Value.String(); value != "" {
98-
return value
99-
}
100-
}
101-
102-
// Fall back to environment variable
103-
return env.Get(cmd.Context(), "DATABRICKS_CONFIG_PROFILE")
104-
}

experimental/aitools/lib/installer/installer.go

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,6 @@ type InstallOptions struct {
6767
Scope string // ScopeGlobal or ScopeProject (default: global)
6868
}
6969

70-
// FetchManifest fetches the skills manifest from the skills repo.
71-
// This is a convenience wrapper that uses the default GitHubManifestSource.
72-
func FetchManifest(ctx context.Context) (*Manifest, error) {
73-
src := &GitHubManifestSource{}
74-
ref := GetSkillsRef(ctx)
75-
return src.FetchManifest(ctx, ref)
76-
}
77-
7870
func fetchSkillFile(ctx context.Context, ref, skillName, filePath string) ([]byte, error) {
7971
url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s/%s/%s",
8072
skillsRepoOwner, skillsRepoName, ref, skillsRepoPath, skillName, filePath)
@@ -303,19 +295,6 @@ func InstallAllSkills(ctx context.Context) error {
303295
return InstallSkillsForAgents(ctx, src, installed, InstallOptions{})
304296
}
305297

306-
// InstallSkill installs a single skill by name for all detected agents.
307-
func InstallSkill(ctx context.Context, skillName string) error {
308-
installed := agents.DetectInstalled(ctx)
309-
if len(installed) == 0 {
310-
printNoAgentsDetected(ctx)
311-
return nil
312-
}
313-
314-
PrintInstallingFor(ctx, installed)
315-
src := &GitHubManifestSource{}
316-
return InstallSkillsForAgents(ctx, src, installed, InstallOptions{SpecificSkills: []string{skillName}})
317-
}
318-
319298
// PrintInstallingFor prints the "Installing..." header with agent names.
320299
func PrintInstallingFor(ctx context.Context, targetAgents []*agents.Agent) {
321300
names := make([]string, len(targetAgents))

experimental/ssh/internal/keys/secrets.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,17 +67,3 @@ func putSecret(ctx context.Context, client *databricks.WorkspaceClient, scope, k
6767
}
6868
return nil
6969
}
70-
71-
// PutSecretInScope creates the secret scope if needed and stores the secret.
72-
// sessionID is the unique identifier for the session (cluster ID for dedicated clusters, connection name for serverless).
73-
func PutSecretInScope(ctx context.Context, client *databricks.WorkspaceClient, sessionID, key, value string) (string, error) {
74-
scopeName, err := CreateKeysSecretScope(ctx, client, sessionID)
75-
if err != nil {
76-
return "", err
77-
}
78-
err = putSecret(ctx, client, scopeName, key, value)
79-
if err != nil {
80-
return "", err
81-
}
82-
return scopeName, nil
83-
}

internal/testcli/golden.go

Lines changed: 0 additions & 24 deletions
This file was deleted.

internal/testutil/helpers.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,3 @@ func RandomName(prefix ...string) string {
2525
sb.WriteString(strings.ReplaceAll(uuid.New().String(), "-", ""))
2626
return sb.String()
2727
}
28-
29-
func ReplaceWindowsLineEndings(s string) string {
30-
return strings.ReplaceAll(s, "\r\n", "\n")
31-
}

libs/apps/prompt/listers.go

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -88,28 +88,6 @@ func ListSecretKeys(ctx context.Context, scope string) ([]ListItem, error) {
8888
return out, nil
8989
}
9090

91-
// ListSQLWarehousesItems returns SQL warehouses as ListItems (reuses same API as ListSQLWarehouses).
92-
func ListSQLWarehousesItems(ctx context.Context) ([]ListItem, error) {
93-
w, err := workspaceClient(ctx)
94-
if err != nil {
95-
return nil, err
96-
}
97-
iter := w.Warehouses.List(ctx, sql.ListWarehousesRequest{})
98-
whs, err := listing.ToSlice(ctx, iter)
99-
if err != nil {
100-
return nil, err
101-
}
102-
out := make([]ListItem, 0, min(len(whs), maxListResults))
103-
for _, wh := range whs {
104-
label := wh.Name
105-
if wh.State != "" {
106-
label = fmt.Sprintf("%s (%s)", wh.Name, wh.State)
107-
}
108-
out = append(out, ListItem{ID: wh.Id, Label: label})
109-
}
110-
return capResults(out), nil
111-
}
112-
11391
// ListSchemas returns UC schemas within a catalog as selectable items.
11492
func ListSchemas(ctx context.Context, catalogName string) ([]ListItem, error) {
11593
w, err := workspaceClient(ctx)

libs/apps/prompt/prompt.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -467,20 +467,6 @@ func promptForPagedResource(ctx context.Context, r manifest.Resource, required b
467467
return singleValueResult(r, value), nil
468468
}
469469

470-
// PromptForWarehouse shows a picker to select a SQL warehouse.
471-
func PromptForWarehouse(ctx context.Context) (string, error) {
472-
var items []ListItem
473-
err := RunWithSpinnerCtx(ctx, "Fetching SQL warehouses...", func() error {
474-
var fetchErr error
475-
items, fetchErr = ListSQLWarehousesItems(ctx)
476-
return fetchErr
477-
})
478-
if err != nil {
479-
return "", fmt.Errorf("failed to fetch SQL warehouses: %w", err)
480-
}
481-
return PromptFromList(ctx, "Select SQL Warehouse", "no SQL warehouses found. Create one in your workspace first", items, true)
482-
}
483-
484470
// resourceTitle returns a prompt title for a resource, including the plugin name
485471
// for context when available (e.g. "Select SQL Warehouse for Analytics").
486472
func resourceTitle(fallback string, r manifest.Resource) string {

0 commit comments

Comments
 (0)