feat: expose per-check alert counts to metric formulas

cmd/vale/command.go

@@ -244,7 +244,16 @@ func printMetrics(args []string, _ *core.CLIFlags) error {
 			"'%s' contains no lintable files", args[0]))
 	}
 
-	computed, _ := linted[0].ComputeMetrics()
+	return printMetricsResult(linted[0])
+}
+
+// printMetricsResult reports f's computed metrics as JSON.
+func printMetricsResult(f *core.File) error {
+	computed, _, _, err := f.ComputeMetrics()
+	if err != nil {
+		return err
+	}
+
 	return printJSON(computed)
 }
 

cmd/vale/metrics_test.go

@@ -0,0 +1,58 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/vale-cli/vale/v3/internal/core"
+)
+
+// TestPrintMetricsResultSucceedsWithoutCollision pins the ordinary,
+// non-colliding path: printMetricsResult must still succeed and print
+// normally when ComputeMetrics finds nothing ambiguous.
+func TestPrintMetricsResultSucceedsWithoutCollision(t *testing.T) {
+	cfg, err := core.NewConfig(&core.CLIFlags{IgnoreGlobal: true})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	f, err := core.NewFile("A document with real prose for a word count.", cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	f.Summary.WriteString("A document with real prose for a word count.")
+
+	if err = printMetricsResult(f); err != nil {
+		t.Fatalf("expected no error, got: %v", err)
+	}
+}
+
+// TestPrintMetricsResultNoLongerErrorsOnSanitizedKeyCollision pins the
+// metric-check-counts redesign's effect on this CLI surface: with check
+// names no longer flattened into Tengo identifiers at all, there is no more
+// sanitized-key collision for ComputeMetrics to detect or for
+// printMetricsResult to propagate specially -- see item 4 of the redesign
+// (cmd/vale/command.go's printMetricsResult should "revert to something
+// much simpler, there's no more collision to propagate specially").
+//
+// This reproduces the f.Metrics = {"words": 999} setup the deleted
+// TestPrintMetricsResultSurfacesCollision (an old-mechanism test pinning
+// the now-removed collision machinery) used to assert an error for, but
+// asserts the opposite outcome: printMetricsResult must now succeed.
+func TestPrintMetricsResultNoLongerErrorsOnSanitizedKeyCollision(t *testing.T) {
+	cfg, err := core.NewConfig(&core.CLIFlags{IgnoreGlobal: true})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	f, err := core.NewFile("A document with real prose for a word count.", cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	f.Summary.WriteString("A document with real prose for a word count.")
+	f.Metrics["words"] = 999
+
+	if err = printMetricsResult(f); err != nil {
+		t.Fatalf("expected no error under the new design (there is no more "+
+			"collision-detection machinery left to trip), got: %v", err)
+	}
+}

internal/check/check_counts.go

@@ -0,0 +1,78 @@
+package check
+
+import (
+	"fmt"
+
+	"github.com/d5/tengo/v2"
+)
+
+// checkCounts is a Tengo object exposing every loaded check's per-document
+// alert count by its real, unflattened name (e.g. "Style.Rule"), added to a
+// `metric` formula's parameters under the "check" key. A formula indexes it
+// directly:
+//
+//	check["AITells.FigurativeOwns"] + check["AITells.HedgingPhrases"] > 3
+//
+// This replaces the old design, where a check name was flattened into a
+// sanitized Tengo identifier (check_Style_Rule) for direct reference in a
+// formula -- a step where two distinct names could sanitize to the same
+// identifier, with no way to tell them apart afterward. Indexing by the
+// real name makes that collision structurally impossible: there's no
+// flattening step left to collide on.
+//
+// It also fixes a silent-typo problem the old design had no way to catch:
+// IndexGet distinguishes a check that's genuinely loaded but simply never
+// fired on this document (reads as 0) from a name that was never loaded at
+// all -- almost always a typo in the formula -- which returns a real error
+// instead of silently reading 0 either way.
+type checkCounts struct {
+	tengo.ObjectImpl
+	// counts holds the raw, unsanitized per-check alert count for this
+	// document, keyed by the check's real name (e.g. "AITells.FigurativeOwns"
+	// -> 3). A check absent from counts simply never fired.
+	counts map[string]int
+	// known holds the set of check names actually loaded for this run --
+	// see core.File.LoadedChecks -- which is what lets IndexGet tell a
+	// never-fired check apart from one that doesn't exist.
+	known map[string]bool
+}
+
+// newCheckCounts builds a checkCounts object from counts (raw per-check
+// alert counts) and known (the set of check names loaded for this run).
+func newCheckCounts(counts map[string]int, known map[string]bool) *checkCounts {
+	return &checkCounts{counts: counts, known: known}
+}
+
+// TypeName returns the name of the type, for Tengo's own error messages and
+// debugging output.
+func (c *checkCounts) TypeName() string {
+	return "check-counts"
+}
+
+// String returns a string representation of the object, for Tengo's own
+// error messages and debugging output.
+func (c *checkCounts) String() string {
+	return "<check counts>"
+}
+
+// IndexGet returns index's alert count as a *tengo.Float -- 0 if it names a
+// check that's loaded but never fired on this document, its real count
+// otherwise -- matching this codebase's existing convention that every other
+// metric value a `metric` formula sees is a float64. Indexing a name that
+// isn't a check genuinely loaded for this run returns a real error naming
+// it, rather than silently reading 0 -- the exact silent-typo failure mode
+// the old, shape-only checkCounterRE match could never catch.
+func (c *checkCounts) IndexGet(index tengo.Object) (tengo.Object, error) {
+	name, ok := tengo.ToString(index)
+	if !ok {
+		return nil, tengo.ErrInvalidIndexType
+	}
+
+	if !c.known[name] {
+		return nil, fmt.Errorf(
+			"%q is not a known check: it isn't defined by any loaded style, "+
+				"so this is likely a typo in the metric formula", name)
+	}
+
+	return &tengo.Float{Value: float64(c.counts[name])}, nil
+}

internal/check/check_counts_test.go

@@ -0,0 +1,114 @@
+package check
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/d5/tengo/v2"
+)
+
+// checkCounts is the target design's replacement for the deleted
+// formulaIdentifiers/identScope/checkCounterRE machinery (see metric.go):
+// rather than flattening every check name into a sanitized Tengo
+// identifier (check_Style_Rule) -- which can collide, since two distinct
+// names can sanitize to the same identifier -- a `metric` formula indexes a
+// single tengo.Object by the check's real, unflattened name:
+//
+//	check["AITells.FigurativeOwns"] + check["AITells.HedgingPhrases"] > 3
+//
+// This makes the collision this branch spent 7 rounds chasing structurally
+// impossible (there's no more name-mangling step to collide), and lets
+// IndexGet distinguish "this check exists and never fired" (0) from "this
+// check name doesn't exist at all" (a real error), fixing the silent-typo
+// problem checkCounterRE's shape-only matching could never catch.
+//
+// newCheckCounts does not exist yet -- this file is the RED-phase
+// specification for it, not a passing test. counts holds the raw,
+// unsanitized per-check alert count (e.g. "AITells.FigurativeOwns" -> 3,
+// the same raw key AddAlert's f.Metrics["check."+a.Check]++ produces once
+// the "check." prefix is stripped); known holds the set of check names
+// actually loaded for this run, which is what lets IndexGet tell a
+// never-fired check apart from a nonexistent one.
+func TestCheckCountsIndexGetOnNeverFiredCheckReturnsZero(t *testing.T) {
+	cc := newCheckCounts(
+		map[string]int{},
+		map[string]bool{"Style.Rule": true},
+	)
+
+	val, err := cc.IndexGet(&tengo.String{Value: "Style.Rule"})
+	if err != nil {
+		t.Fatalf("expected a loaded-but-never-fired check to resolve without "+
+			"error, got: %v", err)
+	}
+
+	got, ok := tengo.ToFloat64(val)
+	if !ok {
+		t.Fatalf("expected a numeric result, got %T (%v)", val, val)
+	}
+	if got != 0 {
+		t.Errorf("expected a never-fired check to read as 0, got %v", got)
+	}
+}
+
+// A check that actually fired must read back its real count, not just a
+// truthy/nonzero placeholder -- the whole point of exposing alert counts to
+// a formula at all.
+func TestCheckCountsIndexGetOnFiredCheckReturnsRealCount(t *testing.T) {
+	cc := newCheckCounts(
+		map[string]int{"Style.Rule": 5},
+		map[string]bool{"Style.Rule": true},
+	)
+
+	val, err := cc.IndexGet(&tengo.String{Value: "Style.Rule"})
+	if err != nil {
+		t.Fatalf("expected a fired, loaded check to resolve without error, got: %v", err)
+	}
+
+	got, ok := tengo.ToFloat64(val)
+	if !ok {
+		t.Fatalf("expected a numeric result, got %T (%v)", val, val)
+	}
+	if got != 5 {
+		t.Errorf("expected the check's real count 5, got %v", got)
+	}
+}
+
+// This is the concrete fix for the design review's most-likely-real-mistake
+// finding: under the old checkCounterRE (`^check_\w+$`, a shape-only regex
+// match against an already-flattened identifier), a misspelled check/rule
+// name in a formula silently evaluated to 0 -- indistinguishable from a
+// real check that simply never fired. Indexing by the check's actual,
+// unflattened name against the set of checks genuinely loaded for this run
+// lets IndexGet tell the two apart and surface a real error instead.
+func TestCheckCountsIndexGetOnUnknownCheckReturnsError(t *testing.T) {
+	cc := newCheckCounts(
+		map[string]int{},
+		map[string]bool{"Style.Rule": true},
+	)
+
+	val, err := cc.IndexGet(&tengo.String{Value: "Style.Typo"})
+	if err == nil {
+		t.Fatalf("expected an error for a check name that isn't loaded, got "+
+			"value %v with no error -- this is the exact silent-typo failure "+
+			"mode the redesign exists to fix", val)
+	}
+	if !strings.Contains(err.Error(), "Style.Typo") {
+		t.Errorf("expected the error to name the unknown check %q, got: %v",
+			"Style.Typo", err)
+	}
+}
+
+// TypeName/String only need to be sane for Tengo's own error messages and
+// debugging output -- not asserted exhaustively, but they must exist and
+// not panic, which ObjectImpl's defaults do (ObjectImpl.TypeName panics
+// with ErrNotImplemented), so checkCounts must actually override both.
+func TestCheckCountsHasATypeNameAndString(t *testing.T) {
+	cc := newCheckCounts(map[string]int{}, map[string]bool{})
+
+	if cc.TypeName() == "" {
+		t.Error("expected a non-empty TypeName")
+	}
+	if cc.String() == "" {
+		t.Error("expected a non-empty String representation")
+	}
+}

internal/check/metric.go

@@ -59,11 +59,19 @@ func (o Metric) Run(_ nlp.Block, f *core.File, _ *core.Config) ([]core.Alert, er
 	ctx, cancel := context.WithTimeout(context.Background(), tengoTimeout)
 	defer cancel()
 
-	parameters, err := f.ComputeMetrics()
+	parameters, rawCheckCounts, hasProse, err := f.ComputeMetrics()
 	if err != nil {
 		return alerts, err
-	} else if len(parameters) == 0 {
-		// empty file.
+	}
+
+	if !hasProse {
+		// A heading-and-code-fence-only document, or any other one with no
+		// prose "words" at all, has nothing for a readability-style formula
+		// to compute: the built-in values it would need (words, characters,
+		// sentences, ...) are never populated in that case (see
+		// ComputeMetrics). Evaluating anyway would fail with an opaque Tengo
+		// "unresolved reference" compile error instead of the graceful no-op
+		// this rule has always had for such a document.
 		return alerts, nil
 	}
 
@@ -75,6 +83,14 @@ func (o Metric) Run(_ nlp.Block, f *core.File, _ *core.Config) ([]core.Alert, er
 		}
 	}
 
+	// Every loaded check's per-document alert count is exposed under the
+	// "check" parameter as an indexable object, rather than flattened into
+	// individual Tengo identifiers -- see checkCounts. A formula reads it as
+	// check["Style.Rule"], which resolves a never-fired check to 0 and a
+	// name that isn't genuinely loaded to a real error, without any risk of
+	// two distinct check names colliding on the same identifier. See #1163.
+	parameters["check"] = newCheckCounts(rawCheckCounts, f.LoadedChecks)
+
 	// The actual result of our formula.
 	//
 	// We need this to allow showing the result in a rule's message.