DEV Community: Tyler Jang

FIXME Please: An Exercise in TODO Linters

Tyler Jang — Fri, 12 Jul 2024 03:13:16 +0000

A few weeks ago, I was talking with a developer in our Community Slack who was interested in adding their own TODO linter. At face value, this is a trivial problem. There are several linters that already support this to varying degrees, and many of them offer decently extensible configuration and their own plugin ecosystems. But the more I thought about it, the more the question piqued my interest. Trunk supports 100+ linters out of the box (OOTB), but which one would solve this problem best? So I set out to evaluate them all. Here are my findings...

To simplify this experiment, we should clarify what makes for a good TODO linter. Depending on your team’s culture, you may want to prevent any TODOs from making it to main, or you may just want to keep tabs on them. But at a minimum, a TODO linter should satisfy the following:

Easily and quickly report what files have “TODO” strings and where
Support multiple languages/file types
Don’t generate additional noise (“mastodon” isn’t a todo)

As a bonus, some TODO linters might:

Require specific syntax for TODO comments (e.g. clang-tidy)
Support other keywords and cases (e.g. FIXME)
Be able to ignore false positives as appropriate (automatically handled with trunk-ignore)

Now that we have our criteria, let’s dive in. All examples (both with and without Trunk) can be found in this sample repo, so feel free to follow along! If you haven’t used Trunk before, you can follow our setup instructions in our docs.

The Sample File

We'll lint this file with all the tools we test in this blog. This file has some real TODO comments and some fake TODOs meant to confuse linters.

# Test Data

A collection of different ways that TODO might show up.

``yaml
# TODO: Make this better
version: 0.1
``

``typescript
// TODO(Tyler): Optimize this
const a = !!!false;
``

<!-- MASTODON is not a fixme -->

## Another Heading

Look at all the ways to check for todo!

<!-- trunk-ignore-begin(todo-grep-wrapped,codespell,cspell,vale,semgrep,trunk-toolbox) -->

Let's ignore this TODO though

<!-- trunk-ignore-end(todo-grep-wrapped,codespell,cspell,vale,semgrep,trunk-toolbox) -->

Per-Language Rules

Let’s try a naive approach. Several linters have built-in rules to check for TODOs (e.g. ruff, ESLint). Many others support plugin ecosystems to add your own rules. Let’s take a look at markdownlint’s approach to this, using the markdownlint-rule-search-replace package. Run trunk check enable markdownlint to get started.

In order to configure the rule, we must modify .markdownlint.json:

{
  "default": true,
  "extends": "markdownlint/style/prettier",
  "search-replace": {
    "rules": [
      {
        "name": "found-todo",
        "message": "Don't use todo",
        "searchPattern": "/TODO/gi"
      }
    ]
  }
}

Then, we can run it and inspect the output:

Note that we have a trunk-ignore to suppress the TODO on line 24.

Markdownlint here gets the job done, but will of course only work on MD files. As soon as you start to add other file types, even YAML or JS, it doesn’t scale, and you’ll lose coverage and consistency, and chasing down the particular incantation to do this for every linter is intractable. Let’s look at some other more sustainable options.

CSpell

CSpell is a relatively extensible code spellchecker. It’s easy to use OOTB, and it runs on all file types. However, it has a high false positive rate and requires that you manually tune it by importing and defining new dictionaries. Let’s see what it takes to turn it into a TODO linter. First, run trunk check enable cspell.

We can define our own dictionary or simply add a list of forbidden words to cspell.yaml:

version: "0.2"
# Suggestions can sometimes take longer on CI machines,
# leading to inconsistent results.
suggestionsTimeout: 5000 # ms
words:
  - "!todo"
  - "!TODO"

We end up with a quick case-insensitive search for TODOs, albeit with some messy suggestions. It gets the job done, but getting it production-ready for the rest of our codebase will usually require curating additional dictionaries. Running it on the sample repo flags 22 additional false positive issues.

codespell

codespell is a code spellchecker that takes a different approach. Much like CSpell, it is prone to false positives, but rather than defining dictionaries of allowlists, it looks for specific common misspellings and provides suggestions. This reduces its false positive rate, but it usually still requires some tuning. Run trunk check enable codespell to get started.

To teach codespell to flag TODOs, we need to define our own dictionary and reference it:

todo_dict.txt

todo->,encountered todo

.codespellrc

[codespell]
dictionary = todo_dict.txt

Still a bit cumbersome, but we can fine-tune the replacements if desired. Let’s examine some other options.

Vale

Vale is a code prose checker. It takes a more opinionated approach to editorial style, and thus can require lots of tuning, but it is very extensible. Let’s have it check for TODOs. Run trunk check enable vale to get started.

Vale has an opinionated, nested structure to define its configuration. For now, we will only do the minimum to check for TODOs:

