Coder Social home page Coder Social logo

exportloopref's Introduction

exportloopref

An analyzer that finds exporting pointers for loop variables. Pin them all!

PkgGoDev Go Report Card Coverage Status Release

What's this?

Sample problem code from: https://github.com/kyoh86/exportloopref/blob/main/testdata/src/simple/simple.go

package main

func main() {
	var intArray [4]*int
	var intSlice []*int
	var intRef *int
	var intStr struct{ x *int }

	println("loop expecting 10, 11, 12, 13")
	for i, p := range []int{10, 11, 12, 13} {
		printp(&p)                      // not a diagnostic
		intSlice = append(intSlice, &p) // want "exporting a pointer for the loop variable p"
		intArray[i] = &p                // want "exporting a pointer for the loop variable p"
		if i%2 == 0 {
			intRef = &p   // want "exporting a pointer for the loop variable p"
			intStr.x = &p // want "exporting a pointer for the loop variable p"
		}
		var vStr struct{ x *int }
		var vArray [4]*int
		var v *int
		if i%2 == 0 {
			v = &p         // not a diagnostic (x is local variable)
			vArray[1] = &p // not a diagnostic (x is local variable)
			vStr.x = &p
		}
		_ = v
	}

	println(`slice expecting "10, 11, 12, 13" but "13, 13, 13, 13"`)
	for _, p := range intSlice {
		printp(p)
	}
	println(`array expecting "10, 11, 12, 13" but "13, 13, 13, 13"`)
	for _, p := range intArray {
		printp(p)
	}
	println(`captured value expecting "12" but "13"`)
	printp(intRef)
}

func printp(p *int) {
	println(*p)
}

In Go, the p variable in the above loops is actually a single variable. So in many case (like the above), using it makes for us annoying bugs.

You can find them with exportloopref, and fix it.

package main

func main() {
	var intArray [4]*int
	var intSlice []*int
	var intRef *int
	var intStr struct{ x *int }

	println("loop expecting 10, 11, 12, 13")
	for i, p := range []int{10, 11, 12, 13} {
    p := p                          // FIX variable into the local variable
		printp(&p)
		intSlice = append(intSlice, &p) 
		intArray[i] = &p
		if i%2 == 0 {
			intRef = &p
			intStr.x = &p
		}
		var vStr struct{ x *int }
		var vArray [4]*int
		var v *int
		if i%2 == 0 {
			v = &p
			vArray[1] = &p
			vStr.x = &p
		}
		_ = v
	}

	println(`slice expecting "10, 11, 12, 13"`)
	for _, p := range intSlice {
		printp(p)
	}
	println(`array expecting "10, 11, 12, 13"`)
	for _, p := range intArray {
		printp(p)
	}
	println(`captured value expecting "12"`)
	printp(intRef)
}

func printp(p *int) {
	println(*p)
}

ref: https://github.com/kyoh86/exportloopref/blob/main/testdata/src/fixed/fixed.go

Sensing policy

I want to make exportloopref as accurately as possible. So some cases of lints will be false-negative.

e.g.

var s Foo
for _, p := range []int{10, 11, 12, 13} {
  s.Bar(&p) // If s stores the pointer, it will be bug.
}

If you want to report all of lints (with some false-positives), you should use looppointer.

Known false negatives

Case 1: pass the pointer to function to export.

Case 2: pass the pointer to local variable, and export it.

package main

type List []*int

func (l *List) AppendP(p *int) {
	*l = append(*l, p)
}

func main() {
	var slice []*int
	list := List{}

	println("loop expect exporting 10, 11, 12, 13")
	for _, v := range []int{10, 11, 12, 13} {
		list.AppendP(&v) // Case 1: wanted "exporting a pointer for the loop variable v", but cannot be found

		p := &v                  // p is the local variable
		slice = append(slice, p) // Case 2: wanted "exporting a pointer for the loop variable v", but cannot be found
	}

	println(`slice expecting "10, 11, 12, 13" but "13, 13, 13, 13"`)
	for _, p := range slice {
		printp(p)
	}
	println(`array expecting "10, 11, 12, 13" but "13, 13, 13, 13"`)
	for _, p := range ([]*int)(list) {
		printp(p)
	}
}

