Before you commit, every commit

An adventure in automated static code analysis

What is static code analysis?

  • "The analysis of computer software that is performed without actually executing programs"
  • Prevent problematic code from being deployed
    • Linting: identify bugs, syntax errors
    • Identify security vulnerabilities
    • Calculate metrics

How do we do static code analysis?

  • Use IDE plugins in your personal development environment
  • Integrate into the CI pipeline
    • SonarQube, pylint, flake8, black, mypy, etc.

Problems with

IDE-based analysis:

  • Doesn't scale well - how many IDEs do we support?
  • Tough to enforce in community/volunteer projects
  • Leaves the door open to individual deviations

Problems with

CI-based analysis:

  • Feedback loop is rather late (often after peer review/merge)
  • If the tool's analysis is asynchronous (SonarQube), it's easy to forget to monitor the results.
  • Automated fixes are a bad idea

super-linter!

Introducing:

from Github

Link

Super-linter pros:

  • Provides earlier feedback loop than the CI pipeline
    • cannot be ignored by rogue developers
  • Supports multiple languages (even in the same repo!)
  • Can be run locally

Super-linter cons (as of June 2020):

  • The only python linter is Pylint
  • No auto-fixing, just alerting

Wouldn't it be cool if...

  • No vendor lock-in
  • Move the feedback loop to the left using git hooks
  • Changes could be automatically applied
    • e.g. automatically replace tabs with spaces
  • Open by design: easily plug in your own checksa

pre-commit

Introducing:

Link

A framework for managing and maintaining multi-language pre-commit* hooks.

What are

pre-commit hooks?

bash$ git init
Initialized empty Git repository in .git/

bash$ ls .git/hooks/
applypatch-msg.sample     pre-applypatch.sample     pre-rebase.sample
commit-msg.sample         pre-commit.sample         pre-receive.sample
fsmonitor-watchman.sample pre-merge-commit.sample   prepare-commit-msg.sample
post-update.sample        pre-push.sample           update.sample

*The pre-commit tool actually supports 10 hook types.

Let's set up a super-linter equivalent using pre-commit

bash$ pip install pre-commit

bash$ cat .pre-commit-config.yaml
repos:
-   repo: https://github.com/PyCQA/pylint
    rev: master
    hooks:
    - id: pylint
    
bash$ pre-commit install

What does `pre-commit install` do?

bash$ head -n 20 .git/hooks/pre-commit

#!/usr/bin/env python
"""File generated by pre-commit: https://pre-commit.com"""
from __future__ import print_function

import distutils.spawn
import os
import subprocess
import sys

# work around https://github.com/Homebrew/homebrew-core/issues/30445
os.environ.pop('__PYVENV_LAUNCHER__', None)

HERE = os.path.dirname(os.path.abspath(__file__))
Z40 = '0' * 40
ID_HASH = '138fd403232d2ddd5efb44317e38bf03'
# start templated
CONFIG = '.pre-commit-config.yaml'
HOOK_TYPE = 'pre-commit'
INSTALL_PYTHON = '/usr/local/opt/python/bin/python3.7'
SKIP_ON_MISSING_CONFIG = False

Let's write some beautiful code:

import sys
import os

name = input('What is your name?\n')

x = True
if x:
	print ('Hi, %s.' % name)
	print ("This is a really long line" + ", and that's okay because" + " lines are allowed to be long, right?")
bash$ git commit -m "example commit"
pylint...................................................................Failed
hookid: pylint

************* Module test
foo/test.py:8:0: W0311: Bad indentation. Found 1 spaces, expected 4 (bad-indentation)
foo/test.py:9:0: C0301: Line too long (109/80) (line-too-long)
foo/test.py:9:0: W0311: Bad indentation. Found 1 spaces, expected 4 (bad-indentation)
foo/test.py:11:0: C0305: Trailing newlines (trailing-newlines)
foo/test.py:6:0: C0103: Constant name "x" doesn't conform to '(([A-Z_][A-Z0-9_]*)|(__.*__))$' pattern (invalid-name)
foo/test.py:1:0: W0611: Unused import sys (unused-import)
foo/test.py:2:0: W0611: Unused import os (unused-import)

