Expand single-line make variables in the lint-once resolver

A recipe of `@$(FMT)` with `FMT := script/fmt-check` gave `make check` two
prettier passes while the suite stayed green: the resolver read the line as
invoking nothing. The shipped Makefile already writes recipes that way
(`@$(YARN) tsc --watch`), so this was a gap in the repo's own house style.

Variables assigned a literal on one line (`:=`, `=`, `?=`) are collected in a
pass of their own and substituted into target and recipe lines. Values needing
evaluation -- another reference, a make function, a `define` body -- are left
verbatim, and the header's not-followed list now says so.

test/packaging/lint-once.test.ts

@@ -31,11 +31,14 @@
 // `include`d makefile, a name outside those charsets, a generated file — is
 // not followed. Recipe lines reached through a make conditional are followed,
 // without evaluating the condition, and so is a recipe written on the target
-// line after a `;`. A recipe reached some other way is not: an `include`d
+// line after a `;`; a recipe built by an `include`d makefile, a pattern rule,
-// makefile, a pattern rule, a target name outside that charset, or a `define`
+// or a target name outside that charset is not. A variable given a literal
-// block pulled into a recipe as `$(NAME)` — that last one is variable
+// value on one line — `VAR := ...`, `VAR = ...`, `VAR ?= ...` — is substituted
-// expansion, and this parser expands no variables, so `$(EXTRA)` in a recipe
+// wherever the Makefile spells it `$(VAR)` or `${VAR}`, because this Makefile
-// is a name it cannot resolve rather than a body it declines to read. Within
+// writes its recipes that way (`@$(YARN) tsc --watch`); the substitution is
+// one pass over literals, so a value that itself names another variable, a
+// value built by a make function (`$(shell ...)`, `$(addprefix ...)`), and the
+// body of a `define`/`endef` block are not expanded and not followed. Within
 // those edges, a prettier call is caught wherever it is added.
 //
 // Two entrypoints are walked, because they cover different graphs: `make check`
@@ -140,10 +143,80 @@ const MAKE_TARGET_NAME = "[a-z][a-z0-9_-]*";
 // alarm someone fixes; missing one is the failure this file exists to prevent.
 const MAKE_CONDITIONAL = /^\s*(?:ifeq|ifneq|ifdef|ifndef|else|endif)\b/;
 
