DEV Community

Cover image for Green is not evidence. Two of my checks were covering less than I thought.
Isamu Arimoto
Isamu Arimoto

Posted on

Green is not evidence. Two of my checks were covering less than I thought.

Someone left this on a post of mine about how I stopped reviewing my own code:

Most teams test the output of a rule (e.g., does this code pass linting?) but rarely test the rule itself.

I'd been writing about pulling decisions into pure functions so they can be tested. They took it one level up: fine, but who tests the thing doing the checking?

I had two answers to that lying around. I just hadn't connected them to each other.

The first one: a typecheck that checked half the repo

We run yarn typecheck in CI on every PR. It had been green for months.

It had also never looked at our server code, or any of our tests.

The root tsconfig.json referenced two of five projects. vue-tsc -b walks the references it's given, so everything else was simply out of scope. Not skipped with a warning — never enumerated in the first place.

Here's the part I want to underline. I fixed the references — and the check stayed green, exactly as green as before. Of course it did; the code was fine. Green looks identical whether you're checking five projects or two, which means it could not tell me whether my fix had worked. So I did this:

const x: number = "nope";
Enter fullscreen mode Exit fullscreen mode

One of those in each of the four areas I cared about, then run the check and confirm it fails.

Where I planted it Before After
server/config/workspace.ts passed ✗ caught
src/utils/focusTrap.ts ✗ caught ✗ caught
test/common/readString.spec.ts passed ✗ caught
test/server/git/prs.spec.ts passed ✗ caught

Three of four had been silently passing. The one that already worked is why nobody noticed — you get some type errors, so the check is obviously running, so you stop wondering. (Fixed now: the root config references all five.)

There's a footnote to this. Our contributing guide had a line saying "yarn typecheck alone passes while CI fails — run all three commands." Somebody had hit this, worked around it in prose, and moved on. A rule in your docs that tells people to be careful is often a bug you can fix in config.

The second one: a rule set to error that reports zero

This one is better, because there is nothing to fix in the config. The config is correct.

We ban type assertions. Not "discourage" — banned, at error severity:

$ npx eslint --print-config src/components/SettingsField.vue \
    | jq '.rules["@typescript-eslint/consistent-type-assertions"]'

[2, {"assertionStyle": "never"}]
Enter fullscreen mode Exit fullscreen mode

Severity 2. Error. For that exact file.

That exact file, line 14:

@input="$emit('update:modelValue', ($event.target as HTMLInputElement).value)"
Enter fullscreen mode Exit fullscreen mode

And:

$ npx eslint src/components/SettingsField.vue
$
Enter fullscreen mode Exit fullscreen mode

Nothing. yarn lint reported 0 errors across the repo.

Both facts are true and neither is a bug. vue-eslint-parser exposes the <template> as a separate AST, and typescript-eslint rules don't walk it. They see <script>. A cast in a template was never in scope for the rule — it just looks like it is, because --print-config will happily tell you the rule is on.

I measured the other frameworks, expecting this to be a template-language problem generally. It isn't:

script side template / JSX side
React, Solid, Preact (.tsx)
Svelte
Astro
Vue

JSX is part of TypeScript's own grammar, so it lands in the same tree. The Svelte and Astro parsers expose their template expressions in a way these rules can visit. Vue keeps them separate. It's an implementation choice, not something inherent to having a template language — which is what I'd assumed before measuring.

And it's fixable. typescript-eslint can't reach that AST, but eslint-plugin-vue can — vue/no-restricted-syntax is the one rule that walks it:

{
  files: ["**/*.vue"],
  rules: {
    "vue/no-restricted-syntax": [
      "error",
      { selector: "TSAsExpression", message: "Narrow in <script>, pass the result to the template." },
      { selector: "TSNonNullExpression", message: "Same." },
    ],
  },
}
Enter fullscreen mode Exit fullscreen mode

One subtlety worth copying: exclude as const.

selector: 'TSAsExpression:not([typeAnnotation.typeName.name="const"])',
Enter fullscreen mode Exit fullscreen mode

consistent-type-assertions already exempts it, and the two halves of one SFC must not disagree about what's banned. It's also not what the ban is for — a const assertion narrows a literal the compiler can already see, rather than claiming a type it couldn't prove.

