From 5a28b3b5cff9c6ebb594728be2feabaab8e2f1e1 Mon Sep 17 00:00:00 2001 From: alex <53851759+alxxjohn@users.noreply.github.com> Date: Sat, 29 Aug 2026 12:40:43 -0400 Subject: [PATCH] Fix scanner precision for route wiring and partial failures --- .../checks/quality/quality_errors.go | 33 ++++ .../checks/quality/quality_precision.go | 22 +++ .../quality/quality_precision_frameworks.go | 30 +++- .../reliability_partial_failure.go | 84 +++++++++- .../quality_error_defensive_multilang_test.go | 6 +- ...uality_ui_false_positive_hardening_test.go | 154 ++++++++++++++++++ tests/checks/reliability_multilang_test.go | 22 ++- tests/checks/reliability_test.go | 99 +++++++++++ 8 files changed, 439 insertions(+), 11 deletions(-) diff --git a/internal/codeguard/checks/quality/quality_errors.go b/internal/codeguard/checks/quality/quality_errors.go index 96117e1..3f172fe 100644 --- a/internal/codeguard/checks/quality/quality_errors.go +++ b/internal/codeguard/checks/quality/quality_errors.go @@ -30,6 +30,7 @@ var ( cleanupIgnoredPattern = regexp.MustCompile(`(?i)(_\s*=\s*[^;\n]*(close|rollback|remove|delete)\s*\(|defer\s+[^;\n]*\.close\s*\(|catch\s*\([^)]*\)\s*\{\s*(?:/\*.*\*/|//.*)?\s*\})`) panicPattern = regexp.MustCompile(`\bpanic\s*\(`) throwRaisePattern = regexp.MustCompile(`(?i)\b(throw|raise)\b`) + nonPartialResultReturnPattern = regexp.MustCompile(`(?i)^return(?:\s+(.+?))?\s*;?$`) ) func errorContractFindings(env support.Context, file string, fn precisionFunction) []core.Finding { @@ -198,6 +199,9 @@ func cleanupIgnoredLine(statements []support.ParsedStatement) (int, bool) { } func partialFailureHiddenLine(fn precisionFunction, loweredBody string) (int, bool) { + if nonPartialResultFunction(fn) { + return 0, false + } if partialFailureSurfacedInResult(loweredBody) { return 0, false } @@ -225,6 +229,35 @@ func partialFailureHiddenLine(fn precisionFunction, loweredBody string) (int, bo return 0, false } +func nonPartialResultFunction(fn precisionFunction) bool { + signature := strings.ToLower(strings.ReplaceAll(fn.Signature, " ", "")) + if !fn.Returns { + return true + } + switch signature { + case "error", "(error)", "void": + return true + } + if strings.Contains(signature, "promise") { + return true + } + for _, statement := range fn.Statements { + line := strings.TrimSpace(firstNonEmptyString(statement.Raw, statement.Text)) + match := nonPartialResultReturnPattern.FindStringSubmatch(line) + if len(match) == 0 { + continue + } + if len(match) == 1 || strings.TrimSpace(match[1]) == "" { + continue + } + value := strings.ToLower(strings.TrimSpace(strings.TrimSuffix(match[1], ";"))) + if value != "nil" && value != "none" && value != "null" && value != "undefined" { + return false + } + } + return signature == "" || signature == "none" +} + func allSettledResultIsReturned(loweredBody string) bool { return containsAny(loweredBody, []string{ "return {", "normalize", "mapsettled", "settledresults", "fulfilled", "rejected", diff --git a/internal/codeguard/checks/quality/quality_precision.go b/internal/codeguard/checks/quality/quality_precision.go index a1e28a8..0d7852f 100644 --- a/internal/codeguard/checks/quality/quality_precision.go +++ b/internal/codeguard/checks/quality/quality_precision.go @@ -114,6 +114,7 @@ func goPrecisionFunction(fset *token.FileSet, fn *ast.FuncDecl, data []byte) pre Name: fn.Name.Name, StartLine: fset.Position(fn.Pos()).Line, EndLine: fset.Position(fn.End()).Line, + Signature: goResultSignature(fn), Params: goParsedParams(fn), Returns: goFuncReturnsValue(fn), } @@ -148,6 +149,27 @@ func goPrecisionFunction(fset *token.FileSet, fn *ast.FuncDecl, data []byte) pre return out } +func goResultSignature(fn *ast.FuncDecl) string { + if fn.Type == nil || fn.Type.Results == nil || len(fn.Type.Results.List) == 0 { + return "" + } + results := make([]string, 0, len(fn.Type.Results.List)) + for _, field := range fn.Type.Results.List { + text := goExprText(field.Type) + if len(field.Names) == 0 { + results = append(results, text) + continue + } + for range field.Names { + results = append(results, text) + } + } + if len(results) == 1 { + return results[0] + } + return "(" + strings.Join(results, ", ") + ")" +} + func goParsedParams(fn *ast.FuncDecl) []support.ParsedParam { if fn.Type == nil || fn.Type.Params == nil { return nil diff --git a/internal/codeguard/checks/quality/quality_precision_frameworks.go b/internal/codeguard/checks/quality/quality_precision_frameworks.go index 47fbba7..aaaafeb 100644 --- a/internal/codeguard/checks/quality/quality_precision_frameworks.go +++ b/internal/codeguard/checks/quality/quality_precision_frameworks.go @@ -7,7 +7,7 @@ import ( ) func isFrameworkOrchestrationBoundary(file string, fn precisionFunction) bool { - return isFrameworkCommandBoundary(file, fn.Name) || isTrackedRouteBoundary(file, fn) || isNestJSRequestBoundary(file) + return isFrameworkCommandBoundary(file, fn.Name) || isTrackedRouteBoundary(file, fn) || isNestJSRequestBoundary(file) || isGoRouterRegistrationBoundary(file, fn) } func isFrameworkConventionalAmbiguousName(file string, fn precisionFunction, name string) bool { @@ -44,3 +44,31 @@ func isTrackedRouteBoundary(file string, fn precisionFunction) bool { body := strings.ToLower(fn.Body) return strings.Contains(body, "nextresponse.") || strings.Contains(body, "response.json(") } + +func isGoRouterRegistrationBoundary(file string, fn precisionFunction) bool { + if !strings.HasSuffix(strings.ToLower(file), ".go") { + return false + } + loweredName := strings.ToLower(strings.Trim(fn.Name, "_$")) + if loweredName == "registerroutes" || strings.HasPrefix(loweredName, "add") && strings.Contains(loweredName, "routes") { + return hasRouterRegistrationEvidence(fn) + } + return false +} + +func hasRouterRegistrationEvidence(fn precisionFunction) bool { + signature := strings.ToLower(fn.Signature) + if strings.Contains(signature, "chi.router") || strings.Contains(signature, "mux.router") || strings.Contains(signature, "gin.engine") { + return true + } + for _, call := range fn.Calls { + lowered := strings.ToLower(call.Callee) + if strings.HasSuffix(lowered, ".route") || strings.HasSuffix(lowered, ".get") || + strings.HasSuffix(lowered, ".post") || strings.HasSuffix(lowered, ".put") || + strings.HasSuffix(lowered, ".patch") || strings.HasSuffix(lowered, ".delete") || + strings.HasSuffix(lowered, ".handle") || strings.HasSuffix(lowered, ".handlefunc") { + return true + } + } + return false +} diff --git a/internal/codeguard/checks/reliability/reliability_partial_failure.go b/internal/codeguard/checks/reliability/reliability_partial_failure.go index 0a93542..1394f1b 100644 --- a/internal/codeguard/checks/reliability/reliability_partial_failure.go +++ b/internal/codeguard/checks/reliability/reliability_partial_failure.go @@ -14,6 +14,11 @@ var ( partialFailureContinuePattern = regexp.MustCompile(`^\s*continue\s*;?\s*(?://.*)?$`) partialFailureSuccessReturn = regexp.MustCompile(`(?i)^\s*return(?:\s+(?:nil|none|null|true|0|\{\}))?\s*;?\s*$`) partialFailurePropagatePattern = regexp.MustCompile(`(?i)\b(return\s+err|return\s+error|raise\b|throw\b)`) + partialFailureGoErrorFunc = regexp.MustCompile(`^\s*func\s+(?:\([^)]*\)\s*)?[A-Za-z_]\w*\s*\([^)]*\)\s*(?:error|\(\s*error\s*\))\s*\{`) + partialFailureScriptVoidFunc = regexp.MustCompile(`\bfunction\s+[A-Za-z_$][\w$]*\s*\([^)]*\)\s*:\s*(?:Promise\s*<\s*void\s*>|void)\s*\{`) + partialFailureCPPVoidFunc = regexp.MustCompile(`^\s*(?:[\w:<>]+\s+)*void\s+[A-Za-z_]\w*\s*\([^)]*\)\s*(?:const\s*)?\{`) + partialFailurePythonDef = regexp.MustCompile(`^\s*def\s+[A-Za-z_]\w*\s*\([^)]*\)\s*(?:->\s*None\s*)?:`) + partialFailureValuedReturn = regexp.MustCompile(`(?i)^\s*return\s+(.+?)\s*;?\s*$`) ) func partialFailureHiddenFindings(env support.Context, file string, data []byte) []core.Finding { @@ -21,6 +26,7 @@ func partialFailureHiddenFindings(env support.Context, file string, data []byte) return nil } lines := strings.Split(strings.ReplaceAll(string(data), "\r\n", "\n"), "\n") + contracts := partialFailureNonResultFunctionLines(lines) loopDepth := 0 pending := 0 findings := make([]core.Finding, 0, 1) @@ -39,12 +45,16 @@ func partialFailureHiddenFindings(env support.Context, file string, data []byte) pending = lineNo } if pending > 0 && partialFailureContinuePattern.MatchString(trimmed) { - findings = append(findings, partialFailureFinding(env, file, pending, "logged failure is skipped and batch processing continues without surfacing partial failure")) + if !contracts[pending] { + findings = append(findings, partialFailureFinding(env, file, pending, "logged failure is skipped and batch processing continues without surfacing partial failure")) + } pending = 0 continue } if pending > 0 && lineNo <= pending+12 && partialFailureSuccessReturn.MatchString(trimmed) { - findings = append(findings, partialFailureFinding(env, file, pending, "logged failure is followed by a success return, hiding partial failure from callers")) + if !contracts[pending] { + findings = append(findings, partialFailureFinding(env, file, pending, "logged failure is followed by a success return, hiding partial failure from callers")) + } pending = 0 continue } @@ -57,6 +67,76 @@ func partialFailureHiddenFindings(env support.Context, file string, data []byte) }) } +func partialFailureNonResultFunctionLines(lines []string) map[int]bool { + out := map[int]bool{} + for start := 0; start < len(lines); start++ { + line := lines[start] + switch { + case partialFailureGoErrorFunc.MatchString(line) || partialFailureScriptVoidFunc.MatchString(line) || partialFailureCPPVoidFunc.MatchString(line): + end := partialFailureBraceFunctionEnd(lines, start) + for idx := start + 1; idx <= end && idx < len(lines); idx++ { + out[idx+1] = true + } + start = end + case partialFailurePythonDef.MatchString(line): + end := partialFailurePythonFunctionEnd(lines, start) + if partialFailurePythonReturnsOnlyNone(lines[start+1 : end+1]) { + for idx := start + 1; idx <= end && idx < len(lines); idx++ { + out[idx+1] = true + } + } + start = end + } + } + return out +} + +func partialFailureBraceFunctionEnd(lines []string, start int) int { + depth := 0 + seenOpen := false + for idx := start; idx < len(lines); idx++ { + line := lines[idx] + depth += strings.Count(line, "{") + if strings.Contains(line, "{") { + seenOpen = true + } + depth -= strings.Count(line, "}") + if seenOpen && depth <= 0 { + return idx + } + } + return len(lines) - 1 +} + +func partialFailurePythonFunctionEnd(lines []string, start int) int { + baseIndent := len(lines[start]) - len(strings.TrimLeft(lines[start], " \t")) + for idx := start + 1; idx < len(lines); idx++ { + trimmed := strings.TrimSpace(lines[idx]) + if trimmed == "" { + continue + } + indent := len(lines[idx]) - len(strings.TrimLeft(lines[idx], " \t")) + if indent <= baseIndent { + return idx - 1 + } + } + return len(lines) - 1 +} + +func partialFailurePythonReturnsOnlyNone(lines []string) bool { + for _, line := range lines { + match := partialFailureValuedReturn.FindStringSubmatch(strings.TrimSpace(line)) + if len(match) != 2 { + continue + } + value := strings.TrimSpace(strings.TrimSuffix(match[1], ";")) + if !strings.EqualFold(value, "none") { + return false + } + } + return true +} + func partialFailureFinding(env support.Context, file string, line int, message string) core.Finding { return newFinding(env, "reliability.partial-failure-hidden", "fail", file, line, 1, message, "medium", "failure_mode", "partial-failure-hidden") } diff --git a/tests/checks/quality_error_defensive_multilang_test.go b/tests/checks/quality_error_defensive_multilang_test.go index 3512149..a45c05c 100644 --- a/tests/checks/quality_error_defensive_multilang_test.go +++ b/tests/checks/quality_error_defensive_multilang_test.go @@ -89,13 +89,15 @@ func TestQualityErrorContractsDetectMultiLanguageSignals(t *testing.T) { "\treturn nil", "}", "", - "func ProcessAll(items []string) error {", + "func ProcessAll(items []string) ([]string, error) {", + "\tprocessed := make([]string, 0, len(items))", "\tfor _, item := range items {", "\t\tif err := sendItem(item); err != nil {", "\t\t\tcontinue", "\t\t}", + "\t\tprocessed = append(processed, item)", "\t}", - "\treturn nil", + "\treturn processed, nil", "}", "", "func DecodeConfig(raw []byte) map[string]string {", diff --git a/tests/checks/quality_ui_false_positive_hardening_test.go b/tests/checks/quality_ui_false_positive_hardening_test.go index a40ae47..e717721 100644 --- a/tests/checks/quality_ui_false_positive_hardening_test.go +++ b/tests/checks/quality_ui_false_positive_hardening_test.go @@ -170,6 +170,160 @@ func TestFunctionCommandQueryMixAllowsReactAndNextBoundaries(t *testing.T) { } } +func TestGoRouteRegistrationAllowsChiWiringAndServiceDeps(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "services", "rest", "share", "share_preview_routes.go"), strings.Join([]string{ + "package share", + "", + "import \"github.com/go-chi/chi/v5\"", + "", + "type Deps struct {", + "\tUsersService UsersService", + "\tCreatorsService CreatorsService", + "\tNotificationsService NotificationsService", + "\tTermsOfServiceURL string", + "}", + "", + "func RegisterRoutes(api chi.Router, deps Deps) {", + "\tapi.Route(\"/share\", func(router chi.Router) {", + "\t\taddSharePreviewRoutes(router, deps)", + "\t})", + "}", + "", + "func addSharePreviewRoutes(router chi.Router, deps Deps) {", + "\trouter.Get(\"/{id}\", func(w ResponseWriter, r Request) {", + "\t\tpreview, err := validateSharePreviewRequest(r)", + "\t\tif err != nil {", + "\t\t\twriteSharePreviewError(w, err)", + "\t\t\treturn", + "\t\t}", + "\t\tcreator := deps.CreatorsService.Lookup(preview.CreatorID)", + "\t\tmedia := selectSharePreviewMedia(preview, creator)", + "\t\twriteSharePreviewResponse(w, metadataForSharePreview(preview, media, deps.TermsOfServiceURL))", + "\t})", + "}", + "", + "type UsersService interface { Lookup(string) Creator }", + "type CreatorsService interface { Lookup(string) Creator }", + "type NotificationsService interface { Notify(string) }", + "type Creator struct{}", + "type Request struct{}", + "type ResponseWriter struct{}", + "type Preview struct { CreatorID string }", + "func validateSharePreviewRequest(Request) (Preview, error) { return Preview{}, nil }", + "func writeSharePreviewError(ResponseWriter, error) {}", + "func selectSharePreviewMedia(Preview, Creator) string { return \"\" }", + "func metadataForSharePreview(Preview, string, string) string { return \"\" }", + "func writeSharePreviewResponse(ResponseWriter, string) {}", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + + assertFindingRuleAbsent(t, report, "Code Quality", "function.command-query-mix") + assertFindingRuleAbsent(t, report, "Code Quality", "function.mixed-abstraction-level") + assertFindingRuleAbsent(t, report, "Code Quality", "function.hidden-mutation") + assertFindingRuleAbsent(t, report, "Code Quality", "naming.generic-identifier") + assertFindingRuleAbsent(t, report, "Code Quality", "quality.ambiguous-name") + assertFindingRuleAbsent(t, report, "Code Quality", "quality.primitive-obsession") +} + +func TestQualityPartialFailureAllowsErrorOnlyBatchResultsAcrossLanguages(t *testing.T) { + cases := []struct { + name string + language string + file string + source []string + }{ + { + name: "go", + language: "go", + file: "records.go", + source: []string{ + "package sample", + "", + "import \"log\"", + "", + "func RecordCollectionView(records []Record) error {", + "\tfor _, record := range records {", + "\t\tif err := persist(record); err != nil {", + "\t\t\tlog.Printf(\"record collection view failed: %v\", err)", + "\t\t\tcontinue", + "\t\t}", + "\t}", + "\treturn nil", + "}", + "", + "type Record struct{}", + "func persist(Record) error { return nil }", + }, + }, + { + name: "typescript", + language: "typescript", + file: "records.ts", + source: []string{ + "export async function recordCollectionView(records: Record[]): Promise {", + " for (const record of records) {", + " try {", + " await persist(record);", + " } catch (error) {", + " console.error(\"record collection view failed\", error);", + " continue;", + " }", + " }", + "}", + "interface Record {}", + "declare function persist(record: Record): Promise;", + }, + }, + { + name: "python", + language: "python", + file: "records.py", + source: []string{ + "import logging", + "", + "def record_collection_view(records) -> None:", + " for record in records:", + " try:", + " persist(record)", + " except RuntimeError as error:", + " logging.warning(\"record collection view failed: %s\", error)", + " continue", + " return None", + }, + }, + { + name: "cpp", + language: "cpp", + file: "records.cpp", + source: []string{ + "#include ", + "#include ", + "", + "void RecordCollectionView(const std::vector& records) {", + " for (const auto& record : records) {", + " if (!persist(record)) {", + " std::cerr << \"record collection view failed\";", + " continue;", + " }", + " }", + "}", + }, + }, + } + 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, tc.language)) + + assertFindingRuleAbsent(t, report, "Code Quality", "error.partial-failure-hidden") + }) + } +} + func TestFunctionMutationRulesAllowWrappedNextRouteHandlersAndRollbackNames(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "src", "app", "api", "apps", "lmp", "new-editor", "route.ts"), strings.Join([]string{ diff --git a/tests/checks/reliability_multilang_test.go b/tests/checks/reliability_multilang_test.go index c9ec053..2e876ef 100644 --- a/tests/checks/reliability_multilang_test.go +++ b/tests/checks/reliability_multilang_test.go @@ -402,17 +402,20 @@ func hiddenPartialFailureCases() []reliabilityLanguageCase { import "log" -func Process(items []Item) error { +func Process(items []Item) ([]Result, error) { + results := make([]Result, 0, len(items)) for _, item := range items { if err := process(item); err != nil { log.Printf("item error: %v", err) continue } + results = append(results, Result{}) } - return nil + return results, nil } type Item struct{} +type Result struct{} func process(Item) error { return nil } `, }, @@ -424,13 +427,15 @@ func process(Item) error { return nil } import logging def process_all(items): + results = [] for item in items: try: process(item) except Exception as error: logging.error("item failed: %s", error) continue - return None + results.append(item) + return results `, }, { @@ -438,7 +443,8 @@ def process_all(items): language: "typescript", file: "batch.ts", source: ` -async function processAll(items: Item[]): Promise { +async function processAll(items: Item[]): Promise { + const results: Result[] = []; for (const item of items) { try { await process(item); @@ -446,9 +452,11 @@ async function processAll(items: Item[]): Promise { console.error("item error", error); continue; } + results.push({}); } - return; + return results; } +interface Result {} `, }, { @@ -457,6 +465,7 @@ async function processAll(items: Item[]): Promise { file: "batch.js", source: ` async function processAll(items) { + const results = []; for (const item of items) { try { await process(item); @@ -464,8 +473,9 @@ async function processAll(items) { console.warn("item error", error); continue; } + results.push(item); } - return; + return results; } `, }, diff --git a/tests/checks/reliability_test.go b/tests/checks/reliability_test.go index 27c367e..117db12 100644 --- a/tests/checks/reliability_test.go +++ b/tests/checks/reliability_test.go @@ -129,6 +129,105 @@ type Card struct{} assertFindingRulePresent(t, report, "Reliability", "reliability.non-idempotent-retry") } +func TestReliabilityPartialFailureAllowsErrorOnlyBatchResultsAcrossLanguages(t *testing.T) { + cases := []struct { + name string + language string + file string + source string + }{ + { + name: "go", + language: "go", + file: "records.go", + source: `package sample + +import "log" + +func RecordCollectionView(records []Record) error { + for _, record := range records { + if err := persist(record); err != nil { + log.Printf("record collection view failed: %v", err) + continue + } + } + return nil +} + +type Record struct{} +func persist(Record) error { return nil } +`, + }, + { + name: "typescript", + language: "typescript", + file: "records.ts", + source: `export async function recordCollectionView(records: Record[]): Promise { + for (const record of records) { + try { + await persist(record); + } catch (error) { + console.error("record collection view failed", error); + continue; + } + } +} + +interface Record {} +declare function persist(record: Record): Promise; +`, + }, + { + name: "python", + language: "python", + file: "records.py", + source: `import logging + +def record_collection_view(records): + for record in records: + try: + persist(record) + except RuntimeError as error: + logging.warning("record collection view failed: %s", error) + continue + return None +`, + }, + { + name: "cpp", + language: "cpp", + file: "records.cpp", + source: `#include +#include + +void RecordCollectionView(const std::vector& records) { + for (const auto& record : records) { + if (!persist(record)) { + std::cerr << "record collection view failed"; + continue; + } + } +} +`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, tc.file), tc.source) + + cfg := reliabilityConfig("reliability-partial-error-only-"+tc.name, dir) + cfg.Targets[0].Language = tc.language + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Reliability", "reliability.partial-failure-hidden") + }) + } +} + func TestReliabilityTypeScriptAllowsCaughtPromiseRejections(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "feature-access-tab.tsx"), `async function fetchNewEditorAccess() {