Repository: ysugimoto/falco (go)

Issue:
[BUG] Parser/Linter error `return;` in `vcl_recv`
**Describe the problem**
A clear and concise description of what the bug is.

False positive linter


```
sub vcl_recv {
   return;
}
```

Falco allows it but [fastly doesnt](https://fiddle.fastly.dev/fiddle/5c934b7c)

Pull request:
add disallow empty return linting rule
Fix #152 

This PR adds a new linter rule that disallows empty return statement in the state-machine method.

Patch:
diff --git a/context/context.go b/context/context.go
index 72dca1e..337dee0 100644
--- a/context/context.go
+++ b/context/context.go
@@ -109,24 +109,28 @@ type Accessor struct {
 }
 
 type Context struct {
+	// private fields
 	curMode        int
 	prevMode       int
 	curName        string
-	Acls           map[string]*types.Acl
-	Backends       map[string]*types.Backend
-	Tables         map[string]*types.Table
-	Directors      map[string]*types.Director
-	Subroutines    map[string]*types.Subroutine
-	Penaltyboxes   map[string]*types.Penaltybox
-	Ratecounters   map[string]*types.Ratecounter
-	Gotos          map[string]*types.Goto
-	Identifiers    map[string]struct{}
 	functions      Functions
 	Variables      Variables
-	RegexVariables map[string]int
-	ReturnType     *types.Type
 	resolver       resolver.Resolver
 	fastlySnippets *FastlySnippet
+
+	// public fields
+	Acls              map[string]*types.Acl
+	Backends          map[string]*types.Backend
+	Tables            map[string]*types.Table
+	Directors         map[string]*types.Director
+	Subroutines       map[string]*types.Subroutine
+	Penaltyboxes      map[string]*types.Penaltybox
+	Ratecounters      map[string]*types.Ratecounter
+	Gotos             map[string]*types.Goto
+	Identifiers       map[string]struct{}
+	RegexVariables    map[string]int
+	ReturnType        *types.Type
+	CurrentSubroutine *ast.SubroutineDeclaration
 }
 
 func New(opts ...Option) *Context {
@@ -157,6 +161,14 @@ func (c *Context) Mode() int {
 	return c.curMode
 }
 
+// Returns true if statement/expression is inside state-mechine subroutine like "vcl_recv", fastly reserved one
+func (c *Context) IsStateMachineMethod() bool {
+	if c.CurrentSubroutine == nil {
+		return false
+	}
+	return IsFastlySubroutine(c.CurrentSubroutine.Name.Value)
+}
+
 func (c *Context) Resolver() resolver.Resolver {
 	if c.resolver == nil {
 		c.resolver = &resolver.EmptyResolver{}
diff --git a/linter/linter.go b/linter/linter.go
index ffb1e0e..a8528ae 100644
--- a/linter/linter.go
+++ b/linter/linter.go
@@ -831,6 +831,13 @@ func (l *Linter) lintSubRoutineDeclaration(decl *ast.SubroutineDeclaration, ctx
 		l.lintFastlyBoilerPlateMacro(decl, ctx, scope)
 	}
 
+	// Store current subroutine in order to be able to access via statements inside
+	ctx.CurrentSubroutine = decl
+	defer func() {
+		// Release it on subroutine linting has ended
+		ctx.CurrentSubroutine = nil
+	}()
+
 	l.lint(decl.Block, cc)
 
 	// We are done linting inside the previous scope so
@@ -1382,8 +1389,16 @@ func (l *Linter) lintReturnStatement(stmt *ast.ReturnStatement, ctx *context.Con
 		expects = append(expects, "deliver")
 	}
 
-	// return statement may not have arguments, then stop linting
+	// If return statement does not have arguemnt, but Fastly requires next state in state-machine method like "vcl_recv"
 	if stmt.ReturnExpression == nil {
+		if ctx.IsStateMachineMethod() {
+			err := &LintError{
+				Severity: ERROR,
+				Token:    stmt.GetMeta().Token,
+				Message:  "Empty return is disallowed in state-machine method",
+			}
+			l.Error(err.Match(DISALLOW_EMPTY_RETURN))
+		}
 		return types.NeverType
 	}
 
diff --git a/linter/rules.go b/linter/rules.go
index 2c48162..d3ddf8a 100644
--- a/linter/rules.go
+++ b/linter/rules.go
@@ -66,6 +66,7 @@ const (
 	UNUSED_DECLARATION                   = "unused/declaration"
 	UNUSED_VARIABLE                      = "unused/variable"
 	UNUSED_GOTO                          = "unused/goto"
+	DISALLOW_EMPTY_RETURN                = "disallow-empty-return"
 )
 
 var references = map[Rule]string{
@@ -100,4 +101,5 @@ var references = map[Rule]string{
 	ERROR_STATEMENT_CODE:             "https://developer.fastly.com/reference/vcl/statements/error/#best-practices-for-using-status-codes-for-errors",
 	SYNTHETIC_STATEMENT_SCOPE:        "https://developer.fastly.com/reference/vcl/statements/synthetic/",
 	SYNTHETIC_BASE64_STATEMENT_SCOPE: "https://developer.fastly.com/reference/vcl/statements/synthetic-base64/",
+	DISALLOW_EMPTY_RETURN:            "https://developer.fastly.com/reference/vcl/subroutines#returning-a-state",
 }


Test patch:
diff --git a/linter/linter_test.go b/linter/linter_test.go
index 2cab3b5..d6ab9b4 100644
--- a/linter/linter_test.go
+++ b/linter/linter_test.go
@@ -2613,3 +2613,40 @@ sub vcl_recv {
 }`
 	assertNoError(t, input)
 }
+
+func TestEmptyReturnStatement(t *testing.T) {
+	t.Run("Error on state-machine-methods", func(t *testing.T) {
+		methodWithMacros := map[string]string{
+			"vcl_recv":    "#FASTLY RECV",
+			"vcl_hash":    "#FASTLY HASH",
+			"vcl_hit":     "#FASTLY HIT",
+			"vcl_miss":    "#FASTLY MISS",
+			"vcl_pass":    "#FASTLY PASS",
+			"vcl_fetch":   "#FASTLY FETCH",
+			"vcl_error":   "#FASTLY ERROR",
+			"vcl_deliver": "#FASTLY DELIVER",
+			"vcl_log":     "#FASTLY LOG",
+		}
+		for method, macro := range methodWithMacros {
+			input := fmt.Sprintf(
+				`
+sub %s {
+	%s
+	return;
+}`, method, macro)
+			assertError(t, input)
+		}
+	})
+
+	t.Run("Pass on other subroutine", func(t *testing.T) {
+		input := `
+sub foo {
+	return;
+}
+sub vcl_recv {
+	#FASTLY RECV
+	call foo;
+}`
+		assertNoError(t, input)
+	})
+}