We ran this on a sibling project first and it found 16 casts that had been outside the gate the entire time the ban was at error. Six were already unnecessary: a v-if above them had started narrowing the type at some point, and nobody re-checked, because nothing was looking.

The rest moved into <script>, which is where a DOM type check belongs anyway:

<input @input="onInput" />
Enter fullscreen mode Exit fullscreen mode
function onInput(e: Event) {
  if (!(e.target instanceof HTMLInputElement)) return;   // bail if it isn't
  emit("update:modelValue", e.target.value);
}
Enter fullscreen mode Exit fullscreen mode

The cast wasn't protecting anything. It was hiding that the check had been written in the wrong place.

Same file, same line, after the rule went in:

$ npx eslint src/components/SettingsField.vue

  18:41  error  Do not use type assertions — narrow in <script> and pass
                the result to the template   vue/no-restricted-syntax

✖ 1 problem (1 error, 0 warnings)
Enter fullscreen mode Exit fullscreen mode

That is the whole point of this post, in one command. The config didn't get stricter — consistent-type-assertions was already at error. What changed is that something is now looking at the place where the violation lives.

Why this keeps happening: the config composes

Neither failure was carelessness, and I don't think either was avoidable by being more careful. Lint and compiler config compose, and composition is where simple pieces stop behaving simply.

Two examples I only found by measuring.

strict doesn't include what you'd guess. It's eight flags. These six are not among them:

noUncheckedIndexedAccess        exactOptionalPropertyTypes
noImplicitReturns               noPropertyAccessFromIndexSignature
noImplicitOverride              noFallthroughCasesInSwitch
Enter fullscreen mode Exit fullscreen mode

The rule that bans as is not in strict. I loaded each typescript-eslint preset and read back what was enabled:

rule recommended strict stylistic recommendedTypeChecked strictTypeChecked
no-explicit-any
no-non-null-assertion
consistent-type-assertions
no-floating-promises
no-unsafe-assignment

It lives in stylistic only. Not in strict, not even in strictTypeChecked. If you reached for strict because casts worried you — which is exactly what I did — you got the opposite of what you wanted, and nothing told you.

Then there's a second-order version, which is the one that actually unsettled me.

sonarjs/different-types-comparison flagged nine comparisons as always-false. All nine were false positives, and they looked like this:

const command = process.argv[2];
if (command === undefined) { /* the rule says this can't happen */ }
Enter fullscreen mode Exit fullscreen mode

Without noUncheckedIndexedAccess, TypeScript types process.argv[2] as string. The rule believed it and concluded the guard was dead. Following the advice would have deleted a real check.

Turning that flag on took it from nine to four — and the remaining four turned out to be genuinely redundant. The rule was never wrong. It was reasoning correctly from a type that was lying to it.

So: when a rule produces false positives, suspect a missing setting upstream before you suspect the rule.

The method, such as it is

Three commands. None of them take more than a few seconds.

# what is ACTUALLY enabled for this file, after all inheritance
npx eslint --print-config src/index.ts

# effective tsconfig, after all extends
./node_modules/.bin/tsc -p tsconfig.app.json --showConfig

# how many errors would this flag produce, before I commit to it
./node_modules/.bin/tsc -p tsconfig.app.json --noEmit --pretty false \
  --noUncheckedIndexedAccess 2>&1 | grep -c "error TS"
Enter fullscreen mode Exit fullscreen mode

Three notes, each from getting it wrong.

Use the project's own tsc. A global one installed by a package manager can be a completely different program wearing the same name, and it will tell you so in a way you won't expect.

Pass --pretty false. With pretty output, tsc writes colour escapes between the two words you're grepping for:

