Shadowing in Go

In the previous post, we discussed why minimizing identifier scope is beneficial for maintainable Go code. Tightly scoped variables reduce cognitive load and make refactoring easier.

However, we might sometimes choose a slightly wider scope to reduce nesting¹ and use early returns². This is generally fine, especially when the same variable would be later reassigned before being reused.

A typical example would be:

 1func lookupSum() (int, error) {
 2	result1, err := lookup1()
 3	if err != nil {
 4		return 0, err
 5	}
 6
 7	if err := check(result1); err != nil {
 8		return 0, err
 9	}
10
11	result2, err := lookup2()
12	if err != nil {
13		return 0, err
14	}
15
16	if err := check(result2); err != nil {
17		return 0, err
18	}
19
20	return result1 + result2, nil
21}

This code demonstrates a common pattern in Go: reusing error variables across sequential operations. But what exactly is happening with err on lines 7 and 16 versus line 11? This brings us to the concept of shadowing.

Shadowing

Variable shadowing occurs when a variable declared in an inner scope has the same name as a variable in an outer scope.³ The inner variable “shadows” the outer one, making the outer variable inaccessible within that block. Variable shadowing is a useful and common feature, frequently used for variables like ctx, err, or ok in sub-scopes.

In Go, however, this is tricky to spot because := can create a new variable with the same name or reuse an existing one, depending on the context.⁴

The lookupSum example above is perfectly fine. The instances where err is shadowed (lines 7 and 16) return immediately. The reassignment at line 11 updates the outer err correctly, so there is no stale value when we check it again at line 12.

Note that the if err := check(... lines are a great use of minimal scope because you can easily move them around; they are self-contained and do not care whether err was previously declared or not.

The tricky part starts when a variable is shadowed, and we later use the outer variable expecting it to hold the value from the inner scope.

Consider this example:

 1func checkedLookup() (int, error) {
 2	value, err := lookup()
 3	if err != nil {
 4		return 0, err
 5	}
 6
 7	// We want to update the outer 'err', but := creates a new one.
 8	if err := check(value); err == nil {
 9		return value, nil
10	}
11
12	// bookkeeping in case the check failed
13	checkFailed(value)
14
15	// Returns the outer err (which is nil), regardless of what check() returns
16	return 0, err
17}

In the code above, the err inside the if statement in line 8 is a new variable. The outer err is never touched, so we return nil in line 16. This bug can be surprisingly hard to spot during a code review.⁵

So how can we catch bugs like this? Let’s look at what tools are available.

Existing Tools

The standard tool for detecting this is shadow⁶:

go install golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow@latest
go vet -vettool=$(which shadow)

Running it on checkedLookup flags an issue:

% go vet -vettool=$(which shadow) -c 0
...go:8:5: declaration of "err" shadows declaration at line 2
8      if err := check(value); err == nil {

However, shadow is noisy. It flags every instance of shadowing, even benign cases that are idiomatic and safe. For the correct lookupSum example shown earlier, we get the same diagnostics as for the erroneous checkedLookup:

% go vet -vettool=$(which shadow) -c 0
...go:7:5: declaration of "err" shadows declaration at line 2
7     if err := check(result1); err != nil {

As noted in the proposal to deprecate shadow⁷, the analyzer suffers from a poor “signal-to-noise ratio,” although “shadowing of local variables […] is a not infrequent source of mistakes in Go.”

A Better Indicator: Usage Detection

The problem isn’t shadowing itself; it is using the outer variable after we accidentally created a shadow variable instead of updating the outer one.

If you shadow a variable but never use the outer variable again, or if you immediately reassign the outer variable before reading it, there is no bug. Shadowing only leads to errors when you read the outer variable expecting it to contain the value from the inner block.

This is precisely where scopeguard takes a different approach.

Instead of flagging the shadowing, scopeguard looks for variables that are used after being shadowed. This heuristic catches the issue shown above while ignoring safe shadowing patterns.

For example, in the problematic checkedLookup function above, the code is flagged because err is returned without being reassigned:

 6	// ... previous code from "checkedLookup"
 7
 8	if err := check(value); err == nil { // Shadowing happens here
 9		return value, nil
10	}
11
12	/*
13	 ...
14	*/
15
16	return 0, err // Use of outer 'err' is flagged

Running scopeguard on this code produces:

% scopeguard -c 0 .
...go:16:12: Identifier 'err' used after previously shadowed (sg:uas)
16     return 0, err
...go:8:5:   After this declaration
8      if err := check(value); err == nil {

In contrast, the correct variant from lookupSum is accepted because err is reassigned before being used:

 3	// ... previous code from "lookupSum"
 4
 5	if err := check(result1); err != nil {
 6		return 0, err
 7	}
 8
 9	result2, err := lookup2() // We reassign 'err' before using it again.
10	if err != nil { // The previous shadowing didn't leave us with a stale value.
11		return 0, err
12	}

An Exercise in Readability

Let’s look at a real-world example. In Argo Workflows TryAcquire we have this line:

273	failedLockName := ""

When I read this, I understand it as “no lock has failed yet, but this will contain the lock’s name if one fails later.” Between this declaration and its usage, scopeguard flags several shadowing instances. Then, 79 lines later, the variable is used in a return statement:

352	return sm.tryAcquireImpl(wf, nil, holderKey, failedLockName, syncItems, lockKeys)

So where between lines 273 and 352 is failedLockName set to an actual failed lock name? Read the code, I’ll wait.

This demonstrates an important point about readability: while the code may be technically correct, the usage after shadowing makes it really hard to read and understand. Does the function need to track failed lock names or not?

A Code Riddle

Let’s finish with a small riddle that demonstrates how subtle shadowing can be. What do you think the following function will return?

 1func calc() (i int, err error) {
 2	for i := range 10 {
 3		j, err := func(i int) (int, error) {
 4			return i + 1, nil
 5		}(i + 2)
 6		if err != nil {
 7			return j + 3, err
 8		}
 9
10		err = func(int) error { return fmt.Errorf("error %d", i+4) }(i + 5)
11	}
12
13	return
14}

While shadow remains silent, scopeguard flags these issues:

% scopeguard -c 0 .
...go:13:2: Identifier 'i' used after previously shadowed (sg:uas)
13     return
...go:2:2:     After this declaration
2      for i := range 10 {
...go:13:2: Identifier 'err' used after previously shadowed (sg:uas)
13     return
...go:3:3:     After this declaration
3              j, err := func(i int) (int, error) {

The function returns 0, nil (Go Playground) despite the error created in the loop, because both i and err in the named return values are shadowed within the loop scope.

Summary

Shadowing allows for concise variable names like err and ctx without having to rename them every time, but it can lead to subtle bugs. By focusing on usage after shadowing, we can catch actual mistakes — where a variable might hold a stale value — without flagging idiomatic code.

If you need to use a variable after shadowing it, the solution is simple: rename the inner variable. This clarifies your intent and ensures that readers (and the compiler) know exactly which value you mean to use.

• ←Previous
• Next→