4

How can I remove shellcheck's warning "SC2154" when linting the shell script?

#!/bin/bash
set -euo pipefail
IFS=$'\n\t'

echo "proxy=$http_proxy" | sudo tee -a /etc/test.txt

The warning is "SC2154: http_proxy is referenced but not assigned."

EDIT: I want to write the environment variable "http_proxy" to the test.txt file using sudo.

Socowi
  • 25,550
  • 3
  • 32
  • 54
user5580578
  • 1,134
  • 1
  • 12
  • 28
  • Not clear, when why you are printing `http_proxy` again in your echo? Please be clear in your question and let us know what you are actually trying to achieve here? – RavinderSingh13 May 27 '20 at 06:26
  • @user5580578 : I don't see that you would write to the variable `http_proxy` anywhere in your script. I can not even see why it would be an **environment** variable (unless it had been exported by the parent process). – user1934428 May 27 '20 at 07:34
  • @user5580578 : And what's the point of removing space as a valid `IFS` in **this** script? I don't see where this would possibly have an effect here. – user1934428 May 27 '20 at 09:19
  • @user1934428 It's extremely common for `http_proxy` to be an environment variable. – Joseph Sible-Reinstate Monica May 27 '20 at 20:23
  • @JosephSible-ReinstateMonica : Wouldn't it then usually be `HTTP_PROXY`? Even then, it needs to be **set** somewhere. If this is a standalone script, this would not be the case. Of course it can be exported by the parent (that's why I mentioned this in my comment). – user1934428 May 28 '20 at 06:29
  • 1
    @user1934428 I've seen it lowercase as often as uppercase, and it's often exported somewhere global like .profile or .bashrc. – Joseph Sible-Reinstate Monica May 28 '20 at 13:57
  • The former is interesting to know. The problem with the latter is that if you write a self-contained script, you can not assume that either `.profile` or `.bashrc` has been sourced before. In this case, I would at least check for the existence of the variable, by using `${http_proxy?:not set}`. – user1934428 May 29 '20 at 06:09
  • @user5580578 : I think you should first make up your mind, what should happen if the variable is undefined. Should then an empty setting be written to test.txt, or do you want to abort with an error message? If the former, I would write a `: ${http_proxy:=}` somewhere before doing the `echo`. If the latter, I would write a `: ${http_proxy?:variable not set}` instead. Both should suppress the warning. – user1934428 May 29 '20 at 06:22

2 Answers2

11

In general, the better solution would be to stick to the convention and name your environment variables in ALL_CAPS. However, in this case I understand that http_proxy is not your environment variable, but is dictated by programs like curl, wget, and so on, so you cannot simply rename it.

You can suppress any warning with a comment:

# shellcheck disable=SC2154
echo "proxy=$http_proxy" | ...

This will ignore the error about http_proxy starting from this line. Subsequent $http_proxy won't give errors either, but other variables will.

To disable the warnings for multiple variables in a central place, put below snippet at the beginning of your script. Warning: Shellcheck directives before the first command in your script will apply to the whole script. As a workaround, put a dummy command (e.g. true) above the directive.

#! /bin/bash
# Using "shellcheck disable=SC2154" here would ignore warnings for all variables
true
# Ignore only warnings for the three proxy variables
# shellcheck disable=SC2154
echo "$http_proxy $https_proxy $no_proxy" > /dev/null
Socowi
  • 25,550
  • 3
  • 32
  • 54
  • 1
    I also would intuitively have used upper case for `HTTP_PROXY`, but as user @JosephSible-ReinstateMonica has pointed out in his comment above, the variable is often used in the lowercase variant too. Indeed, [this](https://www.golinuxcloud.com/set-up-proxy-http-proxy-environment-variable/) and [this](https://www.putorius.net/how-to-set-httpproxy-variable-in-linux.html) article say to use `http_proxy`. After all, it depends on the application which actually makes use of the variable, which spelling is to be expected. – user1934428 May 29 '20 at 06:15
2

Altenatively to suppressing the warning, you can explicitly expand http_proxy to nothing if it is null or unset:

echo "proxy=${http_proxy:-}"
KamilCuk
  • 120,984
  • 8
  • 59
  • 111
  • 1
    I think this would change the behavior of the script (see `set -u pipefail)`. Maybe you could insert an `if`, but I'm not sure whether that will fix the warning. – Socowi May 27 '20 at 06:58
  • @Socowi : Why? the use of `:-` in a parameter expansion does not affect the exit code of `echo`. The `echo` would simply output just a linefeed. – user1934428 May 29 '20 at 06:18
  • @user1934428 When a shell script is run under `set -e` expanding variables that are not set will cause the script to exit. So it could have been the intention of OP to exit the script if `http_proxy` is not set. This solution has different behavior - if the variable is not set, it is substituted for empty. – KamilCuk May 29 '20 at 06:30
  • @KamilCuk : Yes, but usage of `:-` would suppress this. You can try it out with `set -e; echo ${foobar:-}; echo xxx`. That's why I objected against the claim by Socowi, that your solution would change the behaviour. – user1934428 May 29 '20 at 06:37
  • 1
    @user1934428 The `-u` is important too in your example: `bash -c 'set -eu; echo ${foobar:-}; echo xxx'` always prints `xxx` whereas `bash -c 'set -eu; echo foobar; echo xxx'` prints `xxx` if and only if `foobar` is set. I assumed the latter was exactly what OP wanted. – Socowi May 29 '20 at 06:49
  • @Socowi : Right, forgot about the `-u`. But the command to verify that the solution by KamilCuk does not affect the beaviour, would be `bash -c 'set -eu; echo ${foobar:-}; echo xxx'` – user1934428 May 29 '20 at 06:53