Skip to content

Commit

Permalink
Merge pull request #21 from pohly/import-aliases
Browse files Browse the repository at this point in the history
match imports and variables against their canonical name
  • Loading branch information
ashanbrown authored Jan 29, 2023
2 parents c193d6f + cac3c7d commit 1396000
Show file tree
Hide file tree
Showing 21 changed files with 880 additions and 89 deletions.
6 changes: 3 additions & 3 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ version: 2
jobs:
build:
docker:
- image: cimg/go:1.17.7
- image: cimg/go:1.19.5
steps:
- checkout
- run: sudo apt-get update && sudo apt-get -y -qq install python pip
- run: pip install pre-commit
- run: sudo apt-get update && sudo apt-get -y -qq install python3 python3-pip
- run: pip3 install pre-commit
- run: SKIP=no-commit-to-branch pre-commit run -a
- run: go test ./...
- run: make test
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
linters:
enable:
- prealloc
- nakedret
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
repos:
- repo: https://github.com/golangci/golangci-lint
rev: v1.44.2
rev: v1.50.1
hooks:
- id: golangci-lint
8 changes: 8 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
SHELL=bash

setup:
pre-commit install

test:
cd examples && diff <(sed 's|CURDIR|$(CURDIR)|' expected_results.txt) <(go run .. 2>&1 | sed '/^go: downloading/d')

lint:
pre-commit run --all-files

.PHONY: lint test
115 changes: 108 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,121 @@ If no patterns are specified, the default pattern of `^(fmt\.Print.*|print|print
functions (and whole files), that are identifies as Godoc examples (https://blog.golang.org/examples) are excluded from
checking.

A larger set of interesting patterns might include:
By default, patterns get matched against the actual expression as it appears in
the source code. The effect is that ``^fmt\.Print.*$` will not match when that
package gets imported with `import fmt2 "fmt"` and then the function gets
called with `fmt2.Print`.

This makes it hard to match packages that may get imported under a variety of
different names, for example because there is no established convention or the
name is so generic that import aliases have to be used. To solve this,
forbidigo also supports a more advanced mode where it uses type information to
identify what an expression references. This needs to be enabled through the
`analyze_types` command line parameter. Beware this may have a performance
impact because additional information is required for the analysis.

Replacing the literal source code works for items in a package as in the
`fmt2.Print` example above and also for struct fields and methods. For those,
`<package name>.<type name>.<field or method name>` replaces the source code
text. `<package name>` is what the package declares in its `package` statement,
which may be different from last part of the import path:

import "example.com/some/pkg" // pkg uses `package somepkg`
s := somepkg.SomeStruct{}
s.SomeMethod() // -> somepkg.SomeStruct.SomeMethod

Pointers are treated like the type they point to:

var cf *spew.ConfigState = ...
cf.Dump() // -> spew.ConfigState.Dump

When a type is an alias for a type in some other package, the name of that
other package will be used.

An imported identifier gets replaced as if it had been imported without `import .`
*and* also gets matched literally, so in this example both `^ginkgo.FIt$`
and `^FIt$` would catch the usage of `FIt`:

import . "github.com/onsi/ginkgo/v2"
FIt(...) // -> ginkgo.FIt, FIt

Beware that looking up the package name has limitations. When a struct embeds
some other type, references to the inherited fields or methods get resolved
with the outer struct as type:

package foo

type InnerStruct {
SomeField int
}

func (i innerStruct) SomeMethod() {}

type OuterStruct {
InnerStruct
}

* `^fmt\.Print.*$` -- forbid use of Print statements because they are likely just for debugging
* `^fmt\.Errorf$` -- forbid Errorf in favor of using github.com/pkg/errors
* `^ginkgo\.F[A-Z].*$` -- forbid ginkgo focused commands (used for debug issues)
* `^spew\.Dump$` -- forbid dumping detailed data to stdout
* `^fmt\.Errorf(# please use github\.com/pkg/errors)?$` -- forbid Errorf, with a custom message
s := OuterStruct{}
s.SomeMethod() // -> foo.OuterStruct.SomeMethod
i := s.SomeField // -> foo.OuterStruct.SomeField

When a method gets called via some interface, that invocation also only
gets resolved to the interface, not the underlying implementation:

// innerStruct as above

type myInterface interface {
SomeMethod()
}

var i myInterface = InnerStruct{}
i.SomeMethod() // -> foo.myInterface.SomeMethod

Using the package name is simple, but the name is not necessarily unique. For
more advanced cases, it is possible to specify more complex patterns. Such
patterns are strings that contain JSON or YAML for a struct.

The full pattern struct has the following fields:

* `msg`: an additional comment that gets added to the error message when a
pattern matches.
* `pattern`: the regular expression that matches the source code or expanded
expression, depending on the global flag.
* `package`: a regular expression for the full package import path. The package
path includes the package version if the package has a version >= 2. This is
only supported when `analyze_types` is enabled.

To distinguish such patterns from traditional regular expression patterns, the
encoding must start with a `{` or contain line breaks. When using just JSON
encoding, backslashes must get quoted inside strings. When using YAML, this
isn't necessary. The following pattern strings are equivalent:

{p: "^fmt\\.Println$", msg: "do not write to stdout"}

{p: ^fmt\.Println$,
msg: do not write to stdout,
}

{p: ^fmt\.Println$, msg: do not write to stdout}

p: ^fmt\.Println$
msg: do not write to stdout

A larger set of interesting patterns might include:

Note that the linter has no knowledge of what packages were actually imported, so aliased imports will match these patterns.
-* `^fmt\.Print.*$` -- forbid use of Print statements because they are likely just for debugging
-* `^fmt\.Errorf$` -- forbid Errorf in favor of using github.com/pkg/errors
-* `^ginkgo\.F[A-Z].*$` -- forbid ginkgo focused commands (used for debug issues)
-* `^spew\.Dump$` -- forbid dumping detailed data to stdout
-* `^spew.ConfigState\.Dump$` -- also forbid it via a `ConfigState`
-* `^fmt\.Errorf(# please use github\.com/pkg/errors)?$` -- forbid Errorf, with a custom message
-* `{p: ^fmt\.Errorf$, msg: please use github.com/pkg/errors}` -- the same with separate msg field

### Flags
- **-set_exit_status** (default false) - Set exit status to 1 if any issues are found.
- **-exclude_godoc_examples** (default true) - Controls whether godoc examples are identified and excluded
- **-tests** (default true) - Controls whether tests are included
- **-analyze_types** (default false) - Replace literal source code before matching

## Purpose

Expand Down
Loading

0 comments on commit 1396000

Please sign in to comment.