Skip to content

Fix issues reported by shellcheck #5910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Oct 7, 2022
Merged

Fix issues reported by shellcheck #5910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
c0b28b7
shellcheck - fix SC2164, exit on cd failure
pavoljuhas Oct 1, 2022
b31ac12
shellcheck - fix SC2236, needless double negative
pavoljuhas Oct 1, 2022
8ef2006
shellcheck - fix useless use of cat, SC2002
pavoljuhas Oct 1, 2022
77c240e
shellcheck - replace legacy backticks, SC2006
pavoljuhas Oct 1, 2022
d2ceffa
shellcheck - clean unused variables, SC2034
pavoljuhas Oct 1, 2022
52d1b13
shellcheck - fix globing/splitting on expansion, SC2086
pavoljuhas Oct 3, 2022
4ccc7c4
shellcheck - expand variables at trap execution, SC2064
pavoljuhas Oct 3, 2022
05e7940
shellcheck - redundant loop over single value, SC2043
pavoljuhas Oct 3, 2022
8cdfa26
shellcheck - export and assign separately, SC2155
pavoljuhas Oct 3, 2022
eaabe53
shellcheck - ${name} not needed in arithmetic expressions, SC2004
pavoljuhas Oct 3, 2022
2d7bc7f
shellcheck - do not assign array to string, SC2124
pavoljuhas Oct 3, 2022
6e8f539
shellcheck - disable checks for zsh block
pavoljuhas Oct 3, 2022
f80b42c
Merge branch 'master' into appease-shellcheck
pavoljuhas Oct 6, 2022
c44b6bf
Add shebang to identify dev_tools/pypath as bash script
pavoljuhas Oct 6, 2022
8009b9a
Merge branch 'master' into appease-shellcheck
pavoljuhas Oct 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions check/all
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not cd "$(dirname "${BASH_SOURCE[0]}")" || exit?

My local shellcheck says ^-- SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what we are doing, i'm just curious about the creation of the thisdir variable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not cd "$(dirname "${BASH_SOURCE[0]}")"

Such cd would still pass for a failing $(...) command. If that command had no standard output,
the shell would expand it as cd "" and stay in the same working directory.
That could have unexpected effects further below.

Here is how to test -

$ cd /tmp
$ cd "$(false)" || echo "cd failed"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what we are doing, i'm just curious about the creation of the thisdir variable.

This allows us to fail when $(...) fails -

$ thisdir=$(false) || echo directory expression failed

cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

# Parse arguments.
apply_arg=""
Expand All @@ -44,7 +45,7 @@ for arg in "$@"; do
elif [[ "${arg}" == "--apply-format-changes" ]]; then
apply_arg="--apply"
elif [ -z "${rev}" ]; then
if [ "$(git cat-file -t ${arg} 2> /dev/null)" != "commit" ]; then
if [ "$(git cat-file -t "${arg}" 2> /dev/null)" != "commit" ]; then
echo -e "\033[31mNo revision '${arg}'.\033[0m" >&2
exit 1
fi
Expand Down
5 changes: 3 additions & 2 deletions check/asv_run
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

asv run "$@"
11 changes: 6 additions & 5 deletions check/build-changed-protos
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

