Skip to content

Commit

Permalink
Remove support for braced lists in lvalue positions.
Browse files Browse the repository at this point in the history
This was necessary for the legacy assignment syntax when the RHS evaluates to
multiple values, so the only choice is to cram multiple variable names into a
braced list, as in "{a,b}=(cmd) echo $a $b". This is no longer a problem with
the newer assignment commands, like "with a b = (cmd) { echo $a $b }".

This is a fairly obscure feature, and only necessary with the legacy assignment
syntax. Code like `var {a,b} = (cmd)` would now be broken, but a search for `}=`
on GitHub has only found files that also use the legacy assignment syntax and is
broken by its removal anyway. It seems safe enough to remove directly without
deprecation or documenting in the release notes.
  • Loading branch information
xiaq committed Jul 26, 2024
1 parent 2f51ce6 commit 92b5ae7
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 51 deletions.
10 changes: 0 additions & 10 deletions pkg/eval/builtin_special_test.elvts
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,6 @@ Compilation error: new variable $a must not have indices
Compilation error: lvalue may not be composite expressions
[tty]:1:5-8: var a'b'

## braced lists must not have any indices when used as a lvalue ##
~> var {a b}[0] = x y
Compilation error: braced list may not have indices when used as lvalue
[tty]:1:5-12: var {a b}[0] = x y

///////
# set #
///////
Expand Down Expand Up @@ -750,11 +745,6 @@ Exception: x
Exception: foo
[tty]:1:13-21: for x [a] { fail foo }

## more than one iterator ##
~> for {x,y} [] { }
Compilation error: must be exactly one lvalue
[tty]:1:5-9: for {x,y} [] { }

## can't create new variable non-local variable ##
~> for no-such-namespace:x [a b] { }
Compilation error: cannot create variable $no-such-namespace:x; new variables can only be created in the current scope
Expand Down
9 changes: 0 additions & 9 deletions pkg/eval/compile_lvalue.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,6 @@ func (cp *compiler) compileCompoundLValues(ns []*parse.Compound, f lvalueFlag) l
var dummyLValuesGroup = lvaluesGroup{[]lvalue{{}}, -1}

func (cp *compiler) compileIndexingLValue(n *parse.Indexing, f lvalueFlag) lvaluesGroup {
if n.Head.Type == parse.Braced {
// Braced list of lvalues may not have indices.
if len(n.Indices) > 0 {
cp.errorpf(n, "braced list may not have indices when used as lvalue")
return dummyLValuesGroup
}
return cp.compileCompoundLValues(n.Head.Braced, f)
}
// A basic lvalue.
if !parse.ValidLHSVariable(n.Head, true) {
cp.errorpf(n.Head, "lvalue must be valid literal variable names")
return dummyLValuesGroup
Expand Down
65 changes: 33 additions & 32 deletions pkg/parse/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,38 +199,6 @@ func startsForm(r rune) bool {
return IsInlineWhitespace(r) || startsCompound(r, CmdExpr)
}

func ValidLHSVariable(p *Primary, allowSigil bool) bool {
switch p.Type {
case Braced:
// TODO(xiaq): check further inside braced expression
return true
case SingleQuoted, DoubleQuoted:
// Quoted variable names may contain anything
return true
case Bareword:
// Bareword variable names may only contain runes that are valid in raw
// variable names
return validBarewordVariableName(p.Value, allowSigil)
default:
return false
}
}

func validBarewordVariableName(name string, allowSigil bool) bool {
if name == "" {
return false
}
if allowSigil && name[0] == '@' {
name = name[1:]
}
for _, r := range name {
if !allowedInVariableName(r) {
return false
}
}
return true
}

// Redir = { Compound } { '<'|'>'|'<>'|'>>' } { Space } ( '&'? Compound )
type Redir struct {
node
Expand Down Expand Up @@ -676,6 +644,39 @@ func (pn *Primary) variable(ps *parser) {
}
}

// Keep this consistent with the (*Primary).variable above.

// ValidLHSVariable returns whether a [Primary] node containing a variable name
// being used as the LHS of an assignment form without the $ prefix is valid.
func ValidLHSVariable(p *Primary, allowSigil bool) bool {
switch p.Type {
case SingleQuoted, DoubleQuoted:
// Quoted variable names may contain anything
return true
case Bareword:
// Bareword LHS variable are only allowed if they are also valid after a
// $, even if they are valid barewords. For example, a variable named
// a/b must be quoted after $ (as $'a/b'), so for consistency, we also
// require it to be quoted after set (like set 'a/b' = foo) even if a/b
// is a valid bareword.
name := p.Value
if name == "" {
return false
}
if allowSigil && name[0] == '@' {
name = name[1:]
}
for _, r := range name {
if !allowedInVariableName(r) {
return false
}
}
return true
default:
return false
}
}

// The following are allowed in variable names:
// * Anything beyond ASCII that is printable
// * Letters and numbers
Expand Down

0 comments on commit 92b5ae7

Please sign in to comment.