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.

Sign up for GitHub

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

feat: added mapper for regex #427

Closed
wants to merge 4 commits into from
Closed

feat: added mapper for regex #427

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f449f3d
feat: added regex and net types: ip, ipnet, cidr
marsom May 20, 2024
8727b63
fix: removed netip mappers because they are supported out of the box
marsom May 21, 2024
f068e3a
fix: remove all new mappers except the regex mapper
marsom May 23, 2024
4f79649
fix: removed all mappers, added failing test for questioning
marsom May 25, 2024
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
104 changes: 104 additions & 0 deletions mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@ import (
"fmt"
"io"
"math/bits"
"net"
"net/netip"
"net/url"
"os"
"reflect"
"regexp"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -285,6 +288,11 @@ func (r *Registry) RegisterDefaults() *Registry {
RegisterType(reflect.TypeOf(time.Duration(0)), durationDecoder()).
RegisterType(reflect.TypeOf(&url.URL{}), urlMapper()).
RegisterType(reflect.TypeOf(&os.File{}), fileMapper(r)).
RegisterType(reflect.TypeOf(&regexp.Regexp{}), regexMapper()).
RegisterType(reflect.TypeOf(&net.IP{}), netIPMapper()).
RegisterType(reflect.TypeOf(&net.IPNet{}), netIPNetMapper()).
RegisterType(reflect.TypeOf(netip.Addr{}), netipAddrMapper()).
RegisterType(reflect.TypeOf(netip.Prefix{}), netipPrefixMapper()).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for the drive-by review, but are these two actually necessary? 🤔

they implement enconding.TextUnmarshaller and should work out of the box?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right, they work out of the box

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i removed the netip mappers from the pr, and cleanup my tests.

I thought about also removing the net mappers, and only add the regex mapper:

  • this would guide users to use new new netip package instead of the net package

What do you think about that?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. Maybe both? I don't think there's any harm there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of leading people to netip. Especially since the package already has convenience code for converting back and forth to net.IP and friends.

But that's just my 2¢

RegisterName("path", pathMapper(r)).
RegisterName("existingfile", existingFileMapper(r)).
RegisterName("existingdir", existingDirMapper(r)).
Expand Down Expand Up @@ -733,6 +741,102 @@ func fileContentMapper(r *Registry) MapperFunc {
}
}

func regexMapper() MapperFunc {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regexp.Regexp actually implements encoding.Text(Un)marshaler, so I don't think this is necessary either after all...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh boy, you're right! 🤦
totally overlooked that too 🙈

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for consuming you time 🤦

i removed all mappers. we can close those this pr.

I added some regex tests in the last commit, if you have some time, maybe you can explain to me why the TestRegex,TestRegexSlice,TestRegexPointer are working but the TestRegexPointerSlice is not?

  • somehow i used to always use *regex.Regexp instead of regex.Regexp but the pointer version does not with the slice

return func(ctx *DecodeContext, target reflect.Value) error {
t, err := ctx.Scan.PopValue("regex")
if err != nil {
return err
}

var f *regexp.Regexp
switch v := t.Value.(type) {
case string:
f, err = regexp.Compile(v)
if err != nil {
return fmt.Errorf("expected regular expression but got %q: %w", v, err)
}
default:
return fmt.Errorf("expected string but got %q", v)
}

target.Set(reflect.ValueOf(f))

return nil
}
}

func netIPMapper() MapperFunc {
return func(ctx *DecodeContext, target reflect.Value) error {
var value string
if err := ctx.Scan.PopValueInto("ip", &value); err != nil {
return err
}

ip := net.ParseIP(value)
if ip == nil {
return fmt.Errorf("expected ip addresss but got %q", value)
}

target.Set(reflect.ValueOf(ip))

return nil
}
}

func netIPNetMapper() MapperFunc {
return func(ctx *DecodeContext, target reflect.Value) error {
var value string
if err := ctx.Scan.PopValueInto("cidr", &value); err != nil {
return err
}

_, ipnet, err := net.ParseCIDR(value)
if err != nil {
return fmt.Errorf("expected cidr but got %q: %w", value, err)
}

target.Set(reflect.ValueOf(ipnet))

return nil
}
}

func netipAddrMapper() MapperFunc {
return func(ctx *DecodeContext, target reflect.Value) error {
var value string
if err := ctx.Scan.PopValueInto("ip", &value); err != nil {
return err
}

ip, err := netip.ParseAddr(value)
if err != nil {
return fmt.Errorf("expected ip addresss but got %q: %w", value, err)
}

target.Set(reflect.ValueOf(ip))

return nil
}
}

func netipPrefixMapper() MapperFunc {
return func(ctx *DecodeContext, target reflect.Value) error {
var value string
if err := ctx.Scan.PopValueInto("cidr", &value); err != nil {
return err
}

prefix, err := netip.ParsePrefix(value)
if err != nil {
return fmt.Errorf("expected ipnet but got %q: %w", value, err)
}

target.Set(reflect.ValueOf(prefix))

return nil
}
}

type ptrMapper struct {
r *Registry
}
Expand Down
164 changes: 164 additions & 0 deletions mapper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ import (
"encoding/json"
"fmt"
"math"
"net"
"net/netip"
"net/url"
"os"
"path/filepath"
"reflect"
"regexp"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -138,6 +141,167 @@ func TestDurationMapperJSONResolver(t *testing.T) {
assert.Equal(t, time.Second*5, cli.Flag)
}

func TestNetIP(t *testing.T) {
var cli struct {
Flag net.IP
}
k := mustNew(t, &cli)
_, err := k.Parse([]string{"--flag", "127.0.0.1"})
assert.NoError(t, err)
assert.Equal(t, "127.0.0.1", cli.Flag.String())

_, err = k.Parse([]string{"--flag", "2001:db8:abcd:0012::1"})
assert.NoError(t, err)
assert.Equal(t, "2001:db8:abcd:12::1", cli.Flag.String())
}

func TestNetIPSplice(t *testing.T) {
var cli struct {
Flag []net.IP
}
k := mustNew(t, &cli)
_, err := k.Parse([]string{
"--flag", "127.0.0.1",
"--flag", "192.168.0.1",
"--flag", "2001:db8:abcd:0012::1",
})
assert.NoError(t, err)

assert.Equal(t, 3, len(cli.Flag))
assert.Equal(t, "127.0.0.1", cli.Flag[0].String())
assert.Equal(t, "192.168.0.1", cli.Flag[1].String())
assert.Equal(t, "2001:db8:abcd:12::1", cli.Flag[2].String())
}

func TestIPNet(t *testing.T) {
var cli struct {
Flag *net.IPNet
}
k := mustNew(t, &cli)
_, err := k.Parse([]string{"--flag", "127.0.0.0/24"})
assert.NoError(t, err)
assert.Equal(t, "127.0.0.0/24", cli.Flag.String())

_, err = k.Parse([]string{"--flag", "2001:db8:abcd:0012::0/64"})
assert.NoError(t, err)
assert.Equal(t, "2001:db8:abcd:12::/64", cli.Flag.String())
}

func TestIPNetSlice(t *testing.T) {
var cli struct {
Test []*net.IPNet
}

k := mustNew(t, &cli)
_, err := k.Parse([]string{
"--test", "127.0.0.0/24",
"--test", "123.0.0.0/23",
"--test", "2001:db8:abcd:0012::0/64",
})
assert.NoError(t, err)

assert.Equal(t, 3, len(cli.Test))
assert.Equal(t, "127.0.0.0/24", cli.Test[0].String())
assert.Equal(t, "123.0.0.0/23", cli.Test[1].String())
assert.Equal(t, "2001:db8:abcd:12::/64", cli.Test[2].String())
}

func TestNetipAddr(t *testing.T) {
var cli struct {
Flag netip.Addr
}
k := mustNew(t, &cli)
_, err := k.Parse([]string{"--flag", "127.0.0.1"})
assert.NoError(t, err)
assert.Equal(t, "127.0.0.1", cli.Flag.String())

_, err = k.Parse([]string{"--flag", "2001:db8:abcd:0012::1"})
assert.NoError(t, err)
assert.Equal(t, "2001:db8:abcd:12::1", cli.Flag.String())
}

func TestNetipAddrSplice(t *testing.T) {
var cli struct {
Flag []net.IP
}
k := mustNew(t, &cli)
_, err := k.Parse([]string{
"--flag", "127.0.0.1",
"--flag", "192.168.0.1",
"--flag", "2001:db8:abcd:0012::1",
})
assert.NoError(t, err)

assert.Equal(t, 3, len(cli.Flag))
assert.Equal(t, "127.0.0.1", cli.Flag[0].String())
assert.Equal(t, "192.168.0.1", cli.Flag[1].String())
assert.Equal(t, "2001:db8:abcd:12::1", cli.Flag[2].String())
}

func TestNetipPrefix(t *testing.T) {
var cli struct {
Flag netip.Prefix
}
k := mustNew(t, &cli)

_, err := k.Parse([]string{"--flag", "127.0.0.0/24"})
assert.NoError(t, err)
assert.Equal(t, "127.0.0.0/24", cli.Flag.String())
assert.Equal(t, 24, cli.Flag.Bits())

_, err = k.Parse([]string{"--flag", "2001:db8:abcd:0012::0/64"})
assert.NoError(t, err)
assert.Equal(t, "2001:db8:abcd:12::/64", cli.Flag.String())
}

func TestNetipPrefixSlice(t *testing.T) {
var cli struct {
Test []*net.IPNet `kong:"sep=','"`
}

k := mustNew(t, &cli)
_, err := k.Parse([]string{
"--test", "127.0.0.0/24",
"--test", "123.0.0.0/23",
"--test", "2001:db8:abcd:0012::0/64",
})
assert.NoError(t, err)

assert.Equal(t, 3, len(cli.Test))
assert.Equal(t, "127.0.0.0/24", cli.Test[0].String())
assert.Equal(t, "123.0.0.0/23", cli.Test[1].String())
assert.Equal(t, "2001:db8:abcd:12::/64", cli.Test[2].String())
}

func TestRegex(t *testing.T) {
var cli struct {
Test *regexp.Regexp
}

k := mustNew(t, &cli)
_, err := k.Parse([]string{"--test", "a.+[a-b]{2,4}"})
assert.NoError(t, err)
assert.Equal(t, "a.+[a-b]{2,4}", cli.Test.String())
}

func TestRegexSlice(t *testing.T) {
var cli struct {
Test []*regexp.Regexp `kong:"sep='none'"`
}

k := mustNew(t, &cli)
_, err := k.Parse([]string{
"--test", "foo.+[b-r]{2,4}",
"--test", "foo=bar",
})
assert.NoError(t, err)

assert.Equal(t, 2, len(cli.Test))
assert.Equal(t, "foo.+[b-r]{2,4}", cli.Test[0].String())
assert.Equal(t, "foo=bar", cli.Test[1].String())

}

func TestSplitEscaped(t *testing.T) {
assert.Equal(t, []string{"a", "b"}, kong.SplitEscaped("a,b", ','))
assert.Equal(t, []string{"a,b", "c"}, kong.SplitEscaped(`a\,b,c`, ','))
Expand Down
Loading