From 5bdf6a806f314ba920300299ea3e834047c90c61 Mon Sep 17 00:00:00 2001 From: Tiago Natel de Moura Date: Sat, 12 Nov 2016 17:12:18 -0200 Subject: [PATCH] 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 --- cmd/nashfmt/main.go | 8 ++--- internal/sh/shell_test.go | 2 +- parser/parse.go | 47 ++++++++++++++++++++++------- parser/parse_regression_test.go | 17 +++++++++++ parser/parse_test.go | 52 +++++++++++++++++++++++++++++++++ scanner/lex.go | 6 +++- scanner/lex_test.go | 20 +++++++++++++ 7 files changed, 135 insertions(+), 17 deletions(-) diff --git a/cmd/nashfmt/main.go b/cmd/nashfmt/main.go index 5410f3ab..b5aa887e 100644 --- a/cmd/nashfmt/main.go +++ b/cmd/nashfmt/main.go @@ -36,7 +36,7 @@ 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) } @@ -44,7 +44,7 @@ func main() { 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 --git a/internal/sh/shell_test.go b/internal/sh/shell_test.go index 7b2d807a..ae033486 100644 --- a/internal/sh/shell_test.go +++ b/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 --git a/parser/parse.go b/parser/parse.go index 3a0dec4a..513b8eaa 100644 --- a/parser/parse.go +++ b/parser/parse.go @@ -208,7 +208,11 @@ 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() @@ -216,10 +220,16 @@ func (p *Parser) parsePipe(first *ast.CommandNode) (ast.Node, error) { 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,15 +306,15 @@ 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() @@ -304,10 +322,17 @@ cmdLoop: 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 --git a/parser/parse_regression_test.go b/parser/parse_regression_test.go index be3803d5..2317a418 100644 --- a/parser/parse_regression_test.go +++ b/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 --git a/parser/parse_test.go b/parser/parse_test.go index 71b0993c..b6e2158d 100644 --- a/parser/parse_test.go +++ b/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 --git a/scanner/lex.go b/scanner/lex.go index 6ddc7d00..092641f9 100644 --- a/scanner/lex.go +++ b/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 --git a/scanner/lex_test.go b/scanner/lex_test.go index 01c00007..71d55020 100644 --- a/scanner/lex_test.go +++ b/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"},