fatih / faillint Goto Github PK

I'd like to forbid dot imports on some projects, could faillint catch package aliases or explicitly . with -paths?

e.g.

import (
   . "github.com/pkg/errors"
)

Currently faillint only fails on package level (based on the import path). We should also make it possible to fail for a sub-set of exported functions, types or variables (i.e: ast.TypeSpec and ast.ValueSpec should be enough).

Why is this useful? Let's look at this example:

If a user opted to use github.com/pkg/errors not only want they prevent the usage of errors package, they probably also want to avoid using fmt.Errorf and start using errors.Errorf method from the github.com/pkg/errors package. There are many reasons for that. For example the github.com/pkg/errors package includes stack information in each error value. Hence they want to make sure all errors are created with the github.com/pkg/errors package.

We could still use the paths flags and could define the sub-set with a dot identifier. For example the following command could fail for all usages of errors and the usage of fmt.Errorf:

faillint --paths "errors,fmt.Errorf"

If anyone wants to work on this, please comment on the PR before starting on it. I'll work on that at some point otherwise.

Seen at https://github.com/grafana/mimir/actions/runs/8520707102/job/23337387670?pr=7356

Of interest, this is a PR to bump Go version from 1.21 to 1.22.

Since this repo seems moribund, we will try https://github.com/OpenPeeDeeP/depguard.

GOFLAGS="-tags=requires_docker,stringlabels" faillint -paths "github.com/bmizerany/assert=github.com/stretchr/testify/assert,\
	golang.org/x/net/context=context,\
	sync/atomic=go.uber.org/atomic,\
	regexp=github.com/grafana/regexp,\
	github.com/go-kit/kit/log/...=github.com/go-kit/log,\
	github.com/prometheus/client_golang/prometheus.{MultiError}=github.com/prometheus/prometheus/tsdb/errors.{NewMulti},\
	github.com/weaveworks/common/user.{ExtractOrgID}=github.com/grafana/mimir/pkg/tenant.{TenantID,TenantIDs},\
	github.com/weaveworks/common/user.{ExtractOrgIDFromHTTPRequest}=github.com/grafana/mimir/pkg/tenant.{ExtractTenantIDFromHTTPRequest}" ./pkg/... ./cmd/... ./tools/... ./integration/...
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x5b25ef]

goroutine 95 [running]:
go/types.(*Checker).handleBailout(0xc0001b0400, 0xc004179c60)
	/usr/local/go/src/go/types/check.go:367 +0x88
panic({0x7192e0?, 0x9a4b20?})
	/usr/local/go/src/runtime/panic.go:770 +0x132
go/types.(*StdSizes).Sizeof(0x0, {0x7e7e90, 0x9a87e0})
	/usr/local/go/src/go/types/sizes.go:228 +0x30f
go/types.(*Config).sizeof(...)
	/usr/local/go/src/go/types/sizes.go:333
go/types.representableConst.func1({0x7e7e90?, 0x9a87e0?})
	/usr/local/go/src/go/types/const.go:76 +0x9e
go/types.representableConst({0x7e91f0, 0x99d4a0}, 0xc0001b0400, 0x9a87e0, 0xc004178078)
	/usr/local/go/src/go/types/const.go:92 +0x192
[...]
golang.org/x/tools/go/packages.(*loader).loadRecursive(0x6d696d2f72696d69?, 0x6f646e65762f7269?)
	/go/pkg/mod/golang.org/x/[email protected]/go/packages/packages.go:826 +0x4a
golang.org/x/tools/go/packages.(*loader).refine.func2(0x2f33762f676b702f?)
	/go/pkg/mod/golang.org/x/[email protected]/go/packages/packages.go:761 +0x26
created by golang.org/x/tools/go/packages.(*loader).refine in goroutine 1
	/go/pkg/mod/golang.org/x/[email protected]/go/packages/packages.go:760 +0xc9a
make: *** [Makefile:308: lint] Error 2

When using this tool with Go 1.18, I'm seeing errors like

