diff --git a/.gitignore b/.gitignore index 7c7b277..0552de8 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ +nit + *.swp _tools - -nitpicking diff --git a/example/example.go b/example/example.go index 7825dcf..94e0876 100644 --- a/example/example.go +++ b/example/example.go @@ -36,6 +36,6 @@ func Something() { fmt.Println("vim-go") ll.Println("hi") - fmt.Println(nit.Section(0)) + fmt.Println(nit.FileSection(0)) fmt.Println(errors.New("bla")) } diff --git a/file_sections.go b/file_sections.go new file mode 100644 index 0000000..128f459 --- /dev/null +++ b/file_sections.go @@ -0,0 +1,303 @@ +package nit + +import ( + "fmt" + "go/ast" + "go/token" + + "github.com/pkg/errors" +) + +type ( + // FileSection represents a code section. + FileSection uint8 + + // FileSectionMachine represents the state machine determining the right + // order for the sections in file. + FileSectionMachine struct { + current FileSectionTransition + } + + // FileSectionTransition represents one of the 6 valid sections in the + // current file, it defines the State Machine transition rules for that + // concrete section, the order to be expected is: + // "Imports" -> "Types" -> "Consts" -> "Vars" -> "Funcs" -> "Methods" + FileSectionTransition interface { + Imports() (FileSectionTransition, error) + Types() (FileSectionTransition, error) + Consts() (FileSectionTransition, error) + Vars() (FileSectionTransition, error) + Funcs() (FileSectionTransition, error) + Methods() (FileSectionTransition, error) + } + + constsFileSection struct{} + funcsFileSection struct{} + importsFileSection struct{} + methodsFileSection struct{} + typesFileSection struct{} + varsFileSection struct{} +) + +const ( + // FileSectionImports defines the `import` state. + FileSectionImports FileSection = iota // FIXME kill + + // FileSectionTypes defines the `type` state. + FileSectionTypes + + // FileSectionConsts defines the `const` state. + FileSectionConsts + + // FileSectionVars defines the `var` state. + FileSectionVars + + // FileSectionFuncs defines the `func` state. + FileSectionFuncs + + // SectionMethods defines the `method` state. + FileSectionMethods +) + +// NewFileSectionTransition returns a new transition corresponding to the +// received value. +func NewFileSectionMachine(start FileSection) (*FileSectionMachine, error) { + // FIXME: implement tests + c, err := NewFileSectionTransition(start) + if err != nil { + return nil, err + } + return &FileSectionMachine{current: c}, nil +} + +// NewFileSectionTransition returns a new transition corresponding to the +// received value. +func NewFileSectionTransition(s FileSection) (FileSectionTransition, error) { + switch s { + case FileSectionConsts: + return constsFileSection{}, nil + case FileSectionFuncs: + return funcsFileSection{}, nil + case FileSectionImports: + return importsFileSection{}, nil + case FileSectionMethods: + return methodsFileSection{}, nil + case FileSectionTypes: + return typesFileSection{}, nil + case FileSectionVars: + return varsFileSection{}, nil + } + + return nil, errors.New("invalid file section value") +} + +// NewFuncDeclFileSection returns a new State that matches the decl type. +func NewFuncDeclFileSection(decl *ast.FuncDecl) (FileSection, error) { + if decl.Recv == nil { + return FileSectionFuncs, nil + } + return FileSectionMethods, nil +} + +// NewGenDeclFileSection returns a new State that matches the decl type. +func NewGenDeclFileSection(decl *ast.GenDecl) (FileSection, error) { + switch decl.Tok { + case token.IMPORT: + return FileSectionImports, nil + case token.CONST: + return FileSectionConsts, nil + case token.TYPE: + return FileSectionTypes, nil + case token.VAR: + return FileSectionVars, nil + } + + return FileSectionImports, fmt.Errorf("unknown generic declaration node") +} + +// Transition updates the internal state. +func (v *FileSectionMachine) Transition(next FileSection) error { //nolint:gocyclo + var ( + res FileSectionTransition + err error + ) + + switch next { + case FileSectionImports: + res, err = v.current.Imports() + case FileSectionTypes: + res, err = v.current.Types() + case FileSectionConsts: + res, err = v.current.Consts() + case FileSectionVars: + res, err = v.current.Vars() + case FileSectionFuncs: + res, err = v.current.Funcs() + case FileSectionMethods: + res, err = v.current.Methods() + default: + err = errors.Errorf("invalid file section value: %d", next) + } + if err != nil { + return err + } + + v.current = res + return nil +} + +//- + +func (constsFileSection) Consts() (FileSectionTransition, error) { + return constsFileSection{}, nil +} + +func (constsFileSection) Funcs() (FileSectionTransition, error) { + return funcsFileSection{}, nil +} + +func (constsFileSection) Imports() (FileSectionTransition, error) { + return nil, errors.New("imports is invalid, next one must be vars, funcs or methods") +} + +func (constsFileSection) Methods() (FileSectionTransition, error) { + return methodsFileSection{}, nil +} + +func (constsFileSection) Types() (FileSectionTransition, error) { + return nil, errors.New("types is invalid, next one must be vars, funcs or methods") +} + +func (constsFileSection) Vars() (FileSectionTransition, error) { + return varsFileSection{}, nil +} + +//- + +func (funcsFileSection) Consts() (FileSectionTransition, error) { + return nil, errors.New("const is invalid, next one must be funcs or methods") +} + +func (funcsFileSection) Funcs() (FileSectionTransition, error) { + return funcsFileSection{}, nil +} + +func (funcsFileSection) Imports() (FileSectionTransition, error) { + return nil, errors.New("imports is invalid, next one must be funcs or methods") +} + +func (funcsFileSection) Methods() (FileSectionTransition, error) { + return methodsFileSection{}, nil +} + +func (funcsFileSection) Types() (FileSectionTransition, error) { + return nil, errors.New("type is invalid, next one must be funcs or methods") +} + +func (funcsFileSection) Vars() (FileSectionTransition, error) { + return nil, errors.New("vars is invalid, next one must be funcs or methods") +} + +//- + +func (importsFileSection) Consts() (FileSectionTransition, error) { + return constsFileSection{}, nil +} + +func (importsFileSection) Funcs() (FileSectionTransition, error) { + return funcsFileSection{}, nil +} + +func (importsFileSection) Imports() (FileSectionTransition, error) { + return importsFileSection{}, nil +} + +func (importsFileSection) Methods() (FileSectionTransition, error) { + return methodsFileSection{}, nil +} + +func (importsFileSection) Types() (FileSectionTransition, error) { + return typesFileSection{}, nil +} + +func (importsFileSection) Vars() (FileSectionTransition, error) { + return varsFileSection{}, nil +} + +//- + +func (methodsFileSection) Consts() (FileSectionTransition, error) { + return nil, errors.New("consts is invalid, next one must be methods") +} + +func (methodsFileSection) Funcs() (FileSectionTransition, error) { + return nil, errors.New("funcs is invalid, next one must be methods") +} + +func (methodsFileSection) Imports() (FileSectionTransition, error) { + return nil, errors.New("imports is invalid, next one must be methods") +} + +func (methodsFileSection) Methods() (FileSectionTransition, error) { + return methodsFileSection{}, nil +} + +func (methodsFileSection) Types() (FileSectionTransition, error) { + return nil, errors.New("types is invalid, next one must be methods") +} + +func (methodsFileSection) Vars() (FileSectionTransition, error) { + return nil, errors.New("vars is invalid, next one must be methods") +} + +//- + +func (typesFileSection) Consts() (FileSectionTransition, error) { + return constsFileSection{}, nil +} + +func (typesFileSection) Funcs() (FileSectionTransition, error) { + return funcsFileSection{}, nil +} + +func (typesFileSection) Imports() (FileSectionTransition, error) { + return nil, errors.New("imports is invalid, next one must be const, vars, funcs or method") +} + +func (typesFileSection) Methods() (FileSectionTransition, error) { + return methodsFileSection{}, nil +} + +func (typesFileSection) Types() (FileSectionTransition, error) { + return typesFileSection{}, nil +} + +func (typesFileSection) Vars() (FileSectionTransition, error) { + return varsFileSection{}, nil +} + +//- + +func (varsFileSection) Consts() (FileSectionTransition, error) { + return nil, errors.New("types is invalid, next one must be func or methods") +} + +func (varsFileSection) Funcs() (FileSectionTransition, error) { + return funcsFileSection{}, nil +} + +func (varsFileSection) Imports() (FileSectionTransition, error) { + return nil, errors.New("types is invalid, next one must be func or methods") +} + +func (varsFileSection) Methods() (FileSectionTransition, error) { + return methodsFileSection{}, nil +} + +func (varsFileSection) Types() (FileSectionTransition, error) { + return nil, errors.New("types is invalid, next one must be func or methods") +} + +func (varsFileSection) Vars() (FileSectionTransition, error) { + return varsFileSection{}, nil +} diff --git a/file_sections_test.go b/file_sections_test.go new file mode 100644 index 0000000..db8f015 --- /dev/null +++ b/file_sections_test.go @@ -0,0 +1,351 @@ +package nit_test + +import ( + "testing" + + "github.com/MarioCarrion/nit" +) + +func TestNewFileSectionMachine(t *testing.T) { + _, err := nit.NewFileSectionMachine(nit.FileSection(99)) + if err == nil { + t.Fatalf("expected error, got nil") + } +} + +func TestNewFileSectionTransition(t *testing.T) { + _, err := nit.NewFileSectionTransition(nit.FileSection(99)) + if err == nil { + t.Fatalf("expected error, got nil") + } +} + +func TestFileSectionMachine(t *testing.T) { + s := nit.FileSectionMachine{} + + if err := s.Transition(nit.FileSection(99)); err == nil { + t.Fatalf("expected error, got nil") + } +} + +//- + +func TestConstsFileSectionTransition(t *testing.T) { + i, _ := nit.NewFileSectionTransition(nit.FileSectionConsts) + + tests := [...]struct { + name string + transition func() (nit.FileSectionTransition, error) + expectedError bool + }{ + { + "Consts", + i.Consts, + false, + }, + { + "Funcs", + i.Funcs, + false, + }, + { + "Imports", + i.Imports, + true, + }, + { + "Methods", + i.Methods, + false, + }, + { + "Types", + i.Types, + true, + }, + { + "Vars", + i.Vars, + false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(ts *testing.T) { + tr, err := tt.transition() + if (err != nil) != tt.expectedError { + ts.Fatalf("expected error %t, got %s", tt.expectedError, err) + } + if err != nil && tr != nil { + ts.Fatalf("expected nil transition on error") + } + }) + } +} + +func TestFuncsFileSectionTransition(t *testing.T) { + i, _ := nit.NewFileSectionTransition(nit.FileSectionFuncs) + + tests := [...]struct { + name string + transition func() (nit.FileSectionTransition, error) + expectedError bool + }{ + { + "Consts", + i.Consts, + true, + }, + { + "Funcs", + i.Funcs, + false, + }, + { + "Imports", + i.Imports, + true, + }, + { + "Methods", + i.Methods, + false, + }, + { + "Types", + i.Types, + true, + }, + { + "Vars", + i.Vars, + true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(ts *testing.T) { + tr, err := tt.transition() + if (err != nil) != tt.expectedError { + ts.Fatalf("expected error %t, got %s", tt.expectedError, err) + } + if err != nil && tr != nil { + ts.Fatalf("expected nil transition on error") + } + }) + } +} + +func TestImportsFileSectionTransition(t *testing.T) { + i, _ := nit.NewFileSectionTransition(nit.FileSectionImports) + + tests := [...]struct { + name string + transition func() (nit.FileSectionTransition, error) + expectedError bool + }{ + { + "Consts", + i.Consts, + false, + }, + { + "Funcs", + i.Funcs, + false, + }, + { + "Imports", + i.Imports, + false, + }, + { + "Methods", + i.Methods, + false, + }, + { + "Types", + i.Types, + false, + }, + { + "Vars", + i.Vars, + false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(ts *testing.T) { + tr, err := tt.transition() + if (err != nil) != tt.expectedError { + ts.Fatalf("expected error %t, got %s", tt.expectedError, err) + } + if err != nil && tr != nil { + ts.Fatalf("expected nil transition on error") + } + }) + } +} + +func TestMethodsFileSectionTransition(t *testing.T) { + i, _ := nit.NewFileSectionTransition(nit.FileSectionMethods) + + tests := [...]struct { + name string + transition func() (nit.FileSectionTransition, error) + expectedError bool + }{ + { + "Consts", + i.Consts, + true, + }, + { + "Funcs", + i.Funcs, + true, + }, + { + "Imports", + i.Imports, + true, + }, + { + "Methods", + i.Methods, + false, + }, + { + "Types", + i.Types, + true, + }, + { + "Vars", + i.Vars, + true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(ts *testing.T) { + tr, err := tt.transition() + if (err != nil) != tt.expectedError { + ts.Fatalf("expected error %t, got %s", tt.expectedError, err) + } + if err != nil && tr != nil { + ts.Fatalf("expected nil transition on error") + } + }) + } +} + +func TestTypesFileSectionTransition(t *testing.T) { + i, _ := nit.NewFileSectionTransition(nit.FileSectionTypes) + + tests := [...]struct { + name string + transition func() (nit.FileSectionTransition, error) + expectedError bool + }{ + { + "Consts", + i.Consts, + false, + }, + { + "Funcs", + i.Funcs, + false, + }, + { + "Imports", + i.Imports, + true, + }, + { + "Methods", + i.Methods, + false, + }, + { + "Types", + i.Types, + false, + }, + { + "Vars", + i.Vars, + false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(ts *testing.T) { + tr, err := tt.transition() + if (err != nil) != tt.expectedError { + ts.Fatalf("expected error %t, got %s", tt.expectedError, err) + } + if err != nil && tr != nil { + ts.Fatalf("expected nil transition on error") + } + }) + } +} + +func TestVarsFileSectionTransition(t *testing.T) { + i, _ := nit.NewFileSectionTransition(nit.FileSectionVars) + + tests := [...]struct { + name string + transition func() (nit.FileSectionTransition, error) + expectedError bool + }{ + { + "Consts", + i.Consts, + true, + }, + { + "Funcs", + i.Funcs, + false, + }, + { + "Imports", + i.Imports, + true, + }, + { + "Methods", + i.Methods, + false, + }, + { + "Types", + i.Types, + true, + }, + { + "Vars", + i.Vars, + false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(ts *testing.T) { + tr, err := tt.transition() + if (err != nil) != tt.expectedError { + ts.Fatalf("expected error %t, got %s", tt.expectedError, err) + } + if err != nil && tr != nil { + ts.Fatalf("expected nil transition on error") + } + }) + } +} + +//- diff --git a/imports.go b/imports.go new file mode 100644 index 0000000..c083a61 --- /dev/null +++ b/imports.go @@ -0,0 +1,229 @@ +package nit + +import ( + "go/ast" + "go/token" + "strings" + + "github.com/pkg/errors" +) + +type ( + // ImportsSection represents one of the 3 valid `imports` sections. + ImportsSection uint8 + + // ImportsSectionMachine represents the `imports` code organization state + // machine. + ImportsSectionMachine struct { + current ImportsTransition + previous ImportsTransition + } + + // ImportsTransition represents one of the 3 valid `imports` sections, it + // defines the State Machine transition rules for that concrete section, + // the order to be expected is: "Standard" -> "External" -> "Local". + ImportsTransition interface { + External() (ImportsTransition, error) + Local() (ImportsTransition, error) + Standard() (ImportsTransition, error) + } + + // ImportsValidator defines the type including the rules used for validating + // the `imports` section as a whole. + ImportsValidator struct { + localPath string + fsm *ImportsSectionMachine + } + + externalImportsTransition struct{} + localImportsTransition struct{} + standardImportsTransition struct{} +) + +const ( + // ImportsSectionStd represents the Standard Library Packages `imports` section. + ImportsSectionStd ImportsSection = iota + + // ImportsSectionExternal represents the External Packages `imports` section. + ImportsSectionExternal + + // ImportsSectionLocal represents the local Packages `imports` section. + ImportsSectionLocal +) + +// NewImportsSection returns the value representing the corresponding Imports +// section. +func NewImportsSection(path, localPathPrefix string) ImportsSection { + if !strings.Contains(path, ".") { + return ImportsSectionStd + } + if strings.HasPrefix(strings.Replace(path, "\"", "", -1), localPathPrefix) { + return ImportsSectionLocal + } + return ImportsSectionExternal +} + +// NewImportsValidator returns a new instalce of ImporstValidator with the +// local path prefix set. +func NewImportsValidator(localPath string) ImportsValidator { + return ImportsValidator{localPath: localPath} +} + +// NewImportsSectionMachine returns a new ImportsSectionMachine with the +// initial state as `start`. +func NewImportsSectionMachine(start ImportsSection) (*ImportsSectionMachine, error) { + // FIXME: implement tests + c, err := NewImportsTransition(start) + if err != nil { + return nil, err + } + return &ImportsSectionMachine{current: c}, nil +} + +// NewImportsTransition returns a new transition corresponding to the received +// value. +func NewImportsTransition(s ImportsSection) (ImportsTransition, error) { + switch s { + case ImportsSectionStd: + return standardImportsTransition{}, nil + case ImportsSectionExternal: + return externalImportsTransition{}, nil + case ImportsSectionLocal: + return localImportsTransition{}, nil + } + return nil, errors.New("invalid imports value") +} + +//- + +// Current returns the current state. +func (s *ImportsSectionMachine) Current() ImportsTransition { + return s.current +} + +// Previous returns the previous state. +func (s *ImportsSectionMachine) Previous() ImportsTransition { + return s.previous +} + +// Transition updates the internal state. +func (s *ImportsSectionMachine) Transition(next ImportsSection) error { + var ( + res ImportsTransition + err error + ) + + switch next { + case ImportsSectionStd: + res, err = s.current.Standard() + case ImportsSectionExternal: + res, err = s.current.External() + case ImportsSectionLocal: + res, err = s.current.Local() + default: + err = errors.Errorf("invalid imports value: %d", next) + } + if err != nil { + return err + } + + s.previous = s.current + s.current = res + return nil +} + +//- + +// Validate makes sure the implemented `imports` declaration satisfies the +// following rules: +// * Group declaration is parenthesized +// * Packages are separated by a breaking line like this: +// * First standard packages, +// * Next external packages, and +// * Finally local packages +func (i *ImportsValidator) Validate(v *ast.GenDecl, fset *token.FileSet) error { + if !v.Lparen.IsValid() { + return errors.Wrap(errors.New("expected parenthesized declaration"), fset.PositionFor(v.Pos(), false).String()) + } + + lastLine := fset.PositionFor(v.Pos(), false).Line + + for _, t := range v.Specs { + errPrefix := fset.PositionFor(t.Pos(), false).String() + + s, ok := t.(*ast.ImportSpec) + if !ok { + return errors.Wrap(errors.Errorf("invalid token %+v", t), errPrefix) + } + + section := NewImportsSection(s.Path.Value, i.localPath) + if i.fsm == nil { + fsm, err := NewImportsSectionMachine(section) + if err != nil { + return errors.Wrap(errors.Errorf("invalid imports found: %s", err), errPrefix) + } + i.fsm = fsm + } + if err := i.fsm.Transition(section); err != nil { + return errors.Wrap(err, errPrefix) + } + + newLine := fset.PositionFor(t.Pos(), false).Line + + if i.fsm.Current() == i.fsm.Previous() { + if lastLine+1 != newLine { + return errors.Wrap(errors.New("extra line break in section"), errPrefix) + } + } else { + if lastLine+1 == newLine { + return errors.Wrap(errors.New("missing line break in section"), errPrefix) + } + } + + lastLine = newLine + } + + return nil +} + +//- + +func (externalImportsTransition) External() (ImportsTransition, error) { + return externalImportsTransition{}, nil +} + +func (externalImportsTransition) Local() (ImportsTransition, error) { + return localImportsTransition{}, nil +} + +func (externalImportsTransition) Standard() (ImportsTransition, error) { + return nil, errors.New("standard is invalid, next one must be external or local.") +} + +//- + +func (localImportsTransition) External() (ImportsTransition, error) { + return nil, errors.New("external is invalid, next one must be local") +} + +func (localImportsTransition) Local() (ImportsTransition, error) { + return localImportsTransition{}, nil +} + +func (localImportsTransition) Standard() (ImportsTransition, error) { + return nil, errors.New("standard is invalid, next one must be local") +} + +//- + +func (standardImportsTransition) External() (ImportsTransition, error) { + return externalImportsTransition{}, nil +} + +func (standardImportsTransition) Local() (ImportsTransition, error) { + return localImportsTransition{}, nil +} + +func (standardImportsTransition) Standard() (ImportsTransition, error) { + return standardImportsTransition{}, nil +} diff --git a/imports_test.go b/imports_test.go new file mode 100644 index 0000000..310163c --- /dev/null +++ b/imports_test.go @@ -0,0 +1,245 @@ +package nit_test + +import ( + "go/ast" + "go/token" + "testing" + + "github.com/MarioCarrion/nit" +) + +func TestNewImportsSection(t *testing.T) { + tests := [...]struct { + name string + inputPath string + inputLocaBasePrefix string + expected nit.ImportsSection + }{ + { + "ImportsSectionStd", + "fmt", + "github.com/MarioCarrion/nit", + nit.ImportsSectionStd, + }, + { + "ImportsSectionExternal", + "github.com/golang/go", + "github.com/MarioCarrion/nit", + nit.ImportsSectionExternal, + }, + { + "ImportsSectionLocal", + "github.com/MarioCarrion/nit/something", + "github.com/MarioCarrion/nit", + nit.ImportsSectionLocal, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(ts *testing.T) { + if actual := nit.NewImportsSection(tt.inputPath, tt.inputLocaBasePrefix); actual != tt.expected { + ts.Fatalf("expected %+v, actual %+v", tt.expected, actual) + } + }) + } +} + +//nolint:dupl +func TestImportsValidator(t *testing.T) { + tests := [...]struct { + name string + filename string + expectedError bool + }{ + { + "OK", + "imports_valid.go", + false, + }, + { + "Error: parenthesized declaration", + "imports_paren.go", + true, + }, + { + "Error: extra line", + "imports_extra_line.go", + true, + }, + { + "Error: missing line", + "imports_missing_line.go", + true, + }, + { + "Error: invalid group", + "imports_invalid_group.go", + true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(ts *testing.T) { + f, fset, err := newParserFile(ts, tt.filename) + if err != nil { + ts.Fatalf("expected no error, got %s", err) + } + + for _, s := range f.Decls { + switch g := s.(type) { + case *ast.GenDecl: + if g.Tok == token.IMPORT { + validator := nit.NewImportsValidator("github.com/MarioCarrion") + if err := validator.Validate(g, fset); tt.expectedError != (err != nil) { + ts.Fatalf("expected error %t, got %s", tt.expectedError, err) + } + break + } + } + } + }) + } +} + +func TestNewImportsTransition(t *testing.T) { + tests := [...]struct { + name string + input nit.ImportsSection + expectedError bool + }{ + { + "ImportsSectionStd", + nit.ImportsSectionStd, + false, + }, + { + "ImportsSectionExternal", + nit.ImportsSectionExternal, + false, + }, + { + "ImportsSectionLocal", + nit.ImportsSectionLocal, + false, + }, + { + "ImportsSectionLocal", + nit.ImportsSection(3), + true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(ts *testing.T) { + _, err := nit.NewImportsTransition(tt.input) + if (err != nil) != tt.expectedError { + ts.Fatalf("expected error %t, got %s", tt.expectedError, err) + } + }) + } +} + +func TestImportsTransitionExternal(t *testing.T) { + i, _ := nit.NewImportsTransition(nit.ImportsSectionExternal) + + tests := [...]struct { + name string + transition func() (nit.ImportsTransition, error) + expectedError bool + }{ + { + "External", + i.External, + false, + }, + { + "Local", + i.Local, + false, + }, + { + "Standard", + i.Standard, + true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(ts *testing.T) { + _, err := tt.transition() + if (err != nil) != tt.expectedError { + ts.Fatalf("expected error %t, got %s", tt.expectedError, err) + } + }) + } +} + +func TestImportsTransitionLocal(t *testing.T) { + i, _ := nit.NewImportsTransition(nit.ImportsSectionLocal) + + tests := [...]struct { + name string + transition func() (nit.ImportsTransition, error) + expectedError bool + }{ + { + "External", + i.External, + true, + }, + { + "Local", + i.Local, + false, + }, + { + "Standard", + i.Standard, + true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(ts *testing.T) { + _, err := tt.transition() + if (err != nil) != tt.expectedError { + ts.Fatalf("expected error %t, got %s", tt.expectedError, err) + } + }) + } +} + +func TestImportsTransitionStd(t *testing.T) { + i, _ := nit.NewImportsTransition(nit.ImportsSectionStd) + + tests := [...]struct { + name string + transition func() (nit.ImportsTransition, error) + expectedError bool + }{ + { + "External", + i.External, + false, + }, + { + "Local", + i.Local, + false, + }, + { + "Standard", + i.Standard, + false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(ts *testing.T) { + _, err := tt.transition() + if (err != nil) != tt.expectedError { + ts.Fatalf("expected error %t, got %s", tt.expectedError, err) + } + }) + } +} diff --git a/nit_test.go b/nit_test.go new file mode 100644 index 0000000..dcdc914 --- /dev/null +++ b/nit_test.go @@ -0,0 +1,21 @@ +package nit_test + +import ( + "go/ast" + "go/parser" + "go/token" + "path/filepath" + "testing" +) + +func newParserFile(t *testing.T, name string) (*ast.File, *token.FileSet, error) { + t.Helper() + + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, filepath.Join("testdata", name), nil, parser.ParseComments) + if err != nil { + t.Fatalf("expected no error, got %s", err) + } + + return f, fset, nil +} diff --git a/nitpicking.go b/nitpicking.go index 7cd6f4b..77a4979 100644 --- a/nitpicking.go +++ b/nitpicking.go @@ -20,7 +20,7 @@ type ( Nitpicker struct { LocalPath string fset *token.FileSet - fsm SectionMachine + fsm *FileSectionMachine fvalidator *FuncsValidator } ) @@ -73,16 +73,16 @@ func (v *Nitpicker) validateToken(d ast.Decl) error { err error genDecl *ast.GenDecl funcDecl *ast.FuncDecl - nextState Section + nextState FileSection ) switch t := d.(type) { case *ast.GenDecl: genDecl = t - nextState, err = NewGenDeclSection(genDecl) + nextState, err = NewGenDeclFileSection(genDecl) case *ast.FuncDecl: funcDecl = t - nextState, err = NewFuncDeclSection(funcDecl) + nextState, err = NewFuncDeclFileSection(funcDecl) default: return errors.New("unknown declaration state") } @@ -90,32 +90,40 @@ func (v *Nitpicker) validateToken(d ast.Decl) error { return err } + if v.fsm == nil { + fsm, err := NewFileSectionMachine(nextState) + if err != nil { + return errors.Wrap(err, v.fset.PositionFor(d.Pos(), false).String()) + } + v.fsm = fsm + } + if err = v.fsm.Transition(nextState); err != nil { return errors.Wrap(err, v.fset.PositionFor(d.Pos(), false).String()) } switch nextState { - case SectionImports: - validator := &ImportsValidator{LocalPath: v.LocalPath} + case FileSectionImports: + validator := NewImportsValidator(v.LocalPath) if err := validator.Validate(genDecl, v.fset); err != nil { return err } - case SectionTypes: + case FileSectionTypes: validator := &TypesValidator{} if err := validator.Validate(genDecl, v.fset); err != nil { return err } - case SectionConsts: + case FileSectionConsts: validator := &ConstsValidator{} if err := validator.Validate(genDecl, v.fset); err != nil { return err } - case SectionVars: + case FileSectionVars: validator := &VarsValidator{} if err := validator.Validate(genDecl, v.fset); err != nil { return err } - case SectionFuncs: + case FileSectionFuncs: if err := v.fvalidator.Validate(funcDecl, v.fset); err != nil { return err } diff --git a/section.go b/section.go deleted file mode 100644 index 21bbbe8..0000000 --- a/section.go +++ /dev/null @@ -1,77 +0,0 @@ -package nit - -import ( - "fmt" - "go/ast" - "go/token" - "strings" -) - -type ( - // ImportsSection represents an `imports` section - ImportsSection uint8 - - // Section represents a code section. - Section uint8 -) - -const ( - // SectionStart defines the initial State in the validator. - SectionStart Section = iota - // SectionImports defines the `import` state. - SectionImports - // SectionTypes defines the `type` state. - SectionTypes - // SectionConsts defines the `const` state. - SectionConsts - // SectionVars defines the `var` state. - SectionVars - // SectionFuncs defines the `func` state. - SectionFuncs - // SectionMethods defines the `method` state. - SectionMethods -) - -const ( - // ImportsSectionStd represents the Standard Library Packages `imports` section. - ImportsSectionStd ImportsSection = iota - // ImportsSectionExternal represents the External Packages `imports` section. - ImportsSectionExternal - // ImportsSectionLocal represents the local Packages `imports` section. - ImportsSectionLocal -) - -// NewFuncDeclSection returns a new State that matches the decl type. -func NewFuncDeclSection(decl *ast.FuncDecl) (Section, error) { - if decl.Recv == nil { - return SectionFuncs, nil - } - return SectionMethods, nil -} - -// NewGenDeclSection returns a new State that matches the decl type. -func NewGenDeclSection(decl *ast.GenDecl) (Section, error) { - switch decl.Tok { - case token.IMPORT: - return SectionImports, nil - case token.CONST: - return SectionConsts, nil - case token.TYPE: - return SectionTypes, nil - case token.VAR: - return SectionVars, nil - } - - return SectionStart, fmt.Errorf("unknown generic declaration node") -} - -// NewImportsSection returns a new ImportsSection from the path value. -func NewImportsSection(path, localPathPrefix string) ImportsSection { - if !strings.Contains(path, ".") { - return ImportsSectionStd - } - if strings.HasPrefix(strings.Replace(path, "\"", "", -1), localPathPrefix) { - return ImportsSectionLocal - } - return ImportsSectionExternal -} diff --git a/section_test.go b/section_test.go deleted file mode 100644 index 9073b16..0000000 --- a/section_test.go +++ /dev/null @@ -1,128 +0,0 @@ -package nit_test - -import ( - "go/ast" - "go/token" - "testing" - - "github.com/MarioCarrion/nit" -) - -func TestNewGenDeclSection(t *testing.T) { - tests := [...]struct { - name string - input *ast.GenDecl - expected nit.Section - expectedError bool - }{ - { - "Imports", - &ast.GenDecl{Tok: token.IMPORT}, - nit.SectionImports, - false, - }, - { - "Consts", - &ast.GenDecl{Tok: token.CONST}, - nit.SectionConsts, - false, - }, - { - "Type", - &ast.GenDecl{Tok: token.TYPE}, - nit.SectionTypes, - false, - }, - { - "Vars", - &ast.GenDecl{Tok: token.VAR}, - nit.SectionVars, - false, - }, - { - "Error", - &ast.GenDecl{Tok: token.COMMENT}, - nit.SectionStart, - true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(ts *testing.T) { - actual, err := nit.NewGenDeclSection(tt.input) - if (err != nil) != tt.expectedError { - ts.Fatalf("expected no error, got %s", err) - } - if actual != tt.expected { - ts.Fatalf("expected %+v, got %+v", tt.expected, actual) - } - }) - } -} - -func TestNewFuncDeclSection(t *testing.T) { - tests := [...]struct { - name string - input *ast.FuncDecl - expected nit.Section - }{ - { - "Funcs", - &ast.FuncDecl{}, - nit.SectionFuncs, - }, - { - "Methods", - &ast.FuncDecl{Recv: &ast.FieldList{}}, - nit.SectionMethods, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(ts *testing.T) { - actual, err := nit.NewFuncDeclSection(tt.input) - if err != nil { - ts.Fatalf("expected no error, got %s", err) - } - if actual != tt.expected { - ts.Fatalf("expected %+v, got %+v", tt.expected, actual) - } - }) - } -} - -func TestNewImportsSection(t *testing.T) { - tests := [...]struct { - name string - inputPath string - inputLocaBasePrefix string - expected nit.ImportsSection - }{ - { - "ImportsSectionStd", - "fmt", - "github.com/MarioCarrion/nit", - nit.ImportsSectionStd, - }, - { - "ImportsSectionExternal", - "github.com/golang/go", - "github.com/MarioCarrion/nit", - nit.ImportsSectionExternal, - }, - { - "ImportsSectionLocal", - "github.com/MarioCarrion/nit/something", - "github.com/MarioCarrion/nit", - nit.ImportsSectionLocal, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(ts *testing.T) { - if actual := nit.NewImportsSection(tt.inputPath, tt.inputLocaBasePrefix); actual != tt.expected { - t.Fatalf("expected %+v, actual %+v", tt.expected, actual) - } - }) - } -} diff --git a/state_machine.go b/state_machine.go deleted file mode 100644 index 88003d7..0000000 --- a/state_machine.go +++ /dev/null @@ -1,84 +0,0 @@ -package nit - -import ( - "fmt" -) - -type ( - // ImportsSectionMachine represents the `imports` code organization state machine. - ImportsSectionMachine struct { - current ImportsSection - previous ImportsSection - } - - // SectionMachine represents the default sections code organization state machine. - SectionMachine struct { - current Section - } -) - -// NewImportsSectionMachine returns a new ImportsSectionMachine with the initial state as `start` -func NewImportsSectionMachine(start ImportsSection) *ImportsSectionMachine { - return &ImportsSectionMachine{current: start} -} - -// Current returns the current state. -func (s *ImportsSectionMachine) Current() ImportsSection { - return s.current -} - -// Previous returns the previous state. -func (s *ImportsSectionMachine) Previous() ImportsSection { - return s.previous -} - -// Transition updates the internal state. -func (s *ImportsSectionMachine) Transition(next ImportsSection) error { - switch s.current { - case ImportsSectionStd: - // All sections are supported as next ones - case ImportsSectionExternal: - if next != ImportsSectionExternal && next != ImportsSectionLocal { - return fmt.Errorf("next `imports` must be either external or local") - } - case ImportsSectionLocal: - if next != ImportsSectionLocal { - return fmt.Errorf("next `imports` must be local") - } - } - s.previous = s.current - s.current = next - return nil -} - -// Transition updates the internal state. -func (v *SectionMachine) Transition(next Section) error { //nolint:gocyclo - switch v.current { - case SectionStart: - if next != SectionImports && next != SectionTypes && next != SectionConsts && next != SectionVars && next != SectionFuncs { - return fmt.Errorf("next section is invalid") - } - case SectionImports: - if next != SectionTypes && next != SectionConsts && next != SectionVars && next != SectionFuncs { - return fmt.Errorf("next section must either: `type`, `const`, `var` or funcs/methods") - } - case SectionTypes: - if next != SectionConsts && next != SectionVars && next != SectionFuncs && next != SectionMethods { - return fmt.Errorf("next section must either: `const`, `var` or funcs/methods") - } - case SectionConsts: - if next != SectionConsts && next != SectionVars && next != SectionFuncs && next != SectionMethods { - return fmt.Errorf("next section must either: `const`, `var` or funcs/methods") - } - case SectionVars, SectionFuncs: - if next != SectionFuncs && next != SectionMethods { - return fmt.Errorf("next section must either: funcs or methods") - } - case SectionMethods: - if next != SectionMethods { - return fmt.Errorf("next section must be: methods") - } - } - v.current = next - return nil -} diff --git a/state_machine_test.go b/state_machine_test.go deleted file mode 100644 index 69afec8..0000000 --- a/state_machine_test.go +++ /dev/null @@ -1,243 +0,0 @@ -package nit_test - -import ( - "testing" - - "github.com/MarioCarrion/nit" -) - -func TestImportsSectionMachine_Current(t *testing.T) { - i := nit.NewImportsSectionMachine(nit.ImportsSectionStd) - if i.Current() != nit.ImportsSectionStd { - t.Fatalf("expected current value does not match") - } -} - -func TestImportsSectionMachine_Previous(t *testing.T) { - i := nit.NewImportsSectionMachine(nit.ImportsSectionStd) - if err := i.Transition(nit.ImportsSectionLocal); err != nil { - t.Fatalf("expected no error, got %s", err) - } - - if i.Previous() != nit.ImportsSectionStd { - t.Fatalf("expected current value does not match") - } -} - -func TestImportsSectionMachine_Transition(t *testing.T) { - tests := [...]struct { - name string - startState nit.ImportsSection - validStates []nit.ImportsSection - invalidStates []nit.ImportsSection - }{ - { - "Standard", - nit.ImportsSectionStd, - []nit.ImportsSection{ - nit.ImportsSectionStd, - nit.ImportsSectionExternal, - nit.ImportsSectionLocal, - }, - []nit.ImportsSection{}, - }, - { - "External", - nit.ImportsSectionExternal, - []nit.ImportsSection{ - nit.ImportsSectionExternal, - nit.ImportsSectionLocal, - }, - []nit.ImportsSection{nit.ImportsSectionStd}, - }, - { - "Local", - nit.ImportsSectionLocal, - []nit.ImportsSection{nit.ImportsSectionLocal}, - []nit.ImportsSection{ - nit.ImportsSectionStd, - nit.ImportsSectionExternal, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(ts *testing.T) { - for _, s := range tt.validStates { - machine := nit.NewImportsSectionMachine(tt.startState) - if err := machine.Transition(s); err != nil { - t.Fatalf("expected no error, to %s", err) - } - } - - for _, s := range tt.invalidStates { - machine := nit.NewImportsSectionMachine(tt.startState) - if err := machine.Transition(s); err == nil { - t.Fatalf("expected error, got nil") - } - } - }) - } -} - -//nolint: dupl -func TestSectionMachine_Transition(t *testing.T) { - tests := [...]struct { - name string - factory func() nit.SectionMachine - validStates []nit.Section - invalidStates []nit.Section - }{ - { - "Start", - func() nit.SectionMachine { - return nit.SectionMachine{} - }, - []nit.Section{ - nit.SectionImports, - nit.SectionTypes, - nit.SectionConsts, - nit.SectionVars, - nit.SectionFuncs, - }, - []nit.Section{ - nit.SectionStart, - nit.SectionMethods, - nit.Section(99), - }, - }, - { - "Imports", - func() nit.SectionMachine { - v := nit.SectionMachine{} - v.Transition(nit.SectionImports) - return v - }, - []nit.Section{ - nit.SectionTypes, - nit.SectionConsts, - nit.SectionVars, - nit.SectionFuncs, - }, - []nit.Section{ - nit.SectionStart, - nit.SectionImports, - nit.SectionMethods, - }, - }, - { - "Types", - func() nit.SectionMachine { - v := nit.SectionMachine{} - v.Transition(nit.SectionTypes) - return v - }, - []nit.Section{ - nit.SectionConsts, - nit.SectionVars, - nit.SectionFuncs, - nit.SectionMethods, - }, - []nit.Section{ - nit.SectionStart, - nit.SectionImports, - nit.SectionTypes, - }, - }, - { - "Consts", - func() nit.SectionMachine { - v := nit.SectionMachine{} - v.Transition(nit.SectionConsts) - return v - }, - []nit.Section{ - nit.SectionConsts, - nit.SectionVars, - nit.SectionFuncs, - nit.SectionMethods, - }, - []nit.Section{ - nit.SectionStart, - nit.SectionImports, - nit.SectionTypes, - }, - }, - { - "Vars", - func() nit.SectionMachine { - v := nit.SectionMachine{} - v.Transition(nit.SectionVars) - return v - }, - []nit.Section{ - nit.SectionFuncs, - nit.SectionMethods, - }, - []nit.Section{ - nit.SectionStart, - nit.SectionImports, - nit.SectionTypes, - nit.SectionConsts, - nit.SectionVars, - }, - }, - { - "Funcs", - func() nit.SectionMachine { - v := nit.SectionMachine{} - v.Transition(nit.SectionFuncs) - return v - }, - []nit.Section{ - nit.SectionFuncs, - nit.SectionMethods, - }, - []nit.Section{ - nit.SectionStart, - nit.SectionImports, - nit.SectionTypes, - nit.SectionConsts, - nit.SectionVars, - }, - }, - { - "Methods", - func() nit.SectionMachine { - v := nit.SectionMachine{} - v.Transition(nit.SectionTypes) - v.Transition(nit.SectionMethods) - return v - }, - []nit.Section{ - nit.SectionMethods, - }, - []nit.Section{ - nit.SectionStart, - nit.SectionImports, - nit.SectionTypes, - nit.SectionConsts, - nit.SectionVars, - nit.SectionFuncs, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(ts *testing.T) { - for _, s := range tt.validStates { - validator := tt.factory() - if err := validator.Transition(s); err != nil { - ts.Fatalf("expected no error, got %s", err) - } - } - - for _, s := range tt.invalidStates { - validator := tt.factory() - if err := validator.Transition(s); err == nil { - ts.Fatalf("expected error, got nil") - } - } - }) - } -} diff --git a/validator.go b/validator.go index 56c9025..cfffdf6 100644 --- a/validator.go +++ b/validator.go @@ -21,13 +21,6 @@ type ( Comments *BreakComments } - // ImportsValidator defines the type including the rules used for validating - // the `imports` section. - ImportsValidator struct { - LocalPath string - fsm *ImportsSectionMachine - } - // TypesValidator defines the type including the rules used for validating // the `type` sections. TypesValidator struct { @@ -105,54 +98,6 @@ func (f *FuncsValidator) Validate(v *ast.FuncDecl, fset *token.FileSet) error { return nil } -// Validate makes sure the implemented `imports` declaration satisfies the -// following rules: -// * Group declaration is parenthesized -// * Packages are separated by a breaking line like this: -// * First standard packages, -// * Next external packages, and -// * Finally local packages -func (i *ImportsValidator) Validate(v *ast.GenDecl, fset *token.FileSet) error { - if !v.Lparen.IsValid() { - return errors.Wrap(errors.New("expected parenthesized declaration"), fset.PositionFor(v.Pos(), false).String()) - } - - lastLine := fset.PositionFor(v.Pos(), false).Line - - for _, t := range v.Specs { - errPrefix := fset.PositionFor(t.Pos(), false).String() - - s, ok := t.(*ast.ImportSpec) - if !ok { - return errors.Wrap(errors.Errorf("invalid token %+v", t), errPrefix) - } - - section := NewImportsSection(s.Path.Value, i.LocalPath) - if i.fsm == nil { - i.fsm = NewImportsSectionMachine(section) - } - if err := i.fsm.Transition(section); err != nil { - return errors.Wrap(err, errPrefix) - } - - newLine := fset.PositionFor(t.Pos(), false).Line - - if i.fsm.Current() == i.fsm.Previous() { - if lastLine+1 != newLine { - return errors.Wrap(errors.New("extra line break in section"), errPrefix) - } - } else { - if lastLine+1 == newLine { - return errors.Wrap(errors.New("missing line break in section"), errPrefix) - } - } - - lastLine = newLine - } - - return nil -} - // Validate makes sure the implemented `type` declaration satisfies the // following rules: // * Group declaration is parenthesized diff --git a/validator_test.go b/validator_test.go index e453a16..f6f35df 100644 --- a/validator_test.go +++ b/validator_test.go @@ -2,9 +2,7 @@ package nit_test import ( "go/ast" - "go/parser" "go/token" - "path/filepath" "testing" "github.com/MarioCarrion/nit" @@ -191,63 +189,6 @@ func TestTypesValidator_Validate(t *testing.T) { } } -//nolint:dupl -func TestImportsValidator_Validate(t *testing.T) { - tests := [...]struct { - name string - filename string - expectedError bool - }{ - { - "OK", - "imports_valid.go", - false, - }, - { - "Error: parenthesized declaration", - "imports_paren.go", - true, - }, - { - "Error: extra line", - "imports_extra_line.go", - true, - }, - { - "Error: missing line", - "imports_missing_line.go", - true, - }, - { - "Error: invalid group", - "imports_invalid_group.go", - true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(ts *testing.T) { - f, fset, err := newParserFile(ts, tt.filename) - if err != nil { - ts.Fatalf("expected no error, got %s", err) - } - - for _, s := range f.Decls { - switch g := s.(type) { - case *ast.GenDecl: - if g.Tok == token.IMPORT { - validator := nit.ImportsValidator{LocalPath: "github.com/MarioCarrion"} - if err := validator.Validate(g, fset); tt.expectedError != (err != nil) { - ts.Fatalf("expected error %t, got %s", tt.expectedError, err) - } - break - } - } - } - }) - } -} - //nolint:dupl func TestVarsValidator_Validate(t *testing.T) { tests := [...]struct { @@ -304,17 +245,3 @@ func TestVarsValidator_Validate(t *testing.T) { }) } } - -//- - -func newParserFile(t *testing.T, name string) (*ast.File, *token.FileSet, error) { - t.Helper() - - fset := token.NewFileSet() - f, err := parser.ParseFile(fset, filepath.Join("testdata", name), nil, parser.ParseComments) - if err != nil { - t.Fatalf("expected no error, got %s", err) - } - - return f, fset, nil -}