Adds Credo (and associated enhancements)

.credo.exs

@@ -0,0 +1,220 @@
+# This file contains the configuration for Credo and you are probably reading
+# this after creating it with `mix credo.gen.config`.
+#
+# If you find anything wrong or unclear in this file, please report an
+# issue on GitHub: https://github.com/rrrene/credo/issues
+#
+%{
+  #
+  # You can have as many configs as you like in the `configs:` field.
+  configs: [
+    %{
+      #
+      # Run any config using `mix credo -C <name>`. If no config name is given
+      # "default" is used.
+      #
+      name: "default",
+      #
+      # These are the files included in the analysis:
+      files: %{
+        #
+        # You can give explicit globs or simply directories.
+        # In the latter case `**/*.{ex,exs}` will be used.
+        #
+        included: [
+          "lib/",
+          "src/",
+          "web/",
+          "apps/*/lib/",
+          "apps/*/src/",
+          "apps/*/test/",
+          "apps/*/web/"
+        ],
+        excluded: [
+          ~r"/_build/",
+          ~r"/deps/",
+          ~r"/node_modules/",
+          "test/"
+        ]
+      },
+      #
+      # Load and configure plugins here:
+      #
+      plugins: [],
+      #
+      # If you create your own checks, you must specify the source files for
+      # them here, so they can be loaded by Credo before running the analysis.
+      #
+      requires: [],
+      #
+      # If you want to enforce a style guide and need a more traditional linting
+      # experience, you can change `strict` to `true` below:
+      #
+      strict: false,
+      #
+      # To modify the timeout for parsing files, change this value:
+      #
+      parse_timeout: 5000,
+      #
+      # If you want to use uncolored output by default, you can change `color`
+      # to `false` below:
+      #
+      color: true,
+      #
+      # You can customize the parameters of any check by adding a second element
+      # to the tuple.
+      #
+      # To disable a check put `false` as second element:
+      #
+      #     {Credo.Check.Design.DuplicatedCode, false}
+      #
+      checks: %{
+        enabled: [
+          #
+          ## Consistency Checks
+          #
+          {Credo.Check.Consistency.ExceptionNames, []},
+          {Credo.Check.Consistency.LineEndings, []},
+          {Credo.Check.Consistency.ParameterPatternMatching, []},
+          {Credo.Check.Consistency.SpaceAroundOperators, []},
+          {Credo.Check.Consistency.SpaceInParentheses, []},
+          {Credo.Check.Consistency.TabsOrSpaces, []},
+
+          #
+          ## Design Checks
+          #
+          # You can customize the priority of any check
+          # Priority values are: `low, normal, high, higher`
+          #
+          {Credo.Check.Design.AliasUsage,
+           [priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]},
+          # You can also customize the exit_status of each check.
+          # If you don't want TODO comments to cause `mix credo` to fail, just
+          # set this value to 0 (zero).
+          #
+          {Credo.Check.Design.TagTODO, [exit_status: 2]},
+          {Credo.Check.Design.TagFIXME, []},
+
+          #
+          ## Readability Checks
+          #
+          {Credo.Check.Readability.AliasOrder, []},
+          {Credo.Check.Readability.FunctionNames, []},
+          {Credo.Check.Readability.LargeNumbers, []},
+          {Credo.Check.Readability.MaxLineLength, [priority: :low, max_length: 120]},
+          {Credo.Check.Readability.ModuleAttributeNames, []},
+          {Credo.Check.Readability.ModuleDoc, []},
+          {Credo.Check.Readability.ModuleNames, []},
+          {Credo.Check.Readability.ParenthesesInCondition, []},
+          {Credo.Check.Readability.ParenthesesOnZeroArityDefs, []},
+          {Credo.Check.Readability.PipeIntoAnonymousFunctions, []},
+          {Credo.Check.Readability.PredicateFunctionNames, []},
+          {Credo.Check.Readability.PreferImplicitTry, []},
+          {Credo.Check.Readability.RedundantBlankLines, []},
+          {Credo.Check.Readability.Semicolons, []},
+          {Credo.Check.Readability.SpaceAfterCommas, []},
+          {Credo.Check.Readability.StringSigils, []},
+          {Credo.Check.Readability.TrailingBlankLine, []},
+          {Credo.Check.Readability.TrailingWhiteSpace, []},
+          {Credo.Check.Readability.UnnecessaryAliasExpansion, []},
+          {Credo.Check.Readability.VariableNames, []},
+          {Credo.Check.Readability.WithSingleClause, []},
+
+          #
+          ## Refactoring Opportunities
+          #
+          {Credo.Check.Refactor.Apply, false}, # should re-enable at some point
+          {Credo.Check.Refactor.CondStatements, []},
+          {Credo.Check.Refactor.CyclomaticComplexity, false}, # should re-enable at some point
+          {Credo.Check.Refactor.FunctionArity, []},
+          {Credo.Check.Refactor.LongQuoteBlocks, []},
+          {Credo.Check.Refactor.MatchInCondition, []},
+          {Credo.Check.Refactor.MapJoin, []},
+          {Credo.Check.Refactor.NegatedConditionsInUnless, []},
+          {Credo.Check.Refactor.NegatedConditionsWithElse, []},
+          {Credo.Check.Refactor.Nesting, false}, # should re-enable at some point
+          {Credo.Check.Refactor.UnlessWithElse, []},
+          {Credo.Check.Refactor.WithClauses, []},
+          {Credo.Check.Refactor.FilterCount, []},
+          {Credo.Check.Refactor.FilterFilter, []},
+          {Credo.Check.Refactor.RejectReject, []},
+          {Credo.Check.Refactor.RedundantWithClauseResult, []},
+
+          #
+          ## Warnings
+          #
+          {Credo.Check.Warning.ApplicationConfigInModuleAttribute, []},
+          {Credo.Check.Warning.BoolOperationOnSameValues, []},
+          {Credo.Check.Warning.Dbg, []},
+          {Credo.Check.Warning.ExpensiveEmptyEnumCheck, []},
+          {Credo.Check.Warning.IExPry, []},
+          {Credo.Check.Warning.IoInspect, []},
+          {Credo.Check.Warning.MissedMetadataKeyInLoggerConfig, []},
+          {Credo.Check.Warning.OperationOnSameValues, []},
+          {Credo.Check.Warning.OperationWithConstantResult, []},
+          {Credo.Check.Warning.RaiseInsideRescue, []},
+          {Credo.Check.Warning.SpecWithStruct, []},
+          {Credo.Check.Warning.WrongTestFileExtension, []},
+          {Credo.Check.Warning.UnusedEnumOperation, []},
+          {Credo.Check.Warning.UnusedFileOperation, []},
+          {Credo.Check.Warning.UnusedKeywordOperation, []},
+          {Credo.Check.Warning.UnusedListOperation, []},
+          {Credo.Check.Warning.UnusedPathOperation, []},
+          {Credo.Check.Warning.UnusedRegexOperation, []},
+          {Credo.Check.Warning.UnusedStringOperation, []},
+          {Credo.Check.Warning.UnusedTupleOperation, []},
+          {Credo.Check.Warning.UnsafeExec, []}
+        ],
+        disabled: [
+          #
+          # Checks scheduled for next check update (opt-in for now, just replace `false` with `[]`)
+
+          #
+          # Controversial and experimental checks (opt-in, just move the check to `:enabled`
+          #   and be sure to use `mix credo --strict` to see low priority checks)
+          #
+          {Credo.Check.Consistency.MultiAliasImportRequireUse, []},
+          {Credo.Check.Consistency.UnusedVariableNames, []},
+          {Credo.Check.Design.DuplicatedCode, []},
+          {Credo.Check.Design.SkipTestWithoutComment, []},
+          {Credo.Check.Readability.AliasAs, []},
+          {Credo.Check.Readability.BlockPipe, []},
+          {Credo.Check.Readability.ImplTrue, []},
+          {Credo.Check.Readability.MultiAlias, []},
+          {Credo.Check.Readability.NestedFunctionCalls, []},
+          {Credo.Check.Readability.OneArityFunctionInPipe, []},
+          {Credo.Check.Readability.SeparateAliasRequire, []},
+          {Credo.Check.Readability.SingleFunctionToBlockPipe, []},
+          {Credo.Check.Readability.SinglePipe, []},
+          {Credo.Check.Readability.Specs, []},
+          {Credo.Check.Readability.StrictModuleLayout, []},
+          {Credo.Check.Readability.WithCustomTaggedTuple, []},
+          {Credo.Check.Readability.OnePipePerLine, []},
+          {Credo.Check.Refactor.ABCSize, []},
+          {Credo.Check.Refactor.AppendSingleItem, []},
+          {Credo.Check.Refactor.DoubleBooleanNegation, []},
+          {Credo.Check.Refactor.FilterReject, []},
+          {Credo.Check.Refactor.IoPuts, []},
+          {Credo.Check.Refactor.MapMap, []},
+          {Credo.Check.Refactor.ModuleDependencies, []},
+          {Credo.Check.Refactor.NegatedIsNil, []},
+          {Credo.Check.Refactor.PassAsyncInTestCases, []},
+          {Credo.Check.Refactor.PipeChainStart, []},
+          {Credo.Check.Refactor.RejectFilter, []},
+          {Credo.Check.Refactor.VariableRebinding, []},
+          {Credo.Check.Warning.LazyLogging, []},
+          {Credo.Check.Warning.LeakyEnvironment, []},
+          {Credo.Check.Warning.MapGetUnsafePass, []},
+          {Credo.Check.Warning.MixEnv, []},
+          {Credo.Check.Warning.UnsafeToAtom, []}
+
+          # {Credo.Check.Refactor.MapInto, []},
+
+          #
+          # Custom checks can be created using `mix credo.gen.check`.
+          #
+        ]
+      }
+    }
+  ]
+}

