docs/content/introduction/bash_code_style.md

   1 ---
   2 bookHidden: true
   3 title: "Bash Code Style"
   4 ---
   5
   6 The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
   7 "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
   8 "MAY", and "OPTIONAL" in this document are to be interpreted as
   9 described in `BCP 14 <https://tools.ietf.org/html/bcp14>`_
  10 `[RFC2119] <https://tools.ietf.org/html/rfc2119>`_
  11 `[RFC8174] <https://tools.ietf.org/html/rfc8174>`_
  12 when, and only when, they appear in all capitals, as shown here.
  13
  14 This document SHALL describe guidelines for writing reliable, maintainable,
  15 reusable and readable code for CSIT.
  16
  17 # Proposed Style
  18
  19 # File Types
  20
  21 Bash files SHOULD NOT be monolithic. Generally, this document
  22 considers two types of bash files:
  23
  24 + Entry script: Assumed to be called by user,
  25   or a script "external" in some way.
  26
  27   + Sources bash libraries and calls functions defined there.
  28
  29 + Library file: To be sourced by entry scipts, possibly also by other libraries.
  30
  31   + Sources other libraries for functions it needs.
  32
  33     + Or relies on a related file already having sourced that.
  34
  35     + Documentation SHALL imply which case it is.
  36
  37   + Defines multiple functions other scripts can call.
  38
  39 # Safety
  40
  41 + Variable expansions MUST be quoted, to prevent word splitting.
  42
  43   + This includes special "variables" such as "${1}".
  44
  45     + RECOMMENDED even if the value is safe, as in "$?" and "$#".
  46
  47   + It is RECOMMENDED to quote strings in general,
  48     so text editors can syntax-highlight them.
  49
  50     + Even if the string is a numeric value.
  51
  52     + Commands and known options can get their own highlight, no need to quote.
  53
  54       + Example: You do not need to quote every word of
  55         "pip install --upgrade virtualenv".
  56
  57   + Code SHALL NOT quote glob characters you need to expand (obviously).
  58
  59     + OPTIONALLY do not quote adjacent characters (such as dot or fore-slash),
  60       so that syntax highlighting makes them stand out compared to surrounding
  61       ordinary strings.
  62
  63     + Example: cp "logs"/*."log" "."/
  64
  65   + Command substitution on right hand side of assignment are safe
  66     without quotes.
  67
  68     + Note that command substitution limits the scope for quotes,
  69       so it is NOT REQUIRED to escape the quotes in deeper levels.
  70
  71     + Both backtics and "dollar round-bracket" provide command substitution.
  72       The folowing rules are RECOMMENDED:
  73
  74       + For simple constructs, use "dollar round-bracket".
  75
  76       + If there are round brackets in the surrounding text, use backticks,
  77         as some editor highlighting logic can get confused.
  78
  79       + Avoid nested command substitution.
  80
  81         + Put intermediate results into local variables,
  82           use "|| die" on each step of command substitution.
  83
  84   + Code SHOULD NOT be structured in a way where
  85     word splitting is intended.
  86
  87     + Example: Variable holding string of multiple command lines arguments.
  88
  89     + Solution: Array variable should be used in this case.
  90
  91     + Expansion MUST use quotes then: "${name[@]}".
  92
  93     + Word splitting MAY be used when creating arrays from command substitution.
  94
  95 + Code MUST always check the exit code of commands.
  96
  97   + Traditionally, error code checking is done either by "set -e"
  98     or by appending "|| die" after each command.
  99     The first is unreliable, due to many rules affecting "set -e" behavior
 100     (see <https://mywiki.wooledge.org/BashFAQ/105>), but "|| die"
 101     relies on humans identifying each command, which is also unreliable.
 102     When was the last time you checked error code of "echo" command,
 103     for example?
 104
 105     + Another example: "set -e" in your function has no effect
 106       if any ancestor call is done with logical or,
 107       for example in "func || code=$?" construct.
 108
 109   + As there is no reliable method of error detection, and there are two
 110     largely independent unreliable methods, the best what we can do
 111     is to apply both. So, code SHOULD explicitly
 112     check each command (with "|| die" and similar) AND have "set -e" applied.
 113
 114   + Code MUST explicitly check each command, unless the command is well known,
 115     and considered safe (such as the aforementioned "echo").
 116
 117     + The well known commands MUST still be checked implicitly via "set -e".
 118
 119   + See below for specific "set -e" recommendations.
 120
 121 + Code SHOULD use "readlink -e" (or "-f" if target does not exist yet)
 122   to normalize any path value to absolute path without symlinks.
 123   It helps with debugging and identifies malformed paths.
 124
 125 + Code SHOULD use such normalized paths for sourcing.
 126
 127 + When exiting on a known error, code MUST print a longer, helpful message,
 128   in order for the user to fix their situation if possible.
 129
 130 + When error happens at an unexpected place, it is RECOMMENDED for the message
 131   to be short and generic, instead of speculative.
 132
 133 # Bash Options
 134
 135 + Code MUST apply "-x" to make debugging easier.
 136
 137   + Code MAY temporarily supress such output in order to avoid spam
 138     (e.g. in long busy loops), but it is still NOT RECOMMENDED to do so.
 139
 140 + Code MUST apply "-e" for early error detection.
 141
 142   + But code still SHOULD use "|| die" for most commands,
 143     as "-e" has numerous rules and exceptions.
 144
 145   + Code MAY apply "+e" temporarily for commands which (possibly nonzero)
 146     exit code it interested in.
 147
 148     + Code MUST to store "$?" and call "set -e" immediatelly afterwards.
 149
 150     + Code MUST NOT use this approach when calling functions.
 151
 152       + That is because functions are instructed to apply "set -e" on their own
 153         which (when triggered) will exit the whole entry script.
 154
 155         + Unless overriden by ERR trap.
 156           But code SHOULD NOT set any ERR trap.
 157
 158       + If code needs exit code of a function, it is RECOMMENDED to use
 159         pattern 'code="0"; called_function || code="${?}"'.
 160
 161         + In this case, contributor MUST make sure nothing in the
 162           called_function sub-graph relies on "set -e" behavior,
 163           because the call being part of "or construct" disables it.
 164
 165   + Code MAY append "|| true" for benign commands,
 166     when it is clear non-zero exit codes make no difference.
 167
 168     + Also in this case, the contributor MUST make sure nothing within
 169       the called sub-graph depends on "set -e", as it is disabled.
 170
 171 + Code MUST apply "-u" as unset variable is generally a typo, thus an error.
 172
 173   + Code MAY temporarily apply "+u" if a command needs that to pass.
 174
 175     + Virtualenv activation is the only known example so far.
 176
 177 + Code MUST apply "-o pipefail" to make sure "-e" picks errors
 178   inside piped construct.
 179
 180   + Code MAY use "|| true" inside a pipe construct, in the (inprobable) case
 181     when non-zero exit code still results in a meaningful pipe output.
 182
 183 + All together: "set -exuo pipefail".
 184
 185   + Code MUST put that line near start of every file, so we are sure
 186     the options are applied no matter what.
 187
 188     + "Near start" means "before any nontrivial code".
 189
 190     + Basically only copyright is RECOMMENDED to appear before.
 191
 192   + Also code MUST put the line near start of function bodies
 193     and subshell invocations.
 194
 195 # Functions
 196
 197 There are (at least) two possibilities how a code from an external file
 198 can be executed. Either the file contains a code block to execute
 199 on each "source" invocation, or the file just defines functions
 200 which have to be called separately.
 201
 202 This document considers the "function way" to be better,
 203 here are some pros and cons:
 204
 205 + Cons:
 206
 207   + The function way takes more space. Files have more lines,
 208     and the code in function body is one indent deeper.
 209
 210   + It is not easy to create functions for low-level argument manipulation,
 211     as "shift" command in the function code does not affect the caller context.
 212
 213   + Call sites frequently refer to code two times,
 214     when sourcing the definition and when executing the function.
 215
 216   + It is not clear when a library can rely on its relative
 217     to have performed the sourcing already.
 218
 219   + Ideally, each library should detect if it has been sourced already
 220     and return early, which takes even more space.
 221
 222 + Pros:
 223
 224   + Some code blocks are more useful when used as function,
 225     to make call site shorter.
 226
 227     + Examples: Trap functions, "die" function.
 228
 229   + The "import" part and "function" part usually have different side effects,
 230     making the documentation more focused (even if longer overall).
 231
 232   + There is zero risk of argument-less invocation picking arguments
 233     from parent context.
 234
 235     + This safety feature is the main reason for chosing the "function way".
 236
 237     + This allows code blocks to support optional arguments.
 238
 239 + Rules:
 240
 241   + Library files MUST be only "source"d. For example if "tox" calls a script,
 242     it is an entry script.
 243
 244   + Library files (upon sourcing) MUST minimize size effect.
 245
 246     + The only permitted side effects MUST by directly related to:
 247
 248       + Defining functions (without executing them).
 249
 250       + Sourcing sub-library files.
 251
 252   + If a bash script indirectly call another bash script,
 253     it is not a "source" operation, variables are not shared,
 254     so the called script MUST be considered an entry script,
 255     even if it implements logic fitting into a single function.
 256
 257   + Entry scripts SHOULD avoid duplicating any logic.
 258
 259     + Clear duplicated blocks MUST be moved into libraries as functions.
 260
 261     + Blocks with low amount of duplication MAY remain in entry scripts.
 262
 263     + Usual motives for not creating functions are:
 264
 265       + The extracted function would have too much logic for processing
 266         arguments (instead of hardcoding values as in entry script).
 267
 268       + The arguments needed would be too verbose.
 269
 270         + And using "set +x" would take too much vertical space
 271           (when compared to entry script implementation).
 272
 273 # Variables
 274
 275 This document describes two kinds of variables: called "local" and "global".
 276
 277 + Local variables:
 278
 279   + Variable name MUST contain only lower case letters, digits and underscores.
 280
 281   + Code MUST NOT export local variables.
 282
 283   + Code MUST NOT rely on local variables set in different contexts.
 284
 285   + Documentation is NOT REQUIRED.
 286
 287     + Variable name SHOULD be descriptive enough.
 288
 289   + Local variable MUST be initialized before first use.
 290
 291     + Code SHOULD have a comment if a reader might have missed
 292       the initialization.
 293
 294     + Unset local variables when leaving the function.
 295
 296     + Explicitly typeset by "local" builtin command.
 297
 298     + Require strict naming convention, e.g. function_name__variable_name.
 299
 300 + Global variables:
 301
 302   + Variable name MUST contain only upper case letters, digits and underscores.
 303
 304   + They SHOULD NOT be exported, unless external commands need them
 305     (e.g. PYTHONPATH).
 306
 307   + Code MUST document if a function (or its inner call)
 308     reads a global variable.
 309
 310   + Code MUST document if a function (or its inner call)
 311     sets or rewrites a global variable.
 312
 313   + If a function "wants to return a value", it SHOULD be implemented
 314     as the function setting (or rewriting) a global variable,
 315     and the call sites reading that variable.
 316
 317   + If a function "wants to accept an argument", it IS RECOMMENDED
 318     to be implemented as the call sites setting or rewriting global variables,
 319     and the function reading that variables.
 320     But see below for direct arguments.
 321
 322 + Code MUST use curly brackets when referencing variables,
 323   e.g. "${my_variable}".
 324
 325   + It makes related constructs (such as ${name:-default}) less surprising.
 326
 327   + It looks more similar to Robot Framework variables (which is good).
 328
 329 # Arguments
 330
 331 Bash scripts and functions MAY accept arguments, named "${1}", "${2}" and so on.
 332 As a whole available via "$@".
 333 You MAY use "shift" command to consume an argument.
 334
 335 ## Contexts
 336
 337 Functions never have access to parent arguments, but they can read and write
 338 variables set or read by parent contexts.
 339
 340 ### Arguments Or Variables
 341
 342 + Both arguments and global variables MAY act as an input.
 343
 344 + In general, if the caller is likely to supply the value already placed
 345   in a global variable of known name, it is RECOMMENDED
 346   to use that global variable.
 347
 348 + Construct "${NAME:-value}" can be used equally well for arguments,
 349   so default values are possible for both input methods.
 350
 351 + Arguments are positional, so there are restrictions on which input
 352   is optional.
 353
 354 + Functions SHOULD either look at arguments (possibly also
 355   reading global variables to use as defaults), or look at variables only.
 356
 357 + Code MUST NOT rely on "${0}", it SHOULD use "${BASH_SOURCE[0]}" instead
 358   (and apply "readlink -e") to get the current block location.
 359
 360 + For entry scripts, it is RECOMMENDED to use standard parsing capabilities.
 361
 362   + For most Linux distros, "getopt" is RECOMMENDED.
 363
 364 # Working Directory Handling
 365
 366 + Functions SHOULD act correctly without neither assuming
 367   what the currect working directory is, nor changing it.
 368
 369   + That is why global variables and arguments SHOULD contain
 370     (normalized) full paths.
 371
 372   + Motivation: Different call sites MAY rely on different working directories.
 373
 374 + A function MAY return (also with nonzero exit code) when working directory
 375   is changed.
 376
 377   + In this case the function documentation MUST clearly state where (and when)
 378     is the working directory changed.
 379
 380     + Exception: Functions with undocumented exit code.
 381
 382     + Those functions MUST return nonzero code only on "set -e" or "die".
 383
 384       + Note that both "set -e" and "die" by default result in exit of the whole
 385         entry script, but the caller MAY have altered that behavior
 386         (by registering ERR trap, or redefining die function).
 387
 388     + Any callers which use "set +e" or "|| true" MUST make sure
 389       their (and their caller ancestors') assumption on working directory
 390       are not affected.
 391
 392       + Such callers SHOULD do that by restoring the original working directory
 393         either in their code,
 394
 395       + or contributors SHOULD do such restoration in the function code,
 396         (see below) if that is more convenient.
 397
 398   + Motivation: Callers MAY rely on this side effect to simplify their logic.
 399
 400 + A function MAY assume a particular directory is already set
 401   as the working directory (to save space).
 402
 403   + In this case function documentation MUST clearly state what the assumed
 404     working directory is.
 405
 406   + Motivation: Callers MAY call several functions with common
 407     directory of interest.
 408
 409     + Example: Several dowload actions to execute in sequence,
 410       implemented as functions assuming ${DOWNLOAD_DIR}
 411       is the working directory.
 412
 413 + A function MAY change the working directory transiently,
 414   before restoring it back before return.
 415
 416   + Such functions SHOULD use command "pushd" to change the working directory.
 417
 418   + Such functions SHOULD use "trap 'trap - RETURN; popd' RETURN"
 419     imediately after the pushd.
 420
 421     + In that case, the "trap - RETURN" part MUST be included,
 422       to restore any trap set by ancestor.
 423
 424     + Functions MAY call "trap - RETURN; popd" exlicitly.
 425
 426     + Such functions MUST NOT call another pushd (before an explicit popd),
 427       as traps do not stack within a function.
 428
 429 + If entry scripts also use traps to restore working directory (or other state),
 430   they SHOULD use EXIT traps instead.
 431
 432   + That is because "exit" command, as well as the default behavior
 433     of "die" or "set -e" cause direct exit (without skipping function returns).
 434
 435 # Function Size
 436
 437 + In general, code SHOULD follow reasoning similar to how pylint
 438   limits code complexity.
 439
 440 + It is RECOMMENDED to have functions somewhat simpler than Python functions,
 441   as Bash is generally more verbose and less readable.
 442
 443 + If code contains comments in order to partition a block
 444   into sub-blocks, the sub-blocks SHOULD be moved into separate functions.
 445
 446   + Unless the sub-blocks are essentially one-liners,
 447     not readable just because external commands do not have
 448     obvious enough parameters. Use common sense.
 449
 450 # Documentation
 451
 452 + The library path and filename is visible from source sites. It SHOULD be
 453   descriptive enough, so reader do not need to look inside to determine
 454   how and why is the sourced file used.
 455
 456   + If code would use several functions with similar names,
 457     it is RECOMMENDED to create a (well-named) sub-library for them.
 458
 459   + Code MAY create deep library trees if needed, it SHOULD store
 460     common path prefixes into global variables to make sourcing easier.
 461
 462   + Contributors, look at other files in the subdirectory. You SHOULD
 463     improve their filenames when adding-removing other filenames.
 464
 465   + Library files SHOULD NOT have executable flag set.
 466
 467   + Library files SHOULD have an extension .sh (or perhaps .bash).
 468
 469   + It is RECOMMENDED for entry scripts to also have executable flag unset
 470     and have .sh extension.
 471
 472 + Each entry script MUST start with a shebang.
 473
 474   + "#!/bin/usr/env bash" is RECOMMENDED.
 475
 476   + Code SHOULD put an empty line after shebang.
 477
 478   + Library files SHOULD NOT contain a shebang, as "source" is the primary
 479     method to include them.
 480
 481 + Following that, there SHOULD be a block of comment lines with copyright.
 482
 483   + It is a boilerplate, but human eyes are good at ignoring it.
 484
 485   + Overhead for git is also negligible.
 486
 487 + Following that, there MUST be "set -exuo pipefail".
 488
 489   + It acts as an anchor for humans to start paying attention.
 490
 491 Then it depends on script type.
 492
 493 ## Library Documentation
 494
 495 + Following "set -exuo pipefail" SHALL come the "import part" documentation.
 496
 497 + Then SHALL be the import code
 498   ("source" commands and a bare minimum they need).
 499
 500 + Then SHALL be the function definitions, and inside:
 501
 502   + The body SHALL sart with the function documentation explaining API contract.
 503     Similar to Robot [Documentation] or Python function-level docstring.
 504
 505     + See below.
 506
 507   + "set -exuo pipefail" SHALL be the first executable line
 508     in the function body, except functions which legitimely need
 509     different flags. Those SHALL also start with appropriate "set" command(s).
 510
 511   + Lines containing code itself SHALL follow.
 512
 513     + "Code itself" SHALL include comment lines
 514       explaining any non-obvious logic.
 515
 516   + There SHALL be two empty lines between function definitions.
 517
 518 More details on function documentation:
 519
 520 Generally, code SHOULD use comments to explain anything
 521 not obvious from the funtion name.
 522
 523 + Function documentation SHOULD start with short description of function
 524   operation or motivation, but only if not obvious from function name.
 525
 526 + Documentation SHOULD continue with listing any non-obvious side effect:
 527
 528   + Documentation MUST list all read global variables.
 529
 530     + Documentation SHOULD include descriptions of semantics
 531       of global variable values.
 532       It is RECOMMENDED to mention which function is supposed to set them.
 533
 534     + The "include descriptions" part SHOULD apply to other items as well.
 535
 536   + Documentation MUST list all global variables set, unset, reset,
 537     or otherwise updated.
 538
 539   + It is RECOMMENDED to list all hardcoded values used in code.
 540
 541     + Not critical, but can hint at future improvements.
 542
 543   + Documentation MUST list all files or directories read
 544     (so caller can make sure their content is ready).
 545
 546   + Documentation MUST list all files or directories updated
 547     (created, deleted, emptied, otherwise edited).
 548
 549   + Documentation SHOULD list all functions called (so reader can look them up).
 550
 551     + Documentation SHOULD mention where are the functions defined,
 552       if not in the current file.
 553
 554   + Documentation SHOULD list all external commands executed.
 555
 556     + Because their behavior can change "out of bounds", meaning
 557       the contributor changing the implementation of the extrenal command
 558       can be unaware of this particular function interested in its side effects.
 559
 560   + Documentation SHOULD explain exit code (coming from
 561     the last executed command).
 562
 563     + Usually, most functions SHOULD be "pass or die",
 564       but some callers MAY be interested in nonzero exit codes
 565       without using global variables to store them.
 566
 567     + Remember, "exit 1" ends not only the function, but all scripts
 568       in the source chain, so code MUST NOT use it for other purposes.
 569
 570       + Code SHOULD call "die" function instead. This way the caller can
 571         redefine that function, if there is a good reason for not exiting
 572         on function failure.
 573
 574 ## Entry Script Documentation
 575
 576 + After "set -exuo pipefail", high-level description SHALL come.
 577
 578   + Entry scripts are rarely reused, so detailed side effects
 579     are OPTIONAL to document.
 580
 581   + But code SHOULD document the primary side effects.
 582
 583 + Then SHALL come few commented lines to import the library with "die" function.
 584
 585 + Then block of "source" commands for sourcing other libraries needed SHALL be.
 586
 587   + In alphabetical order, any "special" library SHOULD be
 588     in the previous block (for "die").
 589
 590 + Then block os commands processing arguments SHOULD be (if needed).
 591
 592 + Then SHALL come block of function calls (with parameters as needed).
 593
 594 # Other General Recommendations
 595
 596 + Code SHOULD NOT not repeat itself, even in documentation:
 597
 598   + For hardcoded values, a general description SHOULD be written
 599     (instead of copying the value), so when someone edits the value
 600     in the code, the description still applies.
 601
 602   + If affected directory name is taken from a global variable,
 603     documentation MAY distribute the directory description
 604     over the two items.
 605
 606   + If most of side effects come from an inner call,
 607     documentation MAY point the reader to the documentation
 608     of the called function (instead of listing all the side effects).
 609
 610 + But documentation SHOULD repeat it if the information crosses functions.
 611
 612   + Item description MUST NOT be skipped just because the reader
 613     should have read parent/child documentation already.
 614
 615   + Frequently it is RECOMMENDED to copy&paste item descriptions
 616     between functions.
 617
 618   + But sometimes it is RECOMMENDED to vary the descriptions. For example:
 619
 620     + A global variable setter MAY document how does it figure out the value
 621       (without caring about what it will be used for by other functions).
 622
 623     + A global variable reader MAY document how does it use the value
 624       (without caring about how has it been figured out by the setter).
 625
 626 + When possible, Bash code SHOULD be made to look like Python
 627   (or Robot Framework). Those are three primary languages CSIT code relies on,
 628   so it is nicer for the readers to see similar expressions when possible.
 629   Examples:
 630
 631   + Code MUST use indentation, 1 level is 4 spaces.
 632
 633   + Code SHOULD use "if" instead of "&&" constructs.
 634
 635   + For comparisons, code SHOULD use operators such as "!=" (needs "[[").
 636
 637 + Code MUST NOT use more than 80 characters per line.
 638
 639   + If long external command invocations are needed,
 640     code SHOULD use array variables to shorten them.
 641
 642   + If long strings (or arrays) are needed, code SHOULD use "+=" operator
 643     to grow the value over multiple lines.
 644
 645   + If "|| die" does not fit with the command, code SHOULD use curly braces:
 646
 647     + Current line has "|| {",
 648
 649     + Next line has the die commands (indented one level deeper),
 650
 651     + Final line closes with "}" at original intent level.