theochem / cardboardlint

Cheap lint solution for PRs

Introduction

Cardboardlint is a cheap lint solution for pull requests.

It is a non-hosted and customizable tool similar to commercial services like:

• Hound CI: https://houndci.com/
• Stickler CI: https://stickler-ci.com/
• landscape.io: https://landscape.io/
• ...

For more advanced analysis, one could use Coverty Scan. (https://scan.coverity.com/)

Cardboardlint wraps a collection of linters, intended to be used on a project in a Git repository. Cardboardlint is able to report just those messages related to lines that have changed in your development branch, compared to another commit, e.g. the HEAD of the master branch. This is intended to make life easier for contributors to a project: in their pull requests, they will only see linting issues related to the code they have touched. For some linters, cardboardlint also supports automatic fixes of linting issues, optionally restricted to code that has changed in your development branch.

Some example projects that use Cardboardlint:

• Cardboardlint, obviously
• Roberto: https://github.com/theochem/roberto (Cardboardlint also uses Roberto.)
• IOData: https://github.com/theochem/iodata
• CellCutoff: https://github.com/theochem/cellcutoff
• CGrid: https://github.com/theochem/cgrid

Usage

• Install cardboardlint, which requires python 3.6 or 3.7 and PyYaml. You must have Python 3 installed, PyYaml will be installed automatically for you with the instructions below.

  # Install cardboardlint with pip. Any of the following that works for you
  # is fine:
  pip install cardboardlint
  pip install cardboardlint --user
  python3 -m pip install cardboardlint
  python3 -m pip install cardboardlint --user
  # Or install cardboardlint with conda:
  conda install theochem::cardboardlint

• Add a .cardboardlint.yml to the root of your source tree. It should at least contain a linters section with a list of linters, e.g.

  linters:
  - pylint:
      pylintrc: tools/your_custom_pylintrc
  - cppcheck:
  - import:
  ...

  A list of supported linters is easily deduced from the source code. Just look into the cardboardlint/linter_*.py files. Each of these files has a module docstring with some explanations and a DEFAULT_CONFIG dictionary with the available configuration vars.

  You can repeat any linter multiple times with different configuration settings. This can be useful, e.g. when unit tests must be linted differently than the rest of the source code. This is a simple example where unit tests have a different pylint config:

  pre_filefilter: ['+ tools/demo/*.py', '- tools/*', '+ *']
  
  linters:
  - pylint:
      pylintrc: tools/pylintrc
      filefilter: ['- test_*.py', '+ *.py']
      exclude:
  - pylint:
      pylintrc: tools/pylintrc_tests
      filefilter: ['+ test_*.py']
  - import:
  ...

  When cardboardlint starts, it makes a list of files not ignored by git in the current repository. These filenames are first filtered by a so-called pre_filefilter. Files that pass the pre_filefilter are then tested with linter-specific filefilters to end up with a list of files to be checked by a given linter.

  A pre_filefilter or filefilter consists of a list of rules to test if a file should be considered for linting or not. Each rule starts with a possible outcome, + (include) or a - (exclude), followed by a glob pattern. At the moment, the pattern ignores the presence of directory separators and treats the complete path as a single string on which the pattern is tested, using Pythons fnmatch builtin module. The rules are tested in order and when a pattern matches, the corresponding decision is made (include and exclude), without considering subsequent rules. When no patterns give a match, the file is excluded.

  The following tricks might be useful:

  • If you would like to include files that did not match any pattern, add '+ *' as last pattern, which is often useful for the pre_filefilter.
  • If you would like to include all python files, in all directories, use '+ *.py'. The wildcard will also match directories containing the Python file. For example it would match a/b in the path a/b.py.

• Install the linters you intend to run (either locally or in your CI environment). These dependencies are not installed automatically because you may not want to use all of them.

  Conda packages for all supported linters can be found in the main conda channels an in conda-forge (https://anaconda.org/conda-forge). We have added packages to conda-forge for cppcheck and cpplint. All other linters were already available. To install all of them, we can recommend the following commands:

  # Add conda-forga channel with lower priority as the default channels. This
  # prevents your conda env from being flooded by conda-forga packages.
  conda config --append channels conda-forge
  # Install all linters for which cardboardlint has wrappers:
  conda install pycodestyle pydocstyle cppcheck cpplint yamllint flake8 \
                doxygen pylint autopep8, yapf, black

• Run the cardboardlinter, which can be done in several ways:

  # runs all linters and use multiple cpus
  cardboardlinter -n auto
  # runs all linters and only shows messages for changes relative to master
  cardboardlinter -r master
  # run only static linters
  cardboardlinter -f static
  # run only dynamic linters, which require in-place build
  cardboardlinter -f dynamic
  
  # run fixers, which automaticaly solve trivial problems
  cardboardlinter -F
  # run fixers, which automaticaly solve trivial problems, only on those
  # lines that have changed w.r.t. the master branch.
  cardboardlinter -F -r master

• Usage in CI:

  • Travis-CI (in .travis.yml). This will only report messages for lines that have changed in the PR.

    install:
    # Install the latest cardboardlinter
    - if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then
        pip install --upgrade cardboardlint
      fi
    
    script:
    # Run the cardboardlinter, in case of pull requests
    - if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then
        cardboardlinter --refspec $TRAVIS_BRANCH -n auto;
      fi

  • One can also use Roberto to drive the entire build+test+package workflow, which includes linting with Cardboardlint. See https://theochem.github.io/roberto/

Change log

• Version 1.3.1 August 26, 2020

  • Improve interaction with Roberto.
  • Hide duplicate messages.

• Version 1.3.0 April 14, 2019

  • More verbose output. List of selected files for linting is always shown.
  • Fix default location for Python command-line scripts is ./bin, not ./scripts.
  • Wall time of linters is printed.
  • Few cleanups.

• Version 1.2.0 April 12, 2019

  • Added support for fixers. The following liners can also fix problems: header, whitespace, autopep8, yapf, black. The latter three are new.
  • Removed some Python-2 compatibility code.
  • Replace nosetests by pytest.
  • Add more context to README.
  • Add RST linter.

• Version 1.1.0 April 3, 2019

  • Add option for number of processors to use.

• Version 1.0.1 March 30, 2019

  • Add missing config options for cpplint.

• Version 1.0.0 March 27, 2019

  First release, mainly in anticipation of API-breaking changes, which will result in a major version increase. By making a release, we can handle this change in API gracefully. Notable features include:

  • Support for the following linters: cppcheck, ccplint, doxygen, flake8, header (internal), import (internal), namespace (internal), pycodestyle, pydocstyle, pylint, whitespace (internal), yamllint
  • For all supported linters, the output can be restricted to files and (lines in those files) that have changed between two git commits. In pull requests, this will then only show errors that are relevant for the code touched in the PR. This is intended for making life easy for contributors, i.e. to not bother them with linting issues in code they have not touched.
  • Flexible file filtering, essentially a simplified version of rsync's file filtering rules.
  • Consistent and colored output for all linters, making it easy to process linting issues.

cardboardlint's Issues

Simplify Message class

These changes would simplify the code:

• Remove __hash__
• Remove __eq__
• Remove add_context
• Remove @property
• No private attributes, only public

This means that Message instances have to be stored in lists instead of sets.

Add whitespace linter

This is not only useful for python or cpp code but should be checkable for any text file. Hence, tools like pylint and others do not fully cover this need.

Document pre_filefilter feature in README

Project structure that scales better

At the moment, we just have one main script. This may get very big when we add more linters, which is not friendly to work with or to extend. Better structure would be:

• scripts/cardboardlinter with little code (mind the name change)
• cardboardlint/__init__.py empty
• cardboardlint/common.py utility stuff
• cardboardlint/linter_xxx.py code for one linter

The script can use fairly simply discovery of modules to find all linters. No need to code a dictionary explicitly that contains all of them.

Parallelization

Not urgent, but it'd be really nice to allow parallel runs of cardboardlint (ie all the linters run in parallel, usually on different computers).

We wouldn't be able to configure this parallelism via yaml (the project won't know if we test in parallel or not), so it needs to be via command line. But we won't know which tests are configured without reading the yaml on the test machine (which is a lot of extra complexity) so it might be best to simply take an argument like "1/3" and make that linter run the first third of the tests. The next test machine would pass 2/3 and so on.

Implement clang-static-analyzer lint

This is a slow linter, but rather good at catching C/C++ errors:
https://clang-analyzer.llvm.org/

It would be nice to augment cppcheck with this.

Add gcov linter

This is something needed for the CellCutoff project.

Add HEADER linters

Something to test whether source files have the correct HEADER information, related to copyrights

Add namespace linter

See https://github.com/theochem/horton/blob/master/tools/qa/trapdoor_namespace.py

Document PR reviewing guide

There is an extensive guide in the HORTON docs, which we can copy over and modify.

Make namespace linter more fine-grained

Now one can only include or exclude a file, but more fine-grained control is needed. The namespace linter checks for a few things and one cannot decide to enable only a part of them for specific files.

Make config file optional

Some linters support a config file option, but the file always has to be present. Other linters don't support a config file, while the underlying linter supports non-standard config file paths. This would be better:

• If no config file is given, do not pass a default config file as option to the linter. Instead, let the underlying lint program look for the default config files.
• If a config file is given, add the corresponding argument to the linter program.

Make built-in linters robust against encoding issues

See problems described in #80 . Some of the built-in linters assume that the files they read are UTF-8, and crash when this is not the case.

Print out number of files and lines to be checked in diff

Remove python-2 compatibility code

The following can go:

• encoding line on top: # -*- coding: utf-8 -*- and corresponding linter should be adapted
• from __future__ ...

Add a YAML linter

See https://github.com/adrienverge/yamllint

Add cpplint

See https://github.com/theochem/horton/blob/master/tools/qa/trapdoor_cpplint.py

Make use of setuptools entrypoints instead of scripts/cardboardlinter

This makes it easier to build conda packages, and it is also a lighter in terms of project structure.

Use refspec feature for PRs

Currrently the CI performs a full test on the code, instead of just linting the changes in a PR. (Cardboardlint was designed to make this possible, but it should also use it for its own CI.) See #80 for an example of how this leads to a PR in which many unrelated changes are combined.

Apply to self

Apply the linter to itself. It should set the example, right.

Remove cpplint and cppcheck from project

For both linters, conda-force has packages, see:

• https://anaconda.org/conda-forge/cpplint
• https://anaconda.org/conda-forge/cppcheck

Furthermore, cpplint can be installed through pip, thanks to the following initiative: https://github.com/cpplint/cpplint

There is no need to keep our local recipes and copies in cardboardlint.

pycodestylerc ignored

The max line length option in pycodestylerc is being ignored. It still uses 80 regardless of what you put. For an example, see the entry in cellcutoffs.

Add a Markdown linter

E.g. https://github.com/markdownlint/markdownlint There may be better ones.

Add license info

@kimt33 Are you ok with GPLv3?

Add links: examples, other services complementary to cardboardlint

Complementary services

• Coverity: https://scan.coverity.com/
• Codecov.io

Good practices:

• Build and install software on CI, then test

Add the name of the linter to the namespace used in the --filter option

Add Python coverage linter

See https://github.com/theochem/horton/blob/master/tools/qa/trapdoor_coverage.py

Add import linter

See https://github.com/theochem/horton/blob/master/tools/qa/trapdoor_import.py

Matching filename over paths

In matches_filefilter, fnmatch is used to check if the given filename satisfies the given set of patterns using the UNIX filename matching patterns. I'm not too sure if this is correct, but going by how I use filename matching in my terminal (eg ls), I would think that *py search only the current directory, */*py would search all directories within the current directory, and so on. However, fnmatch simply treats the given filename as a string and assumes no directory information. For example, fnmatch('foo/bar.py', '*py') would return True. In fact, we can assume that fnmatch expects a filename specifically rather than the file path.

Right now, each pattern used in matches_filefilter should start with * if the file is located in any of the subdirectories. It's weird that if I want to find all python files (in all directories), my pattern would be *.py, but if I want all python files that start with test_, I would need to write *test_*.py or */test_*.py. I would expect that if *py is valid, test_*.py is also valid.

Add python formatter support

Some examples

• Configurable, but not as robust: https://pypi.org/project/yapf/
• Very robust but "uncompromising": https://github.com/ambv/black
• No experience with the following: https://github.com/hhatto/autopep8

Flake8 linter

The flake8 linter has tons of plugins (shown below) which vastly exceed the number of checks we currently do. The benefit to us is that the output format and ignore rules are consistent, so it will become easy to write a single linter that does it all.

It may be less work to write a single linter for flake8 than it is to reimplement the few linters that we have right now.

flake8-config-4catalyzer (0.2.1)     - 
flake8-formatter-abspath (1.0.0)     - A flake8 formatter plugin that shows the absolute path of files
                                       with warnings
flake8-translation-activate (1.0.2)  - translation activate plugin for flake8
flake8-ugettext-alias (1.1)          - ugettext alias checker plugin for flake8
flake8-format-ansi (0.1.0)           - ANSI error format plugin for flake8.
flake8-plone-api (1.4)               - Checks for code usages that can be replaced with Plone API
                                       method calls.
flake8-author (1.1.4)                - Flake8 __author__ checker
flake8-blind-except (0.1.1)          - A flake8 extension that checks for blind except: statements
flake8-bugbear (17.4.0)              - A plugin for flake8 finding likely bugs and design problems in
                                       your program. Contains warnings that don't belong in pyflakes
                                       and pycodestyle.
flake8-builtins-unleashed (1.3.1)    - builtin override checker plugin for flake8
flake8-builtins (0.4)                - Check for python builtins being used as variables or parameters.
flake8-chart (0.1.5)                 - flake8 stats visualised
gql-checker (0.1)                    - Flake8 and pylama plugin that checks gql GraphQL calls.
flake8-coding (1.3.0)                - Adds coding magic comment checks to flake8
flake8-colors (0.1.6)                - Error highlight plugin for Flake8.
flake8-trailing-commas (0.1.3)       - Flake8 lint for trailing commas.
flake8-commas (0.4.3)                - Flake8 lint for trailing commas.
jones-complexity (0.1.2)             - Jones Complexity checker, plugin for flake8
flake8-comprehensions (1.4.1)        - A flake8 plugin to help you write better list/set/dict
                                       comprehensions.
flake8-copyright (0.2.0)             - Adds copyright checks to flake8
flake8-custom-indent (0.0.0)         - 
flake8-debugger (1.4.0)              - ipdb/pdb statement checker plugin for flake8
flake8-deprecated (1.2.1)            - Warns about deprecated method calls.
flake8-diff (0.2.2)                  - Run flake8 across a set of changed files and filter out
                                       violations occurring only on the lines that were changed.
flake8-rst-docstrings (0.0.5)        - Python docstring reStructuredText (RST) validator
flake8-docstrings (1.1.0)            - Extension for flake8 which uses pydocstyle to check docstrings
flake8-double-quotes (0.0.1)         - Flake8 lint to forbide single quotes.
flake8-exact-pin (0.0.1)             - A flake8 extension that checks for exact pins (e.q.:
                                       `foo==1.5.6`) in setup.py
flake8-module-imports (1.1)          - module-level import checker plugin for flake8
flake8-network-timeout (0.2.0)       - our extension to flake8
flake8-pytest (1.3)                  - pytest assert checker plugin for flake8
flake8-libfaketime (1.1)             - libfaketime checker plugin for flake8
flake8-sorted-keys (0.1.0)           - check keys are sorted in dict literals
flake8-naming (0.0.0)                - 
flake8-single-quotes (0.1.0)         - A Flake8 extension to enforce single-quotes.
flake8-quotes2 (0.0.1)               - flake8 plugin for string quotes checking
flake8-setuptools (0.0.0)            - Flake8 integration with Setuptools
flake8-user-model (1.1)              - user model import checker plugin for flake8
flake8-snippets (0.2)                - A flake8 extension to find any code snippets you don't like
flake8-immediate (0.2)               - Enables immediate output for flake8.
flake8-import-order-spoqa (1.0.1)    - Spoqa's import order style for flake8-import-order
flake8-imports (0.1.1)               - isort extension flake8
flake8-polyfill (1.0.1)              - Polyfill package for Flake8 plugins
flake8-SQL (0.2.0)                   - Flake8 plugin that checks SQL code against opinionated style
                                       rules
flake8-plone-hasattr (0.2.post0)     - Checks for hasattr, which is considered harmful in Plone
                                       projects.
flake8-respect-noqa (0.3)            - Always ignore #noqa lines with flake8.
flake8-future (0.2)                  - A flake8 plugin to warn when any file is missing certain
                                       __future__ imports.
flake8-regex (0.3)                   - Arbitrary regex checker, extension for flake8
flake8-logging-format (0.3.0)        - Flake8 extension to validate (lack of) logging format strings
flake8-string-format (0.2.3)         - string format checker, plugin for flake8
flake8-import-style (0.4.0)          - A flake8 plugin to ensure explicit module imports
flake8-meiqia (0.1.0)                - Python style guideline in Meiqia
flake8-mock (0.3)                    - Provides checking for non-existent mock methods
flake8-pyi (17.3.0)                  - A plugin for flake8 to enable linting .pyi files.
flake8-html (0.4.0)                  - Generate HTML reports of flake8 violations
flake8-mypy (17.3.3)                 - A plugin for flake8 integrating mypy.
flake8-putty (0.4.0)                 - Apply a bit of putty to flake8.
flake8-pep257 (1.0.5)                - Flake8 plugin for the pep257 Python utility.
flake8-mutable (1.2.0)               - mutable defaults flake8 extension
flake8-future-import (0.4.3)         - __future__ import checker, plugin for flake8
flake8-todo (0.7)                    - TODO notes checker, plugin for flake8
pytest-flake8 (0.8.1)                - pytest plugin to check FLAKE8 requirements
flake8-pep3101 (1.1)                 - Checks for old string formatting.
flake8-tidy-imports (1.1.0)          - A flake8 plugin that helps you write tidier imports.
flake8-import-order-fuzeman (1.6.0)  - @fuzeman's import order style for flake8-import-order
flake8-graphql (0.2.5)               - A flake8 plugin to lint your graphql queries
flake8-ownership (1.0.1)             - Checks for author, copyright, and license info.
flake8-junit-report (2.1.0)          - Simple tool that converts a flake8 file to junit format
flake8-print (2.0.2)                 - print statement checker plugin for flake8
flake8-isort (2.2.1)                 - flake8 plugin that integrates isort .
flake8-quotes (0.11.0)               - Flake8 lint for quotes.
flake8-import-order (0.13)           - Flake8 and pylama plugin that checks the ordering of import
                                       statements.
flake8 (3.4.1)                       - the modular source code checker: pep8, pyflakes and co
flake8_dodgy (0.0.1)                 - 
flake8_doubles (0.3)                 - flake8 doubles checker, ensures mock.patch is not used
flake8_strict (0.1.5)                - Flake8 plugin that checks Python code against a set of
                                       opinionated style rules
flake8_truveris (0.3.4)              - Flake8 extension for checking Python code against Truveris's
                                       code style guide
flake8_tuple (0.2.13)                - Check code for 1 element tuple.

Print timing info for each linter

Replaces theochem/horton#119

Add doxygen linter

See https://github.com/theochem/horton/blob/master/tools/qa/trapdoor_doxygen.py

Also lint files with unstaged changes

This needs to be fixed at the filtering level.

Print messages in sorted order

The list of messages printed on screen is easier to read when they are sorted.

Add pydocstyle linter

See https://github.com/theochem/horton/blob/master/tools/qa/trapdoor_pycodestyle.py

Add some testing code to have full coverage on the linters

We could throw some c++ in the tools/testcpp, e.g. https://github.com/LucHermitte/tictactoe/blob/master/tictactoe.cpp, and add a .cardboardlint.yml file to test it. This will give us some useful coverage on cpp linters.

conda package?

Does it make sense to just put this into a conda package? We're installing it right from the pip directly in our Travis.yml, but I don't really see why we can't just install it on the same line as our conda install.

Print out messages in color and assume less wide terminal

Errors may now be easy to miss. We should use an unusual bright color, to make them very visible in travis. Also, the number of terminal columns on travis 128 and it would be nice to keep our messages shorter, e.g. with some nice wrapping.

Extend selection argument to handle other flags

E.g. it would be useful to have flags for each language: cpp and python. The selection argument would ideally accept a general boolean expression. It would then also be better to separate out the N/M trick to run a fraction of the selected linters.

Add an RST linter

See https://github.com/twolfson/restructuredtext-lint

Remove dependency on c3i_test repo on .travis.yml

Package cppcheck needs to be rebuilt without using the c3i_test channel. This channel is very experimental and breaks constantly.

Add PyLint linter

See https://github.com/theochem/horton/blob/master/tools/qa/trapdoor_pylint.py

Add RSync-style include and exclude mechanism for selecting files

The current include and exclude mechanisms may be too simple for larger projects. Instead, having a list of rules as follows would be better:

- 'rule1'
- 'rule2'
...

Each rule starts with a + or - followed by a whitespace and then a glob pattern. The rules are parsed in order for a given file name and the first matching rule is applied. If no rules apply, the file is excluded.

Add better selection argument

From the comments in the code:

    # The next argument should become a lot smarter in future, e.g. select all static or
    # dynamic linters, or a fraction of them, e.g. "dynamic 0%-33%".

Use yaml.safe_load

See https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load(input)-Deprecation

Add pycodestyle linter

See https://github.com/theochem/horton/blob/master/tools/qa/trapdoor_pycodestyle.py

Add a README

Should contain

• instructions on how to use this in a project
• list dependencies

Make installation into a project more future proof

It would be good that the instructions in the README keep working for a long time, even if we decide to structure the project differently, e.g. move code from the main script to a package with modules to keep it overseable.

We could do this with a tools/bootstrap_run.sh script (to be written) to be used as follows:

curl https://github.com/theochem/cardboardlint/tree/master/tools/bootstrap_run.sh | bash

Inside this script we can still decide how to organize things:

• short term: just download the main script and run it
• mid term: get the latest master, install and run
• long term: install with pip and run

We could also add a tools/bootstrap_install.sh to just install without running.

Add "missing last newline" check to whitespace linter