From 8c779447db7baf8e9dfd25fc4003790759af4479 Mon Sep 17 00:00:00 2001
From: Hajime Hoshi <hajimehoshi@gmail.com>
Date: Sun, 30 Aug 2020 21:11:27 +0900
Subject: [PATCH] shader: Friendly error messages when local variable names are
 duplicated

Fixes #1254
---
 internal/shader/shader.go |  6 ++++
 internal/shader/stmt.go   | 18 +++++++++--
 shader_test.go            | 65 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/internal/shader/shader.go b/internal/shader/shader.go
index 91054fb0d..ccbf241e3 100644
--- a/internal/shader/shader.go
+++ b/internal/shader/shader.go
@@ -483,6 +483,12 @@ func (s *compileState) parseVariable(block *block, vs *ast.ValueSpec) ([]variabl
 		}
 
 		name := n.Name
+		for _, v := range append(block.vars, vars...) {
+			if v.name == name {
+				s.addError(vs.Pos(), fmt.Sprintf("duplicated local variable name: %s", name))
+				return nil, nil, nil, false
+			}
+		}
 		vars = append(vars, variable{
 			name: name,
 			typ:  t,
diff --git a/internal/shader/stmt.go b/internal/shader/stmt.go
index 1edf65975..1d968e3e1 100644
--- a/internal/shader/stmt.go
+++ b/internal/shader/stmt.go
@@ -455,8 +455,15 @@ func (cs *compileState) assign(block *block, pos token.Pos, lhs, rhs []ast.Expr,
 			stmts = append(stmts, ss...)
 
 			if define {
+				name := e.(*ast.Ident).Name
+				for _, v := range block.vars {
+					if v.name == name {
+						cs.addError(pos, fmt.Sprintf("duplicated local variable name: %s", name))
+						return nil, false
+					}
+				}
 				v := variable{
-					name: e.(*ast.Ident).Name,
+					name: name,
 				}
 				ts, ok := cs.functionReturnTypes(block, rhs[i])
 				if !ok {
@@ -545,8 +552,15 @@ func (cs *compileState) assign(block *block, pos token.Pos, lhs, rhs []ast.Expr,
 			}
 
 			if define {
+				name := e.(*ast.Ident).Name
+				for _, v := range block.vars {
+					if v.name == name {
+						cs.addError(pos, fmt.Sprintf("duplicated local variable name: %s", name))
+						return nil, false
+					}
+				}
 				v := variable{
-					name: e.(*ast.Ident).Name,
+					name: name,
 				}
 				v.typ = rhsTypes[i]
 				block.vars = append(block.vars, v)
diff --git a/shader_test.go b/shader_test.go
index d7f43d673..bbbed0778 100644
--- a/shader_test.go
+++ b/shader_test.go
@@ -114,3 +114,68 @@ func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
 		}
 	}
 }
+
+func TestShaderShadowing(t *testing.T) {
+	_, err := NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	var position vec4
+	return position
+}
+`))
+	if err == nil {
+		t.Errorf("error must be non-nil but was nil")
+	}
+}
+
+func TestShaderDuplicatedVariables(t *testing.T) {
+	_, err := NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	var foo vec4
+	var foo vec4
+	return foo
+}
+`))
+	if err == nil {
+		t.Errorf("error must be non-nil but was nil")
+	}
+
+	_, err = NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	var foo, foo vec4
+	return foo
+}
+`))
+	if err == nil {
+		t.Errorf("error must be non-nil but was nil")
+	}
+
+	_, err = NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	var foo vec4
+	foo := vec4(0)
+	return foo
+}
+`))
+	if err == nil {
+		t.Errorf("error must be non-nil but was nil")
+	}
+
+	_, err = NewShader([]byte(`package main
+
+func Foo() (vec4, vec4) {
+	return vec4(0), vec4(0)
+}
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	foo, foo := Foo()
+	return foo
+}
+`))
+	if err == nil {
+		t.Errorf("error must be non-nil but was nil")
+	}
+}