Improve the parser on several corner cases: (#138)

- Disallow use of function call in pipelines

The command below now throws error:

  echo "test test" | replace(" ", "|")

- Improve the error for invalid syntaxes (Closes #123)

Signed-off-by: Tiago <[email protected]>

Author: i4ki

Diff for: cmd/nashfmt/main.go

@@ -36,15 +36,15 @@ func main() {
 	file, err = os.Open(fname)
 
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "[ERROR] "+err.Error())
+		fmt.Fprintf(os.Stderr, "error: %s\n", err.Error())
 		os.Exit(1)
 	}
 
 	content, err := ioutil.ReadAll(file)
 
 	if err != nil {
 		file.Close()
-		fmt.Fprintf(os.Stderr, "[ERROR] "+err.Error())
+		fmt.Fprintf(os.Stderr, "error: %s\n", err.Error())
 		os.Exit(1)
 	}
 
@@ -53,7 +53,7 @@ func main() {
 	ast, err := parser.Parse()
 
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "[ERROR] "+err.Error())
+		fmt.Fprintf(os.Stderr, "%s\n", err.Error())
 		file.Close()
 		os.Exit(1)
 	}
@@ -69,7 +69,7 @@ func main() {
 		err = ioutil.WriteFile(fname, []byte(fmt.Sprintf("%s\n", ast.String())), 0666)
 
 		if err != nil {
-			fmt.Fprintf(os.Stderr, "[ERROR] "+err.Error())
+			fmt.Fprintf(os.Stderr, "error: %s\n", err.Error())
 			return
 		}
 	}

Diff for: internal/sh/shell_test.go

@@ -2041,7 +2041,7 @@ func TestExecuteMultilineCmdAssign(t *testing.T) {
 	}
 }
 