func printp(p *int) {
	println(*p)
}

Install

go:

$ go install github.com/kyoh86/exportloopref/cmd/exportloopref@latest

homebrew:

$ brew install kyoh86/tap/exportloopref

gordon:

$ gordon install kyoh86/exportloopref

Usage

exportloopref [-flag] [package]

Flags

Flag Description
-V print version and exit
-all no effect (deprecated)
-c int display offending line with this many lines of context (default -1)
-cpuprofile string write CPU profile to this file
-debug string debug flags, any subset of "fpstv"
-fix apply all suggested fixes
-flags print analyzer flags in JSON
-json emit JSON output
-memprofile string write memory profile to this file
-source no effect (deprecated)
-tags string no effect (deprecated)
-trace string write trace log to this file
-v no effect (deprecated)

LICENSE

MIT License

This is distributed under the MIT License.

exportloopref's People

Contributors

choleraehyq avatar fzambia avatar kyoh86 avatar nkcmr avatar vitaminniy avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar

exportloopref's Issues

Forbid captured loop variables

Hey, what do you think about the mode that forbid capturing loop variables to prevent errors like this:

package main

import (
	"fmt"
)

func main() {
	x := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 0}
	fs := []func(){}
	for _, y := range x {
		// y := y
		fs = append(fs, func() { fmt.Println(y) })
	}
	for _, f := range fs {
		f()
	}
}

Though it won't be necessary when go 1.22 is out: https://go.dev/blog/loopvar-preview

panic in checkUnaryExpr

➜  exportloopref ./...
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x10eb0a9]

goroutine 6091 [running]:
go/ast.(*Object).Pos(0x0, 0xc002801aa0)
        /Users/cholerae/Documents/gopath/go1.15beta1/src/go/ast/scope.go:93 +0x29
github.com/kyoh86/exportloopref.(*Searcher).checkUnaryExpr(0xc008910bf8, 0xc002801ac0, 0xc0056ec500, 0xd, 0x10, 0x0, 0x12c4a01)
        /Users/cholerae/Documents/gocode/exportloopref/exportloopref.go:172 +0xbf
github.com/kyoh86/exportloopref.(*Searcher).Check(0xc008910bf8, 0x136b840, 0xc002801ac0, 0xc0056ec500, 0xd, 0x10, 0x0, 0x1)
        /Users/cholerae/Documents/gocode/exportloopref/exportloopref.go:71 +0xd1
github.com/kyoh86/exportloopref.run.func1(0x136b840, 0xc002801ac0, 0x1, 0xc0056ec500, 0xd, 0x10, 0x1)
        /Users/cholerae/Documents/gocode/exportloopref/exportloopref.go:43 +0x75
golang.org/x/tools/go/ast/inspector.(*Inspector).WithStack(0xc00891c380, 0xc008910c80, 0x5, 0x5, 0xc008910c08)
        /Users/cholerae/Documents/gopath/pkg/mod/golang.org/x/[email protected]/go/ast/inspector/inspector.go:126 +0x142
github.com/kyoh86/exportloopref.run(0xc00891a5a0, 0xc0089164e0, 0x14d1cc0, 0xc008942388, 0xc0012d1dc0)
        /Users/cholerae/Documents/gocode/exportloopref/exportloopref.go:42 +0x273
golang.org/x/tools/go/analysis/internal/checker.(*action).execOnce(0xc0035defa0)
        /Users/cholerae/Documents/gopath/pkg/mod/golang.org/x/[email protected]/go/analysis/internal/checker/checker.go:658 +0x75f
