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 all commits
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
6 changes: 3 additions & 3 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ on:
- v*

env:
GO_VERSION: "1.21"
GO_VERSION: "1.20"

jobs:

Expand Down Expand Up @@ -36,6 +36,6 @@ jobs:
with:
version: latest
args: release --clean
workdir: cmd/go-factory-lint/
workdir: cmd/gofactory/
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
5 changes: 3 additions & 2 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ linters-settings:
- v
- ok
gci:
local-prefixes: github.com/maranqz/go-factory-lint
local-prefixes: github.com/maranqz/gofactory

linters:
enable-all: true
Expand Down 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/gofactory
19 changes: 11 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,25 +1,27 @@
# Factory linter

[![CI](https://github.com/maranqz/go-factory-lint/actions/workflows/ci.yml/badge.svg)](https://github.com/maranqz/go-factory-lint/actions/workflows/ci.yml)
[![Go Report Card](https://goreportcard.com/badge/github.com/maranqz/go-factory-lint)](https://goreportcard.com/report/github.com/maranqz/go-factory-lint?dummy=unused)
[![CI](https://github.com/maranqz/gofactory/actions/workflows/ci.yml/badge.svg)](https://github.com/maranqz/gofactory/actions/workflows/ci.yml)
[![Go Report Card](https://goreportcard.com/badge/github.com/maranqz/gofactory)](https://goreportcard.com/report/github.com/maranqz/gofactory?dummy=unused)
[![MIT License](http://img.shields.io/badge/license-MIT-blue.svg?style=flat)](LICENSE)
[![Coverage Status](https://coveralls.io/repos/github/maranqz/go-factory-lint/badge.svg?branch=main)](https://coveralls.io/github/maranqz/go-factory-lint?branch=main)
[![Coverage Status](https://coveralls.io/repos/github/maranqz/gofactory/badge.svg?branch=main)](https://coveralls.io/github/maranqz/gofactory?branch=main)

The linter checks that the Structures are created by the Factory, and not directly.

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
go install github.com/maranqz/gofactory/cmd/gofactory@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).
- `--packageGlobsOnly` – use a factory to initiate a structure for glob packages only,
[tests](testdata/src/factory/packageGlobsOnly). Doesn't make sense without `--packageGlobs`.

## Example

Expand Down Expand Up @@ -128,6 +130,7 @@ Linter doesn't catch some cases.
2. Local initialization, [example](testdata/src/factory/unimplemented/local/).
3. Named return. If you want to block that case, you can use [nonamedreturns](https://github.com/firefart/nonamedreturns) linter, [example](testdata/src/factory/unimplemented/named_return.go).
4. var declaration, `var initilized nested.Struct` gives structure without factory, [example](testdata/src/factory/unimplemented/var.go).
To block that case, you can use [gopublicfield](github.com/maranqz/gopublicfield) to prevent fill of structure fields.

## TODO

Expand Down
4 changes: 2 additions & 2 deletions cmd/go-factory-lint/main.go → cmd/gofactory/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ package main
import (
"golang.org/x/tools/go/analysis/singlechecker"

"github.com/maranqz/go-factory-lint/v2"
"github.com/maranqz/gofactory"
)

func main() {
singlechecker.Main(factory.NewAnalyzer())
singlechecker.Main(gofactory.NewAnalyzer())
}
79 changes: 17 additions & 62 deletions factory.go
Original file line number Diff line number Diff line change
@@ -1,61 +1,38 @@
package factory
package gofactory

import (
"fmt"
"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
pkgGlobs globsFlag
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},
Name: name,
maranqz marked this conversation as resolved.
Show resolved Hide resolved
Doc: doc,
}

cfg := config{}

analyzer.Flags.Var(&cfg.pkgGlobs, "packageGlobs", packageGlobsDesc)

analyzer.Flags.BoolVar(&cfg.onlyPkgGlobs, "onlyPackageGlobs", false, onlyPkgGlobsDesc)
analyzer.Flags.BoolVar(&cfg.onlyPkgGlobs, "packageGlobsOnly", false, onlyPkgGlobsDesc)

analyzer.Run = run(&cfg)

Expand All @@ -65,17 +42,14 @@ func NewAnalyzer() *analysis.Analyzer {
func run(cfg *config) func(pass *analysis.Pass) (interface{}, error) {
return func(pass *analysis.Pass) (interface{}, error) {
var blockedStrategy blockedStrategy = newAnotherPkg()
if len(cfg.pkgGlobs) > 0 {

pkgGlobs := cfg.pkgGlobs.Value()
if len(pkgGlobs) > 0 {
defaultStrategy := blockedStrategy
if cfg.onlyPkgGlobs {
defaultStrategy = newNilPkg()
}

pkgGlobs, err := compileGlobs(cfg.pkgGlobs.Value())
if err != nil {
return nil, err
}

blockedStrategy = newBlockedPkgs(
pkgGlobs,
defaultStrategy,
Expand Down Expand Up @@ -231,18 +205,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()),
"Use factory for %s.%s", obj.Pkg().Name(), obj.Name(),
maranqz marked this conversation as resolved.
Show resolved Hide resolved
)
}

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 @@ -255,18 +225,3 @@ func containsMatchGlob(globs []glob.Glob, el string) bool {

return false
}

func compileGlobs(globs []string) ([]glob.Glob, error) {
compiledGlobs := make([]glob.Glob, len(globs))

for idx, globString := range globs {
glob, err := glob.Compile(globString)
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

compiledGlobs[idx] = glob
}

return compiledGlobs, nil
}
35 changes: 35 additions & 0 deletions globs_flag.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package gofactory

import (
"fmt"
"strings"

"github.com/gobwas/glob"
)

type globsFlag struct {
globsString []string
globs []glob.Glob
}

func (g globsFlag) String() string {
return strings.Join(g.globsString, ", ")
}

func (g *globsFlag) Set(globString string) error {
globString = strings.TrimSpace(globString)

compiled, err := glob.Compile(globString)
if err != nil {
return fmt.Errorf("unable to compile globs %s: %w", globString, err)
}

g.globsString = append(g.globsString, globString)
g.globs = append(g.globs, compiled)

return nil
}

func (g globsFlag) Value() []glob.Glob {
return g.globs
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module github.com/maranqz/go-factory-lint/v2
module github.com/maranqz/gofactory

go 1.20

Expand Down
35 changes: 16 additions & 19 deletions lint_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package factory_test
package gofactory_test

import (
"path/filepath"
Expand All @@ -7,7 +7,7 @@ import (
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/analysistest"

"github.com/maranqz/go-factory-lint/v2"
"github.com/maranqz/gofactory"
)

func TestLinterSuite(t *testing.T) {
Expand All @@ -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) {
if err := a.Flags.Set("packageGlobs", "factory/onlyPackageGlobs/blocked/**"); err != nil {
t.Fatal(err)
"packageGlobsOnly": {
pkgs: []string{"packageGlobsOnly/main/..."},
prepare: func(t *testing.T, a *analysis.Analyzer) error {
if err := a.Flags.Set("packageGlobs", "factory/packageGlobsOnly/blocked/**"); err != nil {
return err
}

if err := a.Flags.Set("onlyPackageGlobs", "true"); err != nil {
t.Fatal(err)
}
return a.Flags.Set("packageGlobsOnly", "true")
},
},
}
Expand All @@ -55,14 +51,15 @@ func TestLinterSuite(t *testing.T) {
dirs = append(dirs, filepath.Join(testdata, "src", "factory", pkg))
}

analyzer := factory.NewAnalyzer()
analyzer := gofactory.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...)
})
}
}
2 changes: 1 addition & 1 deletion strategy.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package factory
package gofactory

import (
"go/types"
Expand Down
2 changes: 1 addition & 1 deletion testdata/src/factory/packageGlobs/nested/nested.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package nested

import (
"factory/onlyPackageGlobs/blocked"
"factory/packageGlobsOnly/blocked"
)

type Struct struct{}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package blocked

import (
"factory/onlyPackageGlobs/blocked/blocked_nested"
"factory/packageGlobsOnly/blocked/blocked_nested"
)

type Struct struct{}
Expand Down
Loading
Loading