Repository: ysugimoto/falco
Pull request #36 (2021-11-19 16:15:32)

[bugfix] Fix builtin accessor and assign operator
This PR fixes some of bugs:

### some predefined variables should be settable

Some predefined variables should be able to use in `set` and `add` statements. This PR modifies as settable:

- `bereq.method`
- `bereq.request`
- `beresp.grace`
- `resp.status`
- `resp.response`

And add `LF` predefined variable (probably undocumented)

### Fix wrong display code in line included VCL error

On lint error of `unused/declaration` rule, `falco` prints line of main vcl. This PR fixes actual code lines.


Patch:
diff --git a/__generator__/builtin.yml b/__generator__/builtin.yml
index b267578..2f04ed5 100644
--- a/__generator__/builtin.yml
+++ b/__generator__/builtin.yml
@@ -1216,3 +1216,4 @@ uuid.x500:
   reference: "https://developer.fastly.com/reference/vcl/functions/uuid/uuid-x500/"
   on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
   return: STRING
+
diff --git a/__generator__/predefined.yml b/__generator__/predefined.yml
index 4fc7e8c..b0a1eba 100644
--- a/__generator__/predefined.yml
+++ b/__generator__/predefined.yml
@@ -272,6 +272,7 @@ bereq.method:
   reference: "https://developer.fastly.com/reference/vcl/variables/backend-request/bereq-method/"
   on: [PASS, MISS, FETCH]
   get: STRING
+  set: STRING
 
 bereq.proto:
   reference: "https://developer.fastly.com/reference/vcl/variables/backend-request/bereq-proto/"
@@ -282,11 +283,13 @@ bereq.request:
   reference: "https://developer.fastly.com/reference/vcl/variables/backend-request/bereq-request/"
   on: [PASS, MISS, FETCH]
   get: STRING
+  set: STRING
 
 bereq.url:
   reference: "https://developer.fastly.com/reference/vcl/variables/backend-request/bereq-url/"
   on: [PASS, MISS, FETCH]
   get: STRING
+  set: STRING
 
 bereq.url.basename:
   reference: "https://developer.fastly.com/reference/vcl/variables/backend-request/bereq-url-basename/"
@@ -340,7 +343,7 @@ beresp.grace:
   reference: "https://developer.fastly.com/reference/vcl/variables/backend-response/beresp-grace/"
   on: [FETCH]
   get: RTIME
-  set: BOOL
+  set: RTIME
 
 beresp.gzip:
   reference: "https://developer.fastly.com/reference/vcl/variables/backend-response/beresp-gzip/"
@@ -426,6 +429,7 @@ obj.grace:
   reference: "https://developer.fastly.com/reference/vcl/variables/cache-object/obj-grace/"
   on: [HIT, ERROR, LOG]
   get: RTIME
+  set: RTIME
 
 obj.hits:
   reference: "https://developer.fastly.com/reference/vcl/variables/cache-object/obj-hits/"
@@ -1252,11 +1256,13 @@ resp.response:
   reference: "https://developer.fastly.com/reference/vcl/variables/client-response/resp-response/"
   on: [DELIVER, LOG]
   get: STRING
+  set: STRING
 
 resp.status:
   reference: "https://developer.fastly.com/reference/vcl/variables/client-response/resp-status/"
   on: [DELIVER, LOG]
   get: INTEGER
+  set: INTEGER
 
 time.to_first_byte:
   reference: "https://developer.fastly.com/reference/vcl/variables/client-response/time-to-first-byte/"
@@ -2049,3 +2055,8 @@ waf.logged:
   reference: "https://developer.fastly.com/reference/vcl/subroutines/waf-debug-log/"
   on: [LOG]
   get: BOOL
