Skip to content

Commit

Permalink
fix non-alphabetical function-option mismatch
Browse files Browse the repository at this point in the history
fixes switchupcb#29; non-alphabetical output
  • Loading branch information
switchupcb committed Jun 22, 2022
1 parent c7d0408 commit e50ba33
Show file tree
Hide file tree
Showing 8 changed files with 157 additions and 46 deletions.
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ The `setup` file is parsed using an Abstract Syntax Tree. This tree contains the

**Convert** options are defined **outside** of the `type Copygen Interface` and may apply to multiple functions. As a result, all `ast.Comments` must be parsed before `models.Function` and `models.Field` objects can be created. In order to do this, the `type Copygen Interface` is stored, but **NOT** analyzed until the `setup` file is traversed.

There are multiple ways to parse `ast.Comments` into `Options`, but **convert** options require the name of their respective **convert** functions _(which can't be parsed from comments)_. As a result, the most readable, efficient, and least error prone method of parsing `ast.Comments` into `Options `is simply to parse them when discovered; and assign them from a `CommentOptionMap` later. In addition, regex compilation is expensive — [especially in Go](https://github.com/mariomka/regex-benchmark#performance) — and avoided by only compiling unique comments once.
There are multiple ways to parse `ast.Comments` into `Options`, but **convert** options require the name of their respective **convert** functions _(which can't be parsed from comments)_. As a result, the most readable, efficient, and least error prone method of parsing `ast.Comments` into `Options` is to parse them when discovered; and assign them from a `CommentOptionMap` later. In addition, regex compilation is expensive — [especially in Go](https://github.com/mariomka/regex-benchmark#performance) — and avoided by only compiling unique comments once.

#### Copygen Interface

Expand Down Expand Up @@ -130,7 +130,7 @@ If you receive `File is not ... with -...`, use `golangci-lint run --disable-all

### Tests

For information on testing, read [Integration Tests](examples/_tests/).
For information on testing, read [Tests](examples/_tests/).

# Roadmap

Expand Down
41 changes: 12 additions & 29 deletions cli/parser/function.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,38 +13,21 @@ const copygenInterfaceName = "Copygen"

// parseFunctions parses the AST for functions in the setup file.
// astcopygen is used to assign options from *ast.Comments.
func (p *Parser) parseFunctions(astcopygen *ast.InterfaceType) ([]models.Function, error) {

// find the `type Copygen interface` definition in the setup file.
var copygen *types.Interface

setpkg := p.Pkgs[0]
defs := setpkg.TypesInfo.Defs
for k, v := range defs {
if k.Name == copygenInterfaceName {
if it, ok := v.Type().Underlying().(*types.Interface); ok {
copygen = it
}
}
}

if copygen == nil {
return nil, fmt.Errorf("the \"type Copygen interface\" could not be found in the setup file's package")
}

if copygen.NumMethods() == 0 {
func (p *Parser) parseFunctions(copygen *ast.InterfaceType) ([]models.Function, error) {
numMethods := len(copygen.Methods.List)
if numMethods == 0 {
fmt.Println("WARNING: no functions are defined in the \"type Copygen interface\"")
}

// create the models.Function objects
functions := make([]models.Function, copygen.NumMethods())
for i := 0; i < copygen.NumMethods(); i++ {
method := copygen.Method(i)
// create models.Function objects.
functions := make([]models.Function, numMethods)
for i := 0; i < numMethods; i++ {
method := p.Config.SetupPkg.TypesInfo.Defs[copygen.Methods.List[i].Names[0]]

// create the models.Type objects
fieldoptions, manual := getNodeOptions(astcopygen.Methods.List[i], p.Options.CommentOptionMap)
// create models.Type objects.
fieldoptions, manual := getNodeOptions(copygen.Methods.List[i], p.Options.CommentOptionMap)
fieldoptions = append(fieldoptions, p.Options.ConvertOptions...)
parsed, err := parseTypes(method)
parsed, err := parseTypes(method.(*types.Func))
if err != nil {
return nil, fmt.Errorf("an error occurred while parsing the types of function %q.\n%w", method.Name(), err)
}
Expand All @@ -54,15 +37,15 @@ func (p *Parser) parseFunctions(astcopygen *ast.InterfaceType) ([]models.Functio
setTypeOptions(parsed.toTypes, fieldoptions)

// map the function custom options.
var customoptionmap map[string][]string
customoptionmap := make(map[string][]string)
for _, option := range fieldoptions {
customoptionmap, err = options.MapCustomOption(customoptionmap, option)
if err != nil {
fmt.Printf("WARNING: %v\n", err)
}
}

// create the models.Function
// create the models.Function object.
function := models.Function{
Name: method.Name(),
To: parsed.toTypes,
Expand Down
3 changes: 1 addition & 2 deletions cli/parser/keep.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ func (p *Parser) Keep(astFile *ast.File) error {

// remove the `type Copygen interface` function ast.Comments.
comments := getNodeComments(declaration)
err := p.assignFieldOption(comments)
if err != nil {
if err := p.assignFieldOption(comments); err != nil {
return fmt.Errorf("%w", err)
}
trash = append(trash, comments...)
Expand Down
18 changes: 11 additions & 7 deletions cli/parser/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ type Config struct {
// SetupFile represents the setup file as an Abstract Syntax Tree.
SetupFile *ast.File

// SetupPkg represent the setup file's package.
SetupPkg *packages.Package

// Fileset represents the parser's fileset.
Fileset *token.FileSet
}
Expand Down Expand Up @@ -104,19 +107,18 @@ func Parse(gen *models.Generator) error {
return fmt.Errorf("an error occurred parsing the specified .go setup file: %v\n%w", gen.Setpath, err)
}

// Parse the Keep (and set options in the process).
// Parse the setup file's `type Copygen Interface` for the Keep (and set options in the process).
if err := p.Keep(p.Config.SetupFile); err != nil {
return fmt.Errorf("%w", err)
}

// Analyze the `type Copygen Interface` to create models.Function and models.Field objects.
// load package types from the setup file (NOTE: loads different *ast.Files).
// Analyze a new `type Copygen Interface` to create models.Function and models.Field objects.
cfg := &packages.Config{Mode: parserLoadMode}
p.Pkgs, err = packages.Load(cfg, "file="+gen.Setpath)
if err != nil {
return fmt.Errorf("an error occurred while loading the packages for types.\n%w", err)
}
setupPkgPath = p.Pkgs[0].PkgPath
p.Config.SetupPkg = p.Pkgs[0]

// determine the output file package path.
outputPkgs, _ := packages.Load(&packages.Config{Mode: packages.NeedName}, "file="+gen.Outpath)
Expand All @@ -132,9 +134,11 @@ func Parse(gen *models.Generator) error {
}
}

// find a new instance of a copygen AST since the old one has its comments removed.
// find a new instance of a `type Copygen interface` AST from the setup file's
// loaded go/types package (containing different *ast.Files from the Keep)
// since the parsed `type Copygen interface` has its comments removed.
var newCopygen *ast.InterfaceType
for _, decl := range p.Pkgs[0].Syntax[0].Decls {
for _, decl := range p.Config.SetupPkg.Syntax[0].Decls {
switch declaration := decl.(type) {
case *ast.GenDecl:
if it, ok := assertCopygenInterface(declaration); ok {
Expand All @@ -148,7 +152,7 @@ func Parse(gen *models.Generator) error {
return fmt.Errorf("the \"type Copygen interface\" could not be found in the setup file")
}

// create the models.Function objects.
// create models.Function objects.
SetupCache()
if gen.Functions, err = p.parseFunctions(newCopygen); err != nil {
return fmt.Errorf("%w", err)
Expand Down
11 changes: 5 additions & 6 deletions examples/_tests/README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# Examples: Tests

The examples in this folder are used for testing.
Run unit and integration tests using `go test ./_tests` from `cd examples`.

## Integration Tests

The command line interface is straightforward. The loader uses a tested library. The matcher matches fields to other fields, which the generator depends on. Field-matching is heavily dependent on the `parser`, which provides the User Interface for end users _(developers)_. As a result, the `parser` contains the majority of edge cases this program encounters. Testing the entire program from end-to-end is more effective than unit tests _(with the exception of option-parsing)_.

| Test | Description |
| :-------- | :------------------------------------------------------------------- |
Expand All @@ -12,9 +15,5 @@ The examples in this folder are used for testing.
| Duplicate | Defines two structs with duplicate definitions, but not names. |
| Import | Imports a package in the setup file, that the output file exists in. |
| Multi | Tests all types using multiple functions. |
| Option | Tests Generator and Function option-parsing. |

## Integration Tests

The command line interface is straightforward. The loader uses a tested library. The matcher matches fields to other fields, which the generator depends on. Field-matching is heavily dependent on the `parser`, which provides the User Interface for end users _(developers)_. As a result, the `parser` contains the majority of edge cases this program encounters. Testing the entire program from end-to-end is more effective than unit tests for each package.

Run integration tests using `go test ./_tests` from `cd examples`.
26 changes: 26 additions & 0 deletions examples/_tests/option/setup/setup.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Package copygen contains the setup information for copygen generated code.
package copygen

import (
"github.com/switchupcb/copygen/examples/map/domain"
"github.com/switchupcb/copygen/examples/map/models"
)

// Copygen defines the functions that will be generated.
type Copygen interface {
A(*models.Account)
B(*models.User)
// custom comment
C(*domain.Account)
// type basic
D(int)
// type basic
E(string)
// type basic
G(float64)
// type alias
F(byte)
H(rune)
//
Z()
}
11 changes: 11 additions & 0 deletions examples/_tests/option/setup/setup.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Define where the code will be generated.
generated:
setup: ./setup.go
output: ../copygen.go

# Define the optional custom templates used to generate the file (.go, .tmpl supported).
template: ../template/generate.go

# Define custom options (which are passed to generator options) for customization.
custom:
option: The possibilities are endless.
89 changes: 89 additions & 0 deletions examples/_tests/option_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package tests

import (
"reflect"
"testing"

"github.com/switchupcb/copygen/cli"
"github.com/switchupcb/copygen/cli/config"
"github.com/switchupcb/copygen/cli/parser"
)

// TestGeneratorOptions tests whether the Generator Options are parsed from the setup file correctly.
func TestGeneratorOptions(t *testing.T) {
checkwd(t)

env := cli.Environment{
YMLPath: "_tests/option/setup/setup.yml",
Output: false,
Write: false,
}

gen, err := config.LoadYML(env.YMLPath)
if err != nil {
t.Fatalf("Options(%q) error: %v", "Generator", err)
}

want := "The possibilities are endless."
if v, ok := gen.Options.Custom["option"]; ok {
if vs, ok := v.(string); ok {
if vs != want {
t.Fatalf("Options(%q) got %q, want %q", "Generator", vs, want)
}

return
}

t.Fatalf("Options(%q) does not contain a custom option with a string value.", "Generator")
}

t.Fatalf("Options(%q) does not contain a custom option.", "Generator")
}

// TestCustomFunctionOptions tests whether custom Function Options are parsed from the setup file correctly.
func TestCustomFunctionOptions(t *testing.T) {
checkwd(t)

env := cli.Environment{
YMLPath: "_tests/option/setup/setup.yml",
Output: false,
Write: false,
}

gen, err := config.LoadYML(env.YMLPath)
if err != nil {
t.Fatalf("Options(%q) error: %v", "Function", err)
}

if err = parser.Parse(gen); err != nil {
t.Fatalf("Options(%q) error: %v", "Function", err)
}

wanted := []map[string][]string{
make(map[string][]string), // A
make(map[string][]string), // B
{ // C
"custom": []string{"comment"},
},
{ // D
"type": []string{"basic"},
},
{ // E
"type": []string{"basic"},
},
{ // G
"type": []string{"basic"},
},
{ // F
"type": []string{"alias"},
},
make(map[string][]string), // H
make(map[string][]string), // Z
}

for i, function := range gen.Functions {
if !reflect.DeepEqual(function.Options.Custom, wanted[i]) {
t.Fatalf("Options(%q) got %q, want %q", "Function", function.Options.Custom, wanted[i])
}
}
}

0 comments on commit e50ba33

Please sign in to comment.