+// `@$(FMT)` is one token away from being a second prettier pass, and until the
+// variable is resolved it reads as a line that invokes nothing: `make -n check`
+// printed both `script/check` and `script/fmt-check` while this file scored
+// one. `$(MAKE)` was already special-cased below, which is this same rule
+// half-applied to a single name; this is the general form of it.
+//
+// Deliberately only the simple case: a name assigned a literal on one line.
+// `:=` and `=` differ in when make expands them and `?=` in whether it assigns
+// at all, but none of that changes the single value a literal can take, so all
+// three are read the same way. What is not read is anything needing evaluation
+// — a value containing another `$(...)`, a make function, or a `define` body —
+// because expanding those means implementing make, and a half-implementation
+// that resolves a variable to the wrong string would count invocations that do
+// not happen. An unresolved reference is left standing verbatim instead, which
+// is the same dead end as any other unfollowed edge rather than a wrong answer.
+const MAKE_ASSIGNMENT = new RegExp(
+    `^([A-Za-z_][A-Za-z0-9_]*)\\s*(?::=|\\?=|=)\\s*(.*)$`,
+);
+const MAKE_VARIABLE_REFERENCE = /\$[({]([A-Za-z_][A-Za-z0-9_]*)[)}]/g;
+
+// `define`/`endef` bodies are skipped rather than parsed: the body is a
+// multi-line value, so reading its lines as assignments would take whatever
+// `=` they happen to contain and call it a variable.
+const makeVariables = (text: string): Map<string, string> => {
+    const variables = new Map<string, string>();
+    let inDefine = false;
+    for (const raw of text.split("\n")) {
+        if (/^\s*endef\b/.test(raw)) {
+            inDefine = false;
+            continue;
+        }
+        if (/^\s*define\b/.test(raw)) {
+            inDefine = true;
+            continue;
+        }
+        if (inDefine || raw.startsWith("\t")) continue;
+        const assignment = MAKE_ASSIGNMENT.exec(raw.replace(/#.*$/, "").trim());
+        if (assignment === null) continue;
+        const [, name, value] = assignment;
+        if (name === undefined || value === undefined) continue;
+        // A value that is itself a reference is the recursive case, and is not
+        // resolved. Recording it would hand the substitution below a string it
+        // cannot finish expanding.
+        if (/\$[({]/.test(value)) continue;
+        variables.set(name, value.trim());
+    }
+    return variables;
+};
+
+// One pass, and only over names that were assigned a literal: the result is
+// never re-scanned for further references, so this cannot recurse or diverge.
+const expandMakeVariables = (
+    line: string,
+    variables: Map<string, string>,
+): string =>
+    line.replace(
+        MAKE_VARIABLE_REFERENCE,
+        (reference, name: string) => variables.get(name) ?? reference,
+    );
+
 const parseMakefile = (text: string): Map<string, MakeTarget> => {
     const recipes = new Map<string, MakeTarget>();
+    // Collected in a pass of their own because make reads the whole file
+    // before it runs anything: a recipe may spell a variable that the Makefile
+    // assigns further down, and a single pass would leave that one unexpanded
+    // purely because of where its author put it.
+    const variables = makeVariables(text);
     let current: string | null = null;
-    for (const raw of text.split("\n")) {
+    for (const line of text.split("\n")) {
+        // Expanded before parsing rather than only on recipe lines, so a
+        // prerequisite written `check: $(FMT)` resolves the same way a recipe
+        // line does. Assignment lines are expanded too and are unaffected: a
+        // literal value has nothing to substitute.
+        const raw = expandMakeVariables(line, variables);
         if (raw.startsWith("\t")) {
             if (current !== null) {
                 recipes
@@ -710,6 +783,78 @@ describe("the resolver reads what the shell would run", () => {
         });
     });
 
+    // The fifth construct that gave `make check` two prettier passes with the
+    // whole suite green: `FMT := script/fmt-check` and a recipe of `@$(FMT)`.
+    // The shipped Makefile writes recipe lines this way already
+    // (`@$(YARN) tsc --watch`), so an unexpanded variable was a gap in the
+    // house style rather than in an exotic corner of make.
+    it("expands a variable spelled into a recipe line", () => {
+        const recipes = parseMakefile(
+            [
+                "FMT := script/fmt-check",
+                "check:",
+                "\t@$(FMT)",
+                "\t@${FMT}",
+                "\t@script/check",
+            ].join("\n"),
+        );
+        // What `make -n check` prints, in order.
+        expect(recipes.get("check")?.recipe).toEqual([
+            "script/fmt-check",
+            "script/fmt-check",
+            "script/check",
+        ]);
+    });
+
+    it("expands a variable spelled into a prerequisite list", () => {
+        expect(
+            parseMakefile("FMT = fmt-check\ncheck: $(FMT)").get("check")
+                ?.prerequisites,
+        ).toEqual(["fmt-check"]);
+    });
+
+    // A recipe may name a variable the Makefile assigns further down.
+    it("expands a variable assigned after the recipe that uses it", () => {
+        const recipes = parseMakefile(
+            ["check:", "\t@$(FMT)", "FMT ?= script/fmt-check"].join("\n"),
+        );
+        expect(recipes.get("check")?.recipe).toEqual(["script/fmt-check"]);
+    });
+
+    // The edges of the expansion, asserted so the header's not-followed list
+    // is the code's behaviour rather than a claim about it. Each of these
+    // leaves the reference standing verbatim, which reaches nothing — the same
+    // dead end as any other unfollowed edge, and not a wrong resolution.
+    it("leaves a value it cannot resolve to a literal unexpanded", () => {
+        // A make function: resolving it would mean running it.
+        expect(
+            parseMakefile(
+                [
+                    "FMT := $(shell echo script/fmt-check)",
+                    "check:",
+                    "\t@$(FMT)",
+                ].join("\n"),
+            ).get("check")?.recipe,
+        ).toEqual(["$(FMT)"]);
+        // A `define` body is a multi-line value, not a single-line assignment.
+        expect(
+            parseMakefile(
+                [
+                    "define RUNFMT",
+                    "@script/fmt-check",
+                    "endef",
+                    "check:",
+                    "\t$(RUNFMT)",
+                ].join("\n"),
+            ).get("check")?.recipe,
+        ).toEqual(["$(RUNFMT)"]);
+        // An undefined name is not silently emptied.
+        expect(
+            parseMakefile(["check:", "\t@$(NOPE)"].join("\n")).get("check")
+                ?.recipe,
+        ).toEqual(["$(NOPE)"]);
+    });
+
     it("reads a package.json script name that contains a colon", () => {
         // The lookup against `scripts` is what turns this into an edge, and no
         // script in this repo is colon-named, so the charset is asserted on