sync.(*Once).doSlow(0xc0035defa0, 0xc0012d1f90)
        /Users/cholerae/Documents/gopath/go1.15beta1/src/sync/once.go:66 +0xec
sync.(*Once).Do(...)
        /Users/cholerae/Documents/gopath/go1.15beta1/src/sync/once.go:57
golang.org/x/tools/go/analysis/internal/checker.(*action).exec(0xc0035defa0)
        /Users/cholerae/Documents/gopath/pkg/mod/golang.org/x/[email protected]/go/analysis/internal/checker/checker.go:577 +0x65
golang.org/x/tools/go/analysis/internal/checker.execAll.func1(0xc0035defa0)
        /Users/cholerae/Documents/gopath/pkg/mod/golang.org/x/[email protected]/go/analysis/internal/checker/checker.go:565 +0x34
created by golang.org/x/tools/go/analysis/internal/checker.execAll
        /Users/cholerae/Documents/gopath/pkg/mod/golang.org/x/[email protected]/go/analysis/internal/checker/checker.go:571 +0x125
➜  exportloopref -V     
exportloopref: unsupported flag value: -V=true

I'm using 619c505 this version. My code itself can be compiled successfully but exportloopref panic. I'm trying to extract a minimal production.

No reports about using variable on range scope in function literal (deprecated `scopelint` finds it perfectly)

Hi,

scopelint is deprecated and exportloopref is recommended as replacement.
Could the issue be fixed here or is it out of scope of exportloopref analyzer?

Sample code:

package main

import "fmt"

func main() {
	var pipeline []func()

	for _, s := range []string{"a", "b"} {
		pipeline = append(pipeline, func() {
			fmt.Printf("%s\n", s)
		})
	}

	for _, f := range pipeline {
		f()
	}
}

# Output:
# 
# b
# b

Expected analyzer report(like from scopelint):

Using the variable on range scope 's' in function literal

Actual analyzer report: no issues found.

Linter throwing Panic

Linter crashed while building in Jenkins.

[2021-03-31T16:06:22.161Z] + golangci-lint run -c ./golangci.yaml [2021-03-31T16:06:48.663Z] level=warning msg="[linters context] Panic: exportloopref: package \"loggingapi\" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference: goroutine 8231 [running]:\nruntime/debug.Stack(0xf2d9bd, 0x3c, 0xc002296578)\n\truntime/debug/stack.go:24 +0x9f\ngithub.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1(0xc0038a1390)\n\tgithub.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner.go:508 +0x1be\npanic(0xd76f60, 0x1541d70)\n\truntime/panic.go:965 +0x1b9\ngo/ast.(*Object).Pos(0x0, 0xc002296c20)\n\tgo/ast/scope.go:93 +0x29\ngithub.com/kyoh86/exportloopref.(*Searcher).isVar(0xc002296bf0, 0x10429d8, 0xc00c41eff0, 0x104b930, 0xc00f0cc6a0, 0xc00c41eff0)\n\tgithub.com/kyoh86/[email protected]/exportloopref.go:275 +0xb0\ngithub.com/kyoh86/exportloopref.(*Searcher).checkUnaryExpr(0xc002296bf0, 0xc00f0cc700, 0xc0000d6200, 0xd, 0x10, 0x0, 0x0, 0xf5a401)\n\tgithub.com/kyoh86/[email protected]/exportloopref.go:251 +0x3b8\ngithub.com/kyoh86/exportloopref.(*Searcher).Check(0xc002296bf0, 0x1042c58, 0xc00f0cc700, 0xc0000d6200, 0xd, 0x10, 0x0, 0x0, 0xc002296a01)\n\tgithub.com/kyoh86/[email protected]/exportloopref.go:100 +0xee\ngithub.com/kyoh86/exportloopref.run.func1(0x1042c58, 0xc00f0cc700, 0x1, 0xc0000d6200, 0xd, 0x10, 0x1)\n\tgithub.com/kyoh86/[email protected]/exportloopref.go:46 +0x8c\ngolang.org/x/tools/go/ast/inspector.(*Inspector).WithStack(0xc01ced4090, 0xc002296c80, 0x5, 0x5, 0xc00aab9c08)\n\tgolang.org/x/[email protected]/go/ast/inspector/inspector.go:126 +0x142\ngithub.com/kyoh86/exportloopref.run(0xc00dcfdee0, 0x592ac3264, 0x164e260, 0xc00871dbb8, 0x2)\n\tgithub.com/kyoh86/[email protected]/exportloopref.go:45 +0x28b\ngithub.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc0038a1390)\n\tgithub.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner.go:590 +0xa85\ngithub.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()\n\tgithub.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner.go:512 +0x2a\ngithub.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc001e58500, 0xee921b, 0xd, 0xc0037cb770)\n\tgithub.com/golangci/golangci-lint/pkg/timeutils/stopwatch.go:111 +0x4d\ngithub.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0xc0038a1390)\n\tgithub.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner.go:511 +0x91\ngithub.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0xc00c73be50, 0xc0038a1390)\n\tgithub.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner.go:1059 +0x65\ncreated by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze\n\tgithub.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner.go:1054 +0x316\n" [2021-03-31T16:06:48.663Z] level=warning msg="[runner] Can't run linter goanalysis_metalinter: goanalysis_metalinter: exportloopref: package \"loggingapi\" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference" [2021-03-31T16:06:48.663Z] level=error msg="Running error: goanalysis_metalinter: exportloopref: package \"loggingapi\" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference" script returned exit code 3

