diff --git a/.claude/knowledge/local-dev-setup.md b/.claude/knowledge/local-dev-setup.md index 1e090aa..e66f86d 100644 --- a/.claude/knowledge/local-dev-setup.md +++ b/.claude/knowledge/local-dev-setup.md @@ -4,3 +4,4 @@ How to set up, run, and work with this project locally. Non-obvious dependencies - **The self-scan cache lives at `.codeguard/.codeguard/cache.json`, not `.codeguard/cache.json`.** `cache.path` resolves relative to the config directory (`config.containConfigArtifactPaths`), and `make codeguard-ci` uses `-config .codeguard/codeguard.yaml`, so the default `.codeguard/cache.json` nests. When iterating on **rule logic**, delete that file before re-running `make codeguard-ci`: cache keys cover config + file content + the release-bumped scanner-version constant, so a code-only change to a check serves stale findings from the cache and looks like your fix didn't work. - The Makefile runs Go via `env -u GOROOT go` (Makefile:6), so `make` targets work even with the shell's stale GOROOT; direct `go` commands need `export GOROOT=/opt/homebrew/opt/go/libexec && export PATH=$GOROOT/bin:$PATH` first. +- Local `golangci-lint run` can fail from the shell's stale `GOROOT` or unwritable `~/Library/Caches/golangci-lint` even when CI is clean. Use `env -u GOROOT GOCACHE=/private/tmp/codeguard-go-cache GOLANGCI_LINT_CACHE=/private/tmp/golangci-lint-cache golangci-lint run` to reproduce CI lint locally. diff --git a/.claude/knowledge/testing-patterns.md b/.claude/knowledge/testing-patterns.md index 3ea17cc..a0c2163 100644 --- a/.claude/knowledge/testing-patterns.md +++ b/.claude/knowledge/testing-patterns.md @@ -13,3 +13,4 @@ Testing strategies, test infrastructure quirks, how to run/debug specific test s - **Bidirectional (server→client) MCP tests** live in `tests/mcp/sampling_test.go`: the test acts as the MCP client, advertises `sampling`/`roots` at `initialize`, and answers the server's server-initiated requests. stdio uses interactive `StdinPipe`/`StdoutPipe` (not the replay harness). HTTP opens the `GET /mcp` SSE stream (waits for the `: ready` comment to avoid the attach race), reads the request off the stream, and POSTs the response with the matching `Mcp-Session-Id`. propose_fix verification is expected to fail on the throwaway diff — assert the round trip fired, not a verified patch. The HTTP helper passes `-config` via `CODEGUARD_TEST_HTTP_CONFIG`. - **TS tests can be hijacked by the Node semantic engine**: on hosts with a discoverable `typescript.js` (e.g. VS Code installed), TypeScript targets route through the Node runner instead of the per-file Go path. Tests that must exercise the per-file path (tree-sitter differential tests, corpus TS groups) set `CODEGUARD_TYPESCRIPT_LIB_PATH` to an existing-but-invalid lib to force the fallback. - Defensive precision positive fixtures should avoid UI-ish names such as `render*` unless the test is explicitly covering UI suppression. The defensive boundary/null rules intentionally skip React/UI helper contexts, so a fixture named like a renderer can stop emitting the server-side defensive finding the test expects. +- Precision-rule retunes should include both the false-positive fixture and a nearby positive that still fires. Dogfood the local binary against a real target repo before committing; several naming/mutation rules only showed meaningful movement after scanning Legal Nest-style React, route, and integration code together. diff --git a/internal/codeguard/checks/quality/quality_defensive.go b/internal/codeguard/checks/quality/quality_defensive.go index bd7f108..d0c8805 100644 --- a/internal/codeguard/checks/quality/quality_defensive.go +++ b/internal/codeguard/checks/quality/quality_defensive.go @@ -194,14 +194,37 @@ func isValidationOrExtractionHelperName(name string) bool { func hasBoundaryParam(params []support.ParsedParam) bool { for _, param := range params { - name := strings.ToLower(param.Name) - if containsAny(name, []string{"req", "request", "event", "payload", "body", "input"}) { + name := strings.ToLower(strings.Trim(param.Name, "_$")) + typ := strings.ToLower(param.Type) + switch name { + case "req", "event", "payload", "body", "params", "query": return true + case "request": + if typ == "" || isTransportRequestType(typ) { + return true + } + case "input": + if typ == "" || containsAny(typ, []string{"unknown", "any", "record", "json", "request", "payload", "body", "params", "query"}) { + return true + } + default: + if strings.HasSuffix(name, "payload") || strings.HasSuffix(name, "body") || strings.HasSuffix(name, "params") || strings.HasSuffix(name, "query") { + return true + } } } return false } +func isTransportRequestType(typ string) bool { + typ = strings.TrimSpace(strings.ToLower(typ)) + return typ == "request" || + strings.Contains(typ, "nextrequest") || + strings.Contains(typ, "httprequest") || + strings.Contains(typ, "express.request") || + strings.Contains(typ, "incomingmessage") +} + func nullAssumptionLine(file string, fn precisionFunction, loweredBody string) (int, bool) { if isUIHelperOrMappingContext(file, fn) || isReactComponentOrHookBoundary(file, fn) { return 0, false @@ -211,10 +234,10 @@ func nullAssumptionLine(file string, fn precisionFunction, loweredBody string) ( if name == "" || !nullableParam(param) { continue } - if containsAny(loweredBody, []string{name + " == nil", name + " != nil", name + " is none", name + " is not none", name + " === null", name + " !== null", name + " == nullptr", name + " != nullptr"}) { + if nullableParamGuarded(loweredBody, name) { continue } - if containsAny(loweredBody, []string{name + ".", name + "->", name + "[", "*" + name}) { + if nullableUseLine(fn, name) > 0 { return firstUseLine(fn, name), true } } @@ -222,19 +245,67 @@ func nullAssumptionLine(file string, fn precisionFunction, loweredBody string) ( } func nullableParam(param support.ParsedParam) bool { - typ := strings.ToLower(param.Type) - return strings.Contains(typ, "*") || strings.Contains(typ, "optional") || - strings.Contains(typ, "null") || strings.Contains(typ, "none") || - strings.Contains(typ, "maybe") || strings.Contains(typ, "?") + typ := strings.ToLower(strings.TrimSpace(param.Type)) + if typ == "" { + return false + } + if strings.Contains(typ, "*") || strings.Contains(typ, "optional") || strings.Contains(typ, "maybe") { + return true + } + if strings.Contains(typ, "{") && strings.Contains(typ, "}") && + !containsAny(typ, []string{"} | null", "}|null", "} | undefined", "}|undefined", "} | none", "}|none"}) { + return false + } + if strings.HasSuffix(strings.TrimSpace(typ), "[]") && + !regexp.MustCompile(`\]\s*\|\s*(null|undefined|none)\b`).MatchString(typ) { + return false + } + return regexp.MustCompile(`(^|\|)\s*(null|undefined|none)\b|\b(null|undefined|none)\s*\|`).MatchString(typ) || + strings.Contains(typ, "?") +} + +func nullableParamGuarded(loweredBody string, name string) bool { + guards := []string{ + name + " == nil", + name + " != nil", + name + " is none", + name + " is not none", + name + " === null", + name + " !== null", + name + " == null", + name + " != null", + name + " == nullptr", + name + " != nullptr", + "typeof " + name + " === ", + "typeof " + name + " == ", + } + if containsAny(loweredBody, guards) { + return true + } + return regexp.MustCompile(`if\s*\(\s*!\s*` + regexp.QuoteMeta(name) + `\s*\)\s*(?:return|throw|continue|break)\b`).MatchString(loweredBody) } -func firstUseLine(fn precisionFunction, name string) int { +func nullableUseLine(fn precisionFunction, name string) int { for _, statement := range fn.Statements { lowered := strings.ToLower(firstNonEmptyString(statement.Raw, statement.Text)) + if nullableStatementUsesOnlyNullSafeOperators(lowered, name) { + continue + } if containsAny(lowered, []string{name + ".", name + "->", name + "[", "*" + name}) { return statement.Line } } + return 0 +} + +func nullableStatementUsesOnlyNullSafeOperators(statement string, name string) bool { + return containsAny(statement, []string{name + "?.", name + "?.[", name + " ??"}) +} + +func firstUseLine(fn precisionFunction, name string) int { + if line := nullableUseLine(fn, name); line > 0 { + return line + } return fn.StartLine } @@ -251,12 +322,26 @@ func integerOverflowLine(file string, fn precisionFunction, loweredBody string) if !regexp.MustCompile(`(?i)\b(count|size|length|len|capacity|offset|total|bytes)\b`).MatchString(loweredBody) { return 0, false } - if regexp.MustCompile(`[A-Za-z_][\w$]*\s*(\*|\+|<<)\s*[A-Za-z0-9_]`).MatchString(loweredBody) { - return fn.StartLine, true + if !resourceAllocationArithmeticContext(loweredBody) { + return 0, false + } + arithmetic := regexp.MustCompile(`[A-Za-z_][\w$]*\s*(\*|\+|<<)\s*[A-Za-z0-9_]`) + for _, statement := range fn.Statements { + if arithmetic.MatchString(firstNonEmptyString(statement.Raw, statement.Text)) { + return statement.Line, true + } } return 0, false } +func resourceAllocationArithmeticContext(loweredBody string) bool { + return containsAny(loweredBody, []string{ + "buffer.alloc", "allocunsafe", "new uint8array", "new arraybuffer", "new array(", + "make([]", "bytearray(", "vector<", "reserve(", "resize(", "setlength(", "content-length", "contentlength", + "readall", "read_all", "readtoend", "read_to_end", + }) +} + func sequenceCollisionRiskLine(file string, fn precisionFunction, loweredBody string) (int, string, string, bool) { if isSeedOrScriptSourcePath(file) || !sequenceAllocationArithmetic(loweredBody) { return 0, "", core.ConfidenceLow, false @@ -354,8 +439,11 @@ func isSeedOrScriptSourcePath(file string) bool { strings.Contains(normalized, "/backfill") || strings.Contains(normalized, "/import") || strings.HasPrefix(base, "seed") || + strings.Contains(base, "seed") || strings.HasPrefix(base, "backfill") || + strings.Contains(base, "backfill") || strings.HasPrefix(base, "import") || + strings.Contains(base, "import") || strings.HasPrefix(base, "cleanup") } diff --git a/internal/codeguard/checks/quality/quality_errors.go b/internal/codeguard/checks/quality/quality_errors.go index 00b9369..093e939 100644 --- a/internal/codeguard/checks/quality/quality_errors.go +++ b/internal/codeguard/checks/quality/quality_errors.go @@ -302,12 +302,19 @@ func exceptionControlFlowLine(fn precisionFunction) (int, bool) { for idx, statement := range fn.Statements { raw := firstNonEmptyString(statement.Raw, statement.Text) lowered := strings.ToLower(raw) - if !throwRaisePattern.MatchString(raw) || !containsAny(lowered, []string{"invalid", "not found", "missing", "stop", "continue", "break"}) { + if !throwRaisePattern.MatchString(raw) || validationExceptionThrow(lowered) { continue } if strings.Contains(lowered, "panic(") { continue } + controlWord := containsAny(lowered, []string{"stop", "continue", "break"}) + genericLookupSentinel := containsAny(strings.ToLower(fn.Name), []string{"find", "lookup", "get", "control"}) && + containsAny(lowered, []string{"not found", "missing"}) && + containsAny(lowered, []string{"exception", "runtimeerror", "runtime_error", "valueerror"}) + if !controlWord && !genericLookupSentinel { + continue + } if strings.Contains(lowered, " if ") || strings.HasPrefix(strings.TrimSpace(lowered), "if ") || (idx > 0 && strings.HasPrefix(strings.TrimSpace(strings.ToLower(fn.Statements[idx-1].Text)), "if ")) { return statement.Line, true @@ -316,6 +323,17 @@ func exceptionControlFlowLine(fn precisionFunction) (int, bool) { return 0, false } +func validationExceptionThrow(lowered string) bool { + if containsAny(lowered, []string{"stop", "continue", "break"}) { + return false + } + return containsAny(lowered, []string{ + "new trpcerror", "new apperror", "new error", "bad_request", "unauthorized", + "forbidden", "unsupported", "invalid json", "invalid input", "invalid response", + "missing required", "source file not found", + }) +} + func boundaryFunctionName(name string) bool { lowered := strings.ToLower(name) return containsAny(lowered, []string{"handler", "handle", "controller", "route", "api", "endpoint", "render", "respond", "response"}) diff --git a/internal/codeguard/checks/quality/quality_precision.go b/internal/codeguard/checks/quality/quality_precision.go index 20d4c6a..a30b6f7 100644 --- a/internal/codeguard/checks/quality/quality_precision.go +++ b/internal/codeguard/checks/quality/quality_precision.go @@ -477,7 +477,7 @@ func primitiveObsession(fn precisionFunction) bool { } func hiddenSideEffect(file string, fn precisionFunction) bool { - if isFrameworkOrchestrationBoundary(file, fn) || isReactComponentOrNamedHookBoundary(file, fn) || explicitMutationName(fn.Name) { + if isFrameworkOrchestrationBoundary(file, fn) || isReactComponentOrNamedHookBoundary(file, fn) || explicitMutationName(fn.Name) || isUICommandHelperName(file, fn.Name) { return false } if !queryFunctionPrefixPattern.MatchString(strings.ToLower(fn.Name)) { @@ -488,7 +488,7 @@ func hiddenSideEffect(file string, fn precisionFunction) bool { } localTargets := localMutationTargets(fn) for _, call := range directCalls(fn) { - if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call.Callee, localTargets) && !isBuilderAccumulatorMutationCall(fn, call) { + if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call, localTargets) && !isLocalBuilderMutationCall(fn, call) && !isBuilderAccumulatorMutationCall(fn, call) { return true } } @@ -522,7 +522,7 @@ func isDomainLevelCall(callee string) bool { } func commandQueryMix(file string, fn precisionFunction) bool { - if isFrameworkOrchestrationBoundary(file, fn) || isReactComponentOrNamedHookBoundary(file, fn) || explicitMutationName(fn.Name) { + if isFrameworkOrchestrationBoundary(file, fn) || isReactComponentOrNamedHookBoundary(file, fn) || explicitMutationName(fn.Name) || isUICommandHelperName(file, fn.Name) { return false } if !fn.Returns { @@ -537,7 +537,7 @@ func commandQueryMix(file string, fn precisionFunction) bool { } localTargets := localMutationTargets(fn) for _, call := range directCalls(fn) { - if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call.Callee, localTargets) && !isBuilderAccumulatorMutationCall(fn, call) { + if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call, localTargets) && !isLocalBuilderMutationCall(fn, call) && !isBuilderAccumulatorMutationCall(fn, call) { return true } } diff --git a/internal/codeguard/checks/quality/quality_precision_mutation_targets.go b/internal/codeguard/checks/quality/quality_precision_mutation_targets.go index 8b3dddf..154e164 100644 --- a/internal/codeguard/checks/quality/quality_precision_mutation_targets.go +++ b/internal/codeguard/checks/quality/quality_precision_mutation_targets.go @@ -10,7 +10,7 @@ import ( var conventionalMutationBoundaryPattern = regexp.MustCompile(`^(accept|apply|approve|archive|clear|close|commit|deliver|download|drop|ensure|exists|fetch|import|list|notify|open|process|read|reconcile|record|run|seed|submit|sync|toggle|upload)`) -var localAccumulatorExprPattern = regexp.MustCompile(`(?i)^(?:new\s+)?(?:array|formdata|map|object|set|urlsearchparams|weakmap|weakset)\b|^\[|^\{|^make\s*\(|^array\.from\b|\.map\s*\(|\.filter\s*\(|\.reduce\s*\(|\.split\s*\(|cheerio\.load\s*\(|^(?:bytes|strings)\.buffer\b|^strings\.builder\b`) +var localAccumulatorExprPattern = regexp.MustCompile(`(?i)^(?:new\s+)?(?:array|date|filereader|formdata|image|map|object|set|url|urlsearchparams|weakmap|weakset)\b|^\[|^\{|^make\s*\(|^array\.from\b|\.map\s*\(|\.filter\s*\(|\.reduce\s*\(|\.split\s*\(|cheerio\.load\s*\(|document\.createelement\s*\(|^(?:bytes|strings)\.buffer\b|^strings\.builder\b`) func localMutationTargets(fn precisionFunction) map[string]struct{} { params := paramNames(fn) @@ -27,6 +27,10 @@ func localMutationTargets(fn precisionFunction) map[string]struct{} { targets[name] = struct{}{} continue } + if assignmentDerivedFromLocalMutationTarget(assignment, targets) { + targets[name] = struct{}{} + continue + } if fn.Returns && isAccumulatorLikeLocalName(name) && assignmentLooksLocalBuilder(fn, assignment) { targets[name] = struct{}{} } @@ -127,11 +131,27 @@ func assignmentLooksLocalBuilder(fn precisionFunction, assignment support.Parsed strings.Contains(expr, "make") } +func assignmentDerivedFromLocalMutationTarget(assignment support.ParsedAssignment, localTargets map[string]struct{}) bool { + expr := strings.TrimSpace(assignment.Expr) + if expr == "" || len(localTargets) == 0 { + return false + } + for target := range localTargets { + if target == "" { + continue + } + if strings.HasPrefix(expr, target+".") || strings.HasPrefix(expr, target+"->") || strings.HasPrefix(expr, target+"::") { + return true + } + } + return false +} + func isAccumulatorLikeLocalName(name string) bool { lowered := strings.ToLower(strings.Trim(name, "_$")) for _, token := range []string{ "bucket", "buckets", "buffer", "builder", "calendar", "cells", "copy", "doc", - "document", "filter", "filters", "form", "items", "lines", "params", "parts", + "canvas", "ctx", "cursor", "date", "document", "filter", "filters", "form", "img", "items", "lines", "params", "parts", "payload", "primarycells", "query", "result", "rows", "scopes", "sections", "serializer", "text", "urlparams", "values", "csv", "export", "map", "$", } { @@ -177,7 +197,26 @@ func paramNames(fn precisionFunction) map[string]struct{} { return params } -func isLocalMutationCall(callee string, localTargets map[string]struct{}) bool { +func isLocalMutationCall(call support.ParsedCall, localTargets map[string]struct{}) bool { + if isObjectAssignToLocalTarget(call, localTargets) { + return true + } + return isLocalMutationCallee(call.Callee, localTargets) +} + +func isLocalBuilderMutationCall(fn precisionFunction, call support.ParsedCall) bool { + loweredCallee := strings.ToLower(strings.ReplaceAll(strings.TrimSpace(call.Callee), " ", "")) + if loweredCallee == "createhmac" || loweredCallee == "createhash" { + return true + } + if loweredCallee != "update" && !strings.HasSuffix(loweredCallee, ".update") { + return false + } + statement := strings.ToLower(assignmentStatement(fn, call.Line)) + return strings.Contains(statement, "createhmac(") || strings.Contains(statement, "createhash(") +} + +func isLocalMutationCallee(callee string, localTargets map[string]struct{}) bool { if isDerivedCollectionMutationCall(callee) { return true } @@ -191,6 +230,35 @@ func isLocalMutationCall(callee string, localTargets map[string]struct{}) bool { return isLocalMutationTarget(target, localTargets) } +func isObjectAssignToLocalTarget(call support.ParsedCall, localTargets map[string]struct{}) bool { + if !isObjectAssignCall(call) { + return false + } + target := firstCallArgName(call) + return target != "" && isLocalMutationTarget(target, localTargets) +} + +func isObjectAssignCall(call support.ParsedCall) bool { + return strings.EqualFold(strings.TrimSpace(call.Callee), "Object.assign") +} + +func firstCallArgName(call support.ParsedCall) string { + if len(call.Args) == 0 { + return "" + } + arg := strings.TrimSpace(call.Args[0]) + if arg == "" { + return "" + } + if idx := strings.IndexAny(arg, ".[("); idx > 0 { + arg = strings.TrimSpace(arg[:idx]) + } + if regexp.MustCompile(`^[A-Za-z_$][\w$]*$`).MatchString(arg) { + return arg + } + return "" +} + func isDerivedCollectionMutationCall(callee string) bool { lowered := strings.ToLower(strings.ReplaceAll(strings.TrimSpace(callee), " ", "")) return strings.Contains(lowered, ".split.") && (strings.HasSuffix(lowered, ".pop") || strings.HasSuffix(lowered, ".sort") || strings.HasSuffix(lowered, ".reverse")) @@ -231,6 +299,9 @@ func isBuilderAccumulatorMutationCall(fn precisionFunction, call support.ParsedC return false } target := mutationCallTarget(call.Callee) + if isObjectAssignCall(call) { + target = firstCallArgName(call) + } if target == "" { return isBareLocalMutationCall(call.Callee) } diff --git a/internal/codeguard/checks/quality/quality_precision_ui_conventions.go b/internal/codeguard/checks/quality/quality_precision_ui_conventions.go index af8e691..89bae38 100644 --- a/internal/codeguard/checks/quality/quality_precision_ui_conventions.go +++ b/internal/codeguard/checks/quality/quality_precision_ui_conventions.go @@ -197,11 +197,18 @@ func isResourceIdentifierName(name string) bool { func conventionalCardinalityName(name string) bool { base := strings.ToLower(strings.Trim(name, "_$")) switch base { - case "all", "answers", "args", "claims", "columns", "contracts", "docs", "entries", "files", "filtered", "ids", "items", "k", "keys", "krs", "matters", "messages", "next", "out", "params", "props", "records", "risks", "rows", "searchparams", "sections", "source", "status", "thresholds", "users", "versions", "v", "i", "j", "x", "y": + case "accept", "activity", "adjacency", "aliases", "all", "answers", "apply", "args", "arr", "body", "changes", "claims", "claimed", "columns", "comments", "content", "contracts", "counts", "currentpatch", "data", "docs", "entries", "expectedkeys", "files", "filtered", "fixes", "grid", "grouped", "groups", "header", "history", "ids", "input", "inputs", "items", "jobs", "k", "keys", "kindcounts", "known", "krs", "matters", "messages", "model", "newrisks", "next", "nodes", "objectives", "obligations", "openrequests", "out", "params", "parsed", "patch", "policy", "prev", "prisma", "projects", "props", "quarter", "quarters", "records", "requests", "resources", "results", "risks", "roledefaults", "roles", "rows", "schemas", "searchparams", "sections", "skipreasons", "source", "state", "status", "tags", "team", "thresholds", "threads", "tools", "users", "values", "vec", "versions", "where", "window", "v", "i", "j", "x", "y": return true default: return len(name) <= 2 || + strings.HasPrefix(base, "by") || + strings.HasSuffix(base, "dates") || + strings.HasSuffix(base, "fields") || + strings.HasSuffix(base, "filters") || strings.HasSuffix(base, "ids") || + strings.HasSuffix(base, "links") || + strings.HasSuffix(base, "options") || + strings.HasSuffix(base, "points") || strings.HasSuffix(base, "rows") || strings.HasSuffix(base, "items") || strings.HasSuffix(base, "entries") || @@ -209,7 +216,19 @@ func conventionalCardinalityName(name string) bool { strings.HasSuffix(base, "props") || strings.HasSuffix(base, "args") || strings.HasSuffix(base, "columns") || - strings.HasSuffix(base, "sections") + strings.HasSuffix(base, "sections") || + strings.HasSuffix(base, "snapshots") || + strings.HasSuffix(base, "tasks") || + strings.HasSuffix(base, "types") || + strings.HasSuffix(base, "bykey") || + strings.HasSuffix(base, "byrisk") || + strings.HasSuffix(base, "bytext") || + strings.HasSuffix(base, "counts") || + strings.HasSuffix(base, "patch") || + strings.HasSuffix(base, "reasons") || + strings.HasSuffix(base, "risks") || + strings.HasSuffix(base, "soon") || + strings.HasSuffix(base, "weeks") } } @@ -233,6 +252,26 @@ func isUIHelperOrMappingContext(file string, fn precisionFunction) bool { strings.Contains(loweredFile, "/app/") && strings.Contains(loweredFile, "web/") } +func isUICommandHelperName(file string, name string) bool { + normalizedFile := strings.ToLower(strings.ReplaceAll(file, "\\", "/")) + if strings.Contains(normalizedFile, "/api/") || !isScriptLikeSourcePath(file) { + return false + } + if !strings.Contains(normalizedFile, "/_components/") && + !strings.Contains(normalizedFile, "/components/") && + !strings.Contains(normalizedFile, "/screens/") && + !isTSXLikeSourcePath(file) { + return false + } + lowered := strings.ToLower(strings.Trim(name, "_$")) + for _, prefix := range []string{"click", "confirm", "disarm", "finish", "mark", "pick", "prefill", "select", "start"} { + if strings.HasPrefix(lowered, prefix) { + return true + } + } + return false +} + func moduleStatementLooksTopLevel(statement support.ParsedStatement) bool { return statement.Indent == 0 } diff --git a/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go b/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go index a82e7e9..3eb1093 100644 --- a/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go +++ b/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go @@ -41,7 +41,6 @@ var ( unitSuffixPattern = regexp.MustCompile(`(?i)(nanos?|micros?|millis?|ms|seconds?|secs?|s|minutes?|mins?|hours?|hrs?|days?|bytes?|kb|mb|gb|cents?|pennies|usd|eur|gbp|aud|cad)$`) collectionTypePattern = regexp.MustCompile(`(?i)(\[\]|\[\s*\]|array|list|slice|map|dict|record|set|vector|collection|iterable|sequence|promise<[^>]*\[\])`) scalarTypePattern = regexp.MustCompile(`(?i)\b(bool|boolean|char|double|float|float64|int|int32|int64|number|string|str|uint|uint64)\b`) - booleanExprPattern = regexp.MustCompile(`(?i)^(true|false|nil|none|null|undefined|[A-Za-z_$][\w$]*\s*(===|!==|==|!=|<=|>=|<|>)|.*(\band\b|\bor\b|&&|\|\||\binstanceof\b|\bis\s+not\b|\bis\b).*)$`) paramMutationPattern = regexp.MustCompile(`\b([A-Za-z_$][\w$]*)\s*(?:\.|->|\[)`) returnLinePattern = regexp.MustCompile(`(?m)^\s*return(?:\s+([^;\n]+))?`) partialReturnPattern = regexp.MustCompile(`(?i)\breturn\s+[^;\n,]+,\s*(err|error)\b|\breturn\s+\{[^}\n]*(data|result|value)[^}\n]*(err|error)[^}\n]*\}`) @@ -58,7 +57,7 @@ func additionalPrecisionFunctionFindings(env support.Context, file string, fn pr findings = append(findings, precisionWarnFinding(env, functionHiddenMutationRuleID, file, fn.StartLine, fmt.Sprintf("function %s mutates state without an explicit command-style name", fn.Name), core.ConfidenceMedium)) } - if inconsistentReturnContract(fn) { + if !isReactComponentOrHookBoundary(file, fn) && !isScriptEntrypoint(file, fn.Name) && inconsistentReturnContract(fn) { findings = append(findings, precisionWarnFinding(env, functionInconsistentReturnContractRuleID, file, fn.StartLine, fmt.Sprintf("function %s mixes empty and value return shapes; make the success/error contract explicit", fn.Name), core.ConfidenceMedium)) } @@ -66,7 +65,7 @@ func additionalPrecisionFunctionFindings(env support.Context, file string, fn pr findings = append(findings, precisionWarnFinding(env, functionPartialResultRuleID, file, fn.StartLine, fmt.Sprintf("function %s can return a value alongside an error without an explicit partial-result contract", fn.Name), core.ConfidenceMedium)) } - if count, labels := responsibilityCount(fn); count >= 4 { + if count, labels := responsibilityCount(fn); count >= responsibilityThreshold(file, fn) { findings = append(findings, precisionWarnFinding(env, functionMultipleResponsibilitiesRuleID, file, fn.StartLine, fmt.Sprintf("function %s combines %d responsibilities (%s); split orchestration from focused work", fn.Name, count, strings.Join(labels, ", ")), core.ConfidenceMedium)) } @@ -115,11 +114,14 @@ func precisionNamingFindings(env support.Context, file string, fn precisionFunct if item.name == fn.Name && isReactComponentOrHookBoundary(file, fn) { continue } - if isBooleanNameCandidate(item.name, item.typ, item.expr, fn) && !isPredicateName(item.name) && !isAllowedBooleanUIName(file, fn, item.name) { + if isBooleanNameCandidate(item.name, item.typ, fn) && + !isInferredUIBooleanAssignment(file, fn, item.typ, item.expr, item.line) && + !isPredicateName(item.name) && + !isAllowedBooleanUIName(file, fn, item.name) { findings = append(findings, precisionWarnFinding(env, namingBooleanNotPredicateRuleID, file, item.line, fmt.Sprintf("boolean name %q should read as a predicate such as is/has/can/should", item.name), core.ConfidenceMedium)) } - if cardinalityMismatch(item.name, item.typ) { + if cardinalityMismatch(file, fn, item.name, item.typ) { findings = append(findings, precisionWarnFinding(env, namingCardinalityMismatchRuleID, file, item.line, fmt.Sprintf("identifier %q has plural/singular wording that conflicts with its value shape", item.name), core.ConfidenceMedium)) } @@ -176,7 +178,7 @@ func behaviorMismatch(file string, fn precisionFunction) bool { } func hiddenMutation(file string, fn precisionFunction) bool { - if explicitMutationName(fn.Name) || isDomainSideEffectBoundaryName(fn.Name) || isFrameworkOrchestrationBoundary(file, fn) || isScriptEntrypoint(file, fn.Name) { + if explicitMutationName(fn.Name) || isUICommandHelperName(file, fn.Name) || isDomainSideEffectBoundaryName(fn.Name) || isFrameworkOrchestrationBoundary(file, fn) || isScriptEntrypoint(file, fn.Name) { return false } if isReactComponentOrNamedHookBoundary(file, fn) { @@ -200,7 +202,7 @@ func hasLikelyExternalMutationCall(fn precisionFunction) bool { if !mutatingCallPattern.MatchString(call.Callee) { continue } - if isLocalMutationCall(call.Callee, localTargets) || isBuilderAccumulatorMutationCall(fn, call) { + if isLocalMutationCall(call, localTargets) || isLocalBuilderMutationCall(fn, call) || isBuilderAccumulatorMutationCall(fn, call) { continue } target := mutationCallTarget(call.Callee) @@ -240,7 +242,7 @@ func hasLikelyParameterAssignment(fn precisionFunction) bool { func mutatingFunctionEvidence(fn precisionFunction) bool { localTargets := localMutationTargets(fn) for _, call := range directCalls(fn) { - if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call.Callee, localTargets) && !isBuilderAccumulatorMutationCall(fn, call) { + if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call, localTargets) && !isLocalBuilderMutationCall(fn, call) && !isBuilderAccumulatorMutationCall(fn, call) { return true } } @@ -333,6 +335,9 @@ func inconsistentReturnContract(fn precisionFunction) bool { if nextResponseNullableGuardHelper(fn) || nullableParserLookupContract(fn) { return false } + if explicitNullableReturnContract(fn) { + return false + } returns := returnCategories(fn.Body) if returns.total < 2 { return false @@ -340,6 +345,20 @@ func inconsistentReturnContract(fn precisionFunction) bool { return returns.empty && returns.value } +func explicitNullableReturnContract(fn precisionFunction) bool { + signature := strings.ToLower(fn.Signature) + if signature == "" { + return false + } + return containsAny(signature, []string{ + "| null", "|null", "| undefined", "|undefined", + "null>", "undefined>", "optional<", "option<", + "promise<", "result<", + }) && containsAny(fn.Body, []string{ + "return null", "return undefined", "return none", "return nil", + }) +} + func nextResponseNullableGuardHelper(fn precisionFunction) bool { signature := strings.ToLower(fn.Signature) body := strings.ToLower(fn.Body) @@ -443,6 +462,13 @@ func responsibilityCount(fn precisionFunction) (int, []string) { return len(labels), labels } +func responsibilityThreshold(file string, fn precisionFunction) int { + if isReactComponentOrHookBoundary(file, fn) { + return 6 + } + return 4 +} + func classifyResponsibility(text string, record func(string)) { switch { case strings.Contains(text, "validat") || strings.Contains(text, "sanitize"): @@ -488,29 +514,51 @@ func orchestrationDomainMix(fn precisionFunction) bool { return hasInfra && hasDomainDecision } -func isBooleanNameCandidate(name string, typ string, expr string, fn precisionFunction) bool { +func isBooleanNameCandidate(name string, typ string, fn precisionFunction) bool { if name == fn.Name { - return functionLooksBoolean(fn) + if explicitMutationName(name) || explicitNonBooleanFunctionName(name) { + return false + } + return functionReturnLooksBoolean(fn.Signature) } if isBooleanType(typ) { return true } - return expr != "" && booleanExprPattern.MatchString(strings.TrimSpace(expr)) + return false } -func functionLooksBoolean(fn precisionFunction) bool { - if isPredicateName(fn.Name) { - return false - } - for _, statement := range fn.Statements { - text := strings.TrimSpace(strings.TrimSuffix(statement.Text, ";")) - if strings.HasPrefix(text, "return ") && booleanExprPattern.MatchString(strings.TrimSpace(strings.TrimPrefix(text, "return "))) { +func explicitNonBooleanFunctionName(name string) bool { + lowered := strings.ToLower(strings.Trim(name, "_$")) + for _, prefix := range []string{ + "build", "call", "create", "decode", "extract", "fetch", "format", "hydrate", + "load", "lookup", "normalize", "parse", "read", "reject", "render", "resolve", + "serialize", "strip", "to", "write", + } { + if strings.HasPrefix(lowered, prefix) { return true } } return false } +func functionReturnLooksBoolean(signature string) bool { + signature = strings.ToLower(strings.TrimSpace(signature)) + if signature == "" { + return false + } + if idx := strings.LastIndex(signature, "->"); idx >= 0 { + return isBooleanType(signature[idx+len("->"):]) + } + return isBooleanType(signature) +} + +func isInferredUIBooleanAssignment(file string, fn precisionFunction, typ string, expr string, line int) bool { + if expr == "" || isBooleanType(typ) { + return false + } + return isUIHelperOrMappingContext(file, fn) || isReactComponentOrHookBoundary(file, fn) || nearbyUICallbackStatement(fn, line) +} + func isBooleanType(typ string) bool { typ = strings.ToLower(strings.TrimSpace(typ)) return typ == "bool" || typ == "boolean" || strings.Contains(typ, " bool") || strings.Contains(typ, ": boolean") @@ -518,28 +566,56 @@ func isBooleanType(typ string) bool { func isPredicateName(name string) bool { lowered := strings.ToLower(strings.Trim(name, "_$")) - for _, prefix := range []string{"is", "are", "has", "have", "can", "could", "should", "must", "allow", "allows", "enable", "enabled", "disable", "disabled", "needs", "requires", "supports", "valid", "visible", "ready", "show", "matches"} { + for _, prefix := range []string{"is", "are", "has", "have", "can", "could", "should", "must", "allow", "allows", "enable", "enabled", "disable", "disabled", "needs", "requires", "supports", "valid", "verify", "visible", "ready", "show", "matches", "pass", "passes"} { if strings.HasPrefix(lowered, prefix) { return true } } + for _, suffix := range []string{"equal", "equals", "differs", "matches"} { + if strings.HasSuffix(lowered, suffix) { + return true + } + } return false } -func cardinalityMismatch(name string, typ string) bool { +func cardinalityMismatch(file string, fn precisionFunction, name string, typ string) bool { base := strings.ToLower(strings.Trim(name, "_$")) if base == "" || conventionalCardinalityName(base) || configuredPluralDomainAbbreviation(base) || strings.HasSuffix(base, "status") || strings.HasSuffix(base, "class") { return false } + if isUIHelperOrMappingContext(file, fn) && conventionalUICardinalityName(base) { + return false + } + if strings.Contains(typ, "{") || strings.Contains(typ, "}") { + return false + } plural := isPluralName(base) - collection := collectionTypePattern.MatchString(typ) - scalar := scalarTypePattern.MatchString(typ) + collection := collectionTypePattern.MatchString(typ) || strings.Contains(typ, "[") || strings.Contains(typ, "]") + scalar := !collection && scalarTypePattern.MatchString(typ) if plural && scalar && !collection { return true } return !plural && collection && !strings.Contains(base, "map") && !strings.Contains(base, "list") && !strings.Contains(base, "set") } +func conventionalUICardinalityName(base string) bool { + if strings.HasPrefix(base, "by") { + return true + } + switch base { + case "active", "allowed", "display", "form", "result", "scope", "team", "view": + return true + default: + return strings.HasSuffix(base, "bytype") || + strings.HasSuffix(base, "bystatus") || + strings.HasSuffix(base, "bymonth") || + strings.HasSuffix(base, "rows") || + strings.HasSuffix(base, "result") || + strings.HasSuffix(base, "results") + } +} + func isPluralName(name string) bool { if unitSuffixPattern.MatchString(name) { return false diff --git a/internal/codeguard/checks/support/parser_clike.go b/internal/codeguard/checks/support/parser_clike.go index f64c655..c91e161 100644 --- a/internal/codeguard/checks/support/parser_clike.go +++ b/internal/codeguard/checks/support/parser_clike.go @@ -60,11 +60,18 @@ func newCLikeFunction(file *ParsedFile, span clikeSpan, lang CLikeLanguage) *Par if paramsClose > span.paramsOpen { paramText = file.Masked[span.paramsOpen+1 : paramsClose] } + signature := strings.TrimSpace(squashWhitespace(paramText)) + if span.bodyOpen > paramsClose { + trailing := strings.TrimSpace(squashWhitespace(file.Masked[paramsClose+1 : span.bodyOpen])) + if trailing != "" { + signature = strings.TrimSpace(signature + " -> " + trailing) + } + } fn := &ParsedFunction{ Name: span.name, StartLine: LineNumberForOffset(file.Source, span.start), EndLine: LineNumberForOffset(file.Source, span.bodyEnd), - Signature: strings.TrimSpace(squashWhitespace(paramText)), + Signature: signature, Params: parseCLikeParams(paramText, lang), } if span.bodyOpen >= 0 && span.bodyEnd > span.bodyOpen { diff --git a/tests/checks/function_hidden_mutation_noise_test.go b/tests/checks/function_hidden_mutation_noise_test.go index 7676226..885258d 100644 --- a/tests/checks/function_hidden_mutation_noise_test.go +++ b/tests/checks/function_hidden_mutation_noise_test.go @@ -42,6 +42,26 @@ func TestFunctionHiddenMutationAllowsPureLocalMutationAcrossLanguages(t *testing "}", }, }, + { + name: "typescript local date cursor", + language: "typescript", + file: "apps/web/lib/calendar.ts", + source: []string{ + "export function monthBuckets(start: number, end: number) {", + " const buckets: Array<{ start: number; label: string }> = [];", + " const cursor = new Date(start);", + " cursor.setDate(1);", + " cursor.setHours(0, 0, 0, 0);", + " while (cursor.getTime() < end) {", + " const next = new Date(cursor);", + " next.setMonth(next.getMonth() + 1);", + " buckets.push({ start: cursor.getTime(), label: cursor.toISOString() });", + " cursor.setTime(next.getTime());", + " }", + " return buckets;", + "}", + }, + }, { name: "python local list", language: "python", @@ -92,6 +112,7 @@ func TestFunctionHiddenMutationAllowsPureLocalMutationAcrossLanguages(t *testing report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, tc.language)) assertFindingRuleAbsent(t, report, "Code Quality", "function.hidden-mutation") + assertFindingRuleAbsent(t, report, "Code Quality", "function.command-query-mix") }) } } @@ -144,6 +165,62 @@ func TestFunctionHiddenMutationAllowsBuilderParserAccumulatorNames(t *testing.T) "interface Event { day: string }", }, }, + { + name: "local patch object assign", + file: "packages/api/src/lib/portal-field-map.ts", + source: []string{ + "export function answersToMatterPatch(answers: Record) {", + " const patch: Record = {};", + " for (const [key, value] of Object.entries(answers)) {", + " Object.assign(patch, coerce(key, value));", + " }", + " return patch;", + "}", + "declare function coerce(key: string, value: string): Record;", + }, + }, + { + name: "local map entry alias", + file: "apps/web/lib/filter-match.ts", + source: []string{ + "export function passesActiveFilters(active: ActiveFilter[]) {", + " const byId = new Map();", + " for (const filter of active) {", + " const group = byId.get(filter.id);", + " if (group) group.push(filter);", + " else byId.set(filter.id, [filter]);", + " }", + " return byId.size > 0;", + "}", + "interface ActiveFilter { id: string }", + }, + }, + { + name: "local url search params", + file: "apps/web/lib/integration-oauth.ts", + source: []string{ + "export function redirectToIntegrationProfile(origin: string, params: Record) {", + " const url = new URL('/profile', origin);", + " url.hash = 'slack';", + " for (const [key, value] of Object.entries(params)) url.searchParams.set(key, value);", + " return url.toString();", + "}", + }, + }, + { + name: "local crypto hmac builder", + file: "apps/web/lib/oauth-state.ts", + source: []string{ + "import { createHmac, timingSafeEqual } from 'node:crypto';", + "export function verifySignedOAuthState(state: string, expected: string) {", + " const parts = state.split('.');", + " if (parts.length !== 3) return false;", + " const [nonce, userId, signature] = parts as [string, string, string];", + " const digest = createHmac('sha256', expected).update(`${nonce}.${userId}`).digest('hex');", + " return timingSafeEqual(Buffer.from(signature, 'hex'), Buffer.from(digest, 'hex'));", + "}", + }, + }, { name: "parser local object", file: "packages/api/src/lib/contract-summary/parse.ts", diff --git a/tests/checks/function_precision_test.go b/tests/checks/function_precision_test.go index 749b02e..404a078 100644 --- a/tests/checks/function_precision_test.go +++ b/tests/checks/function_precision_test.go @@ -183,6 +183,22 @@ func TestFunctionHiddenMutationWarnsAcrossLanguages(t *testing.T) { } } +func TestFunctionHiddenMutationWarnsForObjectAssignIntoParameter(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "mutation.ts"), strings.Join([]string{ + "export function prepareUser(user: User, patch: Partial): User {", + " Object.assign(user, patch);", + " return user;", + "}", + "interface User { name: string }", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRulePresent(t, report, "Code Quality", "function.hidden-mutation") + assertFindingRulePresent(t, report, "Code Quality", "function.command-query-mix") +} + func TestFunctionHiddenMutationAllowsReactHookLocalStateBoundaries(t *testing.T) { cases := []struct { name string diff --git a/tests/checks/naming_precision_test.go b/tests/checks/naming_precision_test.go index 933ae68..24cc9df 100644 --- a/tests/checks/naming_precision_test.go +++ b/tests/checks/naming_precision_test.go @@ -154,7 +154,7 @@ func TestNamingPredicateAndCardinalityPositiveNegative(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "names.ts"), strings.Join([]string{ "export function evaluate(users: number, user: Array, enabled: boolean): boolean {", - " const active = enabled === true;", + " const active: boolean = enabled === true;", " const isReady = users > 0;", " return active && isReady;", "}", @@ -166,6 +166,119 @@ func TestNamingPredicateAndCardinalityPositiveNegative(t *testing.T) { assertFindingRulePresent(t, report, "Code Quality", "naming.cardinality-mismatch") } +func TestNamingBooleanPredicateSkipsInferredUIAssignments(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "apps/web/components/status-chip.tsx"), strings.Join([]string{ + "export function StatusChip({ status, flag }: { status: string; flag: boolean }) {", + " const active = status === 'ACTIVE';", + " const blocked = status === 'BLOCKED';", + " return {active ? 'Active' : blocked ? 'Blocked' : 'Other'}{flag ? '!' : ''};", + "}", + }, "\n")) + writeFile(t, filepath.Join(dir, "packages/api/src/lib/flag.ts"), strings.Join([]string{ + "export function evaluateFlag(flag: boolean) {", + " return flag;", + "}", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRulePresent(t, report, "Code Quality", "naming.boolean-not-predicate") + assertCodeQualityRuleAbsentForPath(t, report, "naming.boolean-not-predicate", "status-chip.tsx:2") + assertCodeQualityRuleAbsentForPath(t, report, "naming.boolean-not-predicate", "status-chip.tsx:3") +} + +func TestNamingBooleanPredicateAllowsRouteParserAndVerifierHelpers(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "apps/web/app/api/files/upload/route.ts"), strings.Join([]string{ + "export function rejectOversizedMultipartRequest(req: Request): Response | null {", + " if (Number(req.headers.get('content-length') ?? 0) > 1000) return new Response('too large');", + " return null;", + "}", + "export async function readOAuthJson(res: Response): Promise | null> {", + " if (!res.ok) return null;", + " return await res.json();", + "}", + "export function verifyHmac(body: string, signature: string): boolean {", + " return body.length === signature.length;", + "}", + "export function passesActiveFilters(status: string): boolean {", + " return status === 'ACTIVE';", + "}", + "export function datesEqual(a: Date, b: Date): boolean {", + " return a.getTime() === b.getTime();", + "}", + "export function valueDiffers(a: string, b: string): boolean {", + " return a !== b;", + "}", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRuleAbsent(t, report, "Code Quality", "naming.boolean-not-predicate") +} + +func TestNamingBooleanPredicateDoesNotInferFromArrowsOrGenerics(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages/api/src/lib/arrow-values.ts"), strings.Join([]string{ + "export function buildValues(items: T[]) {", + " const timer = setTimeout(() => undefined, 100);", + " const rowClass = items.length > 0 ? 'active' : 'empty';", + " const payload = { values: items.map((item) => String(item)) };", + " return { timer, rowClass, payload };", + "}", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRuleAbsent(t, report, "Code Quality", "naming.boolean-not-predicate") +} + +func TestNamingCardinalityCreditsCollectionTypesBeforeScalarWords(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages/api/src/lib/collections.ts"), strings.Join([]string{ + "export function previousQuarter(quarters: { externalId: string }[] | undefined) {", + " return quarters?.at(0)?.externalId ?? null;", + "}", + "export function confidentialityFilter(responsibleFields: string[]) {", + " return responsibleFields.map((field) => ({ [field]: true }));", + "}", + "export function badName(user: string[]) {", + " return user.length;", + "}", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRulePresent(t, report, "Code Quality", "naming.cardinality-mismatch") + assertCodeQualityRuleAbsentForPath(t, report, "naming.cardinality-mismatch", "collections.ts:1") + assertCodeQualityRuleAbsentForPath(t, report, "naming.cardinality-mismatch", "collections.ts:4") +} + +func TestNamingCardinalityAllowsUICollectiveAndMapNames(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "apps/web/app/risks/_components/use-risk-filters.ts"), strings.Join([]string{ + "export function RiskFilterBar(team: User[], form: FormState[], result: Result[], byMonth: Map, kindByType: Record, allowed: string[]) {", + " const displayRisks: Risk[] = [];", + " return { team, form, result, byMonth, kindByType, allowed, displayRisks };", + "}", + "interface User { id: string }", + "interface FormState { id: string }", + "interface Result { id: string }", + "interface Risk { id: string }", + }, "\n")) + writeFile(t, filepath.Join(dir, "packages/api/src/lib/bad.ts"), strings.Join([]string{ + "export function badName(user: string[]) {", + " return user.length;", + "}", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertCodeQualityRuleAbsentForPath(t, report, "naming.cardinality-mismatch", "use-risk-filters.ts") + assertCodeQualityRulePresentForPathWithMessage(t, report, "naming.cardinality-mismatch", "bad.ts", "user") +} + func TestNamingUnitsAbbreviationsAndImplementationLeak(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "names.py"), strings.Join([]string{ diff --git a/tests/checks/quality_error_defensive_multilang_test.go b/tests/checks/quality_error_defensive_multilang_test.go index 1406960..3512149 100644 --- a/tests/checks/quality_error_defensive_multilang_test.go +++ b/tests/checks/quality_error_defensive_multilang_test.go @@ -311,8 +311,9 @@ func TestQualityDefensiveBoundariesDetectMultiLanguageSignals(t *testing.T) { "\treturn items[0]", "}", "", - "func Allocate(count int) int {", - "\treturn count * 4096", + "func Allocate(count int) []byte {", + "\ttotalBytes := count * 4096", + "\treturn make([]byte, totalBytes)", "}", "", "func AllowByDefault() bool {", @@ -402,7 +403,8 @@ func TestQualityDefensiveBoundariesDetectMultiLanguageSignals(t *testing.T) { " return items[0]", "", "def allocate(count):", - " return count * 4096", + " total_bytes = count * 4096", + " return bytearray(total_bytes)", "", "def allow_by_default():", " return os.environ.get('AUTHZ_DISABLED', 'true')", @@ -447,8 +449,9 @@ func TestQualityDefensiveBoundariesDetectMultiLanguageSignals(t *testing.T) { " return items[0];", "}", "", - "export function allocate(count: number): number {", - " return count * 4096;", + "export function allocate(count: number): Uint8Array {", + " const totalBytes = count * 4096;", + " return new Uint8Array(totalBytes);", "}", "", "export function allowByDefault(): string {", @@ -496,7 +499,7 @@ func TestQualityDefensiveBoundariesDetectMultiLanguageSignals(t *testing.T) { "}", "", "export function first(items) { return items[0]; }", - "export function allocate(count) { return count * 4096; }", + "export function allocate(count) { const totalBytes = count * 4096; return new Uint8Array(totalBytes); }", "export function allowByDefault() { return process.env.AUTHZ_DISABLED || 'true'; }", "", "export function fetchUser(url) {", @@ -534,8 +537,9 @@ func TestQualityDefensiveBoundariesDetectMultiLanguageSignals(t *testing.T) { " return items[0];", "}", "", - "int allocate(int count) {", - " return count * 4096;", + "std::vector allocate(int count) {", + " int totalBytes = count * 4096;", + " return std::vector(totalBytes);", "}", "", "std::string fetchUser(std::string url) {", diff --git a/tests/checks/quality_precision_followup_retune_test.go b/tests/checks/quality_precision_followup_retune_test.go index 7ae251e..83ca45c 100644 --- a/tests/checks/quality_precision_followup_retune_test.go +++ b/tests/checks/quality_precision_followup_retune_test.go @@ -42,6 +42,21 @@ func TestDefensiveIntegerOverflowFollowupRetunes(t *testing.T) { " return new Uint8Array(totalBytes);", "}", }, "\n")) + writeFile(t, filepath.Join(dir, "packages/integrations/src/embeddings/vector.ts"), strings.Join([]string{ + "export function cosine(a: Float32Array, b: Float32Array) {", + " if (a.length !== b.length || a.length === 0) return 0;", + " let dot = 0;", + " for (let i = 0; i < a.length; i++) dot += a[i] * b[i];", + " return dot;", + "}", + }, "\n")) + writeFile(t, filepath.Join(dir, "packages/integrations/src/crypto/envelope.ts"), strings.Join([]string{ + "export function validateHex(ivHex: string, tagHex: string) {", + " return ivHex.length === IV_LEN * 2 && tagHex.length === TAG_LEN * 2;", + "}", + "declare const IV_LEN: number;", + "declare const TAG_LEN: number;", + }, "\n")) report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) @@ -49,10 +64,50 @@ func TestDefensiveIntegerOverflowFollowupRetunes(t *testing.T) { assertCodeQualityRuleAbsentForPath(t, report, "defensive.integer-overflow", "external-id-retry.ts") assertCodeQualityRuleAbsentForPath(t, report, "defensive.integer-overflow", "seed-contracts.ts") assertCodeQualityRuleAbsentForPath(t, report, "defensive.integer-overflow", "date-buckets.ts") + assertCodeQualityRuleAbsentForPath(t, report, "defensive.integer-overflow", "vector.ts") + assertCodeQualityRuleAbsentForPath(t, report, "defensive.integer-overflow", "envelope.ts") assertCodeQualityRulePresentForPathWithMessage(t, report, "defensive.sequence-collision-risk", "external-id-retry.ts", "architectural debt", "database sequence", "transactional allocator") assertCodeQualityRuleAbsentForPath(t, report, "defensive.sequence-collision-risk", "seed-contracts.ts") } +func TestFunctionMultipleResponsibilitiesUsesHigherThresholdForUIBoundaries(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "apps/web/app/claims/claim-detail.tsx"), strings.Join([]string{ + "export function ClaimDetail({ claim, repo, logger, canEdit }: Props) {", + " if (!claim.id) logger.warn('missing id');", + " const rows = claim.items.map((item) => ({ label: item.label }));", + " const owner = repo.find(claim.ownerId);", + " function onSave() { repo.update({ id: claim.id, rows }); }", + " return ;", + "}", + "interface Props { claim: { id: string; ownerId: string; items: Array<{ label: string }> }; repo: Repo; logger: Logger; canEdit: boolean }", + "interface Repo { find(id: string): { name: string }; update(input: unknown): void }", + "interface Logger { warn(message: string): void }", + }, "\n")) + writeFile(t, filepath.Join(dir, "packages/api/src/routers/claim-detail.ts"), strings.Join([]string{ + "export async function buildClaimDetail(input: Input, repo: Repo, logger: Logger, sender: Sender) {", + " validateInput(input);", + " if (!input.id) logger.warn('missing id');", + " const claim = await repo.find(input.id);", + " const rows = claim.items.map((item) => ({ label: item.label }));", + " await repo.update({ id: input.id, rows });", + " await sender.send(rows);", + " return { claim, rows };", + "}", + "interface Input { id: string }", + "interface Claim { items: Array<{ label: string }> }", + "interface Repo { find(id: string): Promise; update(input: unknown): Promise }", + "interface Logger { warn(message: string): void }", + "interface Sender { send(input: unknown): Promise }", + "declare function validateInput(input: Input): void;", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertCodeQualityRuleAbsentForPath(t, report, "function.multiple-responsibilities", "claim-detail.tsx") + assertCodeQualityRulePresentForPathWithMessage(t, report, "function.multiple-responsibilities", "packages/api/src/routers/claim-detail.ts", "combines") +} + func TestAllocateExternalIDIsCommandStyleReturningValue(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "packages/api/src/routers/entity-shared.ts"), strings.Join([]string{ @@ -193,6 +248,94 @@ func TestDefensiveBroadeningSkipsUIBoundsAndInternalORMReads(t *testing.T) { assertCodeQualityRuleAbsentForPath(t, report, "defensive.missing-resource-limit", "search-tools.ts:4") } +func TestDefensiveNullAssumptionCreditsTypeScriptNarrowing(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages/api/src/lib/null-narrowing.ts"), strings.Join([]string{ + "export function fromNullableString(summary: string | null | undefined) {", + " const trimmed = typeof summary === 'string' ? summary.trim() : '';", + " return trimmed;", + "}", + "export function fromNullableDate(value: Date | null) {", + " if (!value) return null;", + " return value.toISOString();", + "}", + "export function fromNullableElements(recipientIds: (string | null | undefined)[]) {", + " return recipientIds.filter((id): id is string => !!id).map((id) => id.toUpperCase());", + "}", + "export function fromNullableField(row: { status: string | null; title: string }) {", + " return row.status === 'ACTIVE' ? row.title : null;", + "}", + "export function unsafeNullableUser(user: { email: string } | null) {", + " return user.email;", + "}", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertCodeQualityRuleAbsentForPath(t, report, "defensive.null-assumption", "null-narrowing.ts:2") + assertCodeQualityRuleAbsentForPath(t, report, "defensive.null-assumption", "null-narrowing.ts:7") + assertCodeQualityRuleAbsentForPath(t, report, "defensive.null-assumption", "null-narrowing.ts:10") + assertCodeQualityRuleAbsentForPath(t, report, "defensive.null-assumption", "null-narrowing.ts:13") + assertCodeQualityRulePresentForPathWithMessage(t, report, "defensive.null-assumption", "null-narrowing.ts:16", "nullable boundary value") +} + +func TestDefensiveBoundaryInputSkipsTypedInternalDTOs(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages/api/src/routers/internal-dtos.ts"), strings.Join([]string{ + "export function buildMyDayResult(inputs: MyDayInputs) {", + " return { count: inputs.myRequests.length };", + "}", + "export function createPrdAnalysis(request: PrdAnalysisRequest) {", + " return { title: request.title, body: request.prdContent };", + "}", + "export function consumeBoundaryInput(input: unknown) {", + " return JSON.stringify(input);", + "}", + "interface MyDayInputs { myRequests: unknown[] }", + "interface PrdAnalysisRequest { title: string; prdContent: string }", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertCodeQualityRuleAbsentForPath(t, report, "defensive.unvalidated-boundary-input", "internal-dtos.ts:1") + assertCodeQualityRuleAbsentForPath(t, report, "defensive.unvalidated-boundary-input", "internal-dtos.ts:4") + assertCodeQualityRulePresentForPathWithMessage(t, report, "defensive.unvalidated-boundary-input", "internal-dtos.ts:7", "boundary input") +} + +func TestExceptionControlFlowSkipsValidationThrows(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages/api/src/routers/validation-errors.ts"), strings.Join([]string{ + "export function validateSourceFile(file: File | null) {", + " if (!file) throw new TRPCError({ code: 'BAD_REQUEST', message: 'Source file not found' });", + " return file.id;", + "}", + "export function requireProfile(profile: Profile | null) {", + " if (!profile) throw new AppError('Attorney not found');", + " return profile.id;", + "}", + "export function parseTokenResponse(payload: unknown) {", + " if (!payload || typeof payload !== 'object') throw new Error('token exchange returned invalid JSON');", + " return payload;", + "}", + "export function findUser(userId: string | null) {", + " if (!userId) throw new Exception('not found');", + " return userId;", + "}", + "interface File { id: string }", + "interface Profile { id: string }", + "declare class TRPCError extends Error { constructor(input: unknown); }", + "declare class AppError extends Error { constructor(message: string); }", + "declare class Exception extends Error { constructor(message: string); }", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertCodeQualityRuleAbsentForPath(t, report, "error.exception-used-for-control-flow", "validation-errors.ts:2") + assertCodeQualityRuleAbsentForPath(t, report, "error.exception-used-for-control-flow", "validation-errors.ts:6") + assertCodeQualityRuleAbsentForPath(t, report, "error.exception-used-for-control-flow", "validation-errors.ts:10") + assertCodeQualityRulePresentForPathWithMessage(t, report, "error.exception-used-for-control-flow", "validation-errors.ts:14", "ordinary branch control") +} + func TestFunctionReturnContractAllowsNullableParserLookupAndExistsHelpers(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "packages/integrations/src/slack/slack-client.ts"), strings.Join([]string{ @@ -216,6 +359,30 @@ func TestFunctionReturnContractAllowsNullableParserLookupAndExistsHelpers(t *tes assertFindingRuleAbsent(t, report, "Code Quality", "function.inconsistent-return-contract") } +func TestFunctionReturnContractAllowsExplicitNullableServiceContracts(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages/integrations/src/anthropic/summarize-content.ts"), strings.Join([]string{ + "export async function buildFileContentBlocks(prompt: string, storageKey?: string | null): Promise {", + " if (!storageKey) return null;", + " return [{ type: 'text', text: prompt }];", + "}", + "export async function fetchMatterContext(userId: string): Promise {", + " if (!process.env.DAINTREE_MCP_URL) return null;", + " return { userId };", + "}", + "export async function autoRoute(pillar: string): Promise {", + " if (!pillar) return null;", + " return 'user_1';", + "}", + "interface Block { type: string; text: string }", + "interface MatterContext { userId: string }", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRuleAbsent(t, report, "Code Quality", "function.inconsistent-return-contract") +} + func TestErrorPartialFailureHiddenCreditsSurfacedDiagnostics(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "packages/integrations/src/gmail/digest-fetch.ts"), strings.Join([]string{ diff --git a/tests/checks/quality_ui_false_positive_hardening_test.go b/tests/checks/quality_ui_false_positive_hardening_test.go index 781ff28..65c1a6c 100644 --- a/tests/checks/quality_ui_false_positive_hardening_test.go +++ b/tests/checks/quality_ui_false_positive_hardening_test.go @@ -51,6 +51,37 @@ func TestQualityAmbiguousNameAllowsReactNativeRenderParams(t *testing.T) { assertFindingRuleAbsent(t, report, "Code Quality", "quality.ambiguous-name") } +func TestFunctionMutationRulesAllowUICommandHelperNamesOnlyInUI(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "apps/web/components/confirm-button.tsx"), strings.Join([]string{ + "export function ConfirmButton() {", + " let armed = false;", + " function disarm() {", + " armed = false;", + " return armed;", + " }", + " function confirmDelete() {", + " armed = true;", + " return armed;", + " }", + " return ;", + "}", + }, "\n")) + writeFile(t, filepath.Join(dir, "packages/api/src/lib/select-user.ts"), strings.Join([]string{ + "export function selectUser(user: User): User {", + " user.selected = true;", + " return user;", + "}", + "interface User { selected: boolean }", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertCodeQualityRuleAbsentForPath(t, report, "function.hidden-mutation", "confirm-button.tsx") + assertCodeQualityRuleAbsentForPath(t, report, "function.command-query-mix", "confirm-button.tsx") + assertCodeQualityRulePresentForPathWithMessage(t, report, "function.hidden-mutation", "select-user.ts", "mutates state") +} + func TestFunctionCommandQueryMixAllowsReactAndNextBoundaries(t *testing.T) { cases := []struct { name string @@ -642,7 +673,8 @@ func TestSmellAndOverflowRulesStillFlagNonUIProductionCode(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "packages/domain/src/account-risk.ts"), strings.Join([]string{ "export function scoreCustomer(customer: Customer, count: number) {", - " const score = count * 4096;", + " const totalBytes = count * 4096;", + " const score = new Uint8Array(totalBytes).byteLength;", " const code = customer.profile.address.country.region.zone.owner.name.toUpperCase();", " return customer.profile.name + customer.profile.email + customer.account.region + customer.account.plan + customer.account.status + code + score;", "}",