diff --git a/linter/expression_linter.go b/linter/expression_linter.go index 432281cd..d42613c8 100644 --- a/linter/expression_linter.go +++ b/linter/expression_linter.go @@ -119,6 +119,7 @@ func (l *Linter) lintGroupedExpression(exp *ast.GroupedExpression, ctx *context. // set req.http.Foo = std.time("xxx", now) + var.someRTime; // valid, string concatenation // // See Fastly fiddle: https://fiddle.fastly.dev/fiddle/befec89e +// nolint: gocognit func (l *Linter) lintStringConcatInfixExpression(exp *ast.InfixExpression, ctx *context.Context) types.Type { var series []*Series @@ -155,17 +156,39 @@ func (l *Linter) lintStringConcatInfixExpression(exp *ast.InfixExpression, ctx * for i := 0; i < len(series); i++ { s := series[i] nt := l.lint(s.Expression, ctx) + meta := s.Expression.GetMeta() - switch s.Expression.(type) { + switch t := s.Expression.(type) { case *ast.String: break case *ast.IfExpression: - if nt != types.StringType { - l.Error(InvalidStringConcatenation(exp.GetMeta(), nt.String()).Match(OPERATOR_CONDITIONAL)) + switch nt { + case types.AclType, types.BackendType: + l.Error(InvalidStringConcatenation(meta, nt.String()).Match(OPERATOR_CONDITIONAL)) + case types.StringType: + break + default: + // If consequence and alternative expression is not STRING type (e.g. INTEGER, FLOAT, BOOL) in if expression, + // The expression must not be a literal + switch { + case isConstantExpression(t.Consequence): + l.Error(InvalidStringConcatenation(t.Consequence.GetMeta(), nt.String()).Match(OPERATOR_CONDITIONAL)) + case isConstantExpression(t.Alternative): + l.Error(InvalidStringConcatenation(t.Alternative.GetMeta(), nt.String()).Match(OPERATOR_CONDITIONAL)) + default: + l.Error(ImplicitTypeConversion(meta, nt, types.StringType)) + } } case *ast.FunctionCallExpression: - if nt != types.StringType { - l.Error(InvalidStringConcatenation(exp.GetMeta(), nt.String()).Match(OPERATOR_CONDITIONAL)) + // If function call expression, arbitrary type except ACL and BACKEND can be used. + // However, without STRING type (e.g INTEGER, FLOAT) will be implicit type conversion + switch nt { + case types.AclType, types.BackendType: + l.Error(InvalidStringConcatenation(meta, nt.String()).Match(OPERATOR_CONDITIONAL)) + case types.StringType: + break + default: + l.Error(ImplicitTypeConversion(meta, nt, types.StringType)) } case *ast.Ident: switch nt { @@ -184,11 +207,12 @@ func (l *Linter) lintStringConcatInfixExpression(exp *ast.InfixExpression, ctx * // If ident value type is RTIME with plus sign, previous type must no TIME type (string concatenation) if s.Operator == "+" || s.Operator == "-" { if ct == types.TimeType { - l.Error(InvalidStringConcatenation(exp.GetMeta(), "RTIME").Match(OPERATOR_CONDITIONAL)) + l.Error(InvalidStringConcatenation(meta, "RTIME").Match(OPERATOR_CONDITIONAL)) + goto OUT } } } - l.Error(ImplicitTypeConversion(exp.GetMeta(), nt, types.StringType)) + l.Error(ImplicitTypeConversion(meta, nt, types.StringType)) case *ast.RTime: // If expression is RTIME literal, follwing condition must be satisfied: // - previous expression is IDENT and the value if TIME type @@ -197,14 +221,14 @@ func (l *Linter) lintStringConcatInfixExpression(exp *ast.InfixExpression, ctx * if s.Operator == "+" || s.Operator == "-" { if _, ok := series[i-1].Expression.(*ast.Ident); ok { // Valid for time addition but we will report as WARNING - l.Error(TimeCalculatation(s.Expression.GetMeta()).Match(TIME_CALCULATION)) + l.Error(TimeCalculatation(meta).Match(TIME_CALCULATION)) goto OUT } } } - l.Error(InvalidStringConcatenation(exp.GetMeta(), "RTIME").Match(OPERATOR_CONDITIONAL)) + l.Error(InvalidStringConcatenation(meta, "RTIME").Match(OPERATOR_CONDITIONAL)) default: - l.Error(InvalidStringConcatenation(exp.GetMeta(), nt.String()).Match(OPERATOR_CONDITIONAL)) + l.Error(InvalidStringConcatenation(meta, nt.String()).Match(OPERATOR_CONDITIONAL)) } OUT: ct = nt diff --git a/linter/expression_linter_test.go b/linter/expression_linter_test.go index 9be91c81..17bb09d9 100644 --- a/linter/expression_linter_test.go +++ b/linter/expression_linter_test.go @@ -399,12 +399,16 @@ func TestStringConcatenation(t *testing.T) { { name: "FunctionCall expression concatenation", statement: `set req.http.Foo = "foo" + std.atoi("foo");`, - isError: true, }, { name: "if expression concatenation, returns string", statement: `set req.http.Foo = "foo" + if(req.http.Bar, "1", "0");`, }, + { + name: "if expression concatenation, returns constant", + statement: `set req.http.Foo = "foo" + if(req.http.Bar, 1, 0);`, + isError: true, + }, { name: "if expression concatenation, returns not string", statement: `set req.http.Foo = "foo" + if(req.http.Bar, 1, 0);`, @@ -526,6 +530,15 @@ func TestStringConcatenationIssue360(t *testing.T) { `, isError: true, }, + { + name: "complicated concatenation for common set-cookie expression", + input: ` + sub vcl_deliver { + #FASTLY deliver + set resp.http.Set-Cookie = "test=abc; domain=fiddle.fastly.dev; path=/; expires=" time.add(now, 3600s) if(req.http.Foo == "bar", std.atoi("1"), std.atoi("2")) ";"; + } + `, + }, } for _, tt := range tests {