Skip to content

Commit

Permalink
fix add statement linter (#22)
Browse files Browse the repository at this point in the history
* fix add statement linter

* add test

* linting

* implement regex string validity
  • Loading branch information
ysugimoto authored Jul 13, 2021
1 parent 6aa4575 commit f37eb0a
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 1 deletion.
4 changes: 3 additions & 1 deletion linter/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,11 @@ func InvalidValue(m *ast.Meta, tt, val string) *LintError {
}
}

// InvalidType raises ERROR due to strict type assertion failed.
// Actually, it cause compile error for that VCL.
func InvalidType(m *ast.Meta, name string, expect, actual types.Type) *LintError {
return &LintError{
Severity: WARNING,
Severity: ERROR,
Token: m.Token,
Message: fmt.Sprintf("%s wants type %s but assign %s", name, expect.String(), actual.String()),
}
Expand Down
28 changes: 28 additions & 0 deletions linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package linter
import (
"fmt"
"net"
"regexp"
"strings"

"github.com/ysugimoto/falco/ast"
Expand Down Expand Up @@ -892,6 +893,22 @@ func (l *Linter) lintAddStatement(stmt *ast.AddStatement, ctx *context.Context)
l.Error(InvalidName(stmt.Ident.GetMeta(), stmt.Ident.Value, "add").Match(ADD_STATEMENT_SYNTAX))
}

// Add statement could use only for HTTP headers.
// https://developer.fastly.com/reference/vcl/statements/add/
if !strings.Contains(stmt.Ident.Value, "req.http.") &&
!strings.Contains(stmt.Ident.Value, "bereq.http.") &&
!strings.Contains(stmt.Ident.Value, "beresp.http.") &&
!strings.Contains(stmt.Ident.Value, "obj.http.") &&
!strings.Contains(stmt.Ident.Value, "resp.http.") {

err := &LintError{
Severity: ERROR,
Token: stmt.Ident.GetMeta().Token,
Message: "Add statement could not use for " + stmt.Ident.Value,
}
l.Error(err.Match(ADD_STATEMENT_SYNTAX))
}

left, err := ctx.Get(stmt.Ident.Value)
if err != nil {
l.Error(&LintError{
Expand Down Expand Up @@ -1179,6 +1196,17 @@ func (l *Linter) lintInfixExpression(exp *ast.InfixExpression, ctx *context.Cont
} else if !expectType(right, types.StringType, types.IPType, types.AclType) {
l.Error(InvalidTypeExpression(exp.GetMeta(), right, types.StringType, types.IPType, types.AclType).Match(OPERATOR_CONDITIONAL))
}
// And, if right expression is STRING, regex must be valid
if v, ok := exp.Right.(*ast.String); ok {
if _, err := regexp.Compile(v.Value); err != nil {
err := &LintError{
Severity: ERROR,
Token: exp.Right.GetMeta().Token,
Message: "regex string is invalid, " + err.Error(),
}
l.Error(err)
}
}
return types.BoolType
case "+":
// Plus operator behaves string concatenation.
Expand Down
19 changes: 19 additions & 0 deletions linter/linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,15 @@ sub foo {
assertError(t, input)
})

t.Run("only can use for HTTP headers", func(t *testing.T) {
input := `
sub foo {
declare local var.FOO STRING;
add var.FOO = "bar";
}`

assertError(t, input)
})
}

func TestLintCallStatement(t *testing.T) {
Expand Down Expand Up @@ -1229,3 +1238,13 @@ sub vcl_recv {
}`
assertNoError(t, input)
}

func TestRegexExpressionIsInvalid(t *testing.T) {
input := `
sub vcl_recv {
#Fastly recv
if (req.url ~ "/foo/(.+") {
}
}`
assertError(t, input)
}

0 comments on commit f37eb0a

Please sign in to comment.