diff --git a/internal/codeguard/checks/design/local_abstraction.go b/internal/codeguard/checks/design/local_abstraction.go index 9eeede6..cfcd74c 100644 --- a/internal/codeguard/checks/design/local_abstraction.go +++ b/internal/codeguard/checks/design/local_abstraction.go @@ -111,6 +111,7 @@ func leakFindings(env support.Context, file string, source string) []core.Findin domainPath := isDomainPath(file) apiPath := isAPIPath(file) handlerPath := isHandlerPath(file) + testOrStubPath := isDesignTestOrStubPath(file) persistenceBoundaryPath := domainPath || apiPath || handlerPath || isContractBoundaryPath(file) for idx, line := range lines { trimmed := strings.TrimSpace(line) @@ -126,7 +127,7 @@ func leakFindings(env support.Context, file string, source string) []core.Findin findings = append(findings, designFinding(env, ruleInfrastructureLeak, file, lineNo, "infrastructure/framework type leaks into a domain or public boundary", core.ConfidenceHigh)) } - if persistenceBoundaryPath && !isPackageAPIImplementationPath(file) && (apiPath || handlerPath || isPublicDeclaration(codeLine)) && + if persistenceBoundaryPath && !isPackageAPIImplementationPath(file) && !testOrStubPath && (apiPath || handlerPath || isPublicDeclaration(codeLine)) && persistenceLeakPattern.MatchString(codeLine) && !allowedGeneratedPersistenceEnumLine(codeLine) && !allowedTypeScriptRecordUtilityLine(codeLine) && !allowedUIPropsDerivedTypeLine(file, codeLine) && !allowedFrameworkDTOBoundaryLine(file, codeLine) { @@ -491,6 +492,21 @@ func isPackageAPIImplementationPath(file string) bool { return strings.Contains(normalized, "/packages/api/src/") || strings.HasPrefix(normalized, "packages/api/src/") } +func isDesignTestOrStubPath(file string) bool { + normalized := strings.ToLower(filepathSlash(file)) + if strings.Contains(normalized, "/test/") || strings.Contains(normalized, "/tests/") || + strings.Contains(normalized, "/testdata/") || strings.Contains(normalized, "/fixtures/") || + strings.Contains(normalized, "/__fixtures__/") || strings.Contains(normalized, "/mocks/") || + strings.Contains(normalized, "/stubs/") { + return true + } + return strings.HasSuffix(normalized, "_test.go") || strings.HasSuffix(normalized, "_test.py") || + strings.HasSuffix(normalized, ".test.ts") || strings.HasSuffix(normalized, ".spec.ts") || + strings.HasSuffix(normalized, ".test.tsx") || strings.HasSuffix(normalized, ".spec.tsx") || + strings.HasSuffix(normalized, ".test.js") || strings.HasSuffix(normalized, ".spec.js") || + strings.HasSuffix(normalized, ".test.jsx") || strings.HasSuffix(normalized, ".spec.jsx") +} + func isContractBoundaryPath(file string) bool { normalized := strings.ToLower(filepathSlash(file)) if isFrontendUIPath(file) { diff --git a/internal/codeguard/checks/quality/quality_defensive.go b/internal/codeguard/checks/quality/quality_defensive.go index 81e4a61..79946ca 100644 --- a/internal/codeguard/checks/quality/quality_defensive.go +++ b/internal/codeguard/checks/quality/quality_defensive.go @@ -130,7 +130,10 @@ func unvalidatedBoundaryInputLine(fn precisionFunction, loweredBody string) (int if !boundaryFunctionName(fn.Name) && !hasBoundaryParam(fn.Params) { return 0, false } - if containsAny(loweredBody, []string{"validate", "schema", "sanitize", "bind", "decodevalid", "zod.", "yup.", "pydantic", "jsonschema"}) { + if validatedBoundaryInputPattern(fn, loweredBody) { + return 0, false + } + if formDataHasContentLengthPreflight(loweredBody) { return 0, false } if containsAny(loweredBody, []string{"request", "req.", "event", "payload", "body", "json", "params", "query"}) { @@ -139,6 +142,22 @@ func unvalidatedBoundaryInputLine(fn precisionFunction, loweredBody string) (int return 0, false } +func formDataHasContentLengthPreflight(loweredBody string) bool { + return strings.Contains(loweredBody, "formdata") && + containsAny(loweredBody, []string{"content-length", "contentlength"}) && + containsAny(loweredBody, []string{"> max", "> limit", "max_upload", "upload too large"}) +} + +func validatedBoundaryInputPattern(fn precisionFunction, loweredBody string) bool { + if containsAny(loweredBody, []string{"validate", "schema", "sanitize", "bind", "decodevalid", "safeparse", "zod.", "yup.", "pydantic", "jsonschema"}) { + return true + } + if regexp.MustCompile(`(?i)\b(parse|assert|guard|ensure|decode)[A-Z_][A-Za-z0-9_]*(?:Input|Payload|Body|Params|Query|Record|Request|Event|Config)?\s*\(`).MatchString(functionRawBody(fn)) { + return true + } + return false +} + func hasBoundaryParam(params []support.ParsedParam) bool { for _, param := range params { name := strings.ToLower(param.Name) @@ -186,6 +205,9 @@ func integerOverflowLine(file string, fn precisionFunction, loweredBody string) if isUIRenderArithmeticContext(file, fn, loweredBody) { return 0, false } + if guardedSequenceCollisionRetry(loweredBody) { + return 0, false + } if containsAny(loweredBody, []string{"maxint", "math.max", "checked", "saturating", "overflow", "limits<", "safeint"}) { return 0, false } @@ -198,6 +220,16 @@ func integerOverflowLine(file string, fn precisionFunction, loweredBody string) return 0, false } +func guardedSequenceCollisionRetry(loweredBody string) bool { + if !containsAny(loweredBody, []string{"p2002", "unique", "collision", "prisma"}) { + return false + } + if !containsAny(loweredBody, []string{"retry", "attempt", "for "}) { + return false + } + return containsAny(loweredBody, []string{"count + 1", "count+1", "externalid", "external_id", "nextid", "next_id"}) +} + func isUIRenderArithmeticContext(file string, fn precisionFunction, loweredBody string) bool { if isUIHelperOrMappingContext(file, fn) { return true @@ -278,6 +310,9 @@ func uncheckedExternalResponseLine(fn precisionFunction, loweredBody string) (in if !externalCallPattern.MatchString(functionRawBody(fn)) { return 0, false } + if urlProtocolAllowlistPattern(loweredBody) { + return 0, false + } if containsAny(loweredBody, []string{"status", ".ok", "err != nil", "if err", "error", "catch", "raise_for_status", "response_code"}) { return 0, false } @@ -287,11 +322,18 @@ func uncheckedExternalResponseLine(fn precisionFunction, loweredBody string) (in return 0, false } +func urlProtocolAllowlistPattern(loweredBody string) bool { + if !containsAny(loweredBody, []string{"new url(", ".protocol"}) { + return false + } + return containsAny(loweredBody, []string{"https:", "http:", "allowedprotocol", "allowed_protocol", "protocols.includes", "includes(url.protocol)", "protocol !==", "protocol !="}) +} + func missingSchemaValidationLine(fn precisionFunction, loweredBody string) (int, bool) { if !jsonDecodePattern.MatchString(functionRawBody(fn)) { return 0, false } - if containsAny(loweredBody, []string{"validate", "schema", "jsonschema", "zod.", "yup.", "pydantic", "isvalid", "required"}) { + if validatedBoundaryInputPattern(fn, loweredBody) || containsAny(loweredBody, []string{"jsonschema", "isvalid", "required"}) { return 0, false } return firstPatternLine(fn, jsonDecodePattern), true @@ -301,12 +343,22 @@ func missingResourceLimitLine(fn precisionFunction, loweredBody string) (int, bo if !resourceReadPattern.MatchString(functionRawBody(fn)) { return 0, false } - if containsAny(loweredBody, []string{"limitreader", "maxbytes", "max_bytes", "content-length", "limit(", "take(", "buffer_size", "quota"}) { + if containsAny(loweredBody, []string{"limitreader", "maxbytes", "max_bytes", "content-length", "contentlength", "limit(", "take(", "buffer_size", "quota"}) { + return 0, false + } + if boundedReadByteLengthCheck(loweredBody) { return 0, false } return firstPatternLine(fn, resourceReadPattern), true } +func boundedReadByteLengthCheck(loweredBody string) bool { + if !containsAny(loweredBody, []string{"arraybuffer", ".text", "readall", ".read"}) { + return false + } + return containsAny(loweredBody, []string{"bytelength", "byte_length", ".length > max", ".length > limit", "buffer.length", "bytes.length"}) +} + func invalidStateTransitionLine(fn precisionFunction, loweredBody string) (int, bool) { if !stateAssignmentPattern.MatchString(functionRawBody(fn)) { return 0, false diff --git a/internal/codeguard/checks/quality/quality_precision.go b/internal/codeguard/checks/quality/quality_precision.go index f63d815..c187db6 100644 --- a/internal/codeguard/checks/quality/quality_precision.go +++ b/internal/codeguard/checks/quality/quality_precision.go @@ -45,7 +45,7 @@ var ( "misc": {}, "stuff": {}, "value": {}, "values": {}, } queryFunctionPrefixPattern = regexp.MustCompile(`^(get|find|list|load|read|lookup|fetch|is|has|can|should|compute|calculate|build|format|parse)`) - mutatingCallPattern = regexp.MustCompile(`(?i)(^|[.>:\-_])(add|append|assign|create|delete|emit|insert|mutate|persist|publish|remove|save|send|set|store|update|upsert|write)([A-Z_:\-.]|$)`) + mutatingCallPattern = regexp.MustCompile(`(?i)(^|[.>:\-_])(add|append|assign|clear|create|delete|emit|insert|mutate|persist|pop|publish|push|push_back|remove|reverse|save|send|set|sort|splice|store|update|upsert|write)([A-Z_:\-.]|$)`) lowLevelOperationPattern = regexp.MustCompile(`(?i)(\bsql\.|\.query\(|\.exec\(|\bhttp\.|\bfetch\(|\baxios\.|\brequests\.|\bjson\.|\bJSON\.|\bos\.Getenv\b|\bprocess\.env\b|\bfs\.|#include\b)`) primitiveTypePattern = regexp.MustCompile(`(?i)\b(string|str|int|int64|float|float64|double|decimal|number|boolean|bool|char|long|short)\b`) domainPrimitiveNamePattern = regexp.MustCompile(`(?i)(id|status|state|type|kind|currency|amount|price|email|phone|country|role|permission|tenant|account|customer|order)`) @@ -372,7 +372,7 @@ func precisionFunctionFindings(env support.Context, file string, fn precisionFun findings = append(findings, precisionWarnFinding(env, namingGenericIdentifierRuleID, file, fn.StartLine, fmt.Sprintf("function name %q is too generic to communicate intent", fn.Name), core.ConfidenceHigh)) } - if isAmbiguousIdentifier(fn.Name) && !isUIConventionalAmbiguousName(file, fn, fn.Name, "", fn.StartLine) { + if isAmbiguousIdentifier(fn.Name) && !isUIConventionalAmbiguousName(file, fn, fn.Name, "", fn.StartLine) && !isLocallyClearAmbiguousName(fn, fn.Name) { findings = append(findings, precisionWarnFinding(env, qualityAmbiguousNameRuleID, file, fn.StartLine, fmt.Sprintf("function name %q is ambiguous without domain context", fn.Name), core.ConfidenceHigh)) } @@ -381,7 +381,7 @@ func precisionFunctionFindings(env support.Context, file string, fn precisionFun findings = append(findings, precisionWarnFinding(env, namingGenericIdentifierRuleID, file, fn.StartLine, fmt.Sprintf("parameter %q is too generic to communicate intent", param.Name), core.ConfidenceHigh)) } - if isAmbiguousIdentifier(param.Name) && !isUIConventionalAmbiguousName(file, fn, param.Name, param.Type, fn.StartLine) { + if isAmbiguousIdentifier(param.Name) && !isUIConventionalAmbiguousName(file, fn, param.Name, param.Type, fn.StartLine) && !isLocallyClearAmbiguousName(fn, param.Name) { findings = append(findings, precisionWarnFinding(env, qualityAmbiguousNameRuleID, file, fn.StartLine, fmt.Sprintf("parameter %q is ambiguous without domain context", param.Name), core.ConfidenceHigh)) } @@ -395,12 +395,12 @@ func precisionFunctionFindings(env support.Context, file string, fn precisionFun findings = append(findings, precisionWarnFinding(env, namingGenericIdentifierRuleID, file, assignment.Line, fmt.Sprintf("identifier %q is too generic to explain its role", assignment.Name), core.ConfidenceHigh)) } - if isAmbiguousIdentifier(assignment.Name) && !isUIConventionalAmbiguousName(file, fn, assignment.Name, "", assignment.Line) { + if isAmbiguousIdentifier(assignment.Name) && !isUIConventionalAmbiguousName(file, fn, assignment.Name, "", assignment.Line) && !isLocallyClearAmbiguousName(fn, assignment.Name) { findings = append(findings, precisionWarnFinding(env, qualityAmbiguousNameRuleID, file, assignment.Line, fmt.Sprintf("identifier %q is ambiguous without domain context", assignment.Name), core.ConfidenceHigh)) } } - if mixedAbstractionLevel(fn) { + if mixedAbstractionLevel(fn) && !isAdapterOrOrchestrationFunction(file, fn) { findings = append(findings, precisionWarnFinding(env, functionMixedAbstractionLevelRuleID, file, fn.StartLine, fmt.Sprintf("function %s mixes orchestration calls with low-level infrastructure operations", fn.Name), core.ConfidenceMedium)) findings = append(findings, precisionWarnFinding(env, qualityMixedAbstractionLevelsRuleID, file, fn.StartLine, @@ -443,6 +443,15 @@ func isAmbiguousIdentifier(name string) bool { return ok } +func isLocallyClearAmbiguousName(fn precisionFunction, name string) bool { + normalized := strings.ToLower(strings.Trim(name, "_$")) + if normalized != "value" && normalized != "values" { + return false + } + loweredName := strings.ToLower(fn.Name) + return containsAny(loweredName, []string{"parse", "normalize", "format", "render", "map", "transform", "compare", "equal", "record", "field", "option"}) +} + func isBooleanParameter(param support.ParsedParam) bool { return strings.EqualFold(strings.TrimSpace(param.Type), "bool") || strings.EqualFold(strings.TrimSpace(param.Type), "boolean") || @@ -474,9 +483,12 @@ func hiddenSideEffect(file string, fn precisionFunction) bool { if !queryFunctionPrefixPattern.MatchString(strings.ToLower(fn.Name)) { return false } + if isAccumulatorBuilderFunctionName(fn.Name) && !hasLikelyExternalMutationCall(fn) { + return false + } localTargets := localMutationTargets(fn) for _, call := range directCalls(fn) { - if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call.Callee, localTargets) { + if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call.Callee, localTargets) && !isBuilderAccumulatorMutationCall(fn, call) { return true } } @@ -516,13 +528,16 @@ func commandQueryMix(file string, fn precisionFunction) bool { if !fn.Returns { return false } + if isAccumulatorBuilderFunctionName(fn.Name) && !hasLikelyExternalMutationCall(fn) { + return false + } name := strings.ToLower(fn.Name) if !queryFunctionPrefixPattern.MatchString(name) && !strings.Contains(fn.Body, "return ") { return false } localTargets := localMutationTargets(fn) for _, call := range directCalls(fn) { - if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call.Callee, localTargets) { + if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call.Callee, localTargets) && !isBuilderAccumulatorMutationCall(fn, call) { return true } } diff --git a/internal/codeguard/checks/quality/quality_precision_duplication.go b/internal/codeguard/checks/quality/quality_precision_duplication.go index d701901..bfab9bb 100644 --- a/internal/codeguard/checks/quality/quality_precision_duplication.go +++ b/internal/codeguard/checks/quality/quality_precision_duplication.go @@ -66,6 +66,14 @@ func duplicatedKnowledgeLineIsDisplayOnly(line string) bool { if strings.Contains(lowered, "classname") || strings.Contains(lowered, "clasname") || strings.Contains(lowered, "class:") { return true } + if strings.Contains(lowered, "class") && strings.Contains(line, "-") { + return true + } + for _, marker := range []string{"css", "style", "styles", "variant", "variants", "tailwind", "stylesheet"} { + if strings.Contains(lowered, marker) { + return true + } + } if strings.Contains(line, "<") && strings.Contains(line, ">") { return true } @@ -91,6 +99,12 @@ func domainKnowledgeLiteralInLine(value string, line string) bool { if numeric, ok := duplicatedKnowledgeNumber(trimmed); ok { return duplicatedKnowledgeNumericLiteral(numeric, line) } + if duplicatedKnowledgeSentinelLiteral(trimmed) { + return false + } + if duplicatedKnowledgeTableOrEnumLiteral(trimmed, line) { + return false + } if duplicatedKnowledgeEnumStatusLiteral(trimmed, line) { return false } @@ -116,9 +130,6 @@ func duplicatedKnowledgeNumericLiteral(number int, line string) bool { } func duplicatedKnowledgeEnumStatusLiteral(value string, line string) bool { - if line == "" { - return false - } trimmed := strings.TrimSpace(value) if trimmed == "" { return false @@ -127,6 +138,12 @@ func duplicatedKnowledgeEnumStatusLiteral(value string, line string) bool { if !enumLike { return false } + if looksLikeAllCapsEnumLiteral(trimmed) { + return true + } + if line == "" { + return false + } loweredLine := strings.ToLower(line) for _, marker := range []string{"enum", "status", "type:", "kind:", "value:", "option", "label", "as const", "satisfies"} { if strings.Contains(loweredLine, marker) { @@ -136,6 +153,48 @@ func duplicatedKnowledgeEnumStatusLiteral(value string, line string) bool { return false } +func duplicatedKnowledgeSentinelLiteral(value string) bool { + trimmed := strings.TrimSpace(value) + return strings.HasPrefix(trimmed, "__") && strings.HasSuffix(trimmed, "__") && len(trimmed) <= 40 +} + +func duplicatedKnowledgeTableOrEnumLiteral(value string, line string) bool { + if !strings.Contains(value, "_") { + return false + } + if len(value) > 48 { + return false + } + parts := strings.Split(value, "_") + if len(parts) > 4 { + return false + } + for _, part := range parts { + if part == "" { + return false + } + } + loweredLine := strings.ToLower(line) + return containsAny(loweredLine, []string{"table", "tablename", "table_name", "enum", "status", "type", "kind", "key:", "value:", "option"}) +} + +func looksLikeAllCapsEnumLiteral(value string) bool { + hasLetter := false + for _, r := range value { + switch { + case r >= 'A' && r <= 'Z': + hasLetter = true + case r >= '0' && r <= '9': + continue + case r == '_' || r == '-' || r == ':': + continue + default: + return false + } + } + return hasLetter && strings.ToUpper(value) == value +} + func duplicatedKnowledgeNumber(value string) (int, bool) { number, err := strconv.Atoi(value) if err != nil { diff --git a/internal/codeguard/checks/quality/quality_precision_mutation_targets.go b/internal/codeguard/checks/quality/quality_precision_mutation_targets.go index 31f367e..8248083 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*\(|^(?:bytes|strings)\.buffer\b|^strings\.builder\b`) +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*\(|^(?:bytes|strings)\.buffer\b|^strings\.builder\b`) func localMutationTargets(fn precisionFunction) map[string]struct{} { params := paramNames(fn) @@ -25,6 +25,22 @@ func localMutationTargets(fn precisionFunction) map[string]struct{} { } if assignmentLooksLocalAccumulator(fn, assignment) { targets[name] = struct{}{} + continue + } + if fn.Returns && isAccumulatorLikeLocalName(name) && assignmentLooksLocalBuilder(fn, assignment) { + targets[name] = struct{}{} + } + } + if fn.Returns && isAccumulatorBuilderFunctionName(fn.Name) { + for _, call := range directCalls(fn) { + target := mutationCallTarget(call.Callee) + if target == "" || !isAccumulatorLikeLocalName(target) { + continue + } + if _, isParam := params[target]; isParam { + continue + } + targets[target] = struct{}{} } } return targets @@ -91,7 +107,52 @@ func assignmentLooksLocalAccumulator(fn precisionFunction, assignment support.Pa return regexp.MustCompile(`(?i)\b(?:const|let|var)\s+`+name+`\b.*=\s*(?:new\s+)?(?:array|formdata|map|object|set|urlsearchparams|weakmap|weakset)\b`).MatchString(statement) || regexp.MustCompile(`(?i)\bvar\s+`+name+`\s+(?:bytes\.buffer|strings\.builder)\b`).MatchString(statement) || regexp.MustCompile(`(?i)\bstd::(?:vector|map|set|unordered_map|unordered_set|stringstream)\b[^;\n]*\b`+name+`\b`).MatchString(statement) || - regexp.MustCompile(`\b`+name+`\s*:=\s*(?:\[\]|\{\}|make\s*\(|(?:bytes|strings)\.Buffer\b|strings\.Builder\b)`).MatchString(statement) + regexp.MustCompile(`\b`+name+`\s*:=\s*(?:\[\]|\{\}|make\s*\(|(?:bytes|strings)\.Buffer\b|strings\.Builder\b)`).MatchString(statement) || + regexp.MustCompile(`(?i)\b(?:const|let|var)\s+`+name+`\b.*=\s*(?:\[|\{|array\.from\b|[^;\n]+\.map\s*\(|[^;\n]+\.filter\s*\(|new\s+urlsearchparams\b)`).MatchString(statement) +} + +func assignmentLooksLocalBuilder(fn precisionFunction, assignment support.ParsedAssignment) bool { + statement := strings.ToLower(assignmentStatement(fn, assignment.Line)) + expr := strings.ToLower(strings.TrimSpace(assignment.Expr)) + if statement == "" && expr == "" { + return false + } + return strings.Contains(statement, "new ") || + strings.Contains(statement, "create") || + strings.Contains(statement, "build") || + strings.Contains(statement, "make") || + strings.Contains(expr, "new ") || + strings.Contains(expr, "create") || + strings.Contains(expr, "build") || + strings.Contains(expr, "make") +} + +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", + "payload", "primarycells", "query", "result", "rows", "scopes", "sections", + "serializer", "text", "urlparams", "values", "csv", "export", "map", + } { + if strings.Contains(lowered, token) { + return true + } + } + return false +} + +func isAccumulatorBuilderFunctionName(name string) bool { + lowered := strings.ToLower(strings.Trim(name, "_$")) + for _, token := range []string{ + "bucket", "build", "collect", "derive", "format", "group", "map", "parse", + "primary", "render", "serialize", "transform", + } { + if strings.Contains(lowered, token) { + return true + } + } + return false } func assignmentStatement(fn precisionFunction, line int) string { @@ -129,7 +190,7 @@ func isLocalMutationCall(callee string, localTargets map[string]struct{}) bool { func isBareLocalMutationCall(callee string) bool { switch strings.TrimSpace(callee) { - case "append", "Set", "Array", "Object", "Map", "WeakMap", "WeakSet": + case "append", "Set", "Array", "Object", "Map", "WeakMap", "WeakSet", "push_back": return true default: return false @@ -154,6 +215,32 @@ func isLocalMutationTarget(name string, localTargets map[string]struct{}) bool { return ok } +func isBuilderAccumulatorMutationCall(fn precisionFunction, call support.ParsedCall) bool { + if !fn.Returns || !isAccumulatorBuilderFunctionName(fn.Name) { + return false + } + target := mutationCallTarget(call.Callee) + if target == "" { + return isBareLocalMutationCall(call.Callee) + } + params := paramNames(fn) + if _, isParam := params[target]; isParam { + return false + } + return isAccumulatorLikeLocalName(target) +} + +func isBuilderAccumulatorAssignment(fn precisionFunction, assignment support.ParsedAssignment) bool { + if !fn.Returns || !isAccumulatorBuilderFunctionName(fn.Name) { + return false + } + params := paramNames(fn) + if _, isParam := params[assignment.Name]; isParam { + return false + } + return isAccumulatorLikeLocalName(assignment.Name) +} + func isFrameworkCommandBoundary(file string, name string) bool { if !isScriptLikeSourcePath(file) { return false diff --git a/internal/codeguard/checks/quality/quality_precision_ui_conventions.go b/internal/codeguard/checks/quality/quality_precision_ui_conventions.go index 9551a3f..f8e4943 100644 --- a/internal/codeguard/checks/quality/quality_precision_ui_conventions.go +++ b/internal/codeguard/checks/quality/quality_precision_ui_conventions.go @@ -171,7 +171,7 @@ func isAllowedBooleanUIName(file string, fn precisionFunction, name string) bool func isConventionalNonPredicateName(name string) bool { switch strings.ToLower(strings.Trim(name, "_$")) { - case "opts", "options", "message", "classname", "class", "icon", "submit", "compare", "parser", "parse", "renderer", "render": + case "asrecord", "cached", "opts", "options", "message", "classname", "class", "icon", "submit", "compare", "parser", "parse", "builder", "build", "renderer", "render": return true default: return false @@ -191,7 +191,7 @@ func isResourceIdentifierName(name string) bool { func conventionalCardinalityName(name string) bool { base := strings.ToLower(strings.Trim(name, "_$")) switch base { - case "answers", "args", "columns", "contracts", "entries", "ids", "items", "k", "matters", "next", "out", "params", "props", "risks", "rows", "searchparams", "sections", "status", "v", "i", "j", "x", "y": + case "answers", "args", "claims", "columns", "contracts", "entries", "files", "ids", "items", "k", "keys", "matters", "messages", "next", "out", "params", "props", "records", "risks", "rows", "searchparams", "sections", "source", "status", "thresholds", "users", "versions", "v", "i", "j", "x", "y": return true default: return len(name) <= 2 || diff --git a/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go b/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go index a09f941..95666b6 100644 --- a/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go +++ b/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go @@ -173,7 +173,7 @@ func behaviorMismatch(file string, fn precisionFunction) bool { } func hiddenMutation(file string, fn precisionFunction) bool { - if explicitMutationName(fn.Name) || isFrameworkOrchestrationBoundary(file, fn) || isScriptEntrypoint(file, fn.Name) { + if explicitMutationName(fn.Name) || isDomainSideEffectBoundaryName(fn.Name) || isFrameworkOrchestrationBoundary(file, fn) || isScriptEntrypoint(file, fn.Name) { return false } if isReactComponentOrNamedHookBoundary(file, fn) { @@ -184,18 +184,65 @@ func hiddenMutation(file string, fn precisionFunction) bool { if isReactLocalStateBoundary(file, fn) && mutatesState && !mutatesParam && onlyReactHookLocalStateMutation(fn) { return false } + if isAccumulatorBuilderFunctionName(fn.Name) && !hasLikelyExternalMutationCall(fn) && !hasLikelyParameterAssignment(fn) { + return false + } return mutatesState || mutatesParam } +func hasLikelyExternalMutationCall(fn precisionFunction) bool { + localTargets := localMutationTargets(fn) + params := paramNames(fn) + for _, call := range directCalls(fn) { + if !mutatingCallPattern.MatchString(call.Callee) { + continue + } + if isLocalMutationCall(call.Callee, localTargets) || isBuilderAccumulatorMutationCall(fn, call) { + continue + } + target := mutationCallTarget(call.Callee) + if target == "" { + continue + } + if _, isParam := params[target]; isParam { + return true + } + if isAccumulatorLikeLocalName(target) { + continue + } + return true + } + return false +} + +func hasLikelyParameterAssignment(fn precisionFunction) bool { + params := paramNames(fn) + if len(params) == 0 { + return false + } + for _, statement := range directStatements(fn) { + line := firstNonEmptyString(statement.Raw, statement.Text) + if !lineHasAssignmentOperator(line) { + continue + } + for _, match := range paramMutationPattern.FindAllStringSubmatch(assignmentLeftHandSide(line), -1) { + if _, ok := params[match[1]]; ok { + return true + } + } + } + return false +} + func mutatingFunctionEvidence(fn precisionFunction) bool { localTargets := localMutationTargets(fn) for _, call := range directCalls(fn) { - if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call.Callee, localTargets) { + if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call.Callee, localTargets) && !isBuilderAccumulatorMutationCall(fn, call) { return true } } for _, assignment := range directAssignments(fn) { - if assignment.Augmented && !isLocalMutationTarget(assignment.Name, localTargets) { + if assignment.Augmented && !isLocalMutationTarget(assignment.Name, localTargets) && !isBuilderAccumulatorAssignment(fn, assignment) { return true } } @@ -217,7 +264,8 @@ func mutatesParameter(fn precisionFunction) bool { if !lineHasAssignmentOperator(line) { continue } - for _, match := range paramMutationPattern.FindAllStringSubmatch(line, -1) { + lhs := assignmentLeftHandSide(line) + for _, match := range paramMutationPattern.FindAllStringSubmatch(lhs, -1) { if _, ok := params[match[1]]; ok { return true } @@ -226,6 +274,27 @@ func mutatesParameter(fn precisionFunction) bool { return false } +func assignmentLeftHandSide(line string) string { + for idx := 0; idx < len(line); idx++ { + if line[idx] != '=' { + continue + } + prev := byte(0) + next := byte(0) + if idx > 0 { + prev = line[idx-1] + } + if idx+1 < len(line) { + next = line[idx+1] + } + if prev == '=' || prev == '!' || prev == '<' || prev == '>' || next == '=' || next == '>' { + continue + } + return line[:idx] + } + return line +} + func lineHasAssignmentOperator(line string) bool { for idx := 0; idx < len(line); idx++ { if line[idx] != '=' { @@ -257,6 +326,38 @@ func explicitMutationName(name string) bool { strings.Contains(lowered, "write") } +func isDomainSideEffectBoundaryName(name string) bool { + lowered := strings.ToLower(strings.TrimSpace(name)) + if lowered == "" { + return false + } + if strings.HasPrefix(lowered, "maybe") && containsAny(lowered, []string{"alert", "notify", "record", "track", "emit"}) { + return true + } + if strings.HasPrefix(lowered, "evaluate") && containsAny(lowered, []string{"abuse", "policy", "rule", "risk", "fraud", "quota", "limit"}) { + return true + } + if strings.HasPrefix(lowered, "load") && containsAny(lowered, []string{"config", "defaults", "settings", "policy"}) { + return true + } + return false +} + +func isAdapterOrOrchestrationFunction(file string, fn precisionFunction) bool { + loweredName := strings.ToLower(strings.Trim(fn.Name, "_$")) + if containsAny(loweredName, []string{"adapter", "bugreport", "bug_report", "slack", "webhook", "sync", "abuseconfig", "abuse_config"}) { + return true + } + if strings.HasPrefix(loweredName, "save") || strings.HasPrefix(loweredName, "insert") || strings.HasPrefix(loweredName, "post") || + strings.HasPrefix(loweredName, "send") || strings.HasPrefix(loweredName, "publish") || strings.HasPrefix(loweredName, "record") { + if containsAny(loweredName, []string{"config", "report", "slack", "webhook", "audit", "event", "job"}) { + return true + } + } + normalized := strings.ToLower(strings.ReplaceAll(file, "\\", "/")) + return containsAny(normalized, []string{"/adapters/", "/adapter/", "/connectors/", "/connector/", "/integrations/", "/webhooks/", "/slack/", "/jobs/"}) +} + func inconsistentReturnContract(fn precisionFunction) bool { returns := returnCategories(fn.Body) if returns.total < 2 { @@ -317,6 +418,9 @@ func partialResult(fn precisionFunction) bool { } func responsibilityCount(fn precisionFunction) (int, []string) { + if isAdapterOrchestrationName(fn.Name) { + return 0, nil + } seen := map[string]struct{}{} record := func(label string) { seen[label] = struct{}{} @@ -339,6 +443,11 @@ func responsibilityCount(fn precisionFunction) (int, []string) { return len(labels), labels } +func isAdapterOrchestrationName(name string) bool { + loweredName := strings.ToLower(strings.Trim(name, "_$")) + return containsAny(loweredName, []string{"abuseconfig", "abuse_config", "bugreport", "bug_report", "slack", "webhook", "adapter"}) +} + func classifyResponsibility(text string, record func(string)) { switch { case strings.Contains(text, "validat") || strings.Contains(text, "sanitize"): @@ -414,7 +523,7 @@ func isBooleanType(typ string) bool { func isPredicateName(name string) bool { lowered := strings.ToLower(strings.Trim(name, "_$")) - for _, prefix := range []string{"is", "has", "have", "can", "could", "should", "must", "allow", "allows", "enable", "enabled", "disable", "disabled", "needs", "requires", "supports", "valid", "visible", "ready"} { + for _, prefix := range []string{"is", "are", "has", "have", "can", "could", "should", "must", "allow", "allows", "enable", "enabled", "disable", "disabled", "needs", "requires", "supports", "valid", "visible", "ready"} { if strings.HasPrefix(lowered, prefix) { return true } diff --git a/internal/codeguard/checks/quality/quality_smells.go b/internal/codeguard/checks/quality/quality_smells.go index 3867333..0a945c4 100644 --- a/internal/codeguard/checks/quality/quality_smells.go +++ b/internal/codeguard/checks/quality/quality_smells.go @@ -379,6 +379,9 @@ func featureEnvyFindings(env support.Context, file string, functions []structura if isStructuralUIRenderingContext(file, fn) || isStructuralMapperOrBuilderContext(fn) { continue } + if isStructuralAdapterOrCrudContext(file, fn) { + continue + } if len(fn.Params) == 0 || fn.Body == "" { continue } @@ -483,6 +486,9 @@ func messageChainFindings(env support.Context, file string, source string, langu if isScriptLikeSourcePath(file) && isLikelyUIFile(file) { return nil } + if isStructuralTraversalUtilityPath(file) { + return nil + } masked := maskForStructuralLanguage(source, language) for idx, line := range strings.Split(masked, "\n") { trimmed := strings.TrimSpace(line) @@ -531,7 +537,9 @@ func looksLikeAllowedTraversalChain(line string) bool { for _, marker := range []string{ "response.", "result.", "payload.", "body.", "json.", "config.", "settings.", "process.env", "import.meta.env", "params.", "query.", "headers.", - "row.", "record.", "dto.", "args.", + "row.", "record.", "dto.", "args.", "urlsearchparams", "searchparams.", + "include:", "select:", "where:", "prisma.", "serialize", "serializer", "json.stringify", + "tojson", ".tojson", } { if strings.Contains(lowered, marker) { return true diff --git a/internal/codeguard/checks/quality/quality_smells_ui.go b/internal/codeguard/checks/quality/quality_smells_ui.go index a2a5c81..3098967 100644 --- a/internal/codeguard/checks/quality/quality_smells_ui.go +++ b/internal/codeguard/checks/quality/quality_smells_ui.go @@ -54,8 +54,10 @@ func isStructuralMapperOrBuilderContext(fn structuralFunction) bool { loweredName := strings.ToLower(strings.Trim(fn.Name, "_$")) mapperName := false for _, token := range []string{ + "adapter", "build", "bucket", "collect", "derive", "format", "group", "map", "normalize", "render", "rows", "serialize", "table", "to", "transform", "writeauditlog", + "cell", "csv", "dto", "export", "prompt", "serializer", } { if strings.Contains(loweredName, token) { mapperName = true @@ -70,13 +72,43 @@ func isStructuralMapperOrBuilderContext(fn structuralFunction) bool { return false } switch strings.ToLower(strings.Trim(dominantName, "_$")) { - case "args", "data", "dto", "input", "item", "message", "payload", "record", "response", "result", "row", "rows", "value": + case "args", "claim", "contract", "data", "dto", "event", "input", "item", "matter", "message", "payload", "record", "response", "result", "row", "rows", "source", "value": return true default: return mapperReturnsConstructedValue(fn.Body) } } +func isStructuralTraversalUtilityPath(file string) bool { + if !isScriptLikeSourcePath(file) { + return false + } + normalized := strings.ToLower(strings.ReplaceAll(file, "\\", "/")) + for _, token := range []string{ + "adapter", "csv", "dto", "export", "mapper", "mappers", "prompt", "serializer", "serializers", + } { + if strings.Contains(normalized, token) { + return true + } + } + return false +} + +func isStructuralAdapterOrCrudContext(file string, fn structuralFunction) bool { + loweredName := strings.ToLower(strings.Trim(fn.Name, "_$")) + if containsAny(loweredName, []string{"abuseconfig", "abuse_config", "abuserules", "abuse_rules", "summarise", "summarize"}) { + return true + } + if strings.HasPrefix(loweredName, "save") || strings.HasPrefix(loweredName, "evaluate") || strings.HasPrefix(loweredName, "summaris") || + strings.HasPrefix(loweredName, "map") || strings.HasPrefix(loweredName, "to") { + if mapperReturnsConstructedValue(fn.Body) || strings.Contains(strings.ToLower(fn.Body), "return ") { + return true + } + } + normalized := strings.ToLower(strings.ReplaceAll(file, "\\", "/")) + return containsAny(normalized, []string{"/adapters/", "/adapter/", "/connectors/", "/connector/", "/serializers/", "/mappers/"}) +} + func mapperReturnsConstructedValue(body string) bool { lowered := strings.ToLower(body) return strings.Contains(lowered, "return {") || diff --git a/internal/codeguard/checks/support/typescript_semantic_runner_core.js b/internal/codeguard/checks/support/typescript_semantic_runner_core.js index ba6f86e..ebe84f9 100644 --- a/internal/codeguard/checks/support/typescript_semantic_runner_core.js +++ b/internal/codeguard/checks/support/typescript_semantic_runner_core.js @@ -242,7 +242,7 @@ function analyzeDesign(sourceFile, relPath) { if (ts.isInterfaceDeclaration(node)) { const members = node.members.length; - if (members > input.max_interface_members) { + if (members > input.max_interface_members && !isAllowedLargeDataShape(node.name.text, relPath)) { pushFinding( "design", sourceFile, @@ -258,7 +258,7 @@ function analyzeDesign(sourceFile, relPath) { if (ts.isTypeAliasDeclaration(node) && ts.isTypeLiteralNode(node.type)) { const members = node.type.members.length; - if (members > input.max_interface_members) { + if (members > input.max_interface_members && !isAllowedLargeDataShape(node.name.text, relPath)) { pushFinding( "design", sourceFile, @@ -274,6 +274,21 @@ function analyzeDesign(sourceFile, relPath) { }); } +function isAllowedLargeDataShape(name, relPath) { + const normalizedName = String(name || "").toLowerCase(); + const normalizedPath = String(relPath || "").replace(/\\/g, "/").toLowerCase(); + if (/(props|row|rows|config|definition|field|fields|dto|record|input|output|response|payload|schema|theme)$/.test(normalizedName)) { + return true; + } + if (/(^|\/)(__fixtures__|fixtures|testdata|types|schemas|dto|config|db|database|prisma)(\/|$)/.test(normalizedPath)) { + return true; + } + if (/\.(test|spec)\.[tj]sx?$/.test(normalizedPath)) { + return true; + } + return false; +} + function classLikeName(node) { return node.name && node.name.text ? node.name.text : "anonymous"; } diff --git a/tests/checks/design_local_abstraction_test.go b/tests/checks/design_local_abstraction_test.go index 258c0ef..238d711 100644 --- a/tests/checks/design_local_abstraction_test.go +++ b/tests/checks/design_local_abstraction_test.go @@ -316,6 +316,42 @@ func TestDesignPersistenceModelLeakAllowsPackageAPIImplementationBoundary(t *tes } } +func TestDesignPersistenceModelLeakIgnoresCommentsTestsAndStubs(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "apps", "web", "app", "api", "claims", "route.test.ts"), strings.Join([]string{ + "// Prisma row comment in test should not be a boundary leak.", + "export type ClaimRow = { id: string };", + "export const prismaStub = { claim: { findMany: async () => [] } };", + }, "\n")) + writeFile(t, filepath.Join(dir, "apps", "web", "app", "api", "claims", "__fixtures__", "row-stub.ts"), strings.Join([]string{ + "// test fixture mentions PrismaClient and row shapes only for stubs.", + "export type ClaimRecord = { id: string };", + }, "\n")) + writeFile(t, filepath.Join(dir, "apps", "web", "app", "api", "claims", "route.ts"), strings.Join([]string{ + "export type ClaimModel = { id: string };", + "export async function GET() {", + " return Response.json({ ok: true });", + "}", + }, "\n")) + + report := runDesignLocalScan(t, designLocalConfig(dir, "typescript")) + + assertFindingRulePresent(t, report, "Design Patterns", "design.persistence-model-leak") + for _, section := range report.Sections { + if section.Name != "Design Patterns" { + continue + } + for _, finding := range section.Findings { + if finding.RuleID != "design.persistence-model-leak" { + continue + } + if strings.Contains(finding.Path, "route.test.ts") || strings.Contains(finding.Path, "__fixtures__") { + t.Fatalf("comments/tests/stubs should not produce persistence leak findings: %+v", finding) + } + } + } +} + func TestDesignPersistenceModelLeakKeepsAPIAndDomainBoundaries(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "apps", "web", "app", "api", "contracts", "route.ts"), strings.Join([]string{ diff --git a/tests/checks/function_hidden_mutation_noise_test.go b/tests/checks/function_hidden_mutation_noise_test.go index 8bdd4c9..7478701 100644 --- a/tests/checks/function_hidden_mutation_noise_test.go +++ b/tests/checks/function_hidden_mutation_noise_test.go @@ -96,6 +96,80 @@ func TestFunctionHiddenMutationAllowsPureLocalMutationAcrossLanguages(t *testing } } +func TestFunctionHiddenMutationAllowsBuilderParserAccumulatorNames(t *testing.T) { + cases := []struct { + name string + file string + source []string + }{ + { + name: "build copy text with array push", + file: "apps/web/lib/copy.ts", + source: []string{ + "export function buildCopyText(claim: Claim) {", + " const parts: string[] = [];", + " parts.push(claim.id);", + " parts.push(claim.status);", + " return parts.join('\\n');", + "}", + "interface Claim { id: string; status: string }", + }, + }, + { + name: "primary cells object array", + file: "apps/web/app/claims/_components/cells.ts", + source: []string{ + "export function primaryCells(row: Row) {", + " const cells = [];", + " cells.push({ key: 'status', value: row.status });", + " cells.push({ key: 'owner', value: row.owner });", + " return cells;", + "}", + "interface Row { status: string; owner: string }", + }, + }, + { + name: "calendar buckets map", + file: "apps/web/lib/calendar.ts", + source: []string{ + "export function buildCalendarBuckets(events: Event[]) {", + " const buckets = new Map();", + " for (const event of events) {", + " const day = event.day;", + " if (!buckets.has(day)) buckets.set(day, []);", + " buckets.get(day)!.push(event);", + " }", + " return buckets;", + "}", + "interface Event { day: string }", + }, + }, + { + name: "parser local object", + file: "packages/api/src/lib/contract-summary/parse.ts", + source: []string{ + "export function parseContractSummary(raw: string) {", + " const result: Record = {};", + " result.raw = raw;", + " return result;", + "}", + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, tc.file), strings.Join(tc.source, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRuleAbsent(t, report, "Code Quality", "function.hidden-mutation") + assertFindingRuleAbsent(t, report, "Code Quality", "function.command-query-mix") + assertFindingRuleAbsent(t, report, "Code Quality", "quality.hidden-side-effect") + }) + } +} + func TestFunctionHiddenMutationStillWarnsForCollaboratorMutationWithLocalPayload(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "mutation.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 4642b00..f550501 100644 --- a/tests/checks/quality_ui_false_positive_hardening_test.go +++ b/tests/checks/quality_ui_false_positive_hardening_test.go @@ -176,7 +176,7 @@ func TestQualityDuplicatedKnowledgeSkipsDisplayStringsAndIncludesLiteral(t *test report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) - finding := firstFindingForRule(t, report, "Code Quality", "quality.duplicated-knowledge") + finding := firstDuplicatedKnowledgeFinding(t, report) if !strings.Contains(finding.Message, "'invoice_policy_code'") { t.Fatalf("expected duplicated literal in message, got %q", finding.Message) } @@ -195,7 +195,7 @@ func TestQualityDuplicatedKnowledgeSkipsTrivialRepeatedNumbers(t *testing.T) { report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) - finding := firstFindingForRule(t, report, "Code Quality", "quality.duplicated-knowledge") + finding := firstDuplicatedKnowledgeFinding(t, report) if strings.Contains(finding.Message, " 0 ") || strings.Contains(finding.Message, " 2 ") { t.Fatalf("expected duplicated domain literal instead of trivial number, got %q", finding.Message) } @@ -221,7 +221,7 @@ func TestQualityDuplicatedKnowledgeSkipsSmallNumbersAndEnumStatusStrings(t *test report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) - finding := firstFindingForRule(t, report, "Code Quality", "quality.duplicated-knowledge") + finding := firstDuplicatedKnowledgeFinding(t, report) if strings.Contains(finding.Message, "CLAIM_APPROVED") || strings.Contains(finding.Message, "25") || strings.Contains(finding.Message, "3") { t.Fatalf("expected strong numeric domain duplicate, got %q", finding.Message) } @@ -230,6 +230,30 @@ func TestQualityDuplicatedKnowledgeSkipsSmallNumbersAndEnumStatusStrings(t *test } } +func TestQualityDuplicatedKnowledgeSkipsSentinelsStylesAndUnmarkedEnums(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "apps/web/app/claims/constants.ts"), strings.Join([]string{ + "export const teamFilterA = '__team__';", + "export const teamFilterB = '__team__';", + "export const baseClass = 'rounded-md border-gray-200';", + "export const activeClass = 'rounded-md border-gray-200';", + "export const firstStatus = 'CLAIM_APPROVED';", + "export const secondStatus = 'CLAIM_APPROVED';", + "export const premiumAmountCents = 1000;", + "export const vipAmountCents = 1000;", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + finding := firstDuplicatedKnowledgeFinding(t, report) + if strings.Contains(finding.Message, "__team__") || strings.Contains(finding.Message, "rounded-md") || strings.Contains(finding.Message, "CLAIM_APPROVED") { + t.Fatalf("expected only strong domain duplicate, got %q", finding.Message) + } + if !strings.Contains(finding.Message, "1000") { + t.Fatalf("expected duplicated money-like numeric literal, got %q", finding.Message) + } +} + func TestNamingCardinalityMismatchAllowsFrameworkConventions(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "apps/web/app/okrs/use-kr-drag.ts"), strings.Join([]string{ @@ -332,8 +356,8 @@ func TestNamingBooleanNotPredicateAllowsHandlersAndResourceIdentifiers(t *testin func TestNamingBooleanNotPredicateAllowsConventionalNonBooleanNames(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "apps/web/app/components/actions.tsx"), strings.Join([]string{ - "export function ActionButton(opts: boolean, message: boolean, className: boolean, Icon: boolean, submit: boolean, compare: boolean, parser: boolean) {", - " return ;", + "export function ActionButton(opts: boolean, message: boolean, className: boolean, Icon: boolean, submit: boolean, compare: boolean, parser: boolean, builder: boolean) {", + " return ;", "}", }, "\n")) @@ -361,6 +385,212 @@ func TestFunctionCommandQueryMixAllowsLocalBuilderMutation(t *testing.T) { assertFindingRuleAbsent(t, report, "Code Quality", "quality.hidden-side-effect") } +func TestFunctionCommandQueryMixAllowsAPICommandReturningResult(t *testing.T) { + cases := []struct { + name string + source []string + }{ + { + name: "createNewFile", + source: []string{ + "export async function createNewFile(repo: Repo, input: Input) {", + " const file = await repo.create(input);", + " await repo.save(file);", + " return file;", + "}", + }, + }, + { + name: "upsertEmbedding", + source: []string{ + "export async function upsertEmbedding(repo: Repo, input: Input) {", + " const embedding = await repo.upsert(input);", + " await repo.record(embedding);", + " return embedding;", + "}", + }, + }, + { + name: "notify", + source: []string{ + "export async function notify(channel: Channel, message: Message) {", + " const delivery = await channel.send(message);", + " await channel.record(delivery);", + " return delivery;", + "}", + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages/api/src/files/actions.ts"), strings.Join(append(tc.source, []string{ + "interface Repo { create(input: Input): Promise; save(input: unknown): Promise; upsert(input: Input): Promise; record(input: unknown): Promise }", + "interface Channel { send(input: Message): Promise; record(input: unknown): Promise }", + "interface Input { id: string }", + "interface Message { id: string }", + }...), "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRuleAbsent(t, report, "Code Quality", "function.command-query-mix") + assertFindingRuleAbsent(t, report, "Code Quality", "quality.hidden-side-effect") + }) + } +} + +func TestQualityPrecisionAllowsDomainSideEffectAndAdapterOrchestrationNames(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages/connectors/src/abuse.ts"), strings.Join([]string{ + "export async function loadAbuseConfig(repo: Repo) {", + " const config = await repo.load();", + " await repo.update(config);", + " return config;", + "}", + "export async function evaluateActionAbuse(evaluator: Evaluator, action: Action) {", + " const result = await evaluator.evaluate(action);", + " await evaluator.record(result);", + " return result;", + "}", + "export async function maybeAlert(alerts: Alerts, result: Result) {", + " if (result.highRisk) await alerts.send(result);", + " return result;", + "}", + "export async function saveAbuseConfig(repo: Repo, input: Input) {", + " const parsed = parseInput(input);", + " await repo.save(parsed);", + " await repo.audit(parsed);", + " return parsed;", + "}", + "export async function postBugReportToSlack(slack: Slack, report: Report) {", + " const body = formatReport(report);", + " await slack.post(body);", + " await slack.record(body);", + " return body;", + "}", + "interface Repo { load(): Promise; update(input: unknown): Promise; save(input: unknown): Promise; audit(input: unknown): Promise }", + "interface Evaluator { evaluate(input: unknown): Promise; record(input: unknown): Promise }", + "interface Alerts { send(input: unknown): Promise }", + "interface Slack { post(input: unknown): Promise; record(input: unknown): Promise }", + "interface Input { id: string }", + "interface Action { id: string }", + "interface Result { highRisk: boolean }", + "interface Report { id: string }", + "declare function parseInput(input: Input): unknown;", + "declare function formatReport(report: Report): unknown;", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + for _, ruleID := range []string{ + "function.hidden-mutation", + "function.multiple-responsibilities", + "function.mixed-abstraction-level", + "quality.mixed-abstraction-levels", + "smell.feature-envy", + } { + assertFindingRuleAbsent(t, report, "Code Quality", ruleID) + } +} + +func TestQualityNamingAllowsCommonConnectorNames(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages/connectors/src/normalizers.ts"), strings.Join([]string{ + "export function asRecord(value: unknown): Record {", + " return typeof value === 'object' && value !== null ? value as Record : {};", + "}", + "export function areStickerPlacementsEqual(source: Placement[], keys: string[], thresholds: Record, cached: boolean) {", + " const value = source.length === keys.length;", + " return cached && value && Object.keys(thresholds).length > 0;", + "}", + "interface Placement { id: string }", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRuleAbsent(t, report, "Code Quality", "naming.boolean-not-predicate") + assertFindingRuleAbsent(t, report, "Code Quality", "naming.cardinality-mismatch") + assertFindingRuleAbsent(t, report, "Code Quality", "quality.ambiguous-name") +} + +func TestQualityDuplicatedKnowledgeSkipsTableAndEnumValueLiterals(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages/connectors/src/constants.ts"), strings.Join([]string{ + "export const abuseTableName = 'lmp_abuse_config';", + "export const auditTableName = 'lmp_abuse_config';", + "export const volumeOptions = [{ value: 'high_volume' }, { value: 'high_volume' }];", + "export const avatarOptions = [{ value: 'custom_avatar' }, { value: 'custom_avatar' }];", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRuleAbsent(t, report, "Code Quality", "quality.duplicated-knowledge") +} + +func TestDefensiveRulesRecognizeValidationAndBoundedReadProofs(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages/connectors/src/boundary.ts"), strings.Join([]string{ + "export function handleToolAction(payload: Record) {", + " const parsed = parseToolActionPayload(payload);", + " return parsed.action;", + "}", + "export async function POST(request: Request) {", + " const contentLength = Number(request.headers.get('content-length') ?? '0');", + " if (contentLength > MAX_UPLOAD_BYTES) throw new Error('upload too large');", + " const form = await request.formData();", + " return Response.json({ ok: true, form });", + "}", + "export async function readBounded(response: Response) {", + " const bytes = await response.arrayBuffer();", + " if (bytes.byteLength > MAX_RESPONSE_BYTES) throw new Error('response too large');", + " return bytes;", + "}", + "export function normalizeWebhookUrl(input: string) {", + " const url = new URL(input);", + " if (url.protocol !== 'https:') throw new Error('invalid protocol');", + " return url;", + "}", + "function parseToolActionPayload(value: Record) {", + " const result = ToolActionSchema.safeParse(value);", + " if (!result.success) throw new Error('invalid payload');", + " return result.data;", + "}", + "declare const ToolActionSchema: { safeParse(value: unknown): { success: boolean; data: { action: string } } };", + "declare const MAX_UPLOAD_BYTES: number;", + "declare const MAX_RESPONSE_BYTES: number;", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRuleAbsent(t, report, "Code Quality", "defensive.unvalidated-boundary-input") + assertFindingRuleAbsent(t, report, "Code Quality", "defensive.missing-schema-validation") + assertFindingRuleAbsent(t, report, "Code Quality", "defensive.missing-resource-limit") + assertFindingRuleAbsent(t, report, "Code Quality", "defensive.unchecked-external-response") +} + +func TestDefensiveIntegerOverflowSkipsGuardedP2002SequenceRetry(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages/api/src/external-id.ts"), strings.Join([]string{ + "export async function allocateExternalId(db: Db) {", + " for (let attempt = 0; attempt < 5; attempt++) {", + " const count = await db.file.count();", + " const externalId = count + 1;", + " try {", + " return await db.file.create({ data: { externalId } });", + " } catch (err: any) {", + " if (err.code !== 'P2002') throw err;", + " }", + " }", + " throw new Error('unique external ID collision retry exhausted');", + "}", + "interface Db { file: { count(): Promise; create(input: unknown): Promise } }", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRuleAbsent(t, report, "Code Quality", "defensive.integer-overflow") +} + func TestReactNativeScreenAllowsUIBooleanAndLocalCollections(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "apps/mobile/src/screens/ClaimsScreen.tsx"), strings.Join([]string{ @@ -495,6 +725,71 @@ func TestStructuralSmellsSkipAPIConfigTraversalAndDTOBuilders(t *testing.T) { assertFindingRuleAbsent(t, report, "Code Quality", "smell.message-chain") } +func TestStructuralSmellsSkipSerializerPrismaPromptAndAdapterHelpers(t *testing.T) { + cases := []struct { + name string + file string + source []string + }{ + { + name: "url search params", + file: "packages/api/src/search/params.ts", + source: []string{ + "export function readSearchParams(request: Request) {", + " const params = new URLSearchParams(request.url);", + " return params.get('team')?.trim()?.toLowerCase()?.replaceAll(' ', '-');", + "}", + }, + }, + { + name: "prisma include select", + file: "packages/api/src/contracts/query.ts", + source: []string{ + "export const contractQuery = {", + " include: { owner: { select: { profile: { select: { department: { select: { name: true } } } } } } },", + "};", + }, + }, + { + name: "csv serializer", + file: "packages/api/src/contracts/export.ts", + source: []string{ + "export function serializeContractCsv(row: Row) {", + " return [row.contract.version.current.owner.profile.name, row.contract.version.current.status.code, row.contract.version.current.timestamps.updatedAt].join(',');", + "}", + "interface Row { contract: any }", + }, + }, + { + name: "prompt adapter", + file: "packages/api/src/ai/prompt-adapter.ts", + source: []string{ + "export function buildClaimPrompt(claim: Claim) {", + " return {", + " title: claim.case.owner.profile.name,", + " email: claim.case.owner.profile.email,", + " status: claim.case.status.current.code,", + " due: claim.case.timeline.current.dueAt,", + " team: claim.case.owner.team.name,", + " };", + "}", + "interface Claim { case: any }", + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, tc.file), strings.Join(tc.source, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + + assertFindingRuleAbsent(t, report, "Code Quality", "smell.message-chain") + assertFindingRuleAbsent(t, report, "Code Quality", "smell.feature-envy") + }) + } +} + func TestSmellAndOverflowRulesStillFlagNonUIProductionCode(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "packages/domain/src/account-risk.ts"), strings.Join([]string{ @@ -513,18 +808,18 @@ func TestSmellAndOverflowRulesStillFlagNonUIProductionCode(t *testing.T) { assertStructuralSmellPresent(t, report, "defensive.integer-overflow") } -func firstFindingForRule(t *testing.T, report codeguard.Report, sectionName string, ruleID string) codeguard.Finding { +func firstDuplicatedKnowledgeFinding(t *testing.T, report codeguard.Report) codeguard.Finding { t.Helper() for _, section := range report.Sections { - if section.Name != sectionName { + if section.Name != "Code Quality" { continue } for _, finding := range section.Findings { - if finding.RuleID == ruleID { + if finding.RuleID == "quality.duplicated-knowledge" { return finding } } } - t.Fatalf("rule %q not found in section %q", ruleID, sectionName) + t.Fatalf("rule %q not found in section %q", "quality.duplicated-knowledge", "Code Quality") return codeguard.Finding{} } diff --git a/tests/checks/typescript_semantic_test.go b/tests/checks/typescript_semantic_test.go index 0e13615..ccba64b 100644 --- a/tests/checks/typescript_semantic_test.go +++ b/tests/checks/typescript_semantic_test.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" "github.com/devr-tools/codeguard/pkg/codeguard" @@ -28,6 +29,83 @@ func TestDesignCheckUsesSemanticTypeScriptAnalyzerForAnonymousDefaultClass(t *te assertFindingRulePresent(t, report, "Design Patterns", "design.typescript.max-methods-per-type") } +func TestDesignCheckSkipsLargeTypeScriptDataContractsAndProps(t *testing.T) { + requireTypeScriptSemanticRuntime(t) + + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "types", "tool.tsx"), strings.Join([]string{ + "export interface ToolDefinition {", + " id: string;", + " name: string;", + " description: string;", + " version: string;", + " owner: string;", + " fields: ToolField[];", + " enabled: boolean;", + "}", + "export interface ToolField {", + " key: string;", + " label: string;", + " type: string;", + " required: boolean;", + " options: string[];", + " defaultValue: string;", + "}", + "export type LmpAbuseConfig = {", + " threshold: number;", + " windowSeconds: number;", + " highVolume: boolean;", + " customAvatar: boolean;", + " alertChannel: string;", + " owner: string;", + "};", + "export interface ToolPanelProps {", + " tool: ToolDefinition;", + " fields: ToolField[];", + " selected: string[];", + " loading: boolean;", + " onSave(): void;", + " onCancel(): void;", + "}", + }, "\n")) + writeFile(t, filepath.Join(dir, "src", "domain", "wide-policy.ts"), strings.Join([]string{ + "export interface WidePolicy {", + " one(): void;", + " two(): void;", + " three(): void;", + " four(): void;", + " five(): void;", + " six(): void;", + "}", + }, "\n")) + + cfg := typeScriptDesignConfig(dir, "typescript") + cfg.Checks.DesignRules.MaxInterfaceMethods = 5 + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRulePresent(t, report, "Design Patterns", "design.typescript.max-interface-members") + for _, section := range report.Sections { + if section.Name != "Design Patterns" { + continue + } + for _, finding := range section.Findings { + if finding.RuleID != "design.typescript.max-interface-members" { + continue + } + if strings.Contains(finding.Message, "ToolDefinition") || + strings.Contains(finding.Message, "ToolField") || + strings.Contains(finding.Message, "LmpAbuseConfig") || + strings.Contains(finding.Message, "ToolPanelProps") { + t.Fatalf("data contracts and prop interfaces should not trip max-interface-members: %+v", finding) + } + } + } +} + func TestQualityCheckUsesSemanticTypeScriptAnalyzerForClassArrowMethods(t *testing.T) { requireTypeScriptSemanticRuntime(t)