+
+LF:
+  reference: "https://developer.fastly.com/reference/vcl/types/string/"
+  on: [RECV, HASH, HIT, MISS, PASS, FETCH, ERROR, DELIVER, LOG]
+  get: STRING
diff --git a/cmd/falco/runner.go b/cmd/falco/runner.go
index 60dd346..d93f95e 100644
--- a/cmd/falco/runner.go
+++ b/cmd/falco/runner.go
@@ -45,6 +45,7 @@ type Runner struct {
 	includePaths []string
 	mainVclFile  string
 	overrides    map[string]linter.Severity
+	lexers       map[string]*lexer.Lexer
 
 	level Level
 
@@ -63,6 +64,7 @@ func NewRunner(mainVcl string, c *Config) (*Runner, error) {
 		context:     context.New(),
 		level:       LevelError,
 		overrides:   make(map[string]linter.Severity),
+		lexers:      make(map[string]*lexer.Lexer),
 	}
 
 	if c.Remote {
@@ -192,7 +194,7 @@ func (r *Runner) Transform(vcls []*plugin.VCL) error {
 }
 
 func (r *Runner) Run() (*RunnerResult, error) {
-	vcls, err := r.run(r.mainVclFile)
+	vcls, err := r.run(r.mainVclFile, true)
 	if err != nil {
 		return nil, err
 	}
@@ -205,7 +207,7 @@ func (r *Runner) Run() (*RunnerResult, error) {
 	}, nil
 }
 
-func (r *Runner) run(vclFile string) ([]*plugin.VCL, error) {
+func (r *Runner) run(vclFile string, isMain bool) ([]*plugin.VCL, error) {
 	fp, err := os.Open(vclFile)
 	if err != nil {
 		return nil, fmt.Errorf("Failed to open file: %s", vclFile)
@@ -224,6 +226,7 @@ func (r *Runner) run(vclFile string) ([]*plugin.VCL, error) {
 		return nil, ErrParser
 	}
 	lx.NewLine()
+	r.lexers[vclFile] = lx
 
 	var vcls []*plugin.VCL
 	// Lint dependent VCLs before execute main VCL
@@ -242,7 +245,7 @@ func (r *Runner) run(vclFile string) ([]*plugin.VCL, error) {
 			}
 		}
 
-		subVcl, err := r.run(file)
+		subVcl, err := r.run(file, false)
 		if err != nil {
 			return nil, err
 		}
@@ -256,7 +259,7 @@ func (r *Runner) run(vclFile string) ([]*plugin.VCL, error) {
 	})
 
 	lt := linter.New()
-	lt.Lint(vcl, r.context)
+	lt.Lint(vcl, r.context, isMain)
 	if len(lt.Errors) > 0 {
 		for _, err := range lt.Errors {
 			le, ok := err.(*linter.LintError)
@@ -306,6 +309,7 @@ func (r *Runner) printLinterError(lx *lexer.Lexer, err *linter.LintError) {
 	}
 	if err.Token.File != "" {
 		file = "in " + err.Token.File + " "
+		lx = r.lexers[err.Token.File]
 	}
 
 	// check severity with overrides
diff --git a/cmd/falco/snippet.go b/cmd/falco/snippet.go
index ff56b16..c805cce 100644
--- a/cmd/falco/snippet.go
+++ b/cmd/falco/snippet.go
@@ -41,7 +41,7 @@ func (s *Snippet) Compile(ctx *context.Context) error {
 		return err
 	}
 	l := linter.New()
-	l.Lint(vcl, ctx)
+	l.Lint(vcl, ctx, false)
 	return nil
 }
 
diff --git a/context/context.go b/context/context.go
index 7f6436e..9f1f4a2 100644
--- a/context/context.go
+++ b/context/context.go
@@ -235,6 +235,24 @@ func (c *Context) AddDirector(name string, director *types.Director) error {
 		},
 	}
 
+	// And, director target backend identifilers should be marked as used
+	for _, d := range director.Decl.Properties {
+		bo, ok := d.(*ast.DirectorBackendObject)
+		if !ok {
+			continue
+		}
+		for _, v := range bo.Values {
+			if v.Key.Value != "backend" {
+				continue
+			}
+			if ident, ok := v.Value.(*ast.Ident); ok {
+				if b, ok := c.Backends[ident.Value]; ok {
+					b.IsUsed = true
+				}
+			}
+		}
+	}
+
 	return nil
 }
 
diff --git a/context/identifilers.go b/context/identifilers.go
index 0d6e0f5..c12d435 100644
--- a/context/identifilers.go
+++ b/context/identifilers.go
@@ -38,5 +38,12 @@ func builtinIdentifiers() map[string]struct{} {
 		"standard":  {},
 		"url":       {},
 		"url_nopad": {},
+
+		// use for setcookie.get_value_by_name argument of ID
+		"req":    {},
+		"bereq":  {},
+		"obj":    {},
+		"beresp": {},
+		"resp":   {},
 	}
 }
diff --git a/context/predefined.go b/context/predefined.go
index 134b93f..703c744 100644
--- a/context/predefined.go
+++ b/context/predefined.go
@@ -8,6 +8,16 @@ import (
 
 func predefinedVariables() Variables {
 	return Variables{
+		"LF": &Object{
+			Items: map[string]*Object{},
+			Value: &Accessor{
+				Get:       types.StringType,
+				Set:       types.NeverType,
+				Unset:     false,
+				Scopes:    RECV | HASH | HIT | MISS | PASS | FETCH | ERROR | DELIVER | LOG,
+				Reference: "",
+			},
+		},
 		"backend": &Object{
 			Items: map[string]*Object{
 				"conn": &Object{
@@ -410,7 +420,7 @@ func predefinedVariables() Variables {
 					Items: map[string]*Object{},
 					Value: &Accessor{
 						Get:       types.StringType,
-						Set:       types.NeverType,
+						Set:       types.StringType,
 						Unset:     false,
 						Scopes:    PASS | MISS | FETCH,
 						Reference: "https://developer.fastly.com/reference/vcl/variables/backend-request/bereq-method/",
@@ -430,7 +440,7 @@ func predefinedVariables() Variables {
 					Items: map[string]*Object{},
 					Value: &Accessor{
 						Get:       types.StringType,
-						Set:       types.NeverType,
+						Set:       types.StringType,
 						Unset:     false,
 						Scopes:    PASS | MISS | FETCH,
 						Reference: "https://developer.fastly.com/reference/vcl/variables/backend-request/bereq-request/",
@@ -491,7 +501,7 @@ func predefinedVariables() Variables {
 					},
 					Value: &Accessor{
 						Get:       types.StringType,
-						Set:       types.NeverType,
+						Set:       types.StringType,
 						Unset:     false,
 						Scopes:    PASS | MISS | FETCH,
 						Reference: "https://developer.fastly.com/reference/vcl/variables/backend-request/bereq-url/",
@@ -609,7 +619,7 @@ func predefinedVariables() Variables {
 					Items: map[string]*Object{},
 					Value: &Accessor{
 						Get:       types.RTimeType,
-						Set:       types.BoolType,
+						Set:       types.RTimeType,
 						Unset:     false,
 						Scopes:    FETCH,
 						Reference: "https://developer.fastly.com/reference/vcl/variables/backend-response/beresp-grace/",
@@ -2466,7 +2476,7 @@ func predefinedVariables() Variables {
 					Items: map[string]*Object{},
 					Value: &Accessor{
 						Get:       types.RTimeType,
-						Set:       types.NeverType,
+						Set:       types.RTimeType,
 						Unset:     false,
 						Scopes:    HIT | ERROR | LOG,
 						Reference: "https://developer.fastly.com/reference/vcl/variables/cache-object/obj-grace/",
@@ -3341,7 +3351,7 @@ func predefinedVariables() Variables {
 					Items: map[string]*Object{},
 					Value: &Accessor{
 						Get:       types.StringType,
-						Set:       types.NeverType,
+						Set:       types.StringType,
 						Unset:     false,
 						Scopes:    DELIVER | LOG,
 						Reference: "https://developer.fastly.com/reference/vcl/variables/client-response/resp-response/",
@@ -3382,7 +3392,7 @@ func predefinedVariables() Variables {
 					Items: map[string]*Object{},
 					Value: &Accessor{
 						Get:       types.IntegerType,
-						Set:       types.NeverType,
+						Set:       types.IntegerType,
 						Unset:     false,
 						Scopes:    DELIVER | LOG,
 						Reference: "https://developer.fastly.com/reference/vcl/variables/client-response/resp-status/",
diff --git a/linter/linter.go b/linter/linter.go
index e6fa7b7..f5c4ee6 100644
--- a/linter/linter.go
+++ b/linter/linter.go
@@ -34,18 +34,21 @@ func (l *Linter) Error(err error) {
 
 // Expose lint function to call from external program.
 // It means this method is bootstrap, called only once.
-func (l *Linter) Lint(node ast.Node, ctx *context.Context) types.Type {
+func (l *Linter) Lint(node ast.Node, ctx *context.Context, isMain bool) types.Type {
 	if ctx == nil {
 		ctx = context.New()
 	}
 
 	l.lint(node, ctx)
 
-	// After whole VCLs have been linted, check all definitions are exactly used.
-	l.lintUnusedTables(ctx)
-	l.lintUnusedAcls(ctx)
-	l.lintUnusedBackends(ctx)
-	l.lintUnusedSubroutines(ctx)
+	// After whole VCLs have been linted in main VCL, check all definitions are exactly used.
+	if isMain {
+		println("main vcl")
+		l.lintUnusedTables(ctx)
+		l.lintUnusedAcls(ctx)
+		l.lintUnusedBackends(ctx)
+		l.lintUnusedSubroutines(ctx)
+	}
 
 	return types.NeverType
 }
@@ -679,14 +682,34 @@ func (l *Linter) lintSetStatement(stmt *ast.SetStatement, ctx *context.Context)
 	case "||=":
 		l.lintLogicalOROperator(stmt.Operator, left, right)
 	default: // "="
-		if left != right {
-			l.Error(InvalidType(stmt.Value.GetMeta(), stmt.Ident.Value, left, right))
-		}
+		l.lintAssignOperator(stmt.Value, stmt.Ident.Value, left, right)
 	}
 
 	return types.NeverType
 }
 
+func (l *Linter) lintAssignOperator(value ast.Expression, name string, left, right types.Type) {
+	switch left {
+	case types.StringType:
+		switch right {
+		case types.StringType:
+			return
+		case types.FloatType, types.RTimeType, types.IntegerType, types.BoolType, types.NullType:
+			// Note: on right expression of set statement, could not assign with literal,
+			// but can assign with variable/identity as stringified value
+			if isLiteralExpression(value) {
+				l.Error(InvalidType(value.GetMeta(), name, left, right))
+			}
+		default:
+			l.Error(InvalidType(value.GetMeta(), name, left, right))
+		}
+	default:
+		if left != right {
+			l.Error(InvalidType(value.GetMeta(), name, left, right))
+		}
+	}
+}
+
 func (l *Linter) lintAdditionOperator(op *ast.Operator, left, right types.Type) {
 	switch left {
 	case types.IntegerType, types.FloatType, types.RTimeType:
@@ -1003,9 +1026,6 @@ func (l *Linter) lintAddStatement(stmt *ast.AddStatement, ctx *context.Context)
 	}
 
 	right := l.lint(stmt.Value, ctx)
-	if left != right {
-		l.Error(InvalidType(stmt.Value.GetMeta(), stmt.Ident.Value, left, right))
-	}
 
 	// Commonly, add statement operator must be "="
 	if stmt.Operator.Operator != "=" {
@@ -1016,6 +1036,7 @@ func (l *Linter) lintAddStatement(stmt *ast.AddStatement, ctx *context.Context)
 		}
 		l.Error(err.Match(OPERATOR_ASSIGNMENT))
 	}
+	l.lintAssignOperator(stmt.Value, stmt.Ident.Value, left, right)
 
 	return types.NeverType
 }
@@ -1435,7 +1456,7 @@ func (l *Linter) lintFunctionCallExpression(exp *ast.FunctionCallExpression, ctx
 		case types.RTimeType:
 			// fuzzy type check: some builtin function expects RTIME type,
 			// then actual argument type could be STRING because VCL TIME type could be parsed from STRING.
-			if !expectType(arg, types.TimeType, types.StringType) {
+			if !expectType(arg, types.RTimeType, types.TimeType, types.StringType) {
 				l.Error(FunctionArgumentTypeMismatch(
 					exp.Function.GetMeta(), exp.Function.String(), i+1, v, arg,
 				).Match(FUNCTION_ARGUMENT_TYPE).Ref(fn.Reference))


Test patch:
diff --git a/context/context_test.go b/context/context_test.go
index 635f593..dfe1ff2 100644
--- a/context/context_test.go
+++ b/context/context_test.go
@@ -3,6 +3,7 @@ package context
 import (
 	"testing"
 
+	"github.com/ysugimoto/falco/ast"
 	"github.com/ysugimoto/falco/types"
 )
 
@@ -174,7 +175,11 @@ func TestDynamicVariableExist(t *testing.T) {
 		if _, err := c.Get("director.example.healthy"); err == nil {
 			t.Errorf("expected error but got nil")
 		}
-		c.AddDirector("example", &types.Director{})
+		c.AddDirector("example", &types.Director{
+			Decl: &ast.DirectorDeclaration{
+				Properties: []ast.Expression{},
+			},
+		})
 		if v, err := c.Get("director.example.healthy"); err != nil {
 			t.Errorf("expected nil but got error: %s", err)
 		} else if v != types.BoolType {
diff --git a/linter/linter_test.go b/linter/linter_test.go
index a763573..47ba7a5 100644
--- a/linter/linter_test.go
+++ b/linter/linter_test.go
@@ -1289,7 +1289,7 @@ sub bar {
 		}
 
 		l := New()
-		l.Lint(vcl, context.New())
+		l.Lint(vcl, context.New(), true)
 		if len(l.Errors) == 0 {
 			t.Errorf("Expect one lint error but empty returned")
 		}
@@ -1305,7 +1305,7 @@ acl foo {}
 		}
 
 		l := New()
-		l.Lint(vcl, context.New())
+		l.Lint(vcl, context.New(), true)
 		if len(l.Errors) == 0 {
 			t.Errorf("Expect one lint error but empty returned")
 		}
@@ -1327,7 +1327,7 @@ sub bar {
 		}
 
 		l := New()
-		l.Lint(vcl, context.New())
+		l.Lint(vcl, context.New(), true)
 		if len(l.Errors) == 0 {
 			t.Errorf("Expect one lint error but empty returned")
 		}
@@ -1343,7 +1343,7 @@ table foo {}
 		}
 
 		l := New()
-		l.Lint(vcl, context.New())
+		l.Lint(vcl, context.New(), true)
 		if len(l.Errors) == 0 {
 			t.Errorf("Expect one lint error but empty returned")
 		}
@@ -1365,7 +1365,7 @@ sub vcl_recl {
 		}
 
 		l := New()
-		l.Lint(vcl, context.New())
+		l.Lint(vcl, context.New(), true)
 		if len(l.Errors) == 0 {
 			t.Errorf("Expect one lint error but empty returned")
 		}
@@ -1381,7 +1381,7 @@ backend foo {}
 		}
 
 		l := New()
-		l.Lint(vcl, context.New())
+		l.Lint(vcl, context.New(), true)
 		if len(l.Errors) == 0 {
 			t.Errorf("Expect one lint error but empty returned")
 		}
@@ -1403,7 +1403,7 @@ sub vcl_recl {
 		}
 
 		l := New()
-		l.Lint(vcl, context.New())
+		l.Lint(vcl, context.New(), true)
 		if len(l.Errors) == 0 {
 			t.Errorf("Expect one lint error but empty returned")
 		}
@@ -1419,7 +1419,7 @@ sub foo {}
 		}
 
 		l := New()
-		l.Lint(vcl, context.New())
+		l.Lint(vcl, context.New(), true)
 		if len(l.Errors) == 0 {
 			t.Errorf("Expect one lint error but empty returned")
 		}
@@ -1440,7 +1440,7 @@ sub vcl_recv {
 		}
 
 		l := New()
-		l.Lint(vcl, context.New())
+		l.Lint(vcl, context.New(), true)
 		if len(l.Errors) == 0 {
 			t.Errorf("Expect one lint error but empty returned")
 		}
@@ -1459,7 +1459,7 @@ sub vcl_recv {
 		}
 
 		l := New()
-		l.Lint(vcl, context.New())
+		l.Lint(vcl, context.New(), true)
 		if len(l.Errors) == 0 {
 			t.Errorf("Expect one lint error but empty returned")
 		}