-func TestExecuteMuliReturnUnfinished(t *testing.T) {
+func TestExecuteMultiReturnUnfinished(t *testing.T) {
 	shell, err := NewShell()
 
 	if err != nil {

Diff for: parser/parse.go

@@ -208,18 +208,28 @@ func (p *Parser) parsePipe(first *ast.CommandNode) (ast.Node, error) {
 	if n.IsMulti() {
 		it = p.peek()
 		if it.Type() != token.RParen {
-			return nil, errors.NewUnfinishedCmdError(p.name, it)
+			if it.Type() == token.EOF {
+				return nil, errors.NewUnfinishedCmdError(p.name, it)
+			}
+
+			return nil, newParserError(it, p.name, "Unexpected symbol '%s'", it)
 		}
 
 		p.ignore()
 	}
 
 	it = p.peek()
 
-	if it.Type() == token.Semicolon {
-		p.ignore()
+	if it.Type() == token.RBrace {
+		return n, nil
 	}
 
+	if it.Type() != token.Semicolon {
+		return nil, newParserError(it, p.name, "Unexpected symbol %s", it)
+	}
+
+	p.ignore()
+
 	return n, nil
 }
 
@@ -234,7 +244,7 @@ func (p *Parser) parseCommand(it scanner.Token) (ast.Node, error) {
 	}
 
 	if it.Type() != token.Ident && it.Type() != token.Arg {
-		if isMulti {
+		if isMulti && it.Type() == token.EOF {
 			return nil, errors.NewUnfinishedCmdError(p.name, it)
 		}
 
@@ -249,6 +259,14 @@ cmdLoop:
 
 		switch typ := it.Type(); {
 		case typ == token.RBrace:
+			if p.openblocks > 0 {
+				if p.insidePipe {
+					p.insidePipe = false
+				}
+
+				return n, nil
+			}
+
 			break cmdLoop
 		case isValidArgument(it):
 			arg, err := p.getArgument(true, true)
@@ -288,26 +306,33 @@ cmdLoop:
 		}
 	}
 
-	if p.insidePipe {
-		p.insidePipe = false
-	}
-
 	it = p.peek()
 
 	if isMulti {
 		if it.Type() != token.RParen {
-			return nil, errors.NewUnfinishedCmdError(p.name, it)
+			if it.Type() == token.EOF {
+				return nil, errors.NewUnfinishedCmdError(p.name, it)
+			}
+
+			return nil, newParserError(it, p.name, "Unexpected symbol '%s'", it)
 		}
 
 		p.ignore()
 
 		it = p.peek()
 	}
 
-	if it.Type() == token.Semicolon {
-		p.ignore()
+	if p.insidePipe {
+		p.insidePipe = false
+		return n, nil
+	}
+
+	if it.Type() != token.Semicolon {
+		return nil, newParserError(it, p.name, "Unexpected symbol '%s'", it)
 	}
 
+	p.ignore()
+
 	return n, nil
 }
 

Diff for: parser/parse_regression_test.go

@@ -244,3 +244,20 @@ func TestParseIssue108(t *testing.T) {
 
 	parserTestTable("parse issue #108", `cat spec.ebnf | grep -i rfork`, expected, t, false)
 }
+
+func TestParseIssue123(t *testing.T) {
+	parser := NewParser("invalid cmd assignment", `IFS <= ("\n")`)
+
+	_, err := parser.Parse()
+
+	if err == nil {
+		t.Errorf("Must fail...")
+		return
+	}
+
+	expected := "invalid cmd assignment:1:9: Unexpected token STRING. Expecting IDENT or ARG"
+	if err.Error() != expected {
+		t.Fatalf("Error string differs. Expecting '%s' but got '%s'",
+			expected, err.Error())
+	}
+}

Diff for: parser/parse_test.go

@@ -947,6 +947,46 @@ func TestParseFnBasic(t *testing.T) {
 }`, expected, t, true)
 }
 
+func TestParseInlineFnDecl(t *testing.T) {
+	expected := ast.NewTree("fn")
+	ln := ast.NewBlockNode(token.NewFileInfo(1, 0))
+
+	fn := ast.NewFnDeclNode(token.NewFileInfo(1, 0), "cd")
+	tree := ast.NewTree("fn body")
+	lnBody := ast.NewBlockNode(token.NewFileInfo(1, 0))
+	echo := ast.NewCommandNode(token.NewFileInfo(1, 11), "echo", false)
+	echo.AddArg(ast.NewStringExpr(token.NewFileInfo(1, 16), "hello", true))
+	lnBody.Push(echo)
+
+	tree.Root = lnBody
+	fn.SetTree(tree)
+
+	// root
+	ln.Push(fn)
+	expected.Root = ln
+
+	parserTestTable("inline fn", `fn cd() { echo "hello" }`,
+		expected, t, false)
+
+	test := ast.NewCommandNode(token.NewFileInfo(1, 26), "test", false)
+	test.AddArg(ast.NewStringExpr(token.NewFileInfo(1, 32), "-d", false))
+	test.AddArg(ast.NewStringExpr(token.NewFileInfo(1, 35), "/etc", false))
+
+	pipe := ast.NewPipeNode(token.NewFileInfo(1, 11), false)
+	pipe.AddCmd(echo)
+	pipe.AddCmd(test)
+	lnBody = ast.NewBlockNode(token.NewFileInfo(1, 0))
+	lnBody.Push(pipe)
+	tree.Root = lnBody
+	fn.SetTree(tree)
+	ln = ast.NewBlockNode(token.NewFileInfo(1, 0))
+	ln.Push(fn)
+	expected.Root = ln
+
+	parserTestTable("inline fn", `fn cd() { echo "hello" | test -d /etc }`,
+		expected, t, false)
+}
+
 func TestParseBindFn(t *testing.T) {
 	expected := ast.NewTree("bindfn")
 	ln := ast.NewBlockNode(token.NewFileInfo(1, 0))
@@ -1216,3 +1256,15 @@ func TestMultiPipe(t *testing.T) {
 	awk "{print AAAAAAAAAAAAAAAAAAAAAA}"
 )`, expected, t, true)
 }
+
+func TestFunctionPipes(t *testing.T) {
+	parser := NewParser("invalid pipe with functions",
+		`echo "some thing" | replace(" ", "|")`)
+
+	_, err := parser.Parse()
+
+	if err == nil {
+		t.Error("Must fail. Function must be bind'ed to command name to use in pipe.")
+		return
+	}
+}

Diff for: scanner/lex.go

@@ -372,7 +372,11 @@ func lexStart(l *Lexer) stateFn {
 			l.emit(token.Arg)
 		}
 
-		l.addSemicolon = true
+		if next == eof && l.openParens > 0 {
+			l.addSemicolon = false
+		} else {
+			l.addSemicolon = true
+		}
 
 		return lexStart
 	case isArgument(r):

Diff for: scanner/lex_test.go

@@ -484,6 +484,26 @@ func TestLexerPipe(t *testing.T) {
 	testTable("testPipe with redirection", `go tool vet -h > out.log | grep log`, expected, t)
 }
 
+func TestPipeFunctions(t *testing.T) {
+	expected := []Token{
+		{typ: token.Ident, val: "echo"},
+		{typ: token.String, val: "some thing"},
+		{typ: token.Pipe, val: "|"},
+		{typ: token.Ident, val: "replace"},
+		{typ: token.LParen, val: "("},
+		{typ: token.String, val: " "},
+		{typ: token.Comma, val: ","},
+		{typ: token.String, val: "|"},
+		{typ: token.RParen, val: ")"},
+		{typ: token.Semicolon, val: ";"},
+		{typ: token.EOF},
+	}
+
+	testTable("test pipe with function",
+		`echo "some thing" | replace(" ", "|")`,
+		expected, t)
+}
+
 func TestLexerUnquoteArg(t *testing.T) {
 	expected := []Token{
 		{typ: token.Ident, val: "echo"},