faillint: internal error: package "time" without types was imported from "github.com/github/my-project-repo/constants"

This might be fixed by bumping golang.org/x/tools to a newer version

We have some use cases where we would like to blacklist an import path prefix so that all import packages belonging to that prefix are blacklisted as well. Would be of any interest to add such feature to the project?

Example of README:

As an example, you could enforce the usage of github.com/pkg/errors instead of the errors package.

The Github project github.com/pkg/errors is archived.

I am new to Go, but maybe today it is the other way around: You want to enforce that the standard errors package get used?

Maybe update the README?

It would be nice to have a flag to exclude a package from a scan.

Example

faillint -paths google.golang.org/grpc/status ./pkg/... -exclude ./pkg/server/...

instead of having to do

faillint -paths google.golang.org/grpc/status ./pkg/log/... ./pkg/metrics/... ./pkg/internal/... # and so on and so forth

Hello,

what would be the recommended approach for ignoring the vendor directory while scanning the rest of the project?

Would this be a possible upcoming feature?

Thanks.

if I run this target without build it fails with the error in the title.
https://github.com/arribada/i360/blob/master/Makefile#L108

Here is a github action.
https://github.com/arribada/i360/runs/1800460336?check_suite_focus=true

When I add build as a prerequisite for this target it works fine.

Would it be possible to ignore test files. I had a quick look at the code. I saw a way to disable the test for test packages but could not see how I would achieve this for a single test file.

I'll try to have a better look at the code maybe I can find something there.

I'd like a way to block github.com/jmoiron/sqlx, struct DB, function Select, Exec, Must*

• Must* functions are dangerous in a service context (assuming you don't have recover middleware)
• Select, Exec, and a few others don't take a context, we need to use *Context( for cancellation aware versions.

The issue is that these aren't top level functions or vars, but are nested inside an *sqlx.DB.

Wanted expression:

github.com/jmoiron/sqlx.DB.{Select,Exec,Must*}

running faillint-v1.7.0 -paths "errors=github.com/pkg/errors" ./...
in this repo results in the title error.

go version go1.17.2 linux/amd64

Can see the github actions runs for more details.

Imagine wanting to avoid a package but, in practice, needing to be able to tolerate exceptions to it (while using faillint in a CI pipeline). Much like many linting tools I wonder if the import could be annotated like,

import (
    "github.com/gogo/protobuf/proto" // faillint: gogo is tolerated here because XYZ.
)

So if we had -path "github.com/gogo/protobuf/proto=github.com/golang/protobuf/proto" but faillint saw a // faillint: annotation it would suppress its objection (and if no objections exit zero).

Rather than printing a warning and a suggestion, it should be possible to allow user to specify an option (e.g. -fix) so that directly replace the offending import path with the suggested one.

It would be nice if -paths can be parsed from a config file (YAML, JSON, etc). :)

Hey 👋

Any ideas on how to deal with generated files? How can we easily ignore those without many faillint invocations?

/home/bwplotka/Repos/thanos/pkg/store/storepb/types.pb.go:18:13: declaration "Errorf" from package "fmt" shouldn't be used, suggested: "github.com/pkg/errors.{Errorf}"

I tried find . -type f -name "*.go" | grep -v vendor/ | grep -vE '*.pb.go' | xargs $(FAILLINT) -paths $(FAILLINT_PATHS) but got:

-: named files must all be in one directory; have ./test/e2e/ and ./test/e2e/e2ethanos/
faillint: error during loading

We have some tests with a buildtag // +build requires_docker at the beginning of the file and I've just realised these files are skipped by faillint and I can't find a way to specify build tags (I also checked a bit the source code). Is there any way to get it working on source files with build tags?

Thank you for this tool! I have a use case where I want to ban time.Sleep from test functions to avoid flaky tests. Instead of time.Sleep with a hard-coded timeout, I want to push others to use functions that would retry some operations continuously until some deadline. For that, faillint should be able to operate only on Test*, Benchmark*, and Fuzz* functions. What do you think? Would you be open to adding such an option?