------------------------------------
Your code has been rated at 0.00/10

Some hooks fix code for us!!

bash$ cat .pre-commit-config.yaml
repos:
    
# black: python code formatting
-   repo: https://github.com/psf/black
    rev: stable
    hooks:
    - id: black

# python import sorting, remove unused imports
-   repo: https://github.com/sqlalchemyorg/zimports/
    rev: 0.2.0
    hooks:
    -   id: zimports

-   repo: https://github.com/PyCQA/pylint
    rev: master
    hooks:
    - id: pylint
bash$ git commit -m "example commit"
black....................................................................Failed
hookid: black

Files were modified by this hook. Additional output:

reformatted foo/test.py
All done! ✨ 🍰 ✨
1 file reformatted.

zimports.................................................................Failed
hookid: zimports

Files were modified by this hook. Additional output:

[Writing]       foo/test.py ([20% of lines are imports] [source +0L/-2L] [2 imports removed in 0.0038 sec])

pylint...................................................................Failed
hookid: pylint

************* Module test
foo/test.py:4:0: C0103: Constant name "x" doesn't conform to '(([A-Z_][A-Z0-9_]*)|(__.*__))$' pattern (invalid-name)

------------------------------------------------------------------
Your code has been rated at 8.00/10 (previous run: 0.00/10, +8.00)
bash$ git diff foo/test.py
diff --git a/foo/test.py b/foo/test.py
index 1b50268..ab066ec 100644
--- a/foo/test.py
+++ b/foo/test.py
@@ -1,11 +1,11 @@
-import sys
-import os

-name = input('What is your name?\n')
+name = input("What is your name?\n")

 x = True
 if x:
-       print ('Hi, %s.' % name)
-       print ("This is a really long line" + ", and that's okay because" + " lines are allowed to be long, right?")
-
-
+    print("Hi, %s." % name)
+    print(
+        "This is a really long line"
+        + ", and that's okay because"
+        + " lines are allowed to be long, right?"
+    )

A "quick" feature summary

  • Open by design, easy to add and create new hooks
  • Supports several *execution platforms*:
    • shell, Docker, Golang, Node, Perl, Ruby, Rust, Swift
    • and Python, obviously
  • Lint more than just programming languages
  • Can run in CI using `pre-commit run --all-files`
  • Many configuration options for tweaking/overriding hook functionality to suit your project.

Pre-commit cons

  • Doesn't support every language natively (notably JVM)
    • but shell and docker are sufficient workarounds
  • Requires installation locally
    • If a developer doesn't install locally, they may push code that deviates from the project norms.
    • Workaround - put pre-commit in the CI pipeline as well.

A quick aside about the creator,

Anthony Sottile

What hooks are available for Python?

  • There are many hooks for Python
    • pylint, flake8, autoflake, black, mypy
    • requirements, jinja, docstrings
    • bandit, safety security tests
  • There are many generic hooks for everyone
    • detect private keys/credentials
    • consistent end of line format
    • check for >>>> merge conflicts
    • enforce commit message format
    • check file permissions

What about my friends that write their code in _____?

There are currently 348 hooks tracked by pre-commit, including hooks for:

 

  • Ansible, Chef, Puppet, Terraform, and Cloudformation
  • Perl, PHP, and Bash
  • Lua, R, and Nim
  • CMake, CPP, Clang, and Makefile
  • Mac Admin (jamf, munki, autopkg)

Hook Repository Link

Pre-commit isn't the only option

*but it has the best community support

  • pre-commit
  • Github super-linter
  • lefthook (ruby first, supports python)
  • husky, lint-staged (javascript only)

The pre-commit hook phase of the developer workflow is a great place to do static code analysis.

 

  • A team-wide standard using configuration as code
  • Provides a faster feedback loop than the CI pipeline

In Conclusion

Before you commit

By m3brown

Made with Slides.com

Before you commit

  • 1,020

More from m3brown