.vale.ini

StylesPath = "styles"

MinAlertLevel = suggestion
Packages = base

[*]
BasedOnStyles = Vale, base

styles/base/todo.yml

extends: existence
message: Don't use TODO
level: warning
scope: [raw, text]
tokens:
  - TODO

If you’re already using Vale, and you’re willing to eat the cost of configuration, it can work quite well! Additionally, you can easily customize which file types and scopes it applies to. Let’s try a few more.

Semgrep

Semgrep is a static analysis tool that offers semantic-aware grep. It catches a number of vulnerabilities out of the box, and it’s fairly extensible. It handles most file types, although anecdotally it struggles in some edge cases (e.g. C++ macros, networkless settings). Run trunk check enable semgrep to get started.

Thankfully, Semgrep is configured pretty easily and lets us just specify words or patterns to check for. We can add a config file like so:

.semgrep.yaml

rules:
  - id: check-for-todo
    languages:
      - generic
    severity: ERROR
    message: Don't use TODO
    pattern-either:
      - pattern: TODO
      - pattern: todo

It works pretty well!! And we can customize it however we want in their playground, even modifying our pattern to require specific TODO styling. Semgrep seems like a decent contender for a best-effort solution, but let’s give a couple more a try.

trunk-toolbox

trunk-toolbox is our open-source homegrown linter Swiss Army knife. It supports a few different rules, including searching for TODO and FIXME. It works on all file types and is available just by running trunk check enable trunk-toolbox.

Enable TODO checking in toolbox.toml:

[todo]
enabled = true

This immediately accomplishes the stated goal of a TODO linter–if you just want to find TODOs, just use trunk-toolbox–but it isn’t configurable beyond that.

Grep Linter

Let’s take this one step further. How difficult is it to prototype a solution from scratch? Building a wrapper around grep is the no-brainer solution for this, so let’s start with that.

At its simplest, we can build something like:

.trunk/trunk.yaml

lint:
  definitions:
    - name: todo-grep-linter
      description: Uses grep to look for TODOs
      files: [ALL]
      commands:
        - name: lint
          run: bash -c "grep -E -i 'TODO\W' --line-number --with-filename ${target}"
          output: pass_fail
          success_codes: [0, 1]

This pass_fail linter will just report when we have TODOs. In order to get line numbers, we can wrap this in a script and make it a regex linter with an output that Trunk Check understands:

todo_grep.sh

#!/bin/bash

set -euo pipefail

LINT_TARGET="${1}"

TODO_REGEX="TODO\W"
GREP_FORMAT="([^:]*):([0-9]+):(.*)"
PARSER_FORMAT="\1:\2:0: [error] Found TODO in line (TODO)"

grep -o -E "${TODO_REGEX}" --line-number --with-filename "${LINT_TARGET}" | sed -E "s/${GREP_FORMAT}/${PARSER_FORMAT}/"

.trunk/trunk.yaml

lint:
  definitions:
    - name: todo-grep-wrapped
      description: Uses grep to look for TODOs
      files: [ALL]
      commands:
        - name: lint
          run: sh ${cwd}/todo_grep.sh ${target}
          output: regex
          parse_regex: "((?P<path>.*):(?P<line>-?\\d+):(?P<col>-?\\d+): \\[(?P<severity>.*)\\] (?P<message>.*) \\((?P<code>.*)\\))"
          success_codes: [0, 1]

It’s a bit messy, but it gets the job done. It’s another thing to maintain, but you can tune it as much as you want. We’ll definitely be using one of the pre-built solutions, though.

What did we learn?

There are more than a couple of reasonable options, and depending on your appetite for configuration vs. plug-and-play, some make more sense than others. But overall, using an existing language-agnostic tool performs much better.

And regardless of your preference, all of these options can be super-charged by Trunk. Using githooks and CI gating, you can prevent TODOs from ever landing if that’s your taste. Or, you can burn them down incrementally, only tackling new issues with Hold the Line. You can always make TODOs a non-blocking threshold if need be, or turn them on for yourself without blocking your team.

We all end up with more TODOs than we’d like, but it’s important to build processes that track them (and if necessary gate them) so they don’t get out of hand, just like any other linting issue. There are lots of reasonable options to choose from, but it’s important to make an informed decision when adopting a generalizable approach to linting.

If this post interests you, come check out our other linter definitions in our open-source plugins repo or come chat with us on Slack!

Safely Upgrading our Open Source Dependencies at Scale

Tyler Jang — Fri, 28 Jun 2024 19:03:20 +0000

