Detect problematic defer statements with function calls

• Playground Link

Description

This rule detects a common anti-pattern in Go testing code where defer statements contain function calls with parameters that are evaluated immediately instead of when the defer executes.

In Go, defer schedules a function call to be executed when the surrounding function returns. However, the arguments to the deferred function are evaluated immediately when the defer statement is encountered, not when the defer executes.

This is particularly problematic when using assertion libraries in tests. For example:

defer require.NoError(t, failpoint.Disable("some/path"))

In this case, failpoint.Disable("some/path") is called immediately when the defer statement is reached, not when the function exits. This means the failpoint is disabled right after being enabled, making the test ineffective.

Pattern

ast-grep \
  --lang go \
  --pattern '{ defer $A.$B(t, failpoint.$M($$$)) } \
  --selector defer_statement'

Example

func TestIssue16696(t *testing.T) {
	alarmRatio := vardef.MemoryUsageAlarmRatio.Load()
	vardef.MemoryUsageAlarmRatio.Store(0.0)
	defer vardef.MemoryUsageAlarmRatio.Store(alarmRatio)
	require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/executor/sortexec/testSortedRowContainerSpill", "return(true)"))
	defer require.NoError(t,
	   failpoint.Disable(
		"github.com/pingcap/tidb/pkg/executor/sortexec/testSortedRowContainerSpill"
	))
	require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/executor/join/testRowContainerSpill", "return(true)"))
	defer require.NoError(t,
		failpoint.Disable("github.com/pingcap/tidb/pkg/executor/join/testRowContainerSpill"))
}

Fix

The correct way to defer a function with parameters is to wrap it in an anonymous function:

defer func() {
    require.NoError(t, failpoint.Disable("some/path"))
}()

Contributed by

Inspired by YangKeao's tweet about this common pitfall in TiDB codebase.