False positive when ranging over a slice of pointers

package main

import (
	"fmt"
)

type Node struct {
	name string
}

type Nodes []*Node

type Graph struct {
	nodes Nodes
}

func main() {
	var graph Graph
	var s *string

	graph.nodes = Nodes{&Node{"hello"}, &Node{"world"}, &Node{"foo"}, &Node{"bar"}, &Node{"baz"}}

	for i, n := range graph.nodes {
		if i == 2 {
			s = &n.name // foo.go:23:9: exporting a pointer for the loop variable n
		}
	}

	fmt.Println(*s)
}

This is valid code because n takes new pointer values, so &n.name points correctly to different names.

fails to find out what scopelint does

Hi,

As suggested on scopelint's site its obsolete now, exportloopref is what someone should use. So I am trying it on very simple code and it doesn't seem to work as expected.

Here is the code I am trying

package main

import (
	"fmt"
)

func main() {
	fmt.Println("Hello!")

	letters := []string{"a", "b", "c"}
	funcs := []func(){}
	for _, l := range letters {
		funcs = append(funcs, func(){
			fmt.Println(l)
		})
	}
	for _, f := range funcs {
		f()
	}
}

scopelint sees the error

$ scopelint ./...
main.go:14:16: Using the variable on range scope "l" in function literal
Found 1 lint problems; failing.

but exportloopref doesn't report anything. like scopelint, I expect same error from exportloopref.

$ exportloopref  ./...

What might be going wrong?

exportloopref may be able to check decendants member like `a.b.c.d...`

#2 is the problem that when a parent of Selector (i.e. a of the a.b) is the pointer, exportloopref reports false-positive.

In past, I thought that exportloopref cannot report &a.b.c.d since it cannot check each of the layer in the Selectors is not a pointer.
But now, I found the solution ( *analysis.Pass.TypeInfo ).
So maybe I can check that.

False positive when exporting a pointer to a loop variable field.

Like so:

package main

import "fmt"

func main() {
	type S struct {
		I int
	}

	structs := []S{S{1}, S{2}}

	var myI *int
	for _, s := range structs {
		myI = &s.I
	}

	fmt.Println(*myI)
}

We do this a fair amount in my team's code, and it feels awkward to introduce a second variable like

i := s.I
myI = &i

or to use a //nolint directive.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.