Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Cosmetic fixes #7

Merged
merged 3 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ issues:
- path: lint_test
linters:
- varnamelen
- wrapcheck
- linters:
- lll
source: "^(?: |\t)*// "
Expand All @@ -45,4 +46,4 @@ issues:
- linters:
- godot
- lll
source: "// ?TODO "
source: "// ?TODO "
13 changes: 13 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
.PHONY: fmt lint clean.test test test.clean

CVPKG=go list ./... | grep -v mocks | grep -v internal/
GO_TEST=go test `$(CVPKG)` -race
COVERAGE_FILE="coverage.out"

all: fmt lint test install

fmt:
go fmt ./...

lint:
golangci-lint run --fix ./...

clean.test:
go clean --testcache

Expand All @@ -12,3 +22,6 @@ test.clean: clean.test test

test.coverage:
$(GO_TEST) -covermode=atomic -coverprofile=$(COVERAGE_FILE)

install:
go install ./cmd/go-factory-lint
10 changes: 6 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,18 @@ The linter checks that the Structures are created by the Factory, and not direct
The checking helps to provide invariants without exclusion and helps avoid creating an invalid object.


## Use
## Usage

Installation
### Installation

go install github.com/maranqz/go-factory-lint/cmd/go-factory-lint@latest

### Options

- `--packageGlobs` - list of glob packages, which can create structures without factories inside the glob package. By default, all structures from another package should be created by factories, [tests](testdata/src/factory/packageGlobs).
- `onlyPackageGlobs` - use a factory to initiate a structure for glob packages only, [tests](testdata/src/factory/onlyPackageGlobs).
maranqz marked this conversation as resolved.
Show resolved Hide resolved
- `--packageGlobs` – list of glob packages, which can create structures without factories inside the glob package.
By default, all structures from another package should be created by factories, [tests](testdata/src/factory/packageGlobs).
- `--onlyPackageGlobs` – use a factory to initiate a structure for glob packages only,
[tests](testdata/src/factory/onlyPackageGlobs). Doesn't make sense without `--packageGlobs`.

## Example

Expand Down
54 changes: 14 additions & 40 deletions factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,50 +5,28 @@ import (
"go/ast"
"go/types"
"log"
"strings"

"github.com/gobwas/glob"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
)

type config struct {
pkgGlobs stringsFlag
onlyPkgGlobs bool
}

type stringsFlag []string
maranqz marked this conversation as resolved.
Show resolved Hide resolved

func (s stringsFlag) String() string {
return strings.Join(s, ", ")
}

func (s *stringsFlag) Set(value string) error {
*s = append(*s, value)

return nil
}

func (s stringsFlag) Value() []string {
res := make([]string, 0, len(s))

for _, str := range s {
res = append(res, strings.TrimSpace(str))
}

return res
}

const (
packageGlobsDesc = "List of glob packages, which can create structures without factories inside the glob package"
onlyPkgGlobsDesc = "Use a factory to initiate a structure for glob packages only."
name = "gofactory"
doc = "Blocks the creation of structures directly, without a factory."

packageGlobsDesc = "list of glob packages, which can create structures without factories inside the glob package"
onlyPkgGlobsDesc = "use a factory to initiate a structure for glob packages only"
)

