address review feedback, add a lot of comments :-), call shellcheck o…

…n the completion scripts, fix a bug in compopt

data/completion/complete.sh

@@ -1,10 +1,31 @@
 # -*- bash -*-
 
-# _complete_from_snap serialises the tab completion request and sends it off to
-# the appropriate 'snap run --command=complete', and de-serialises the response
-# into the usual tab completion result.
+# _complete_from_snap performs the tab completion request by calling the
+# appropriate 'snap run --command=complete' with serialized args, and
+# deserializes the response into the usual tab completion result.
+#
+# How snap command completion works is:
+# 1. snappy's complete.sh is sourced into the user's shell environment
+# 2. user performs '<command> <tab>'. If '<command>' is a snap command,
+#    proceed to step '3', otherwise perform normal bash completion
+# 3. run 'snap run --command=complete ...', converting bash completion
+#    environment into serialized command line arguments
+# 4. 'snap run --command=complete ...' exec()s 'etelpmoc.sh' within the snap's
+#    runtime environment and confinement
+# 5. 'etelpmoc.sh' takes the serialized command line arguments from step '3'
+#    and puts them back into the bash completion environment variables
+# 6. 'etelpmoc.sh' sources the snap's 'completer' script, performs the bash
+#    completion and serializes the resulting completion environment variables
+#    by printing to stdout the results in a format that snappy's complete.sh
+#    will understand, then exits
+# 7. control returns to snappy's 'complete.sh' and it deserializes the output
+#    from 'etelpmoc.sh', validates the results and puts the validated results
+#    into the bash completion environment variables
+# 8. bash displays the results to the user
 _complete_from_snap() {
     {
+        # De-serialize the output of 'snap run --command=complete ...' into the format
+        # bash expects:
         read -a opts
         # opts is expected to be a series of compopt options
         if [[ ${#opts[@]} -gt 0 ]]; then
@@ -15,7 +36,7 @@ _complete_from_snap() {
 
             for i in "${opts[@]}"; do
                 if ! [[ "$i" =~ ^[a-z]+$ ]]; then
-                    # non-alphanumeric option; something awry
+                    # only lowercase alpha characters allowed
                     return 2
                 fi
             done
@@ -33,19 +54,24 @@ _complete_from_snap() {
         esac
 
         read sep
-        if [ "$sep" ]; then
+        if [ -n "$sep" ]; then
             # non-blank separator? madness!
             return 2
         fi
         local oldIFS="$IFS"
 
         if [ ! "$bounced" ]; then
             local IFS=$'\n'
-            COMPREPLY=( $( \grep -v '[[:cntrl:];?*{}]' ) )
+            # Ignore any suspicious results that are uncommon in filenames and that
+            # might be used to trick the user. A whitelist approach would be better
+            # but is impractical with UTF-8 and common characters like quotes.
+            COMPREPLY=( $( command grep -v '[[:cntrl:];&?*{}]' ) )
             IFS="$oldIFS"
         fi
 
         if [[ ${#opts[@]} -gt 0 ]]; then
+            # shellcheck disable=SC2046
+            # (we *want* word splitting to happen here)
             compopt $(printf " -o %s" "${opts[@]}")
         fi
         if [ "$bounced" ]; then

data/completion/etelpmoc.sh

@@ -3,8 +3,13 @@
 # etelpmoc is the backwards half of complete: it de-serialises the tab
 # completion request into the appropriate environs expected by the tab
 # completion tools, performs whatever action is wanted, and serialises the
-# result. It accomplishes this by a mixture of aliases and functions overriding
-# the builtin completion commands.
+# result. It accomplishes this by having functions override the builtin
+# completion commands.
+#
+# this always runs "inside", in the same environment you get when doing "snap
+# run --shell", and snap-exec is the one setting the first argument to the
+# completion script set in the snap. The rest of the arguments come through
+# from snap-run --command=complete <snap> <args...>
 
 _die() {
     echo "$*" >&2
@@ -19,6 +24,7 @@ if [[ "${#@}" -lt 8 ]]; then
     _die "USAGE: $0 <script> <COMP_TYPE> <COMP_KEY> <COMP_POINT> <COMP_CWORD> <COMP_WORDBREAKS> <COMP_LINE> cmd [args...]"
 fi
 
+# De-serialize the command line arguments and populate tab completion environment
 _compscript="$1"
 shift
 COMP_TYPE="$1"
@@ -46,15 +52,22 @@ if [[ ! -f "$_compscript" ]]; then
     _die "ERROR: completion script does not exist"
 fi
 
+# Source the bash-completion library functions and common completion setup
 . /usr/share/bash-completion/bash_completion
-
+# Now source the snap's 'completer' script itself
 . "$_compscript"
 
-# _compopts is an associative array, which keys are options.
+# _compopts is an associative array, which keys are options. The options are
+# described in bash(1)'s description of the -o option to the "complete"
+# builtin, and they affect how the completion options are presented to the user
+# (e.g. adding a slash for directories, whether to add a space after the
+# completion, etc). These need setting in the user's environemnt so need
+# serializing separately from the completions themselves.
 declare -A _compopts
 
 # wrap compgen, setting _compopts for any options given.
-xcompgen() {
+# (as these options need handling separately from the completions)
+compgen() {
     local opt
 
     while getopts :o: opt; do
@@ -64,15 +77,12 @@ xcompgen() {
                 ;;
         esac
     done
-    # aliases are not checked if the command is quoted, and a backslash counts.
-    \compgen "$@"
+    builtin compgen "$@"
 }
-alias compgen=xcompgen
-shopt -s expand_aliases
 
 # compopt replaces the original compopt with one that just sets/unsets entries
 # in _compopts
-compopt() (
+compopt() {
     local i
 
     for ((i=0; i<$#; i++)); do
@@ -87,14 +97,19 @@ compopt() (
                 ;;
         esac
     done
-)
+}
 
 _compfunc="_minimal"
 _compact=""
 # this is a lot more complicated than it should be, but it's how you
 # get the result of 'complete -p "$1"' into an array, splitting it as
 # the shell would.
 readarray -t _comp < <(xargs -n1 < <(complete -p "$1") )
+# _comp is now an array of the appropriate 'complete' invocation, word-split as
+# the shell would, so we can now inspect it with getopts to determine the
+# appropriate completion action.
+# Unfortunately shellcheck doesn't know about readarray:
+# shellcheck disable=SC2154
 if [[ "${_comp[*]}" ]]; then
     while getopts :abcdefgjksuvA:C:W:o:F: opt "${_comp[@]:1}"; do
         case "$opt" in
@@ -144,7 +159,7 @@ if [[ "${_comp[*]}" ]]; then
                 _compfunc="$OPTARG"
                 ;;
             W)
-                readarray -t COMPREPLY < <( \compgen -W "$OPTARG" -- "${COMP_WORDS[$COMP_CWORD]}" )
+                readarray -t COMPREPLY < <( builtin compgen -W "$OPTARG" -- "${COMP_WORDS[$COMP_CWORD]}" )
                 _compfunc=""
                 ;;
             *)
@@ -166,7 +181,7 @@ esac
 
 if [ ! "$_bounce" ]; then
     if [ "$_compact" ]; then
-        readarray -t COMPREPLY < <( \compgen -A "$_compact" -- "${COMP_WORDS[$COMP_CWORD]}" )
+        readarray -t COMPREPLY < <( builtin compgen -A "$_compact" -- "${COMP_WORDS[$COMP_CWORD]}" )
     elif [ "$_compfunc" ]; then
         # execute completion function (or the command if -C)
         $_compfunc

run-checks

@@ -107,6 +107,11 @@ if [ "$STATIC" = 1 ]; then
         go vet $pkg
     done
 
+    if which shellcheck 2>/dev/null; then
+        echo Checking shell scripts...
+        shellcheck -s bash data/completion/*.sh
+    fi
+
     echo Checking spelling errors
     go get -u github.com/client9/misspell/cmd/misspell
     for file in $(ls . | grep -v 'vendor\|po'); do

tests/lib/snaps/complexion/bin/complexion

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 echo "Greetings from inside ${SNAP:?}."
 if [ "$#" -gt "0" ]; then
     echo "Arguments given:"