.github/workflows/elixir.yml

@@ -12,15 +12,11 @@ env:
 jobs:
   mix_test:
     name: mix test (Elixir ${{matrix.elixir}} | OTP ${{matrix.otp}})
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-20.04
     strategy:
       fail-fast: false
       matrix:
         include:
-          - elixir: '1.5.x'
-            otp: 20.3.8.26
-          - elixir: '1.6.6'
-            otp: 21.3.8.24
           - elixir: '1.7.x'
             otp: 22.3.4.26
           - elixir: '1.8.x'
@@ -37,15 +33,13 @@ jobs:
             otp: 25.1
           - elixir: '1.14.x'
             otp: 25.1
-            #warnings_as_errors: true # temporarily disabled due to separate issue that requires fixing, see: https://github.com/nccgroup/sobelow/issues/115
-            check_formatted: true
 
     steps:
     - name: Setup Elixir
       uses: erlef/setup-beam@v1
       with:
-        otp-version: ${{matrix.otp}}
-        elixir-version: ${{matrix.elixir}}
+        otp-version: ${{ matrix.otp }}
+        elixir-version: ${{ matrix.elixir }}
 
     - name: Checkout Code
       uses: actions/checkout@v3
@@ -56,5 +50,19 @@ jobs:
         mix local.rebar --force
         mix deps.get --only test
 