Trunk Check installs and manages over a hundred linters, code formatters, and other code quality tools. The underlying tools are open source projects that each run on their own release schedule and, like all software, sometimes have bugs. Figuring out which of our 100+ tools are safe to upgrade for our customers is a really hard problem. We have to validate every new version of every tool we support, and their accompanying runtimes and upstream dependencies, before shipping it to our customers. With so many integrations, across all different languages, it’s important to ensure compatibility and continuity as new versions are released. Even something as simple as changing the name of a cli option could break thousands of workflows. Here's how we solved the problem of testing, validating, and automating our linter upgrades.

Black Box Integration Testing

Within the our execution model, each linter can have a runtime, a download, a parser, and configuration for how to run. With so many linter idiosyncrasies, it isn't practical to assert on the behavior of each individual step or linter output; instead, we rely on black box testing.

Our aim is to guarantee Trunk output continuity. When a new linter version is released, it should have the same or a similar set of diagnostics as its predecessor. Fortunately, this problem lends itself well to Jest snapshots. We can capture the JSON result of a linter’s trunk check run, or the contents of a formatted file, and use that as our source of truth.

Versioned Linter Snapshots

When we first run tests on a linter, we save a snapshot of its Trunk output, named with the linter’s version. Subsequent test runs will use the most recent matching snapshot for our assertions, allowing us to keep a historical record of linter output and maintain compatibility.

We detect the latest version of a linter by querying its runtime or by using GitHub APIs. Using our snapshot setup, and the assumption that the latest release of a linter will pass the most recent snapshot (in practice, true about 95% of the time for new releases), we can run nightly tests on the latest versions of all our supported linters. If a linter fails, we can investigate and adapt as needed. If it passes, we can upload its version to our own Release Version Service, which provides the source of truth for linter upgrades.

Example of Linters Breaking

With this system in place, let’s see what happens when a linter breaks compatibility. Trunk integrated with the linter tidy when it was released at known_good_version 1.0.0. So, we generated a snapshot named tidy_v1.0.0.check.shot:

{“issues”: [{
      "linter": "tidy",
      "code": "missing-return",
      "file": “test_data/adder.py",
      "line": "8",
      "column": "12",
      "level": "LEVEL_HIGH",
      "message": "’add’ does not return a value"
    }
]}

We can see that running tidy with Trunk generates one file issue for adder.py. We expect new versions to return the same result - anything else would indicate there was a failure or regression worth investigating.

After we generated this snapshot, tidy then released versions 1.0.1, 1.1.0, and 1.1.1, all of which had an identical output for our test data, so each passed our tests. No need to generate any new snapshots. One week later, tidy released a new version 1.2.0, which renamed one of its CLI arguments. Suddenly, our test reports a failure:

{"issues": [],
"failures": [{
  "report": `tidy exited with exit_code=2
    stdout: (none)
    stderr: |
      usage: tidy [options]
      tidy: error: Unrecognized option found: --verify`
}]}

Trunk doesn’t know how to run tidy@1.2.0. In response, we can add a new versioned subcommand to Trunk configuration, and it runs again!

lint:
  definitions:
    - name: tidy
      …
      commands:
        - name: lint
          version: “>=1.2.0”
          output: sarif
          run: tidy --sarif --verify-target {target}
          success_codes: [0, 1]
        - name: lint
          version: “>=1.0.0”
          output: sarif
          run: tidy --sarif --verify {target}
          success_codes: [0, 1]

The output also changed slightly, so we can create another snapshot, and name it tidy_v1.2.0.check.shot. Going forward, new releases of tidy will use this snapshot, but we always have the option to go back in time and verify that older versions still work as intended, or we can add more test data if we need to. Snapshots are lightweight enough that we can store historical information as long as we need, and Trunk accommodates hermetically installing and running old and new linter versions.

Keeping your linters up to date should be painless. You shouldn’t have to worry about missing downloads, compatibility issues, or sudden flakiness. Trunk will only ever upgrade you to validated linter versions that have passed snapshot tests, sparing you the burden of any bleeding-edge headaches.

Paying Dividends in the Real World

With this system in place, we have already caught several bugs before they could hit user repos.

Semgrep released a binary that was broken on macOS. We identified this issue and helped push for a quick fix.
Ansible-lint introduced a bug that executed user playbooks. We blocked this version from installing on user machines until a fix was landed.
Kube-linter changed their download URL schema. Within hours, we had landed a fix supporting this new URL.
Trunk runs tools hermetically, and several tools (e.g. nixpkgs-fmt, golangci-lint, gofmt) deprecated support for older runtimes. Seeing this, we automatically upgraded the runtimes for users running these linters.
Using older snapshots of these linters, we can block any potential regression before it happens, remaining backwards-compatible whenever possible.

How to Contribute

Trunk’s plugins repository is open-source, and we are always welcoming new contributions for additional linter integrations and improvements, as well as new actions. If you want to try out Trunk for yourself, it's just one shell command away.