Shell: Using function with parameters in if

Question

I'm trying to execute the code below but when I try to use my function in the if statement I get the -bash: [: too many arguments error. Why is it happening?

Thank you in advance!

notContainsElement () {
  local e match="$1"
  shift
  for e; do [[ "$e" == "$match" ]] && return 1; done
  return 0
}

list=( "pears" "apples" "bananas" "oranges" )
blacklist=( "oranges" "apples" )
docheck=1

for fruit in "${list[@]}"
do
    if [ notContainsElement "$fruit" "${blacklist[@]}" -a $docheck = 1 ]
    then
        echo $fruit
    fi
done

Solution 1

When using if [ ... ] you are actually using the [ utility (which is the same as test but requires that the last argument is ]).

[ does not understand to run your function, it expects strings. Fortunately, you don't need to use [ at all here (for the function at least):

if [ "$docheck" -eq 1 ] && notContainsElement "$fruit" "${blacklist[@]}"; then
  ...
fi

Note that I'm also checking the integer first, so that we may avoid calling the function at all if $docheck is not 1.

This works because if takes an arbitrary command and decides what to do from the exit status of that command. Here we use a [ ... ] test together with a call to your function, with && in-between, creating a compound command. The compound command's exit status would be true if both the [ ... ] test and the function returned zero as their exit statuses, signalling success.

As a style note, I would not have the function test whether the array does not contain the element but whether if does contain the element, and then

if [ "$docheck" -eq 1 ] && ! contains "$fruit" "${blacklist[@]}"; then ...

Having a function test a negative will mess up logic in cases where you do want to test whether the array contains the element (if ! notContainsElement ...).

Solution 2

try

if notContainsElement "$fruit" "${blacklist[@]}" && test "$docheck" = 1
then

• -a option is neither a shell or a test option

here you have two part test

notContainsElement "$fruit" "${blacklist[@]}"
test $docheck = 1 ## or [ $docheck = 1 ]

you link then in if using

if cmd1 && cmd2

as pointed out -a is a test option, but can only be used with other test option, thus you can use

if [ "$a" -lt "$b" -a "$a" -lt "$c" ]

to test that $a is lower than both $b and $c, but you cannot used other command within test scope.

Comments

[MoonCheese62]

Use shellcheck.net (or its offline version). It finds the issue mentioned in the answers albeit with a far less detailed explanation and without a solution.

[Арсений Черенков]

arg, @Kusalananda was fastest gun in world wild web again ;)

[Kusalananda]

RSI is the price I pay for that.

[hyde]

-a is test option, it means AND.

[Kusalananda]

@hyde Except it has been marked obsolescent in the POSIX standard.

[schily]

It is obsolete only because it is not recommended to be used since complex expressions are subject to problems in case that parameters are expanded shell variables. Since test is built into shells since the early 1980s, it is not slower but more reliable to use &&and || together with more testcalls. Note that in the mid 1980s, it took one second to call the external test utility and this is why people liked to see complex test expressions.

[Charles Duffy]

@Archemar, ...consider fixing the quoting bugs still present in your examples (unquoted $docheck, unquoted $a/$b/etc in the last example).

[Scott - Слава Україні]

(1) This fails in the case that one of the list fruits is contained within one of the blacklist fruits.  For example, if we change blacklist to ( "oranges" "pineapples" ), the output of your script doesn’t change. … (Cont’d)

[Scott - Слава Україні]

(Cont’d) …  (2) I don’t quite understand what you mean by “I think this is simpler, but lacks the && logic which many prefer.”  It’s often easier to make things simpler by leaving out functionality.  As Albert Einstein may or may not have said, “Everything should be made as simple as possible, but no simpler.”  Why did you leave out the &&?  (3) When you post code, please indent it appropriately.

[Wastrel]

Well, I tried to use the 4-space indent but it seems that it didn't work. I see what you mean about the pineapples, though.

[Charles Duffy]

It also can't distinguish between a single list entry two words, and two subsequent items, two and words. And if your list contained * as an entry, it would match everything. (And because you aren't quoting the $f in what should be echo "$f", if you did try to echo such an element, it would be replaced with a list of filenames in the current directory).

[Арсений Черенков]

I quoted variable, because thou shall quotest thy variables how when OP set docheck to numerical value, or if I know that a b,c are numerical, I don't see the point

[Kusalananda]

@Archemar Unquoted numerical strings would still be susceptible to word splitting if a mischievous user sets IFS so that it includes digits.

[Wastrel]

I've made some edits. This is not trivial when the data in the arrays is arbitrary. Suppose "apples" is in the blacklist and "Granny Smith apples" is an element of the other array. I think the OP's code has similar issues. The arrays have to be made of elements that "work."