Skip to content

Commit e50ba33

Browse files
author
switchupcb
committed
fix non-alphabetical function-option mismatch
fixes #29; non-alphabetical output
1 parent c7d0408 commit e50ba33

File tree

8 files changed

+157
-46
lines changed

8 files changed

+157
-46
lines changed

CONTRIBUTING.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ The `setup` file is parsed using an Abstract Syntax Tree. This tree contains the
4444

4545
**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.
4646

47-
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.
47+
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.
4848

4949
#### Copygen Interface
5050

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

131131
### Tests
132132

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

135135
# Roadmap
136136

cli/parser/function.go

+12-29
Original file line numberDiff line numberDiff line change
@@ -13,38 +13,21 @@ const copygenInterfaceName = "Copygen"
1313

1414
// parseFunctions parses the AST for functions in the setup file.
1515
// astcopygen is used to assign options from *ast.Comments.
16-
func (p *Parser) parseFunctions(astcopygen *ast.InterfaceType) ([]models.Function, error) {
17-
18-
// find the `type Copygen interface` definition in the setup file.
19-
var copygen *types.Interface
20-
21-
setpkg := p.Pkgs[0]
22-
defs := setpkg.TypesInfo.Defs
23-
for k, v := range defs {
24-
if k.Name == copygenInterfaceName {
25-
if it, ok := v.Type().Underlying().(*types.Interface); ok {
26-
copygen = it
27-
}
28-
}
29-
}
30-
31-
if copygen == nil {
32-
return nil, fmt.Errorf("the \"type Copygen interface\" could not be found in the setup file's package")
33-
}
34-
35-
if copygen.NumMethods() == 0 {
16+
func (p *Parser) parseFunctions(copygen *ast.InterfaceType) ([]models.Function, error) {
17+
numMethods := len(copygen.Methods.List)
18+
if numMethods == 0 {
3619
fmt.Println("WARNING: no functions are defined in the \"type Copygen interface\"")
3720
}
3821

39-
// create the models.Function objects
40-
functions := make([]models.Function, copygen.NumMethods())
41-
for i := 0; i < copygen.NumMethods(); i++ {
42-
method := copygen.Method(i)
22+
// create models.Function objects.
23+
functions := make([]models.Function, numMethods)
24+
for i := 0; i < numMethods; i++ {
25+
method := p.Config.SetupPkg.TypesInfo.Defs[copygen.Methods.List[i].Names[0]]
4326

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

5639
// map the function custom options.
57-
var customoptionmap map[string][]string
40+
customoptionmap := make(map[string][]string)
5841
for _, option := range fieldoptions {
5942
customoptionmap, err = options.MapCustomOption(customoptionmap, option)
6043
if err != nil {
6144
fmt.Printf("WARNING: %v\n", err)
6245
}
6346
}
6447

65-
// create the models.Function
48+
// create the models.Function object.
6649
function := models.Function{
6750
Name: method.Name(),
6851
To: parsed.toTypes,

cli/parser/keep.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ func (p *Parser) Keep(astFile *ast.File) error {
2727

2828
// remove the `type Copygen interface` function ast.Comments.
2929
comments := getNodeComments(declaration)
30-
err := p.assignFieldOption(comments)
31-
if err != nil {
30+
if err := p.assignFieldOption(comments); err != nil {
3231
return fmt.Errorf("%w", err)
3332
}
3433
trash = append(trash, comments...)

cli/parser/parse.go

+11-7
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ type Config struct {
2626
// SetupFile represents the setup file as an Abstract Syntax Tree.
2727
SetupFile *ast.File
2828

29+
// SetupPkg represent the setup file's package.
30+
SetupPkg *packages.Package
31+
2932
// Fileset represents the parser's fileset.
3033
Fileset *token.FileSet
3134
}
@@ -104,19 +107,18 @@ func Parse(gen *models.Generator) error {
104107
return fmt.Errorf("an error occurred parsing the specified .go setup file: %v\n%w", gen.Setpath, err)
105108
}
106109

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

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

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

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

151-
// create the models.Function objects.
155+
// create models.Function objects.
152156
SetupCache()
153157
if gen.Functions, err = p.parseFunctions(newCopygen); err != nil {
154158
return fmt.Errorf("%w", err)

examples/_tests/README.md

+5-6
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
# Examples: Tests
22

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

5+
## Integration Tests
6+
7+
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)_.
58

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

16-
## Integration Tests
17-
18-
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.
19-
20-
Run integration tests using `go test ./_tests` from `cd examples`.

examples/_tests/option/setup/setup.go

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Package copygen contains the setup information for copygen generated code.
2+
package copygen
3+
4+
import (
5+
"github.com/switchupcb/copygen/examples/map/domain"
6+
"github.com/switchupcb/copygen/examples/map/models"
7+
)
8+
9+
// Copygen defines the functions that will be generated.
10+
type Copygen interface {
11+
A(*models.Account)
12+
B(*models.User)
13+
// custom comment
14+
C(*domain.Account)
15+
// type basic
16+
D(int)
17+
// type basic
18+
E(string)
19+
// type basic
20+
G(float64)
21+
// type alias
22+
F(byte)
23+
H(rune)
24+
//
25+
Z()
26+
}
+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# Define where the code will be generated.
2+
generated:
3+
setup: ./setup.go
4+
output: ../copygen.go
5+
6+
# Define the optional custom templates used to generate the file (.go, .tmpl supported).
7+
template: ../template/generate.go
8+
9+
# Define custom options (which are passed to generator options) for customization.
10+
custom:
11+
option: The possibilities are endless.

examples/_tests/option_test.go

+89
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
package tests
2+
3+
import (
4+
"reflect"
5+
"testing"
6+
7+
"github.com/switchupcb/copygen/cli"
8+
"github.com/switchupcb/copygen/cli/config"
9+
"github.com/switchupcb/copygen/cli/parser"
10+
)
11+
12+
// TestGeneratorOptions tests whether the Generator Options are parsed from the setup file correctly.
13+
func TestGeneratorOptions(t *testing.T) {
14+
checkwd(t)
15+
16+
env := cli.Environment{
17+
YMLPath: "_tests/option/setup/setup.yml",
18+
Output: false,
19+
Write: false,
20+
}
21+
22+
gen, err := config.LoadYML(env.YMLPath)
23+
if err != nil {
24+
t.Fatalf("Options(%q) error: %v", "Generator", err)
25+
}
26+
27+
want := "The possibilities are endless."
28+
if v, ok := gen.Options.Custom["option"]; ok {
29+
if vs, ok := v.(string); ok {
30+
if vs != want {
31+
t.Fatalf("Options(%q) got %q, want %q", "Generator", vs, want)
32+
}
33+
34+
return
35+
}
36+
37+
t.Fatalf("Options(%q) does not contain a custom option with a string value.", "Generator")
38+
}
39+
40+
t.Fatalf("Options(%q) does not contain a custom option.", "Generator")
41+
}
42+
43+
// TestCustomFunctionOptions tests whether custom Function Options are parsed from the setup file correctly.
44+
func TestCustomFunctionOptions(t *testing.T) {
45+
checkwd(t)
46+
47+
env := cli.Environment{
48+
YMLPath: "_tests/option/setup/setup.yml",
49+
Output: false,
50+
Write: false,
51+
}
52+
53+
gen, err := config.LoadYML(env.YMLPath)
54+
if err != nil {
55+
t.Fatalf("Options(%q) error: %v", "Function", err)
56+
}
57+
58+
if err = parser.Parse(gen); err != nil {
59+
t.Fatalf("Options(%q) error: %v", "Function", err)
60+
}
61+
62+
wanted := []map[string][]string{
63+
make(map[string][]string), // A
64+
make(map[string][]string), // B
65+
{ // C
66+
"custom": []string{"comment"},
67+
},
68+
{ // D
69+
"type": []string{"basic"},
70+
},
71+
{ // E
72+
"type": []string{"basic"},
73+
},
74+
{ // G
75+
"type": []string{"basic"},
76+
},
77+
{ // F
78+
"type": []string{"alias"},
79+
},
80+
make(map[string][]string), // H
81+
make(map[string][]string), // Z
82+
}
83+
84+
for i, function := range gen.Functions {
85+
if !reflect.DeepEqual(function.Options.Custom, wanted[i]) {
86+
t.Fatalf("Options(%q) got %q, want %q", "Function", function.Options.Custom, wanted[i])
87+
}
88+
}
89+
}

0 commit comments

Comments
 (0)