From f6dc2faa344464d6b1bbab288921d07f9288def3 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 26 May 2022 16:02:38 +0200 Subject: [PATCH 1/3] Slightly improve error messages --- constraints.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/constraints.go b/constraints.go index d97d340..25c1d47 100644 --- a/constraints.go +++ b/constraints.go @@ -52,7 +52,7 @@ func ParseConstraint(in string) (Constraint, error) { n := peek() if !isIdentifier(n) && !isVersionSeparator(n) { if start == curr { - return nil, fmt.Errorf("invalid version") + return nil, fmt.Errorf("invalid version: unexpected char '%c'", rune(n)) } return Parse(in[start:curr]) } @@ -79,7 +79,7 @@ func ParseConstraint(in string) (Constraint, error) { } skipSpace() if c := next(); c != ')' { - return nil, fmt.Errorf("unexpected char at: %s", in[curr-1:]) + return nil, fmt.Errorf("unexpected char: %c", rune(c)) } return expr, nil case '=': From 13bff22c60b68b05b8699b0f6ccdd92ab03baf89 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 26 May 2022 16:04:24 +0200 Subject: [PATCH 2/3] Consider '=' constraint implicit if operator is missing --- constraints.go | 15 ++++++++++++--- constraints_test.go | 32 ++++++++++++++++++++++++++------ 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/constraints.go b/constraints.go index 25c1d47..38aa992 100644 --- a/constraints.go +++ b/constraints.go @@ -56,7 +56,7 @@ func ParseConstraint(in string) (Constraint, error) { } return Parse(in[start:curr]) } - curr++ + next() } } @@ -65,14 +65,16 @@ func ParseConstraint(in string) (Constraint, error) { terminal = func() (Constraint, error) { skipSpace() - switch next() { + switch peek() { case '!': + next() expr, err := terminal() if err != nil { return nil, err } return &Not{expr}, nil case '(': + next() expr, err := constraint() if err != nil { return nil, err @@ -83,12 +85,14 @@ func ParseConstraint(in string) (Constraint, error) { } return expr, nil case '=': + next() v, err := version() if err != nil { return nil, err } return &Equals{v}, nil case '>': + next() if peek() == '=' { next() v, err := version() @@ -104,6 +108,7 @@ func ParseConstraint(in string) (Constraint, error) { return &GreaterThan{v}, nil } case '<': + next() if peek() == '=' { next() v, err := version() @@ -119,7 +124,11 @@ func ParseConstraint(in string) (Constraint, error) { return &LessThan{v}, nil } default: - return nil, fmt.Errorf("unexpected char at: %s", in[curr-1:]) + v, err := version() + if err != nil { + return nil, err + } + return &Equals{v}, nil } } diff --git a/constraints_test.go b/constraints_test.go index 2cba5e4..e3960db 100644 --- a/constraints_test.go +++ b/constraints_test.go @@ -85,23 +85,47 @@ func TestConstraintsParser(t *testing.T) { good := []goodStringTest{ {"", ""}, // always true {"=1.3.0", "=1.3.0"}, + {"1.3.0", "=1.3.0"}, + {"!1.0.0", "!(=1.0.0)"}, {" =1.3.0 ", "=1.3.0"}, + {" 1.3.0 ", "=1.3.0"}, {"=1.3.0 ", "=1.3.0"}, + {"1.3.0 ", "=1.3.0"}, {" =1.3.0", "=1.3.0"}, + {" 1.3.0", "=1.3.0"}, {">=1.3.0", ">=1.3.0"}, {">1.3.0", ">1.3.0"}, {"<=1.3.0", "<=1.3.0"}, {"<1.3.0", "<1.3.0"}, {"(=1.4.0)", "=1.4.0"}, + {"(1.4.0)", "=1.4.0"}, {"!(=1.4.0)", "!(=1.4.0)"}, + {"!(1.4.0)", "!(=1.4.0)"}, {"!(((=1.4.0)))", "!(=1.4.0)"}, + {"!(((1.4.0)))", "!(=1.4.0)"}, {"=1.2.4 && =1.3.0", "(=1.2.4 && =1.3.0)"}, + {"1.2.4 && 1.3.0", "(=1.2.4 && =1.3.0)"}, {"=1.2.4 && =1.3.0 && =1.2.0", "(=1.2.4 && =1.3.0 && =1.2.0)"}, + {"1.2.4 && 1.3.0 && 1.2.0", "(=1.2.4 && =1.3.0 && =1.2.0)"}, {"=1.2.4 && =1.3.0 || =1.2.0", "((=1.2.4 && =1.3.0) || =1.2.0)"}, + {"1.2.4 && 1.3.0 || 1.2.0", "((=1.2.4 && =1.3.0) || =1.2.0)"}, {"=1.2.4 || =1.3.0 && =1.2.0", "(=1.2.4 || (=1.3.0 && =1.2.0))"}, {"(=1.2.4 || =1.3.0) && =1.2.0", "((=1.2.4 || =1.3.0) && =1.2.0)"}, + {"(1.2.4 || 1.3.0) && 1.2.0", "((=1.2.4 || =1.3.0) && =1.2.0)"}, {"(=1.2.4 || !>1.3.0) && =1.2.0", "((=1.2.4 || !(>1.3.0)) && =1.2.0)"}, + {"(1.2.4 || !>1.3.0) && 1.2.0", "((=1.2.4 || !(>1.3.0)) && =1.2.0)"}, {"!(=1.2.4 || >1.3.0) && =1.2.0", "(!(=1.2.4 || >1.3.0) && =1.2.0)"}, + {"!(1.2.4 || >1.3.0) && 1.2.0", "(!(=1.2.4 || >1.3.0) && =1.2.0)"}, + {">1.0.0 && 2.0.0", "(>1.0.0 && =2.0.0)"}, + {">1.0.0 && =2.0.0", "(>1.0.0 && =2.0.0)"}, + {">1.0.0 || 2.0.0", "(>1.0.0 || =2.0.0)"}, + {">1.0.0 || =2.0.0", "(>1.0.0 || =2.0.0)"}, + {"(>1.0.0) || 2.0.0", "(>1.0.0 || =2.0.0)"}, + {"(>1.0.0) || =2.0.0", "(>1.0.0 || =2.0.0)"}, + {">1.0.0 || (2.0.0)", "(>1.0.0 || =2.0.0)"}, + {">1.0.0 || (=2.0.0)", "(>1.0.0 || =2.0.0)"}, + {"((>1.0.0) || (2.0.0))", "(>1.0.0 || =2.0.0)"}, + {"((>1.0.0) || (=2.0.0))", "(>1.0.0 || =2.0.0)"}, } for i, test := range good { in := test.In @@ -115,7 +139,6 @@ func TestConstraintsParser(t *testing.T) { } bad := []string{ - "1.0.0", "= 1.0.0", ">= 1.0.0", "> 1.0.0", @@ -124,19 +147,16 @@ func TestConstraintsParser(t *testing.T) { ">>1.0.0", ">1.0.0 =2.0.0", ">1.0.0 &", - "!1.0.0", - ">1.0.0 && 2.0.0", ">1.0.0 | =2.0.0", "(>1.0.0 | =2.0.0)", "(>1.0.0 || =2.0.0", - ">1.0.0 || 2.0.0", } for i, s := range bad { in := s t.Run(fmt.Sprintf("BadString%03d", i), func(t *testing.T) { p, err := ParseConstraint(in) - require.Nil(t, p) - require.Error(t, err) + require.Nil(t, p, "parsing: %s", in) + require.Error(t, err, "parsing: %s", in) fmt.Printf("'%s' parse error: %s\n", in, err) }) } From 17ed0f5b9022cf26141c28521e5413daf2c586bb Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 26 May 2022 16:08:53 +0200 Subject: [PATCH 3/3] Increase test coverage --- constraints_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/constraints_test.go b/constraints_test.go index e3960db..ab28abe 100644 --- a/constraints_test.go +++ b/constraints_test.go @@ -150,6 +150,11 @@ func TestConstraintsParser(t *testing.T) { ">1.0.0 | =2.0.0", "(>1.0.0 | =2.0.0)", "(>1.0.0 || =2.0.0", + "!1.0.0.0", + "!1.0.0 && !1.0.0.0", + "(!1.0.0 && !1.0.0", + "!1.0.0 || !1.0.0.0", + "(!1.0.0 || !1.0.0", } for i, s := range bad { in := s