Skip to content

Enable linter for exhaustive coverage in enum switch statements - #3641

Merged
pietern merged 4 commits into
mainfrom
exhaustive-linter
Sep 23, 2025
Merged

Enable linter for exhaustive coverage in enum switch statements#3641
pietern merged 4 commits into
mainfrom
exhaustive-linter

Conversation

@pietern

Copy link
Copy Markdown
Contributor

Changes

Linter will error for switch statements on an enum where not all enum values are covered.

The statement can still use a default: clause to explicitly fall through.

Why

Observation here: #3546 (comment)

Comment threadbundle/run/job.go
return nil, fmt.Errorf("run timed out: %s", run.State.StateMessage)

// TODO: handle other result states.
default:

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few more states we need to handle. This is the only real problem that this linter surfaced.

}

// Infof creates a new info diagnostic.
func Infof(format string, args ...any) Diagnostics {

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out this wasn't used anywhere.

dst.SetString(strconv.FormatFloat(src.MustFloat(), 'f', -1, 64))
return nil
default:
// Fall through to the error case.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking / style question: should we just move error case inside default? then comment won't be needed.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For most of the other types, there are other cases that also fall through to the error case.

If we move it into the default, we need to duplicate the error case into those other branches.

switch kind {
case reflect.Struct, reflect.Map:
expectedJsonType = "object"
case reflect.Slice:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reflect.Array belongs here as well?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That maps to fixed-size arrays. We don't use those anywhere to my knowledge (not for JSON marshalling/unmarshalling at least). The patch here matches the existing logic.

@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Sep 22, 2025

Copy link
Copy Markdown
Collaborator

Run: 17949026941

Env✅​pass🔄​flaky🙈​skip
✅​aws linux311529
✅​aws windows312528
✅​aws-ucws linux423427
✅​aws-ucws windows424426
✅​azure linux311528
✅​azure windows312527
✅​azure-ucws linux423426
✅​azure-ucws windows424425
🔄​gcp linux3082530
✅​gcp windows311529
Test Namegcp linux
TestWorkspaceFilesExtensionsDirectoriesAreNotNotebooks🔄​flaky
TestWorkspaceFilesExtensionsNotebooksAreNotDeletedAsFiles🔄​flaky

@pietern
pietern added this pull request to the merge queueSep 23, 2025
Merged via the queue into main with commit 7842331Sep 23, 2025
13 checks passed
@pietern
pietern deleted the exhaustive-linter branch September 23, 2025 15:08
denik pushed a commit that referenced this pull request May 20, 2026
## Changes
Linter will error for switch statements on an enum where not all enum
values are covered.
The statement can still use a `default:` clause to explicitly fall
through.
## Why
Observation here:
#3546 (comment)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@pietern@eng-dev-ecosystem-bot@denik@andrewnester