+    - name: Hex Audit
+      run: mix hex.audit
+
+    - name: Check Formatting
+      if: ${{ matrix.elixir == '1.14.x' }} # we only care about formatting for latest version of Elixir
+      run: mix format --check-formatted
+
+    - name: Compiles w/o Warnings
+      if: ${{ matrix.elixir == '1.14.x' }} # we only care about warnings for latest version of Elixir
+      run: mix compile --warnings-as-errors
+
+    - name: Credo
+      run: mix credo --all --strict
+
     - name: Run Tests
       run: mix test

CHANGELOG.md

@@ -1,5 +1,21 @@
 # Changelog
 
+## v0.13.0
+  * Removed
+    * Support for minimum Elixir versions 1.5 & 1.6 (**POTENTIALLY BREAKING** - only applies if you relied on Elixir 1.5 or 1.6, 1.7+ is still supported)
+  * Enhancements
+    * Fixed all `credo` warnings
+    * Implemented all `credo` "Code Readability" adjustments
+    * Took advantage of _some_ `credo` refactoring opportunities
+    * Added (sub)module documentation that was missing for some vulnerabilities and unified presentation of others
+  * Misc
+    * Added `mix credo --strict` to project
+    * Improvements to GitHub CI
+      * Hex Audit
+      * Compiler Warnings as Errors
+      * Checks Formatting
+    * Added helper `mix test.all` alias
+
 ## v0.12.2
   * Bug fixes
     * Removed `:castore` and introduced `:verify_none` to quiet warning and unblock escript usage, see [#133](https://github.com/nccgroup/sobelow/issues/133) for more context on why this is necessary

lib/mix/tasks/sobelow.ex

@@ -192,14 +192,17 @@ defmodule Mix.Tasks.Sobelow do
 
   # This diff check is strictly used for testing/debugging and
   # isn't meant for general use.
+  #
+  # Useful for comapring the output of two different runs of Sobelow
   def run_diff(argv) do
     diff_idx = Enum.find_index(argv, fn i -> i === "--diff" end)
     {_, list} = List.pop_at(argv, diff_idx)
     {diff_target, list} = List.pop_at(list, diff_idx)
-    args = Enum.join(list, " ") |> to_charlist()
-    diff_target = to_charlist(diff_target)
-    :os.cmd('mix sobelow ' ++ args ++ ' > sobelow.tempdiff')
-    IO.puts(:os.cmd('diff sobelow.tempdiff ' ++ diff_target))
+    args = Enum.join(list, " ")
+    diff_target = to_string(diff_target)
+    System.shell("mix sobelow #{args} > sobelow.tempdiff")
+    {diff, _} = System.shell("diff sobelow.tempdiff #{diff_target}")
+    IO.puts(diff)
   end
 
   def set_env(key, value) do
@@ -279,9 +282,10 @@ defmodule Mix.Tasks.Sobelow do
   defp out_format("", format), do: format
 
   defp out_format(_out, format) do
-    cond do
-      format in ["json", "quiet", "sarif"] -> format
-      true -> "json"
+    if format in ["json", "quiet", "sarif"] do
+      format
+    else
+      "json"
     end
   end
 end