# Figure out which revision to compare against.
if [ ! -z "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t $1 2> /dev/null)" != "commit" ]; then
if [ -n "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then
echo -e "\033[31mNo revision '$1'.\033[0m" >&2
exit 1
fi
Expand Down Expand Up @@ -63,7 +64,7 @@ dev_tools/build-protos.sh
# but the error logic will still work.
uncommitted=$(git status --porcelain 2>/dev/null | grep -E "^?? cirq-google" | cut -d " " -f 3)

if [[ ! -z "$uncommitted" ]]; then
if [[ -n "$uncommitted" ]]; then
echo -e "\033[31mERROR: Uncommitted generated files found! Please generate and commit these files using dev_tools/build-protos.sh:\033[0m"
for generated in $uncommitted
do
Expand Down
5 changes: 3 additions & 2 deletions check/doctest
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

source dev_tools/pypath

Expand Down
9 changes: 5 additions & 4 deletions check/format-incremental
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?


# Parse arguments.
Expand All @@ -46,7 +47,7 @@ for arg in "$@"; do
elif [[ "${arg}" == "--all" ]]; then
only_changed=0
elif [ -z "${rev}" ]; then
if [ "$(git cat-file -t ${arg} 2> /dev/null)" != "commit" ]; then
if [ "$(git cat-file -t "${arg}" 2> /dev/null)" != "commit" ]; then
echo -e "\033[31mNo revision '${arg}'.\033[0m" >&2
exit 1
fi
Expand Down Expand Up @@ -82,7 +83,7 @@ if (( only_changed == 1 )); then

# Get the modified, added and moved python files.
IFS=$'\n' read -r -d '' -a format_files < \
<(git diff --name-only --diff-filter=MAR ${rev} -- '*.py' ':(exclude)cirq-google/cirq_google/cloud/*' ':(exclude)*_pb2.py')
<(git diff --name-only --diff-filter=MAR "${rev}" -- '*.py' ':(exclude)cirq-google/cirq_google/cloud/*' ':(exclude)*_pb2.py')
else
echo -e "Formatting all python files." >&2
IFS=$'\n' read -r -d '' -a format_files < \
Expand Down
5 changes: 3 additions & 2 deletions check/misc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
################################################################################

# Get the working directory to the repo root.
cd "$( dirname "${BASH_SOURCE[0]}" )"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

# Check for non-contrib references to contrib.
results=$(grep -Rl "\bcirq\.contrib\b" cirq-core | grep -v "cirq/contrib" | grep -v "__")
Expand Down
10 changes: 6 additions & 4 deletions check/mypy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

CONFIG_FILE='mypy.ini'

CIRQ_PACKAGES=$(env PYTHONPATH=. python dev_tools/modules.py list --mode package-path)
read -r -a CIRQ_PACKAGES < \
<(env PYTHONPATH=. python dev_tools/modules.py list --mode package-path)

echo -e -n "\033[31m"
mypy --config-file=dev_tools/conf/$CONFIG_FILE "$@" $CIRQ_PACKAGES dev_tools examples
mypy --config-file=dev_tools/conf/$CONFIG_FILE "$@" "${CIRQ_PACKAGES[@]}" dev_tools examples
result=$?
echo -e -n "\033[0m"

Expand Down
7 changes: 4 additions & 3 deletions check/nbformat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ for arg in "$@"; do
done

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

pip show tensorflow-docs > /dev/null || exit 1

Expand All @@ -35,7 +36,7 @@ FORMAT_CMD="python3 -m tensorflow_docs.tools.nbfmt --indent=1"
# Test the notebooks
unformatted=$($FORMAT_CMD --test docs 2>&1 | grep "\- docs" || true)
needed_changes=0
if [ ! -z "${unformatted}" ]; then
if [ -n "${unformatted}" ]; then
needed_changes=1
if (( only_print == 0 )); then
$FORMAT_CMD docs
Expand Down
5 changes: 3 additions & 2 deletions check/npm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

npm --prefix 'cirq-web/cirq_ts' "$@"
6 changes: 3 additions & 3 deletions check/npx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?

cd 'cirq-web/cirq_ts'
cd "${topdir}/cirq-web/cirq_ts" || exit $?
npx "$@"
10 changes: 6 additions & 4 deletions check/pylint
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

CIRQ_MODULES=$(env PYTHONPATH=. python dev_tools/modules.py list --mode package-path)
read -r -a CIRQ_MODULES < \
<(env PYTHONPATH=. python dev_tools/modules.py list --mode package-path)

# Add dev_tools to $PYTHONPATH so that pylint can find custom checkers
env PYTHONPATH=dev_tools pylint --jobs=0 --rcfile=dev_tools/conf/.pylintrc "$@" $CIRQ_MODULES dev_tools examples
env PYTHONPATH=dev_tools pylint --jobs=0 --rcfile=dev_tools/conf/.pylintrc "$@" "${CIRQ_MODULES[@]}" dev_tools examples
11 changes: 6 additions & 5 deletions check/pylint-changed-files
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

# Figure out which revision to compare against.
if [ ! -z "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t $1 2> /dev/null)" != "commit" ]; then
if [ -n "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then
echo -e "\033[31mNo revision '$1'.\033[0m" >&2
exit 1
fi
Expand All @@ -55,7 +56,7 @@ fi

typeset -a changed
IFS=$'\n' read -r -d '' -a changed < \
<(git diff --name-only ${rev} -- '*.py' ':(exclude)cirq-google/cirq_google/cloud/*' ':(exclude)*_pb2.py' \
<(git diff --name-only "${rev}" -- '*.py' ':(exclude)cirq-google/cirq_google/cloud/*' ':(exclude)*_pb2.py' \
| grep -E "^(cirq|dev_tools|examples).*.py$"
)

Expand Down
5 changes: 3 additions & 2 deletions check/pytest
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

# Run in parallel by default. Pass the `-n0` option for a single-process run.
# (the last `-n` option wins)
Expand Down
4 changes: 2 additions & 2 deletions check/pytest-and-incremental-coverage
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pytest_result=$?

# assume successful cover_result in case coverage is not run
cover_result=0
if (( $ANALYZE_COV )); then
if (( ANALYZE_COV )); then
# Convert to .py,cover files.
coverage annotate

Expand All @@ -92,7 +92,7 @@ if (( $ANALYZE_COV )); then
fi

# Report result.
if (( ${pytest_result} || ${cover_result} )); then
if (( pytest_result || cover_result )); then
exit 1
fi
exit 0
15 changes: 8 additions & 7 deletions check/pytest-changed-files
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,19 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

# Figure out which branch to compare against.
rest=$@
if [ ! -z "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t $1 2> /dev/null)" != "commit" ]; then
rest=( "$@" )
if [ -n "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then
echo -e "\033[31mNo revision '$1'.\033[0m" >&2
exit 1
fi
rev=$1
rest=${@:2}
rest=( "${@:2}" )
elif [ "$(git cat-file -t upstream/master 2> /dev/null)" == "commit" ]; then
rev=upstream/master
elif [ "$(git cat-file -t origin/master 2> /dev/null)" == "commit" ]; then
Expand Down Expand Up @@ -77,4 +78,4 @@ fi

source dev_tools/pypath

pytest ${rest} "${changed[@]}"
pytest "${rest[@]}" "${changed[@]}"
4 changes: 2 additions & 2 deletions check/pytest-changed-files-and-incremental-coverage
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ cd "$( dirname "${BASH_SOURCE[0]}" )" || exit 1
cd "$(git rev-parse --show-toplevel)" || exit 1

# Figure out which revision to compare against.
if [ ! -z "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t $1 2> /dev/null)" != "commit" ]; then
if [ -n "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then
echo -e "\033[31mNo revision '$1'.\033[0m" >&2
exit 1
fi
Expand Down
5 changes: 3 additions & 2 deletions check/ts-build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

check/npx webpack --mode production "$@"
4 changes: 2 additions & 2 deletions check/ts-build-current
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ check/ts-build
# Find the changed bundle.js files, if any
untracked=$(git status --porcelain 2>/dev/null | grep "cirq-web/cirq_ts/dist/" | cut -d " " -f 3)

if [[ ! -z "$untracked" ]]; then
if [[ -n "$untracked" ]]; then
echo -e "\033[31mERROR: Uncommitted changes to bundle file(s) found! Please commit these files:\033[0m"
for generated in $untracked
do
Expand All @@ -35,4 +35,4 @@ if [[ ! -z "$untracked" ]]; then
exit 1
fi

exit 0
exit 0
5 changes: 3 additions & 2 deletions check/ts-coverage
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

check/npm run coverage "$@"
5 changes: 3 additions & 2 deletions check/ts-lint
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

check/npm run lint "$@"
5 changes: 3 additions & 2 deletions check/ts-lint-and-format
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

check/npm run fix "$@"
5 changes: 3 additions & 2 deletions check/ts-test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

check/npm run test "$@"
5 changes: 3 additions & 2 deletions check/ts-test-e2e
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
################################################################################

# Get the working directory to the repo root.
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(git rev-parse --show-toplevel)"
thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $?
topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $?
cd "${topdir}" || exit $?

check/npm run test-e2e "$@"
6 changes: 3 additions & 3 deletions dev_tools/conf/pip-install-minimal-for-pytest-changed-files.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ set -e
cd "$( dirname "${BASH_SOURCE[0]}" )"
cd "$(git rev-parse --show-toplevel)"

REQS="-r dev_tools/requirements/pytest-minimal.env.txt"
reqs=( -r dev_tools/requirements/pytest-minimal.env.txt )

# Install contrib requirements only if needed.
changed=$(git diff --name-only origin/master | grep "cirq/contrib" || true)
[ "${changed}" = "" ] || REQS="$REQS -r cirq-core/cirq/contrib/requirements.txt"
[ "${changed}" = "" ] || reqs+=( -r cirq-core/cirq/contrib/requirements.txt )

pip install $REQS
pip install "${reqs[@]}"
Loading