- \x1b[91merror\x1b[0m\x1b[90m TS2322: \x1b[0mType 'string' is not …
Enter fullscreen mode Exit fullscreen mode

The literal string error TS isn't in there any more, so grep -c "error TS" returns 0 — which reads exactly like "no errors." Mine didn't drop colour when piped, either. (And grep -c counts matching lines, so treat it as a gauge, not a census.)

Point -p at a real project, not a solution file. This one is the same bug as the whole post, and I walked into it while writing this section. Our root tsconfig.json is files: [] plus five references — a solution file. -p doesn't follow references, so it compiles nothing and exits clean. I planted one error in server/ and measured:

command errors reported
tsc -p tsconfig.json --noEmit (the root) 0
tsc -b tsconfig.json 72
tsc -p tsconfig.server.json --noEmit 1

Zero, from a repo with a deliberate type error in it. If your root is a solution file, -p on it is a green light that means nothing — use -b, or name the leaf project.

Then the part no command does for you:

Break it on purpose. --print-config tells you what the config says. It cannot tell you whether the rule can reach your code, and -p can't tell you whether it compiled any. The only thing that answers either question is planting a violation and watching the check fail.

What turned up once the checks were actually running

The point of all this is that the checks then find things. A sample of what came out:

  • A type claimed a field the API never sent. /api/session/:id doesn't return id. Adding a runtime guard broke four tests, which is how we found out. The type had said id was there since the beginning.
  • fetchJson<T> returned whatever the caller named it. No validation. If you wrote fetchJson<Config>(...), you got Config — as a claim, not a fact.
  • Config was validated on save and not on load. Broken entries came straight back in at startup.
  • String(x ?? ""), seventeen times. When x is an object you get "[object Object]", no exception. That string was used as a lookup key and rendered as a session title. The real damage isn't the display — it's that missing and corrupted stop being distinguishable.
  • An await on a synchronous function. It waited for nothing and told every future reader "this line is I/O."
  • A sort whose answer depended on who ran it. A rule suggested localeCompare for filenames — which are zero-padded dates and ISO timestamps. Locale order would have made the result machine-dependent. Following that advice would have introduced the bug.
  • A guard that could never fire. matchAll always populates index, per spec.

And on the server side, where I cleared 145 of the 407 no-unsafe-* findings, they came from just three entry points:

entry point why it's any
await import(name) a dynamic import with a computed specifier returns any, and everything reached through it stays outside the type checker
JSON.parse(...) returns any
req.body Express types it any

All three are "a value from outside." One validating function at each boundary cleared all 145. The client side had a wider spread — socket frames, Response.json() — but the shape was the same every time: a boundary where an untyped value walked in and nothing stopped to look at it.

If you only do one thing from this post, grep for those three.

One trap in there: typeof x === "function" doesn't narrow enough. You get Function, and calling a Function returns any, so the result escapes again.

Not everything is worth turning on

Two flags I measured and deliberately left off.

noImplicitReturns — 58 findings, and nearly all of them are this:

if (bad) return res.status(400).json({ error });
res.json(result);   // no explicit return
Enter fullscreen mode Exit fullscreen mode

That's correct Express. Nobody reads a handler's return value. Satisfying the flag means adding 58 meaningless returns and closing zero holes.

noPropertyAccessFromIndexSignature — 1,785 findings, all of them obj.keyobj["key"]. Access safety is unchanged.

Compare with noUncheckedIndexedAccess: 118 findings in shipped code, every one of them "write down what the code already assumed." All 118 were worth it.

The count isn't the signal. The question is whether the work closes a hole, and 118 real fixes beat 1,785 renames.

(Tests got their own answer: noUncheckedIndexedAccess is off there. A test indexing its own fixture with rows[0] doesn't need T | undefined — the test is the thing guaranteeing that value. That's 233 findings that were pure noise. exactOptionalPropertyTypes stays on in tests, because that one is about meaning, not fixture ergonomics.)

The thing I'd tell myself six months ago

Both failures had the same shape, and so did the sibling project's — where yarn lint turned out never to reach scripts/, batch/ or config/. That's 21 files, 92 errors, nine of them the very casts we'd banned. The code that decides whether a PR can merge had no gate on it.

Gates don't fail loudly. A gate that isn't running produces the same output as a gate that's running and finding nothing. There is no error message for "this never executed," because from the inside those two states look identical.

So the commenter had it right, and their framing is better than what I'd written. Testing that your code passes the check is not the same as testing the check.

You can't fix that by reading configs more carefully. You fix it by making the check fail on purpose, once, and watching.


Anything you've found this way, I'd like to hear it — particularly if your green was hiding something dumber than mine.

Top comments (0)