Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

SA4008: false positive for stub implementations #1231

Open
dominikh opened this issue Mar 27, 2022 · 0 comments
Open

SA4008: false positive for stub implementations #1231

dominikh opened this issue Mar 27, 2022 · 0 comments
Labels
false-positive needs-decision We have to decide if this check is feasible and desirable

Comments

@dominikh
Copy link
Owner

Input:

package main

type S struct{}

func (s *S) Len() int { return 0 }
func (s *S) Foo()     { panic("unsupported") }

func foo() {
	var s S
	for i := 0; i < s.Len(); i++ {
		s.Foo()
	}
}

Output:

baz.go:13:14: variable in loop condition never changes (SA4008)

We flag the loop because s.Foo panics unconditionally and the loop executes at most once. However, in the real code, the implementation of S differs for Go 1.17 and Go 1.18, with the Go 1.17 implementation being a stub. The loop isn't incorrect. With the stub implementation, Len() returns zero, the loop never executes and the panic never triggers. The loop variable doesn't change, but it isn't expected to. In the real implementation, Len returns arbitrary values, and Foo doesn't panic unconditionally.

The core problem is that of infeasible paths; we're complaining about a combination of conditions that can never occur under real execution.

There are two workarounds:

  • Try to understand the loop condition, determine that the loop never executes, and don't flag it. That is, only flag loops that execute at least once.
  • Since this is only a problem with build tags, require the user to make use of the upcoming build matrix and report merging features.
@dominikh dominikh added needs-decision We have to decide if this check is feasible and desirable false-positive labels Mar 27, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
false-positive needs-decision We have to decide if this check is feasible and desirable
Projects
None yet
Development

No branches or pull requests

1 participant