func NewAnalyzer() *analysis.Analyzer {
analyzer := &analysis.Analyzer{
Name: "gofactory",
Doc: "Blocks the creation of structures directly, without a factory.",
Requires: []*analysis.Analyzer{inspect.Analyzer},
maranqz marked this conversation as resolved.
Show resolved Hide resolved
Name: name,
Doc: doc,
}

cfg := config{}
Expand Down Expand Up @@ -231,18 +209,14 @@ func (v *visitor) checkBrackets(expr ast.Expr, identObj types.Object) {
func (v *visitor) report(node ast.Node, obj types.Object) {
v.pass.Reportf(
node.Pos(),
fmt.Sprintf(`Use factory for %s.%s`, obj.Pkg().Name(), obj.Name()),
maranqz marked this conversation as resolved.
Show resolved Hide resolved
"Use factory for %s.%s", obj.Pkg().Name(), obj.Name(),
)
}

func (v *visitor) unexpectedCode(node ast.Node) {
fset := v.pass.Fset
pos := fset.Position(node.Pos())

log.Printf("Unexpected code in %s:%d:%d, please report to the developer with example.\n",
fset.File(node.Pos()).Name(),
pos.Line,
pos.Column,
log.Printf("%s: unexpected code in %s, please report to the developer with example.\n",
name,
v.pass.Fset.Position(node.Pos()),
maranqz marked this conversation as resolved.
Show resolved Hide resolved
)
}

Expand All @@ -260,12 +234,12 @@ func compileGlobs(globs []string) ([]glob.Glob, error) {
compiledGlobs := make([]glob.Glob, len(globs))

for idx, globString := range globs {
glob, err := glob.Compile(globString)
compiled, err := glob.Compile(globString)
maranqz marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, fmt.Errorf("unable to compile globs %s: %w", glob, err)
maranqz marked this conversation as resolved.
Show resolved Hide resolved
return nil, fmt.Errorf("unable to compile glob %q: %w", globString, err)
}

compiledGlobs[idx] = glob
compiledGlobs[idx] = compiled
}

return compiledGlobs, nil
Expand Down
23 changes: 10 additions & 13 deletions lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,25 @@ func TestLinterSuite(t *testing.T) {

tests := map[string]struct {
pkgs []string
prepare func(t *testing.T, a *analysis.Analyzer)
prepare func(t *testing.T, a *analysis.Analyzer) error
}{
"simple": {pkgs: []string{"simple/..."}},
"casting": {pkgs: []string{"casting/..."}},
"generic": {pkgs: []string{"generic/..."}},
"packageGlobs": {
pkgs: []string{"packageGlobs/..."},
prepare: func(t *testing.T, a *analysis.Analyzer) {
if err := a.Flags.Set("packageGlobs", "factory/packageGlobs/blocked/**"); err != nil {
t.Fatal(err)
maranqz marked this conversation as resolved.
Show resolved Hide resolved
}
prepare: func(t *testing.T, a *analysis.Analyzer) error {
return a.Flags.Set("packageGlobs", "factory/packageGlobs/blocked/**")
},
},
"onlyPackageGlobs": {
pkgs: []string{"onlyPackageGlobs/main/..."},
prepare: func(t *testing.T, a *analysis.Analyzer) {
prepare: func(t *testing.T, a *analysis.Analyzer) error {
if err := a.Flags.Set("packageGlobs", "factory/onlyPackageGlobs/blocked/**"); err != nil {
t.Fatal(err)
return err
}

if err := a.Flags.Set("onlyPackageGlobs", "true"); err != nil {
t.Fatal(err)
}
return a.Flags.Set("onlyPackageGlobs", "true")
},
},
}
Expand All @@ -58,11 +54,12 @@ func TestLinterSuite(t *testing.T) {
analyzer := factory.NewAnalyzer()

if tt.prepare != nil {
tt.prepare(t, analyzer)
if err := tt.prepare(t, analyzer); err != nil {
t.Fatal(err)
}
}

analysistest.Run(t, testdata,
analyzer, dirs...)
analysistest.Run(t, testdata, analyzer, dirs...)
})
}
}
25 changes: 25 additions & 0 deletions strings_flag.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package factory

import "strings"

type stringsFlag []string

func (s stringsFlag) String() string {
return strings.Join(s, ", ")
}

func (s *stringsFlag) Set(value string) error {
*s = append(*s, value)

return nil
}

func (s stringsFlag) Value() []string {
res := make([]string, 0, len(s))

for _, str := range s {
res = append(res, strings.TrimSpace(str))
}

return res
}
Loading