diff --git a/internal/sh/builtin/len.go b/internal/sh/builtin/len.go index 5a15ef23..e380e0d9 100644 --- a/internal/sh/builtin/len.go +++ b/internal/sh/builtin/len.go @@ -10,7 +10,7 @@ import ( type ( lenFn struct { - arg sh.Obj + arg sh.Collection } ) @@ -29,12 +29,7 @@ func lenresult(res int) []sh.Obj { } func (l *lenFn) Run(in io.Reader, out io.Writer, err io.Writer) ([]sh.Obj, error) { - if l.arg.Type() == sh.ListType { - arglist := l.arg.(*sh.ListObj) - return lenresult(len(arglist.List())), nil - } - argstr := l.arg.(*sh.StrObj) - return lenresult(len(argstr.Str())), nil + return lenresult(l.arg.Len()), nil } func (l *lenFn) SetArgs(args []sh.Obj) error { @@ -43,11 +38,11 @@ func (l *lenFn) SetArgs(args []sh.Obj) error { } obj := args[0] - - if obj.Type() != sh.ListType && obj.Type() != sh.StringType { - return errors.NewError("lenfn expects a list or a string, but a %s was provided", obj.Type()) + col, err := sh.NewCollection(obj) + if err != nil { + return errors.NewError("len:error[%s]", err) } - l.arg = obj + l.arg = col return nil } diff --git a/internal/sh/shell.go b/internal/sh/shell.go index 4592895d..357c01de 100644 --- a/internal/sh/shell.go +++ b/internal/sh/shell.go @@ -529,41 +529,37 @@ func (shell *Shell) ExecFile(path string) error { } func (shell *Shell) setvar(name *ast.NameNode, value sh.Obj) error { - finalObj := value - if name.Index != nil { - if list, ok := shell.Getvar(name.Ident); ok { - index, err := shell.evalIndex(name.Index) - - if err != nil { - return err - } - - if list.Type() != sh.ListType { - return errors.NewEvalError(shell.filename, - name, "Indexed assigment on non-list type: Variable %s is %s", - name.Ident, - list.Type()) - } + if name.Index == nil { + shell.Setvar(name.Ident, value) + return nil + } - lobj := list.(*sh.ListObj) - lvalues := lobj.List() + obj, ok := shell.Getvar(name.Ident) + if !ok { + return errors.NewEvalError(shell.filename, + name, "Variable %s not found", name.Ident) + } - if index >= len(lvalues) { - return errors.NewEvalError(shell.filename, - name, "List out of bounds error. List has %d elements, but trying to access index %d", - len(lvalues), index) - } + index, err := shell.evalIndex(name.Index) + if err != nil { + return err + } - lvalues[index] = value - finalObj = sh.NewListObj(lvalues) - } else { - return errors.NewEvalError(shell.filename, - name, "Variable %s not found", name.Ident) - } + col, err := sh.NewWriteableCollection(obj) + if err != nil { + return errors.NewEvalError(shell.filename, name, err.Error()) } - shell.Setvar(name.Ident, finalObj) + err = col.Set(index, value) + if err != nil { + return errors.NewEvalError( + shell.filename, + name, + "error[%s] setting var", + err, + ) + } return nil } @@ -1486,9 +1482,9 @@ func (shell *Shell) evalIndexedVar(indexVar *ast.IndexExpr) (sh.Obj, error) { return nil, err } - if v.Type() != sh.ListType { - return nil, errors.NewEvalError(shell.filename, indexVar.Var, - "Invalid indexing of non-list variable: %s (%+v)", v.Type(), v) + col, err := sh.NewCollection(v) + if err != nil { + return nil, errors.NewEvalError(shell.filename, indexVar.Var, err.Error()) } indexNum, err := shell.evalIndex(indexVar.Index) @@ -1496,17 +1492,11 @@ func (shell *Shell) evalIndexedVar(indexVar *ast.IndexExpr) (sh.Obj, error) { return nil, err } - vlist := v.(*sh.ListObj) - values := vlist.List() - - if indexNum < 0 || indexNum >= len(values) { - return nil, errors.NewEvalError(shell.filename, - indexVar.Var, - "Index out of bounds. len(%s) == %d, but given %d", indexVar.Var.Name, - len(values), indexNum) + val, err := col.Get(indexNum) + if err != nil { + return nil, errors.NewEvalError(shell.filename, indexVar.Var, err.Error()) } - - return values[indexNum], nil + return val, nil } func (shell *Shell) evalArgIndexedVar(indexVar *ast.IndexExpr) ([]sh.Obj, error) { @@ -1515,9 +1505,9 @@ func (shell *Shell) evalArgIndexedVar(indexVar *ast.IndexExpr) ([]sh.Obj, error) return nil, err } - if v.Type() != sh.ListType { - return nil, errors.NewEvalError(shell.filename, indexVar.Var, - "Invalid indexing of non-list variable: %s (%+v)", v.Type(), v) + col, err := sh.NewCollection(v) + if err != nil { + return nil, errors.NewEvalError(shell.filename, indexVar.Var, err.Error()) } indexNum, err := shell.evalIndex(indexVar.Index) @@ -1525,18 +1515,11 @@ func (shell *Shell) evalArgIndexedVar(indexVar *ast.IndexExpr) ([]sh.Obj, error) return nil, err } - vlist := v.(*sh.ListObj) - values := vlist.List() - - if indexNum < 0 || indexNum >= len(values) { - return nil, errors.NewEvalError(shell.filename, - indexVar.Var, - "Index out of bounds. len(%s) == %d, but given %d", indexVar.Var.Name, - len(values), indexNum) + retval, err := col.Get(indexNum) + if err != nil { + return nil, errors.NewEvalError(shell.filename, indexVar.Var, err.Error()) } - retval := values[indexNum] - if indexVar.IsVariadic { if retval.Type() != sh.ListType { return nil, errors.NewEvalError(shell.filename, @@ -1545,7 +1528,7 @@ func (shell *Shell) evalArgIndexedVar(indexVar *ast.IndexExpr) ([]sh.Obj, error) retlist := retval.(*sh.ListObj) return retlist.List(), nil } - return []sh.Obj{values[indexNum]}, nil + return []sh.Obj{retval}, nil } func (shell *Shell) evalVariable(a ast.Expr) (sh.Obj, error) { @@ -2213,14 +2196,18 @@ func (shell *Shell) executeFor(n *ast.ForNode) ([]sh.Obj, error) { return nil, err } - if obj.Type() != sh.ListType { + col, err := sh.NewCollection(obj) + if err != nil { return nil, errors.NewEvalError(shell.filename, - inExpr, "Invalid variable type in for range: %s", obj.Type()) + inExpr, "error[%s] trying to iterate", err) } - objlist := obj.(*sh.ListObj) - - for _, val := range objlist.List() { + for i := 0; i < col.Len(); i++ { + val, err := col.Get(i) + if err != nil { + return nil, errors.NewEvalError(shell.filename, + inExpr, "unexpected error[%s] during iteration", err) + } shell.Setvar(id, val) objs, err := shell.executeTree(n.Tree(), false) diff --git a/internal/sh/shell_test.go b/internal/sh/shell_test.go index 2e240be6..d0c55aff 100644 --- a/internal/sh/shell_test.go +++ b/internal/sh/shell_test.go @@ -524,59 +524,6 @@ func TestExecuteCmdMultipleAssignment(t *testing.T) { } } -// IFS *DO NOT* exists anymore. -// This tests only assure things works as expected (IFS has no power) -func TestExecuteCmdAssignmentIFSDontWork(t *testing.T) { - for _, test := range []execTestCase{ - { - "ifs", - `IFS = (" ") -range <= echo 1 2 3 4 5 6 7 8 9 10 - -for i in $range { - echo "i = " + $i -}`, - "", "", - ":4:9: Invalid variable type in for range: StringType", - }, - { - "ifs", - `IFS = (";") -range <= echo "1;2;3;4;5;6;7;8;9;10" - -for i in $range { - echo "i = " + $i -}`, - "", "", - ":4:9: Invalid variable type in for range: StringType", - }, - { - "ifs", - `IFS = (" " ";") -range <= echo "1;2;3;4;5;6 7;8;9;10" - -for i in $range { - echo "i = " + $i -}`, - "", "", - ":4:9: Invalid variable type in for range: StringType", - }, - { - "ifs", - `IFS = (" " "-") -range <= echo "1;2;3;4;5;6;7-8;9;10" - -for i in $range { - echo "i = " + $i -}`, - "", "", - ":4:9: Invalid variable type in for range: StringType", - }, - } { - testExec(t, test) - } -} - func TestExecuteRedirection(t *testing.T) { f, teardown := setup(t) defer teardown() diff --git a/sh/obj.go b/sh/obj.go index c84d671b..d5a625f0 100644 --- a/sh/obj.go +++ b/sh/obj.go @@ -29,25 +29,72 @@ type ( StrObj struct { objType - str string + runes []rune + } + + Collection interface { + Len() int + Get(index int) (Obj, error) + } + + WriteableCollection interface { + Set(index int, val Obj) error } ) +func NewCollection(o Obj) (Collection, error) { + sizer, ok := o.(Collection) + if !ok { + return nil, fmt.Errorf( + "SizeError: trying to get size from type %s which is not a collection", + o.Type(), + ) + } + return sizer, nil +} + +func NewWriteableCollection(o Obj) (WriteableCollection, error) { + indexer, ok := o.(WriteableCollection) + if !ok { + return nil, fmt.Errorf( + "IndexError: trying to use a non write/indexable type %s to write on index: ", + o.Type(), + ) + } + return indexer, nil +} + func (o objType) Type() objType { return o } func NewStrObj(val string) *StrObj { return &StrObj{ - str: val, + runes: []rune(val), objType: StringType, } } -func (o *StrObj) Str() string { return o.str } +func (o *StrObj) Str() string { return string(o.runes) } func (o *StrObj) String() string { return o.Str() } +func (o *StrObj) Get(index int) (Obj, error) { + if index >= o.Len() { + return nil, fmt.Errorf( + "IndexError: Index[%d] out of range, string size[%d]", + index, + o.Len(), + ) + } + + return NewStrObj(string(o.runes[index])), nil +} + +func (o *StrObj) Len() int { + return len(o.runes) +} + func NewFnObj(val FnDef) *FnObj { return &FnObj{ fn: val, @@ -66,6 +113,33 @@ func NewListObj(val []Obj) *ListObj { } } +func (o *ListObj) Len() int { + return len(o.list) +} + +func (o *ListObj) Set(index int, value Obj) error { + if index >= len(o.list) { + return fmt.Errorf( + "IndexError: Index[%d] out of range, list size[%d]", + index, + len(o.list), + ) + } + o.list[index] = value + return nil +} + +func (o *ListObj) Get(index int) (Obj, error) { + if index >= len(o.list) { + return nil, fmt.Errorf( + "IndexError: Index out of bounds, index[%d] but list size[%d]", + index, + len(o.list), + ) + } + return o.list[index], nil +} + func (o *ListObj) List() []Obj { return o.list } func (o *ListObj) String() string { diff --git a/tests/cfg.go b/tests/cfg.go new file mode 100644 index 00000000..ea6c164e --- /dev/null +++ b/tests/cfg.go @@ -0,0 +1,3 @@ +package tests + +const nashcmd = "../cmd/nash/nash" diff --git a/tests/doc.go b/tests/doc.go new file mode 100644 index 00000000..64f2ef6b --- /dev/null +++ b/tests/doc.go @@ -0,0 +1,18 @@ +// Package tests contains all nash tests that are blackbox. +// What would be blackbox ? These are tests that are targeted +// directly on top of the language using only the shell API, +// they are end to end in the sense that they will exercise +// a lot of different packages on a single test. +// +// The objective of these tests is to have a compreensive set +// of tests that are coupled only the language specification +// and not to how the language is implemented. These allows +// extremely aggressive refactorings to be made without +// incurring in any changes on the tests. +// +// There are disadvantages but discussing integration VS unit +// testing here is not the point (there are also unit tests). +// +// Here even tests that involves the script calling syscalls like +// exit are allowed without interfering with the results of other tests +package tests diff --git a/tests/internal/assert/doc.go b/tests/internal/assert/doc.go new file mode 100644 index 00000000..5e970bba --- /dev/null +++ b/tests/internal/assert/doc.go @@ -0,0 +1,2 @@ +// Package assert has some common assert functions +package assert diff --git a/tests/internal/assert/equal.go b/tests/internal/assert/equal.go new file mode 100644 index 00000000..ce4b8e81 --- /dev/null +++ b/tests/internal/assert/equal.go @@ -0,0 +1,20 @@ +package assert + +import ( + "strings" + "testing" +) + +func EqualStrings(t *testing.T, want string, got string) { + // TODO: could use t.Helper here, but only on Go 1.9 + if want != got { + t.Fatalf("wanted[%s] but got[%s]", want, got) + } +} + +func ContainsString(t *testing.T, str string, sub string) { + // TODO: could use t.Helper here, but only on Go 1.9 + if !strings.Contains(str, sub) { + t.Fatalf("[%s] is not a substring of [%s]", sub, str) + } +} diff --git a/tests/internal/assert/error.go b/tests/internal/assert/error.go new file mode 100644 index 00000000..6bb74901 --- /dev/null +++ b/tests/internal/assert/error.go @@ -0,0 +1,9 @@ +package assert + +import "testing" + +func NoError(t *testing.T, err error, operation string) { + if err != nil { + t.Fatalf("error[%s] %s", err, operation) + } +} diff --git a/tests/internal/sh/shell.go b/tests/internal/sh/shell.go new file mode 100644 index 00000000..22ca0f84 --- /dev/null +++ b/tests/internal/sh/shell.go @@ -0,0 +1,46 @@ +// Package shell makes it easier to run nash scripts for test purposes +package sh + +import ( + "bytes" + "io" + "io/ioutil" + "os" + "os/exec" + "testing" + + "github.com/NeowayLabs/nash/tests/internal/assert" +) + +// Exec runs the script code and returns the result of it. +func Exec( + t *testing.T, + nashpath string, + scriptcode string, + scriptargs ...string, +) (string, string, error) { + scriptfile, err := ioutil.TempFile("", "testshell") + assert.NoError(t, err, "creating tmp file") + + defer func() { + err := scriptfile.Close() + assert.NoError(t, err, "closing tmp file") + err = os.Remove(scriptfile.Name()) + assert.NoError(t, err, "deleting tmp file") + }() + + _, err = io.Copy(scriptfile, bytes.NewBufferString(scriptcode)) + assert.NoError(t, err, "writing script code to tmp file") + + scriptargs = append([]string{scriptfile.Name()}, scriptargs...) + cmd := exec.Command(nashpath, scriptargs...) + + stdout := bytes.Buffer{} + stderr := bytes.Buffer{} + + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + err = cmd.Run() + return stdout.String(), stderr.String(), err +} diff --git a/tests/internal/tester/tester.go b/tests/internal/tester/tester.go new file mode 100644 index 00000000..2fd48f9a --- /dev/null +++ b/tests/internal/tester/tester.go @@ -0,0 +1,52 @@ +// Package tester makes it easy to run multiple +// script test cases. +package tester + +import ( + "testing" + + "github.com/NeowayLabs/nash/tests/internal/assert" + "github.com/NeowayLabs/nash/tests/internal/sh" +) + +type TestCase struct { + Name string + ScriptCode string + ExpectStdout string + ExpectStderrToContain string + Fails bool +} + +func Run(t *testing.T, nashcmd string, cases ...TestCase) { + + for _, tcase := range cases { + t.Run(tcase.Name, func(t *testing.T) { + stdout, stderr, err := sh.Exec(t, nashcmd, tcase.ScriptCode) + if !tcase.Fails { + if err != nil { + t.Fatalf( + "error[%s] stdout[%s] stderr[%s]", + err, + stdout, + stderr, + ) + } + + if stderr != "" { + t.Fatalf( + "unexpected stderr[%s], on success no stderr is expected", + stderr, + ) + } + } + + if tcase.ExpectStdout != "" { + assert.EqualStrings(t, tcase.ExpectStdout, stdout) + } + + if tcase.ExpectStderrToContain != "" { + assert.ContainsString(t, stderr, tcase.ExpectStderrToContain) + } + }) + } +} diff --git a/tests/listindex_test.go b/tests/listindex_test.go new file mode 100644 index 00000000..22990980 --- /dev/null +++ b/tests/listindex_test.go @@ -0,0 +1,81 @@ +package tests + +import ( + "testing" + + "github.com/NeowayLabs/nash/tests/internal/tester" +) + +func TestListIndexing(t *testing.T) { + tester.Run(t, nashcmd, + tester.TestCase{ + Name: "PositionalAccess", + ScriptCode: ` + a = ("1" "2") + echo $a[0] + echo $a[1] + `, + ExpectStdout: "1\n2\n", + }, + tester.TestCase{ + Name: "PositionalAssigment", + ScriptCode: ` + a = ("1" "2") + a[0] = "9" + a[1] = "p" + echo $a[0] + $a[1] + `, + ExpectStdout: "9p\n", + }, + tester.TestCase{ + Name: "PositionalAccessWithVar", + ScriptCode: ` + a = ("1" "2") + i = "0" + echo $a[$i] + i = "1" + echo $a[$i] + `, + ExpectStdout: "1\n2\n", + }, + tester.TestCase{ + Name: "Iteration", + ScriptCode: ` + a = ("1" "2" "3") + for x in $a { + echo $x + } + `, + ExpectStdout: "1\n2\n3\n", + }, + tester.TestCase{ + Name: "IterateEmpty", + ScriptCode: ` + a = () + for x in $a { + exit("1") + } + echo "ok" + `, + ExpectStdout: "ok\n", + }, + tester.TestCase{ + Name: "IndexOutOfRangeFails", + ScriptCode: ` + a = ("1" "2" "3") + echo $a[3] + `, + Fails: true, + ExpectStderrToContain: "IndexError", + }, + tester.TestCase{ + Name: "IndexEmptyFails", + ScriptCode: ` + a = () + echo $a[0] + `, + Fails: true, + ExpectStderrToContain: "IndexError", + }, + ) +} diff --git a/tests/stringindex_test.go b/tests/stringindex_test.go new file mode 100644 index 00000000..b2337ecd --- /dev/null +++ b/tests/stringindex_test.go @@ -0,0 +1,141 @@ +package tests + +import ( + "testing" + + "github.com/NeowayLabs/nash/tests/internal/tester" +) + +func TestStringIndexing(t *testing.T) { + tester.Run(t, nashcmd, + tester.TestCase{ + Name: "IterateEmpty", + ScriptCode: ` + a = "" + for x in $a { + exit("1") + } + echo "ok" + `, + ExpectStdout: "ok\n", + }, + tester.TestCase{ + Name: "IndexEmptyFails", + ScriptCode: ` + a = "" + echo $a[0] + `, + Fails: true, + ExpectStderrToContain: "IndexError", + }, + tester.TestCase{ + Name: "IsImmutable", + ScriptCode: ` + a = "12" + a[0] = "2" + echo $a + `, + Fails: true, + ExpectStderrToContain: "IndexError", + }, + ) +} +func TestStringIndexingASCII(t *testing.T) { + tester.Run(t, nashcmd, + tester.TestCase{Name: "PositionalAccess", + ScriptCode: ` + a = "12" + echo $a[0] + echo $a[1] + `, + ExpectStdout: "1\n2\n", + }, + tester.TestCase{ + Name: "PositionalAccessReturnsString", + ScriptCode: ` + a = "12" + x = $a[0] + $a[1] + echo $x + `, + ExpectStdout: "12\n", + }, + tester.TestCase{ + Name: "Len", + ScriptCode: ` + a = "12" + l <= len($a) + echo $l + `, + ExpectStdout: "2\n", + }, + tester.TestCase{ + Name: "Iterate", + ScriptCode: ` + a = "123" + for x in $a { + echo $x + } + `, + ExpectStdout: "1\n2\n3\n", + }, + tester.TestCase{ + Name: "IndexOutOfRangeFails", + ScriptCode: ` + a = "123" + echo $a[3] + `, + Fails: true, + ExpectStderrToContain: "IndexError", + }, + ) +} + +func TestStringIndexingNonASCII(t *testing.T) { + tester.Run(t, nashcmd, + tester.TestCase{Name: "PositionalAccess", + ScriptCode: ` + a = "⌘⌘" + echo $a[0] + echo $a[1] + `, + ExpectStdout: "⌘\n⌘\n", + }, + tester.TestCase{ + Name: "Iterate", + ScriptCode: ` + a = "⌘⌘" + for x in $a { + echo $x + } + `, + ExpectStdout: "⌘\n⌘\n", + }, + tester.TestCase{ + Name: "PositionalAccessReturnsString", + ScriptCode: ` + a = "⌘⌘" + x = $a[0] + $a[1] + echo $x + `, + ExpectStdout: "⌘⌘\n", + }, + tester.TestCase{ + Name: "Len", + ScriptCode: ` + a = "⌘⌘" + l <= len($a) + echo $l + `, + ExpectStdout: "2\n", + }, + tester.TestCase{ + Name: "IndexOutOfRangeFails", + ScriptCode: ` + a = "⌘⌘" + echo $a[2] + `, + Fails: true, + ExpectStderrToContain: "IndexError", + }, + ) +}