Skip to content
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.

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

Favor single quote's where possible #2346

Open
2 tasks done
oliv3r opened this issue Oct 4, 2021 · 13 comments
Open
2 tasks done

Favor single quote's where possible #2346

oliv3r opened this issue Oct 4, 2021 · 13 comments

Comments

@oliv3r
Copy link

oliv3r commented Oct 4, 2021

For new checks and feature suggestions

I've been a 'double quote' guy for all my live, as C says, a string is surrounded by double quotes' ;) Single quotes are for characters! However in shell, this is not true. As most, I was stubborn and used double quotes whenever anyway ;)

After years, I've come to the realization, from a safety/security point of view, it actually does make sense to favor single quotes in scripts. It indicates 'no processing/expansion on the content of this string shall be done' (right?). As such, the choice (recommendation) should be, that whenever a variable does not contain anything that ought to be processed, favor single quotes.

Linters for example, such as pep8 also do this afaik as well.

@brother
Copy link
Collaborator

brother commented Oct 4, 2021

As such, the choice (recommendation) should be, that whenever a variable does not contain
anything that ought to be processed, favor single quotes.

(bash)

echo 'big up'
myvar='hello'

# rather than

echo "I know my BGP"
myothervar="world"

# and be careful with processing

echo "$myvar $myothervar"
echo '$myvar $myothervar'

Interesting to see if there are data from the big set @koalaman uses for testing that would indicate that this would be a good choice. For my own code I think I more often have processing involved and mostly need to avoid spacing issues and so on. But I haven't looked.

@koalaman
Copy link
Owner

koalaman commented Oct 8, 2021

This is a pretty subjective style debate that it doesn't make sense for ShellCheck to pick sides in, but it could potentially be an explicitly enabled optional suggestion.

@felipecrs
Copy link
Contributor

+1 to be an optional rule, preferably with "Did you mean" suggestions so that it can be auto-fixed with VS Code ShellCheck on save.

@oliv3r
Copy link
Author

oliv3r commented Oct 14, 2021

@koalaman mostly true and I agree, as an optional it makes a lot of sense.

A bit off-topiic, but speaking of optional's, could you do something like enable="all !favor_single_quotes" for example?

@felipecrs
Copy link
Contributor

Or:

enable=all
disable=favor-single-quotes

@oliv3r
Copy link
Author

oliv3r commented Jan 7, 2022

Ah yeah, that looks good to me too.

@felipecrs
Copy link
Contributor

felipecrs commented Jan 7, 2022

I changed my mind, I think this should be an optional rule just like prefer-double-brackets. :-P

@oliv3r
Copy link
Author

oliv3r commented Jan 7, 2022

haha, but all enables all optionals no? or is it

enable=all, prefer-double-brackets

cause that's kind of weird :p

@felipecrs
Copy link
Contributor

Lol

You are absolutely right. I confused myself. :-)

@ramses0
Copy link

ramses0 commented Feb 6, 2023

Please see the linked issue #2687 ... I believe a strong case can be made that ' should be preferred for as many strings as possible. Similar preferences and reasoning can be found in other linters (eg: https://eslint.org/docs/latest/rules/quotes ):

"""Object option:

"avoidEscape": true allows strings to use single-quotes or double-quotes so long as the string contains a quote that would have to be escaped otherwise"""

@oliv3r
Copy link
Author

oliv3r commented Apr 4, 2023

BTW, we already have the opposite in SC2016 :)

@ramses0
Copy link

ramses0 commented Apr 5, 2023

@oliv3r - is this then just a matter of "patches welcome"?

@oliv3r
Copy link
Author

oliv3r commented Apr 6, 2023

@ramses0 I don't know? That's up to the maintainer; but I would expect it to be so?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants