openstreetmap-si/GursAddressesForOSM

View on GitHub

Showing 8 of 8 total issues

Function readShapefileToMap has a Cognitive Complexity of 28 (exceeds 20 allowed). Consider refactoring.
Open

func readShapefileToMap(shapeFileName string, keyColumnName, valueColumnName string) map[string]string {
    result := make(map[string]string)

    shapeReader, err := shp.Open(shapeFileName)
    if err != nil {
Severity: Minor
Found in gursShp2geoJson.go - About 1 hr to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

Avoid deeply nested control flow statements.
Open

                    if !loaded {
                        log.Printf("Possible new abbreviation in %s: %s,%s", valueColumnName, valueUtf, valueUtf)
                    }
Severity: Major
Found in gursShp2geoJson.go - About 45 mins to fix

    Double quote to prevent globbing and word splitting.
    Open

        MUNDIRURL=$(echo -n $MUNDIR|sed 's/Č/%25C4%258C/g'|sed 's/Ž/%25C5%25BD/g'|sed 's/Š/%25C5%25A0/g' |sed 's/č/%25C4%258D/g'|sed 's/ž/%25C5%25BE/g'|sed 's/š/%25C5%25A1/g' )
    Severity: Minor
    Found in summarize.sh by shellcheck

    Double quote to prevent globbing and word splitting.

    Problematic code:

    echo $1
    for i in $*; do :; done # this done and the next one also applies to expanding arrays.
    for i in $@; do :; done

    Correct code:

    echo "$1"
    for i in "$@"; do :; done # or, 'for i; do'

    Rationale

    The first code looks like "print the first argument". It's actually "Split the first argument by IFS (spaces, tabs and line feeds). Expand each of them as if it was a glob. Join all the resulting strings and filenames with spaces. Print the result."

    The second one looks like "iterate through all arguments". It's actually "join all the arguments by the first character of IFS (space), split them by IFS and expand each of them as globs, and iterate on the resulting list". The third one skips the joining part.

    Quoting variables prevents word splitting and glob expansion, and prevents the script from breaking when input contains spaces, line feeds, glob characters and such.

    Strictly speaking, only expansions themselves need to be quoted, but for stylistic reasons, entire arguments with multiple variable and literal parts are often quoted as one:

    $HOME/$dir/dist/bin/$file        # Unquoted (bad)
    "$HOME"/"$dir"/dist/bin/"$file"  # Minimal quoting (good)
    "$HOME/$dir/dist/bin/$file"      # Canonical quoting (good)

    When quoting composite arguments, make sure to exclude globs and brace expansions, which lose their special meaning in double quotes: "$HOME/$dir/src/*.c" will not expand, but "$HOME/$dir/src"/*.c will.

    Note that $( ) starts a new context, and variables in it have to be quoted independently:

    echo "This $variable is quoted $(but this $variable is not)"
    echo "This $variable is quoted $(and now this "$variable" is too)"

    Exceptions

    Sometimes you want to split on spaces, like when building a command line:

    options="-j 5 -B"
    make $options file

    Just quoting this doesn't work. Instead, you should have used an array (bash, ksh, zsh):

    options=(-j 5 -B) # ksh: set -A options -- -j 5 -B
    make "${options[@]}" file

    or a function (POSIX):

    make_with_flags() { make -j 5 -B "$@"; }
    make_with_flags file

    To split on spaces but not perform glob expansion, Posix has a set -f to disable globbing. You can disable word splitting by setting IFS=''.

    Similarly, you might want an optional argument:

    debug=""
    [[ $1 == "--trace-commands" ]] && debug="-x"
    bash $debug script

    Quoting this doesn't work, since in the default case, "$debug" would expand to one empty argument while $debug would expand into zero arguments. In this case, you can use an array with zero or one elements as outlined above, or you can use an unquoted expansion with an alternate value:

    debug=""
    [[ $1 == "--trace-commands" ]] && debug="yes"
    bash ${debug:+"-x"} script

    This is better than an unquoted value because the alternative value can be properly quoted, e.g. wget ${output:+ -o "$output"}.


    As always, this warning can be [[ignore]]d on a case-by-case basis.

    this is especially relevant when BASH many not be available for the array work around. For example, use in eval or in command options where script has total control of the variables...

    FLAGS="-av -e 'ssh -x' --delete --delete-excluded"
    ...
    # shellcheck disable=SC2086
    eval rsync $FLAGS ~/dir remote_host:dir

    Notice

    Original content from the ShellCheck https://github.com/koalaman/shellcheck/wiki.

    Not following: venv/bin/activate: openFile: does not exist (No such file or directory)
    Open

    source venv/bin/activate
    Severity: Minor
    Found in conflate.sh by shellcheck

    Not following: (error message here)

    Reasons include: file not found, no permissions, not included on the command line, not allowing shellcheck to follow files with -x, etc.

    Problematic code:

    source somefile

    Correct code:

    # shellcheck disable=SC1091
    source somefile

    Rationale:

    ShellCheck, for whichever reason, is not able to access the source file.

    This could be because you did not include it on the command line, did not use shellcheck -x to allow following other files, don't have permissions or a variety of other problems.

    Feel free to ignore the error with a [[directive]].

    Exceptions:

    If you're fine with it, ignore the message with a [[directive]].

    Notice

    Original content from the ShellCheck https://github.com/koalaman/shellcheck/wiki.

    2: cannot find package "github.com/jonas-p/go-shp" in any of:
    Open

        shp "github.com/jonas-p/go-shp"
    Severity: Minor
    Found in gursShp2geoJson.go by govet

    Double quote to prevent globbing and word splitting.
    Open

        BASENAMEURL=$(echo -n $BASENAME|sed 's/Č/%25C4%258C/g'|sed 's/Ž/%25C5%25BD/g'|sed 's/Š/%25C5%25A0/g' |sed 's/č/%25C4%258D/g'|sed 's/ž/%25C5%25BE/g'|sed 's/š/%25C5%25A1/g' )
    Severity: Minor
    Found in summarize.sh by shellcheck

    Double quote to prevent globbing and word splitting.

    Problematic code:

    echo $1
    for i in $*; do :; done # this done and the next one also applies to expanding arrays.
    for i in $@; do :; done

    Correct code:

    echo "$1"
    for i in "$@"; do :; done # or, 'for i; do'

    Rationale

    The first code looks like "print the first argument". It's actually "Split the first argument by IFS (spaces, tabs and line feeds). Expand each of them as if it was a glob. Join all the resulting strings and filenames with spaces. Print the result."

    The second one looks like "iterate through all arguments". It's actually "join all the arguments by the first character of IFS (space), split them by IFS and expand each of them as globs, and iterate on the resulting list". The third one skips the joining part.

    Quoting variables prevents word splitting and glob expansion, and prevents the script from breaking when input contains spaces, line feeds, glob characters and such.

    Strictly speaking, only expansions themselves need to be quoted, but for stylistic reasons, entire arguments with multiple variable and literal parts are often quoted as one:

    $HOME/$dir/dist/bin/$file        # Unquoted (bad)
    "$HOME"/"$dir"/dist/bin/"$file"  # Minimal quoting (good)
    "$HOME/$dir/dist/bin/$file"      # Canonical quoting (good)

    When quoting composite arguments, make sure to exclude globs and brace expansions, which lose their special meaning in double quotes: "$HOME/$dir/src/*.c" will not expand, but "$HOME/$dir/src"/*.c will.

    Note that $( ) starts a new context, and variables in it have to be quoted independently:

    echo "This $variable is quoted $(but this $variable is not)"
    echo "This $variable is quoted $(and now this "$variable" is too)"

    Exceptions

    Sometimes you want to split on spaces, like when building a command line:

    options="-j 5 -B"
    make $options file

    Just quoting this doesn't work. Instead, you should have used an array (bash, ksh, zsh):

    options=(-j 5 -B) # ksh: set -A options -- -j 5 -B
    make "${options[@]}" file

    or a function (POSIX):

    make_with_flags() { make -j 5 -B "$@"; }
    make_with_flags file

    To split on spaces but not perform glob expansion, Posix has a set -f to disable globbing. You can disable word splitting by setting IFS=''.

    Similarly, you might want an optional argument:

    debug=""
    [[ $1 == "--trace-commands" ]] && debug="-x"
    bash $debug script

    Quoting this doesn't work, since in the default case, "$debug" would expand to one empty argument while $debug would expand into zero arguments. In this case, you can use an array with zero or one elements as outlined above, or you can use an unquoted expansion with an alternate value:

    debug=""
    [[ $1 == "--trace-commands" ]] && debug="yes"
    bash ${debug:+"-x"} script

    This is better than an unquoted value because the alternative value can be properly quoted, e.g. wget ${output:+ -o "$output"}.


    As always, this warning can be [[ignore]]d on a case-by-case basis.

    this is especially relevant when BASH many not be available for the array work around. For example, use in eval or in command options where script has total control of the variables...

    FLAGS="-av -e 'ssh -x' --delete --delete-excluded"
    ...
    # shellcheck disable=SC2086
    eval rsync $FLAGS ~/dir remote_host:dir

    Notice

    Original content from the ShellCheck https://github.com/koalaman/shellcheck/wiki.

    For loops over find output are fragile. Use find -exec or a while read loop.
    Open

    for gursGeoJson in $(find "$DIRNAME" -name '*-gurs.geojson');
    Severity: Minor
    Found in summarize.sh by shellcheck

    For loops over find output are fragile. Use find -exec or a while read loop.

    Problematic code:

    for file in $(find mydir -mtime -7 -name '*.mp3')
    do
      let count++
      echo "Playing file no. $count"
      play "$file"
    done
    echo "Played $count files"

    This will fail for filenames containing spaces and similar, such as My File.mp3, and has a series of potential globbing issues depending on other filenames in the directory like (if you have MyFile2.mp3 and MyFile[2014].mp3, the former file will play twice and the latter will not play at all).

    Correct code:

    There are many possible fixes, each with its pros and cons.

    The most general fix (that requires the least amount of thinking to apply) is having find output a \0 separated list of files and consuming them in a while read loop:

    while IFS= read -r -d '' file
    do
      let count++
      echo "Playing file no. $count"
      play "$file"
    done <   <(find mydir -mtime -7 -name '*.mp3' -print0)
    echo "Played $count files"

    In usage it's very similar to the for loop: it gets its output from a find statement, it executes a shell script body, it allows updating/aggregating variables, and the variables are available when the loop ends.

    It requires Bash, and works with GNU, Busybox, OS X, FreeBSD and OpenBSD find, but not POSIX find.

    If find is just matching globs recursively

    If you don't need find logic like -mtime -7 and just use it to match globs recursively (all *.mp3 files under a directory), you can instead use globstar and nullglob instead of find, and still use a for loop:

    shopt -s globstar nullglob
    for file in mydir/**/*.mp3
    do
      let count++
      echo "Playing file no. $count"
      play "$file"
    done
    echo "Played $count files"

    This is bash 4 specific.

    For POSIX

    If you need POSIX compliance, this is a fair approach:

    find mydir ! -name "$(printf "*\n*")" -name '*.mp3' > tmp
    while IFS= read -r file
    do
      let count++
      echo "Playing file #$count"
      play "$file"
    done < tmp
    rm tmp
    echo "Played $count files"

    The only problem is for filenames containing line feeds. A ! -name "$(printf "*\n*")" has been added to simply skip these files, just in case there are any.

    If you don't need variables to be available after the loop (here, if you don't need to print the final play count at the end), you can skip the tmp file and just pipe from find to while.

    For simple commands with no aggregation

    If you don't need a shell script loop body or any form of variable like if we only wanted to play the file, we can dramatically simplify while maintaining POSIX compatibility:

    # Simple and POSIX
    find mydir -name '*.mp3' -exec play {} \;

    This does not allow things like let counter++ because let is a shell builtin, not an external command.

    For shell commands with no aggregation

    If we do need a shell script body but no aggregation, you can do the above but invoking sh (this is still POSIX):

    find mydir -name '*.mp3' -exec sh -c '
        echo "Playing ${1%.mp3}"
        play "$1"
      ' sh {} \;

    This would not be possible without sh, because ${1%.mp3} is a shell construct that find can't evaluate by itself. If we had tried to let counter++ in this loop, we would have found that the value never changes.

    Note that using + instead of \;, and using an embedded for file in "$@" loop rather than "$1", will not allow aggregating variables. This is because for large lists, find will invoke the command multiple times, each time with some chunk of the input.

    Rationale:

    for var in $(find ...) loops rely on word splitting and will evaluate globs, which will wreck havoc with filenames containing whitespace or glob characters.

    find -exec for i in glob and find+while do not rely on word splitting, so they avoid this problem.

    Exceptions

    If you know about and carefully apply IFS=$'\n' and set -f, you could choose to ignore this message.

    Notice

    Original content from the ShellCheck https://github.com/koalaman/shellcheck/wiki.

    TODO found
    Open

    ## TODO
    Severity: Minor
    Found in README.md by fixme
    Severity
    Category
    Status
    Source
    Language