Skip to content

Commit

Permalink
all: start using Go 1.21 std APIs
Browse files Browse the repository at this point in the history
A lot of simplifications this time around.
  • Loading branch information
mvdan committed Feb 10, 2024
1 parent 9c56ff3 commit 65756a2
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 68 deletions.
6 changes: 1 addition & 5 deletions expand/braces.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@ func Braces(word *syntax.Word) []*syntax.Word {

fromLit := br.Elems[0].Lit()
toLit := br.Elems[1].Lit()
zeros := extraLeadingZeros(fromLit)
// TODO: use max when we can assume Go 1.21
if z := extraLeadingZeros(toLit); z > zeros {
zeros = z
}
zeros := max(extraLeadingZeros(fromLit), extraLeadingZeros(toLit))

from, err1 := strconv.Atoi(fromLit)
to, err2 := strconv.Atoi(toLit)
Expand Down
3 changes: 2 additions & 1 deletion expand/environ.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package expand

import (
"runtime"
"slices"
"sort"
"strings"
)
Expand Down Expand Up @@ -152,7 +153,7 @@ func ListEnviron(pairs ...string) Environ {
// listEnvironWithUpper implements ListEnviron, but letting the tests specify
// whether to uppercase all names or not.
func listEnvironWithUpper(upper bool, pairs ...string) Environ {
list := append([]string{}, pairs...)
list := slices.Clone(pairs)
if upper {
// Uppercase before sorting, so that we can remove duplicates
// without the need for linear search nor a map.
Expand Down
4 changes: 2 additions & 2 deletions expand/environ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestListEnviron(t *testing.T) {
{
name: "Empty",
pairs: nil,
want: []string{},
want: nil,
},
{
name: "Simple",
Expand Down Expand Up @@ -62,7 +62,7 @@ func TestListEnviron(t *testing.T) {
gotEnv := listEnvironWithUpper(tc.upper, tc.pairs...)
got := []string(gotEnv.(listEnviron))
if !reflect.DeepEqual(got, tc.want) {
t.Fatalf("ListEnviron(%t, %q) wanted %q, got %q",
t.Fatalf("ListEnviron(%t, %q) wanted %#v, got %#v",
tc.upper, tc.pairs, tc.want, got)
}
})
Expand Down
3 changes: 3 additions & 0 deletions expand/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -731,12 +731,14 @@ func (cfg *Config) quotedElemFields(pe *syntax.ParamExp) []string {
switch vr := cfg.Env.Get(name); vr.Kind {
case Indexed:
keys := make([]string, 0, len(vr.Map))
// TODO: maps.Keys if it makes it into Go 1.23
for key := range vr.List {
keys = append(keys, strconv.Itoa(key))
}
return keys
case Associative:
keys := make([]string, 0, len(vr.Map))
// TODO: maps.Keys if it makes it into Go 1.23
for key := range vr.Map {
keys = append(keys, key)
}
Expand All @@ -757,6 +759,7 @@ func (cfg *Config) quotedElemFields(pe *syntax.ParamExp) []string {
case Indexed:
return vr.List
case Associative:
// TODO: maps.Values if it makes it into Go 1.23
elems := make([]string, 0, len(vr.Map))
for _, elem := range vr.Map {
elems = append(elems, elem)
Expand Down
17 changes: 4 additions & 13 deletions interp/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"io"
"io/fs"
"maps"
"math/rand"
"os"
"path/filepath"
Expand Down Expand Up @@ -667,9 +668,7 @@ func (r *Runner) Reset() {
if r.Vars == nil {
r.Vars = make(map[string]expand.Variable)
} else {
for k := range r.Vars {
delete(r.Vars, k)
}
clear(r.Vars)
}
// TODO(v4): Use the supplied Env directly if it implements enough methods.
r.writeEnv = &overlayEnviron{parent: r.Env}
Expand Down Expand Up @@ -818,17 +817,9 @@ func (r *Runner) Subshell() *Runner {
// Env vars aren't copied; setVar will copy lists and maps as needed.
oenv := &overlayEnviron{parent: r.writeEnv}
r2.writeEnv = oenv
r2.Funcs = make(map[string]*syntax.Stmt, len(r.Funcs))
for k, v := range r.Funcs {
r2.Funcs[k] = v
}
r2.Funcs = maps.Clone(r.Funcs)
r2.Vars = make(map[string]expand.Variable)
if l := len(r.alias); l > 0 {
r2.alias = make(map[string]alias, l)
for k, v := range r.alias {
r2.alias[k] = v
}
}
r2.alias = maps.Clone(r.alias)

r2.dirStack = append(r2.dirBootstrap[:0], r.dirStack...)
r2.fillExpandConfig(r.ectx)
Expand Down
16 changes: 5 additions & 11 deletions interp/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ package interp

import (
"fmt"
"maps"
"os"
"runtime"
"slices"
"strconv"
"strings"

Expand Down Expand Up @@ -228,9 +230,8 @@ func (r *Runner) setVar(name string, index syntax.ArithmExpr, vr expand.Variable
case expand.String:
list = append(list, cur.Str)
case expand.Indexed:
// TODO: slices.Clone
// TODO: only clone when inside a subshell and getting a var from outside for the first time
list = append([]string(nil), cur.List...)
list = slices.Clone(cur.List)
case expand.Associative:
// if the existing variable is already an AssocArray, try our
// best to convert the key to a string
Expand All @@ -240,13 +241,8 @@ func (r *Runner) setVar(name string, index syntax.ArithmExpr, vr expand.Variable
}
k := r.literal(w)

// TODO: maps.Clone
// TODO: only clone when inside a subshell and getting a var from outside for the first time
m2 := make(map[string]string, len(cur.Map))
for k, vr := range cur.Map {
m2[k] = vr
}
cur.Map = m2
cur.Map = maps.Clone(cur.Map)
cur.Map[k] = valStr
r.setVarInternal(name, cur)
return
Expand Down Expand Up @@ -355,9 +351,7 @@ func (r *Runner) assignVal(as *syntax.Assign, valType string) expand.Variable {
}
elemValues[i].index = index
index += len(elemValues[i].values)
if index > maxIndex {
maxIndex = index
}
maxIndex = max(maxIndex, index)
}
// Flatten down the values.
strs := make([]string, maxIndex)
Expand Down
6 changes: 3 additions & 3 deletions syntax/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ func FuzzQuote(f *testing.F) {
var shellProgram string
switch lang {
case LangBash:
hasBash51(t)
requireBash52(t)
shellProgram = "bash"
case LangPOSIX:
hasDash059(t)
requireDash059(t)
shellProgram = "dash"
case LangMirBSDKorn:
hasMksh59(t)
requireMksh59(t)
shellProgram = "mksh"
case LangBats:
t.Skip() // bats has no shell and its syntax is just bash
Expand Down
59 changes: 26 additions & 33 deletions syntax/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,43 +164,36 @@ func TestMain(m *testing.M) {
}

var (
storedHasBash52 bool
onceHasBash52 sync.Once
onceHasBash52 = sync.OnceValue(func() bool {
return cmdContains("version 5.2", "bash", "--version")
})

storedHasDash059 bool
onceHasDash059 sync.Once
onceHasDash059 = sync.OnceValue(func() bool {
// dash provides no way to check its version, so we have to
// check if it's new enough as to not have the bug that breaks
// our integration tests.
// This also means our check does not require a specific version.
return cmdContains("Bad subst", "dash", "-c", "echo ${#<}")
})

storedHasMksh59 bool
onceHasMksh59 sync.Once
onceHasMksh59 = sync.OnceValue(func() bool {
return cmdContains(" R59 ", "mksh", "-c", "echo $KSH_VERSION")
})
)

func hasBash51(tb testing.TB) {
onceHasBash52.Do(func() {
storedHasBash52 = cmdContains("version 5.2", "bash", "--version")
})
if !storedHasBash52 {
tb.Skipf("bash 5.1 required to run")
func requireBash52(tb testing.TB) {
if !onceHasBash52() {
tb.Skipf("bash 5.2 required to run")
}
}

func hasDash059(tb testing.TB) {
// dash provides no way to check its version, so we have to
// check if it's new enough as to not have the bug that breaks
// our integration tests.
// This also means our check does not require a specific version.
onceHasDash059.Do(func() {
storedHasDash059 = cmdContains("Bad subst", "dash", "-c", "echo ${#<}")
})
if !storedHasDash059 {
func requireDash059(tb testing.TB) {
if !onceHasDash059() {
tb.Skipf("dash 0.5.9+ required to run")
}
}

func hasMksh59(tb testing.TB) {
onceHasMksh59.Do(func() {
storedHasMksh59 = cmdContains(" R59 ", "mksh", "-c", "echo $KSH_VERSION")
})
if !storedHasMksh59 {
func requireMksh59(tb testing.TB) {
if !onceHasMksh59() {
tb.Skipf("mksh 59 required to run")
}
}
Expand Down Expand Up @@ -265,7 +258,7 @@ func TestParseBashConfirm(t *testing.T) {
if testing.Short() {
t.Skip("calling bash is slow.")
}
hasBash51(t)
requireBash52(t)
i := 0
for _, c := range append(fileTests, fileTestsNoPrint...) {
if c.Bash == nil {
Expand All @@ -283,7 +276,7 @@ func TestParsePosixConfirm(t *testing.T) {
if testing.Short() {
t.Skip("calling dash is slow.")
}
hasDash059(t)
requireDash059(t)
i := 0
for _, c := range append(fileTests, fileTestsNoPrint...) {
if c.Posix == nil {
Expand All @@ -301,7 +294,7 @@ func TestParseMirBSDKornConfirm(t *testing.T) {
if testing.Short() {
t.Skip("calling mksh is slow.")
}
hasMksh59(t)
requireMksh59(t)
i := 0
for _, c := range append(fileTests, fileTestsNoPrint...) {
if c.MirBSDKorn == nil {
Expand All @@ -319,7 +312,7 @@ func TestParseErrBashConfirm(t *testing.T) {
if testing.Short() {
t.Skip("calling bash is slow.")
}
hasBash51(t)
requireBash52(t)
for _, c := range shellTests {
want := c.common
if c.bsmk != nil {
Expand All @@ -340,7 +333,7 @@ func TestParseErrPosixConfirm(t *testing.T) {
if testing.Short() {
t.Skip("calling dash is slow.")
}
hasDash059(t)
requireDash059(t)
for _, c := range shellTests {
want := c.common
if c.posix != nil {
Expand All @@ -358,7 +351,7 @@ func TestParseErrMirBSDKornConfirm(t *testing.T) {
if testing.Short() {
t.Skip("calling mksh is slow.")
}
hasMksh59(t)
requireMksh59(t)
for _, c := range shellTests {
want := c.common
if c.bsmk != nil {
Expand Down

0 comments on commit 65756a2

Please sign in to comment.