From 355fbe532133cf134226e0047c1b062dea43e73a Mon Sep 17 00:00:00 2001 From: alex <53851759+alxxjohn@users.noreply.github.com> Date: Sat, 29 Aug 2026 15:40:22 -0400 Subject: [PATCH 1/2] fix quality analyzer false positives --- .../checks/quality/quality_history.go | 37 ++++ .../checks/quality/quality_precision.go | 92 +++++++-- .../quality_precision_workstreams_cd.go | 119 +++++++++-- internal/codeguard/core/config_rule_types.go | 6 + tests/checks/maintainability_history_test.go | 5 +- tests/checks/naming_precision_test.go | 5 +- .../quality_v174_false_positive_test.go | 186 ++++++++++++++++++ tests/checks/smell_history_test.go | 10 +- 8 files changed, 424 insertions(+), 36 deletions(-) create mode 100644 tests/checks/quality_v174_false_positive_test.go diff --git a/internal/codeguard/checks/quality/quality_history.go b/internal/codeguard/checks/quality/quality_history.go index f7cf8e18..e28b426f 100644 --- a/internal/codeguard/checks/quality/quality_history.go +++ b/internal/codeguard/checks/quality/quality_history.go @@ -56,6 +56,10 @@ func maintainabilityHistoryFindings(ctx context.Context, env support.Context, ta if env.Mode != core.ScanModeDiff { return nil } + cfg := env.Config.Checks.QualityRules.MaintainabilityHistory + if cfg.Enabled != nil && !*cfg.Enabled { + return nil + } changed := changedFilesForTarget(env, target) if len(changed) == 0 { return nil @@ -93,9 +97,42 @@ func maintainabilityHistoryFindings(ctx context.Context, env support.Context, ta } return findings[i].Message < findings[j].Message }) + if len(findings) > 0 { + env.PutArtifact(maintainabilityHistoryArtifact(target, findings)) + } + if cfg.ReportAsFindings == nil || !*cfg.ReportAsFindings { + return nil + } return findings } +func maintainabilityHistoryArtifact(target core.TargetConfig, findings []core.Finding) core.Artifact { + entries := make([]core.FileRiskEntry, 0, len(findings)) + for idx, finding := range findings { + entries = append(entries, core.FileRiskEntry{ + Path: finding.Path, + Rank: idx + 1, + Score: 1, + Components: []core.FileRiskComponent{{ + Label: finding.RuleID, + Weight: 1, + Count: 1, + Contribution: 1, + Detail: finding.Message, + }}, + }) + } + return core.Artifact{ + ID: "maintainability_history." + support.ArtifactSafeID(target.Name), + Kind: "maintainability_history", + Language: target.Language, + Target: target.Path, + PRHotspots: &core.PRHotspotsArtifact{ + Hotspots: entries, + }, + } +} + func historyRuleFindings(env support.Context, rel string, line int, metric history.FileChangeMetrics, hints fileMaintainabilityHints) []core.Finding { findings := make([]core.Finding, 0, 7) metadata := historyMetadata(metric, hints) diff --git a/internal/codeguard/checks/quality/quality_precision.go b/internal/codeguard/checks/quality/quality_precision.go index ed7117c8..f6890289 100644 --- a/internal/codeguard/checks/quality/quality_precision.go +++ b/internal/codeguard/checks/quality/quality_precision.go @@ -60,17 +60,19 @@ var ( ) type precisionFunction struct { - Name string - StartLine int - EndLine int - Signature string - Params []support.ParsedParam - Assignments []support.ParsedAssignment - Calls []support.ParsedCall - Statements []support.ParsedStatement - Nested []precisionLineRange - Body string - Returns bool + Name string + Receiver string + StartLine int + EndLine int + Signature string + Params []support.ParsedParam + Assignments []support.ParsedAssignment + Calls []support.ParsedCall + Statements []support.ParsedStatement + Nested []precisionLineRange + Body string + Returns bool + ImplementsInterfaceSignature bool } func localPrecisionEnabled(env support.Context) bool { @@ -88,9 +90,11 @@ func excessiveParameterFinding(env support.Context, file string, fn functionMetr func goPrecisionFindings(env support.Context, file string, fset *token.FileSet, parsed *ast.File, data []byte) []core.Finding { findings := make([]core.Finding, 0) + interfaceMethods := goInterfaceMethodSignatures(parsed) ast.Inspect(parsed, func(n ast.Node) bool { if node, ok := n.(*ast.FuncDecl); ok { fn := goPrecisionFunction(fset, node, data) + fn.ImplementsInterfaceSignature = interfaceMethods[goInterfaceMethodKey(fn.Name, fn.Params, fn.Signature)] findings = append(findings, precisionFunctionFindings(env, file, fn)...) if node.Body != nil { findings = append(findings, goDefensiveFindings(env, file, fset, node.Body)...) @@ -117,6 +121,7 @@ func goPrecisionFindings(env support.Context, file string, fset *token.FileSet, func goPrecisionFunction(fset *token.FileSet, fn *ast.FuncDecl, data []byte) precisionFunction { out := precisionFunction{ Name: fn.Name.Name, + Receiver: goReceiverType(fn), StartLine: fset.Position(fn.Pos()).Line, EndLine: fset.Position(fn.End()).Line, Signature: goResultSignature(fn), @@ -133,6 +138,12 @@ func goPrecisionFunction(fset *token.FileSet, fn *ast.FuncDecl, data []byte) pre } ast.Inspect(fn.Body, func(n ast.Node) bool { switch node := n.(type) { + case *ast.FuncLit: + out.Nested = append(out.Nested, precisionLineRange{ + Start: fset.Position(node.Pos()).Line, + End: fset.Position(node.End()).Line, + }) + return false case *ast.AssignStmt: out.Assignments = append(out.Assignments, goAssignments(fset, node)...) case *ast.ValueSpec: @@ -193,6 +204,56 @@ func goParsedParams(fn *ast.FuncDecl) []support.ParsedParam { return params } +func goReceiverType(fn *ast.FuncDecl) string { + if fn.Recv == nil || len(fn.Recv.List) == 0 { + return "" + } + return strings.TrimPrefix(goExprText(fn.Recv.List[0].Type), "*") +} + +func goInterfaceMethodSignatures(parsed *ast.File) map[string]bool { + out := map[string]bool{} + for _, decl := range parsed.Decls { + gen, ok := decl.(*ast.GenDecl) + if !ok || gen.Tok != token.TYPE { + continue + } + for _, spec := range gen.Specs { + typeSpec, ok := spec.(*ast.TypeSpec) + if !ok { + continue + } + iface, ok := typeSpec.Type.(*ast.InterfaceType) + if !ok || iface.Methods == nil { + continue + } + for _, method := range iface.Methods.List { + methodType, ok := method.Type.(*ast.FuncType) + if !ok { + continue + } + methodDecl := &ast.FuncDecl{Type: methodType} + signature := goResultSignature(methodDecl) + params := goParsedParams(methodDecl) + for _, name := range method.Names { + out[goInterfaceMethodKey(name.Name, params, signature)] = true + } + } + } + } + return out +} + +func goInterfaceMethodKey(name string, params []support.ParsedParam, signature string) string { + parts := make([]string, 0, len(params)+2) + parts = append(parts, name) + for _, param := range params { + parts = append(parts, strings.ReplaceAll(param.Type, " ", "")) + } + parts = append(parts, strings.ReplaceAll(signature, " ", "")) + return strings.Join(parts, "\x00") +} + func goExprText(expr ast.Expr) string { if expr == nil { return "" @@ -452,8 +513,10 @@ func precisionFunctionFindings(env support.Context, file string, fn precisionFun findings = append(findings, additionalPrecisionFunctionFindings(env, file, fn)...) if !isUIHelperOrMappingContext(file, fn) && !isSeedOrScriptSourcePath(file) && !isFrontendLibraryPath(file) && !isDomainSideEffectBoundaryName(fn.Name) && !isValidationOrExtractionHelperName(fn.Name) && primitiveObsession(fn) { - findings = append(findings, precisionWarnFinding(env, qualityPrimitiveObsessionRuleID, file, fn.StartLine, - fmt.Sprintf("function %s passes several domain concepts as raw primitives", fn.Name), core.ConfidenceMedium)) + if !fn.ImplementsInterfaceSignature { + findings = append(findings, precisionWarnFinding(env, qualityPrimitiveObsessionRuleID, file, fn.StartLine, + fmt.Sprintf("function %s passes several domain concepts as raw primitives", fn.Name), core.ConfidenceMedium)) + } } if hiddenSideEffect(file, fn) { findings = append(findings, precisionWarnFinding(env, qualityHiddenSideEffectRuleID, file, fn.StartLine, @@ -584,6 +647,9 @@ func commandQueryMix(file string, fn precisionFunction) bool { if isQualityFixturePath(file) { return false } + if explicitRepositoryCommandResultContract(file, fn) { + return false + } if isFrameworkOrchestrationBoundary(file, fn) || isReactComponentOrNamedHookBoundary(file, fn) || isUIHelperOrMappingContext(file, fn) || isScriptEntrypoint(file, fn.Name) || isSeedOrScriptSourcePath(file) || isAdapterOrOrchestrationFunction(file, fn) || isPostgresRepositoryPath(file) || isSecurityOrConfigUtilityFunction(file, fn) || explicitMutationName(fn.Name) || isUICommandHelperName(file, fn.Name) || isDomainSideEffectBoundaryName(fn.Name) { return false } diff --git a/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go b/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go index 9ab69a09..a6a3b09c 100644 --- a/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go +++ b/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go @@ -2,6 +2,7 @@ package quality import ( "fmt" + "path/filepath" "regexp" "sort" "strings" @@ -62,7 +63,7 @@ func additionalPrecisionFunctionFindings(env support.Context, file string, fn pr 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)) } - if !isUIHelperOrMappingContext(file, fn) && !isSeedOrScriptSourcePath(file) && partialResult(fn) { + if !isUIHelperOrMappingContext(file, fn) && !isSeedOrScriptSourcePath(file) && partialResult(file, fn) { 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)) } @@ -452,10 +453,13 @@ func inconsistentReturnContract(fn precisionFunction) bool { if explicitNullableReturnContract(fn) { return false } + if strings.EqualFold(strings.TrimSpace(fn.Signature), "error") { + return false + } if standardGoResultErrorContract(fn) && !returnsNonZeroValueWithError(fn) { return false } - returns := returnCategories(fn.Body) + returns := returnCategories(fn) if returns.total < 2 { return false } @@ -508,13 +512,16 @@ type returnShapeCounts struct { value bool } -func returnCategories(body string) returnShapeCounts { +func returnCategories(fn precisionFunction) returnShapeCounts { out := returnShapeCounts{} - for _, match := range returnLinePattern.FindAllStringSubmatch(body, -1) { + for _, match := range returnLinePattern.FindAllStringSubmatchIndex(fn.Body, -1) { + if len(match) < 4 || returnLineIsNested(fn, bodyOffsetLine(fn, match[0])) { + continue + } out.total++ expr := "" - if len(match) > 1 { - expr = strings.TrimSpace(match[1]) + if match[2] >= 0 { + expr = strings.TrimSpace(fn.Body[match[2]:match[3]]) } if expr == "" || isEmptyReturnExpr(expr) { out.empty = true @@ -545,15 +552,24 @@ func isEmptyReturnExpr(expr string) bool { } } -func partialResult(fn precisionFunction) bool { +func partialResult(file string, fn precisionFunction) bool { loweredName := strings.ToLower(fn.Name) - if strings.Contains(loweredName, "partial") || strings.Contains(loweredName, "try") || explicitResultObjectContract(fn) { + if strings.Contains(loweredName, "partial") || strings.Contains(loweredName, "try") || explicitResultObjectContract(fn) || + explicitRepositoryCommandResultContract(file, fn) { + return false + } + if strings.EqualFold(strings.TrimSpace(fn.Signature), "error") { return false } if standardGoResultErrorContract(fn) && !returnsNonZeroValueWithError(fn) { return false } - return partialReturnPattern.MatchString(fn.Body) + for _, match := range partialReturnPattern.FindAllStringIndex(fn.Body, -1) { + if !returnLineIsNested(fn, bodyOffsetLine(fn, match[0])) { + return true + } + } + return false } func standardGoResultErrorContract(fn precisionFunction) bool { @@ -573,13 +589,68 @@ func standardGoResultErrorFirstType(fn precisionFunction) string { return first } +func explicitRepositoryCommandResultContract(file string, fn precisionFunction) bool { + if standardGoResultErrorFirstType(fn) != "bool" { + return false + } + if !hasRepositoryCommandPrefix(fn.Name) || !hasRepositoryCommandContext(file, fn) { + return false + } + return goBoolErrorPathsReturnFalse(fn) +} + +func hasRepositoryCommandPrefix(name string) bool { + for _, prefix := range []string{ + "Create", "Update", "Delete", "Insert", "Upsert", "Save", "Unsave", + "Like", "Unlike", "Record", "Mark", "Set", "Enable", "Disable", + } { + if strings.HasPrefix(name, prefix) { + return true + } + } + return false +} + +func hasRepositoryCommandContext(file string, fn precisionFunction) bool { + path := "/" + strings.ToLower(filepath.ToSlash(file)) + if strings.Contains(path, "/repository") || strings.Contains(path, "/repositories") || + strings.Contains(path, "/storage/") || strings.Contains(path, "/postgres/") { + return true + } + return strings.Contains(strings.ToLower(fn.Receiver), "repository") +} + +func goBoolErrorPathsReturnFalse(fn precisionFunction) bool { + for _, match := range returnLinePattern.FindAllStringSubmatchIndex(fn.Body, -1) { + if len(match) < 4 || match[2] < 0 || returnLineIsNested(fn, bodyOffsetLine(fn, match[0])) { + continue + } + expr := strings.TrimSpace(fn.Body[match[2]:match[3]]) + parts := strings.Split(expr, ",") + if len(parts) < 2 { + continue + } + errExpr := strings.ToLower(strings.TrimSpace(strings.TrimSuffix(strings.Join(parts[1:], ","), ";"))) + if errExpr == "nil" || errExpr == "" { + continue + } + if !strings.Contains(errExpr, "err") && !strings.Contains(errExpr, "error") { + continue + } + if strings.ToLower(strings.TrimSpace(parts[0])) != "false" { + return false + } + } + return true +} + func returnsNonZeroValueWithError(fn precisionFunction) bool { firstType := standardGoResultErrorFirstType(fn) - for _, match := range returnLinePattern.FindAllStringSubmatch(fn.Body, -1) { - if len(match) < 2 { + for _, match := range returnLinePattern.FindAllStringSubmatchIndex(fn.Body, -1) { + if len(match) < 4 || match[2] < 0 || returnLineIsNested(fn, bodyOffsetLine(fn, match[0])) { continue } - expr := strings.TrimSpace(match[1]) + expr := strings.TrimSpace(fn.Body[match[2]:match[3]]) parts := strings.Split(expr, ",") if len(parts) < 2 { continue @@ -596,6 +667,20 @@ func returnsNonZeroValueWithError(fn precisionFunction) bool { return false } +func returnLineIsNested(fn precisionFunction, line int) bool { + return callInNestedFunction(fn, line) +} + +func bodyOffsetLine(fn precisionFunction, offset int) int { + if offset < 0 { + return fn.StartLine + } + if offset > len(fn.Body) { + offset = len(fn.Body) + } + return fn.StartLine + strings.Count(fn.Body[:offset], "\n") +} + func isGoZeroReturnExpr(resultType string, expr string) bool { resultType = strings.TrimSpace(strings.ToLower(resultType)) expr = strings.TrimSpace(strings.TrimSuffix(expr, ";")) @@ -716,10 +801,7 @@ func orchestrationDomainMix(file string, fn precisionFunction) bool { func isBooleanNameCandidate(name string, typ string, fn precisionFunction) bool { if name == fn.Name { - if explicitMutationName(name) || explicitNonBooleanFunctionName(name) { - return false - } - return functionReturnLooksBoolean(fn.Signature) + return false } if isBooleanType(typ) { return true @@ -782,8 +864,9 @@ func isPredicateName(name string) bool { } } for _, suffix := range []string{ - "allowed", "changed", "compatible", "complete", "differs", "equal", - "equals", "forbidden", "included", "matches", "readable", "supported", + "allowed", "changed", "compatible", "complete", "differs", "enabled", + "equal", "equals", "exists", "forbidden", "included", "matches", + "present", "readable", "supported", "valid", } { if strings.HasSuffix(lowered, suffix) { return true diff --git a/internal/codeguard/core/config_rule_types.go b/internal/codeguard/core/config_rule_types.go index cb45b191..acd6c143 100644 --- a/internal/codeguard/core/config_rule_types.go +++ b/internal/codeguard/core/config_rule_types.go @@ -15,9 +15,15 @@ type QualityRulesConfig struct { CoverageDelta CoverageDeltaConfig `json:"coverage_delta,omitempty" yaml:"coverage_delta,omitempty"` CPPTooling CPPToolingConfig `json:"cpp_tooling,omitempty" yaml:"cpp_tooling,omitempty"` LocalPrecision *bool `json:"local_precision,omitempty" yaml:"local_precision,omitempty"` + MaintainabilityHistory MaintainabilityHistoryConfig `json:"maintainability_history,omitempty" yaml:"maintainability_history,omitempty"` Naming QualityNamingConfig `json:"naming,omitempty" yaml:"naming,omitempty"` } +type MaintainabilityHistoryConfig struct { + Enabled *bool `json:"enabled,omitempty" yaml:"enabled,omitempty"` + ReportAsFindings *bool `json:"report_as_findings,omitempty" yaml:"report_as_findings,omitempty"` +} + // QualityDeadCodeConfig enables toolchain-backed dead-code evidence. It is // intentionally separate from AIChecks.DeadCode, which stays a fast heuristic // pass for obvious unreachable code and AI leftovers. diff --git a/tests/checks/maintainability_history_test.go b/tests/checks/maintainability_history_test.go index 3ca68897..3f0937ad 100644 --- a/tests/checks/maintainability_history_test.go +++ b/tests/checks/maintainability_history_test.go @@ -84,7 +84,10 @@ func partnerFunctionName(path string) string { func TestMaintainabilityHistoryHotspotRulesUseGitEvidence(t *testing.T) { dir := initMaintainabilityHistoryRepo(t) - report := runMaintainabilityDeltaScan(t, qualityPrecisionConfig(dir)) + cfg := qualityPrecisionConfig(dir) + on := true + cfg.Checks.QualityRules.MaintainabilityHistory.ReportAsFindings = &on + report := runMaintainabilityDeltaScan(t, cfg) assertFindingRulePresent(t, report, "Code Quality", "maintainability.hotspot") assertFindingRulePresent(t, report, "Code Quality", "maintainability.high-churn-hotspot") diff --git a/tests/checks/naming_precision_test.go b/tests/checks/naming_precision_test.go index 24cc9df1..9a7e1f97 100644 --- a/tests/checks/naming_precision_test.go +++ b/tests/checks/naming_precision_test.go @@ -153,10 +153,11 @@ func TestNamingBehaviorMismatchWarnsAcrossLanguages(t *testing.T) { 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 {", + "export function evaluate(users: number, user: Array, enabled: boolean, flag: boolean): boolean {", " const active: boolean = enabled === true;", + " const success: boolean = active;", " const isReady = users > 0;", - " return active && isReady;", + " return success && isReady && flag;", "}", }, "\n")) diff --git a/tests/checks/quality_v174_false_positive_test.go b/tests/checks/quality_v174_false_positive_test.go new file mode 100644 index 00000000..9dc2795a --- /dev/null +++ b/tests/checks/quality_v174_false_positive_test.go @@ -0,0 +1,186 @@ +package checks_test + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func TestGoNestedCallbackReturnsDoNotLeakIntoParentReturnContract(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "repository", "save.go"), strings.Join([]string{ + "package repository", + "", + "import (", + "\t\"context\"", + "\t\"fmt\"", + ")", + "", + "type Pool interface{}", + "type Tx interface{}", + "", + "func WithTxValue(ctx context.Context, pool Pool, fn func(Tx) (bool, error)) (bool, error) {", + "\treturn fn(nil)", + "}", + "", + "func SaveThing(ctx context.Context, pool Pool, failed bool, err error) error {", + "\tvalue, err := WithTxValue(ctx, pool, func(tx Tx) (bool, error) {", + "\t\tif failed {", + "\t\t\treturn false, err", + "\t\t}", + "\t\treturn true, nil", + "\t})", + "\tif err != nil {", + "\t\treturn fmt.Errorf(\"save thing: %w\", err)", + "\t}", + "\t_ = value", + "\treturn nil", + "}", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + + assertFindingRuleAbsent(t, report, "Code Quality", "function.inconsistent-return-contract") + assertFindingRuleAbsent(t, report, "Code Quality", "function.partial-result") +} + +func TestGoRepositoryCommandBoolErrorContractIsExplicitMutationBoundary(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "internal", "postgres", "repository", "delete.go"), strings.Join([]string{ + "package repository", + "", + "import (", + "\t\"context\"", + "\t\"fmt\"", + ")", + "", + "type Repository struct { queries Queries }", + "type Queries interface { DeleteThing(context.Context, string) (int64, error) }", + "", + "func (r *Repository) DeleteThing(ctx context.Context, thingID string) (bool, error) {", + "\taffected, err := r.queries.DeleteThing(ctx, thingID)", + "\tif err != nil {", + "\t\treturn false, fmt.Errorf(\"delete thing: %w\", err)", + "\t}", + "\treturn affected > 0, nil", + "}", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + + assertFindingRuleAbsent(t, report, "Code Quality", "function.partial-result") + assertFindingRuleAbsent(t, report, "Code Quality", "function.inconsistent-return-contract") + assertFindingRuleAbsent(t, report, "Code Quality", "function.command-query-mix") +} + +func TestGoPrimitiveObsessionSkipsInterfaceImplementationSignature(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "internal", "postgres", "repository", "collections.go"), strings.Join([]string{ + "package repository", + "", + "import \"context\"", + "", + "type CollectionRepository interface {", + "\tDeleteCollectionItem(ctx context.Context, collectionID string, itemType string, itemID string) (bool, error)", + "}", + "", + "type Repository struct{}", + "", + "func (r *Repository) DeleteCollectionItem(ctx context.Context, collectionID string, itemType string, itemID string) (bool, error) {", + "\treturn false, nil", + "}", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + + assertFindingRuleAbsent(t, report, "Code Quality", "quality.primitive-obsession") +} + +func TestMaintainabilityHistoryDefaultsToAdvisoryArtifactsNotFindings(t *testing.T) { + dir := initMaintainabilityHistoryRepo(t) + + report := runMaintainabilityDeltaScan(t, qualityPrecisionConfig(dir)) + + for _, ruleID := range []string{ + "maintainability.hotspot", + "maintainability.high-churn-hotspot", + "maintainability.repeat-defect-area", + "maintainability.unstable-interface", + "maintainability.change-amplification", + "smell.shotgun-surgery-history", + "smell.divergent-change-history", + } { + assertFindingRuleAbsent(t, report, "Code Quality", ruleID) + } + if history := maintainabilityHistoryArtifact(report); history == nil || len(history.Hotspots) == 0 { + t.Fatalf("expected maintainability history advisory artifact, got %#v", report.Artifacts) + } +} + +func TestBooleanNotPredicateRequiresStrongBooleanValueEvidence(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "packages", "api", "src", "booleans.ts"), strings.Join([]string{ + "export function RequireCurrentUser(req: Request): User {", + " return req.user;", + "}", + "export function RawStringField(value: string): Field {", + " return { value };", + "}", + "export function fileExists(path: string): boolean {", + " return path.length > 0;", + "}", + "export function boolFromAny(value: unknown): boolean {", + " return Boolean(value);", + "}", + "export function ipInCIDRs(ip: string, cidrs: string[]): boolean {", + " return cidrs.includes(ip);", + "}", + "export function lookupUser(id: string): [User, boolean] {", + " return [{ id }, true];", + "}", + "interface Request { user: User }", + "interface User { id: string }", + "interface Field { value: string }", + }, "\n")) + writeFile(t, filepath.Join(dir, "internal", "postgres", "repository", "commands.go"), strings.Join([]string{ + "package repository", + "", + "import \"context\"", + "", + "type Repository struct{}", + "", + "func (r *Repository) SaveThing(ctx context.Context, thingID string) (bool, error) {", + "\treturn true, nil", + "}", + }, "\n")) + + cfg := codeguard.ExampleConfig() + cfg.Name = "boolean-retune" + cfg.Targets = []codeguard.TargetConfig{ + {Name: "ts", Path: dir, Language: "typescript"}, + {Name: "go", Path: dir, Language: "go"}, + } + cfg.Checks.Quality = true + cfg.Checks.Design = false + cfg.Checks.Security = false + cfg.Checks.Prompts = false + cfg.Checks.CI = false + off := false + cfg.Checks.Context = &off + cfg.Cache.Enabled = &off + + report := runQualityPrecisionScan(t, cfg) + + assertFindingRuleAbsent(t, report, "Code Quality", "naming.boolean-not-predicate") +} + +func maintainabilityHistoryArtifact(report codeguard.Report) *codeguard.PRHotspotsArtifact { + for _, artifact := range report.Artifacts { + if artifact.Kind == "maintainability_history" && artifact.PRHotspots != nil { + return artifact.PRHotspots + } + } + return nil +} diff --git a/tests/checks/smell_history_test.go b/tests/checks/smell_history_test.go index 7c6ada55..97873e0a 100644 --- a/tests/checks/smell_history_test.go +++ b/tests/checks/smell_history_test.go @@ -8,7 +8,10 @@ import ( func TestSmellHistoryRulesUseCoChangeEvidence(t *testing.T) { dir := initMaintainabilityHistoryRepo(t) - report := runMaintainabilityDeltaScan(t, qualityPrecisionConfig(dir)) + cfg := qualityPrecisionConfig(dir) + on := true + cfg.Checks.QualityRules.MaintainabilityHistory.ReportAsFindings = &on + report := runMaintainabilityDeltaScan(t, cfg) assertFindingRulePresent(t, report, "Code Quality", "smell.shotgun-surgery-history") assertFindingRulePresent(t, report, "Code Quality", "smell.divergent-change-history") @@ -28,7 +31,10 @@ func TestSmellHistoryRulesUseCoChangeEvidence(t *testing.T) { func TestChangeAmplificationDeterministicMetadataOrdering(t *testing.T) { dir := initMaintainabilityHistoryRepo(t) - report := runMaintainabilityDeltaScan(t, qualityPrecisionConfig(dir)) + cfg := qualityPrecisionConfig(dir) + on := true + cfg.Checks.QualityRules.MaintainabilityHistory.ReportAsFindings = &on + report := runMaintainabilityDeltaScan(t, cfg) finding := findFinding(t, report, "Code Quality", "maintainability.change-amplification") if got := finding.Metadata["top_partners"]; !strings.HasPrefix(got, "partner_a.go:") { From a9d7403e1de7bde90ecd92593201fdec34811575 Mon Sep 17 00:00:00 2001 From: alex <53851759+alxxjohn@users.noreply.github.com> Date: Sat, 29 Aug 2026 15:46:16 -0400 Subject: [PATCH 2/2] fix lint after boolean naming retune --- .../quality_precision_workstreams_cd.go | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go b/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go index a6a3b09c..9c7968cf 100644 --- a/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go +++ b/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go @@ -809,31 +809,6 @@ func isBooleanNameCandidate(name string, typ string, fn precisionFunction) bool return false } -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