Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions docs/adr/003-cross-tool-contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,30 @@ have:
audit does not yet implement staleness heuristics (keyword/path/minFileCount).
This is a placeholder for parity, not a promise it will be populated soon.

`ScanResult.migrationInventory` mirrors flaglint-js's `MigrationInventoryItem`
field-for-field (`file`, `line`, `column`, `launchDarklyMethod`,
`callExpression`, `rangeStart`, `rangeEnd`, `flagKeyExpression`,
`staticFlagKey`, `isDynamic`, `valueType`, `fallbackExpression`,
`evaluationContextExpression`, `safelyAutomatable`, `manualReviewReason`) —
richer, migration-focused detail a future `migrate` command would need to
safely rewrite a call, not just report it (flaglint-go has no `migrate`
command yet — see ADR 002's Phase 1 scope). One deliberate omission:
flaglint-js's `isAwaited?: boolean` has no Go equivalent (no async/await) and
is never emitted. Unlike flaglint-js (whose generic `variation()`/
`isFeatureEnabled()` calls require inferring `valueType` from the fallback
argument's runtime type), every Go SDK method name is already type-specific,
so `valueType` is always derived directly from the method name — flaglint-go
never actually produces `"unknown-fallback"` as a `manualReviewReason` as a
result (kept in the type for cross-tool consistency regardless). A
`--strict-types`-only usage detected via the "forwarding function" pattern
(ADR 006) has no corresponding `migrationInventory` item: its call site
doesn't directly show the LD method's own `(key, context, fallback)`
arguments, so an item for it would misrepresent what's actually safe to
rewrite. Every other usage — Phase 1 and interface-satisfaction/transitive-
factory-wrapping `--strict-types` findings alike — gets one, since their call
sites are all ordinary `client.Method(key, context, fallback)` shapes
regardless of how the receiver's identity was proven.

## Consequences

- Any PR to flaglint-go that changes fingerprint format, exit codes, config
Expand Down
4 changes: 3 additions & 1 deletion internal/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,9 @@ func TestCLI_emptyScanProducesEmptyArraysNotNull(t *testing.T) {
if err != nil {
t.Fatalf("scan failed: %v", err)
}
for _, field := range []string{`"uniqueFlags":null`, `"usages":null`, `"warnings":null`} {
// json.MarshalIndent inserts a space after the colon, so the check must
// match that exactly, not compact-JSON spacing.
for _, field := range []string{`"uniqueFlags": null`, `"usages": null`, `"warnings": null`, `"migrationInventory": null`} {
if strings.Contains(string(out), field) {
t.Errorf("output contains %q — empty results must serialize as [], not null:\n%s", field, out)
}
Expand Down
22 changes: 19 additions & 3 deletions internal/reporter/reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,32 @@ func TestRender_unsupportedFormat(t *testing.T) {
}

func TestRender_json_emptyArraysAreNotNull(t *testing.T) {
out, err := Render(types.ScanResult{}, Options{Format: FormatJSON})
// Explicit empty (non-nil) slices, matching what Scan() itself actually
// produces for a clean scan — NOT a bare types.ScanResult{}, whose
// zero-value slice fields are nil by construction and would marshal as
// `null` regardless of any tag on these fields; that would make this
// test unable to ever catch the real regression it exists to guard
// against: an accidentally-added `omitempty` tag on one of these
// fields, which turns a non-nil empty slice's `[]` output into total
// field omission instead.
result := types.ScanResult{
UniqueFlags: []string{},
Usages: []types.FlagUsage{},
Warnings: []types.ScanWarning{},
MigrationInventory: []types.MigrationInventoryItem{},
}
out, err := Render(result, Options{Format: FormatJSON})
if err != nil {
t.Fatalf("Render() error = %v", err)
}
// A raw string check, not just json.Unmarshal round-tripping: Go's
// decoder would happily turn a `null` array back into an empty slice,
// masking the exact bug this guards (nil slices marshaling as `null`
// instead of `[]`, which breaks a consumer's `.map()`/`jq` pipeline on
// the JSON *text* itself).
for _, field := range []string{`"uniqueFlags":null`, `"usages":null`, `"warnings":null`} {
// the JSON *text* itself). json.MarshalIndent inserts a space after
// the colon, so the check must match that exactly, not compact-JSON
// spacing.
for _, field := range []string{`"uniqueFlags": null`, `"usages": null`, `"warnings": null`, `"migrationInventory": null`} {
if strings.Contains(out, field) {
t.Errorf("output contains %q — empty slices must marshal as [], not null, to match flaglint-js's array semantics:\n%s", field, out)
}
Expand Down
132 changes: 132 additions & 0 deletions internal/scanner/migration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
// Package scanner (this file): builds MigrationInventoryItem records —
// migrationInventory is a richer, migration-focused view of a call site
// than FlagUsage, additive to flaglint-go's JSON output for cross-tool
// parity with flaglint-js (see docs/adr/003-cross-tool-contract.md and
// flaglint-js's src/scanner/index.ts, which this deliberately mirrors
// field-for-field). flaglint-go has no `migrate` command yet (ADR 002's
// Phase 1 scope), so nothing in this package *consumes* these records —
// they exist so a future migrate command (or another tool reading this
// JSON) has the same rewrite-relevant detail flaglint-js already provides.
package scanner

import (
"go/ast"
"go/token"

"github.com/flaglint/flaglint-go/internal/types"
)

// migrationValueTypes maps each known LaunchDarkly Go SDK method name to
// the OpenFeature evaluation-method category its return value maps to.
// Unlike flaglint-js (whose generic variation()/isFeatureEnabled() calls
// require inferring this from the fallback argument's runtime type), every
// Go SDK method name already fully determines this — no fallback
// inspection needed, and "unknown-fallback" (a real flaglint-js reason)
// can never actually be produced here as a result.
var migrationValueTypes = map[string]types.MigrationValueType{
"BoolVariation": types.MigrationValueBoolean, "BoolVariationCtx": types.MigrationValueBoolean,
"BoolVariationDetail": types.MigrationValueBoolean, "BoolVariationDetailCtx": types.MigrationValueBoolean,
"StringVariation": types.MigrationValueString, "StringVariationCtx": types.MigrationValueString,
"StringVariationDetail": types.MigrationValueString, "StringVariationDetailCtx": types.MigrationValueString,
"IntVariation": types.MigrationValueNumber, "IntVariationCtx": types.MigrationValueNumber,
"IntVariationDetail": types.MigrationValueNumber, "IntVariationDetailCtx": types.MigrationValueNumber,
"Float64Variation": types.MigrationValueNumber, "Float64VariationCtx": types.MigrationValueNumber,
"Float64VariationDetail": types.MigrationValueNumber, "Float64VariationDetailCtx": types.MigrationValueNumber,
"JSONVariation": types.MigrationValueObject, "JSONVariationCtx": types.MigrationValueObject,
"JSONVariationDetail": types.MigrationValueObject, "JSONVariationDetailCtx": types.MigrationValueObject,
}

// migrationDetailMethods is every "*VariationDetail(Ctx)" method name —
// always manual-review (returns ldreason.EvaluationDetail, no direct
// OpenFeature equivalent), matching flaglint-js's detail-method reason.
var migrationDetailMethods = map[string]bool{
"BoolVariationDetail": true, "BoolVariationDetailCtx": true,
"StringVariationDetail": true, "StringVariationDetailCtx": true,
"IntVariationDetail": true, "IntVariationDetailCtx": true,
"Float64VariationDetail": true, "Float64VariationDetailCtx": true,
"JSONVariationDetail": true, "JSONVariationDetailCtx": true,
}

// exprText extracts node's exact source text and byte-offset range from
// src, using fset to convert AST positions to offsets. Returns ok == false
// for a nil node or a position outside src's bounds (never expected in
// practice — pos/end always come from nodes parsed out of src itself — but
// checked rather than assumed, since a malformed range would otherwise
// panic on the slice below).
func exprText(fset *token.FileSet, src []byte, node ast.Expr) (text string, start, end int, ok bool) {
if node == nil {
return "", 0, 0, false
}
startPos := fset.Position(node.Pos())
endPos := fset.Position(node.End())
if startPos.Offset < 0 || endPos.Offset > len(src) || startPos.Offset > endPos.Offset {
return "", 0, 0, false
}
return string(src[startPos.Offset:endPos.Offset]), startPos.Offset, endPos.Offset, true
}

// buildMigrationInventoryItem builds one MigrationInventoryItem for call,
// a confirmed LaunchDarkly Go SDK call site (spec/callTypeName already
// resolved by the caller — see fileDetector.detect, scanner.go). src is
// this file's raw source bytes (parsedFile.src), needed to extract each
// expression's exact source text alongside its already-parsed AST shape.
func buildMigrationInventoryItem(fset *token.FileSet, src []byte, relPath string, call *ast.CallExpr, spec methodSpec, callTypeName string, pos token.Position, flagKey string, isDynamic bool) types.MigrationInventoryItem {
item := types.MigrationInventoryItem{
File: relPath,
Line: pos.Line,
Column: pos.Column,
LaunchDarklyMethod: types.CallType(callTypeName),
IsDynamic: isDynamic,
}
if text, start, end, ok := exprText(fset, src, call); ok {
item.CallExpression = text
item.RangeStart = start
item.RangeEnd = end
}

// Bulk call (AllFlagsState) — no flag key; args[0] is the evaluation
// context, matching flaglint-js's identical handling of its bulk-call
// equivalent.
if spec.keyArgIndex == -1 {
item.ValueType = types.MigrationValueUnknown
if len(call.Args) > 0 {
if text, _, _, ok := exprText(fset, src, call.Args[0]); ok {
item.EvaluationContextExpression = text
}
}
item.ManualReviewReason = types.MigrationReasonBulkInventoryCall
return item
}

if spec.keyArgIndex < len(call.Args) {
if text, _, _, ok := exprText(fset, src, call.Args[spec.keyArgIndex]); ok {
item.FlagKeyExpression = text
}
}
if !isDynamic {
item.StaticFlagKey = flagKey
}
if ctxIdx := spec.keyArgIndex + 1; ctxIdx < len(call.Args) {
if text, _, _, ok := exprText(fset, src, call.Args[ctxIdx]); ok {
item.EvaluationContextExpression = text
}
}
if fallbackIdx := spec.keyArgIndex + 2; fallbackIdx < len(call.Args) {
if text, _, _, ok := exprText(fset, src, call.Args[fallbackIdx]); ok {
item.FallbackExpression = text
}
}

item.ValueType = migrationValueTypes[callTypeName]

switch {
case isDynamic:
item.ManualReviewReason = types.MigrationReasonDynamicKey
case migrationDetailMethods[callTypeName]:
item.ManualReviewReason = types.MigrationReasonDetailMethod
default:
item.SafelyAutomatable = true
}

return item
}
48 changes: 36 additions & 12 deletions internal/scanner/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ type parsedFile struct {
file *ast.File
imports sdkImports

// src is this file's raw source bytes, kept alongside its already-
// parsed AST so migrationInventory (migration.go) can extract each
// call/argument's exact source text — go/parser discards the original
// text once parsing succeeds, so this has to be retained separately if
// anything downstream needs it.
src []byte

// typesInfo is real go/types information for this file's package, only
// populated by ScanStrict (strict.go) — nil for an ordinary Scan. See
// fileContext.typesInfo (identity.go) and docs/adr/005-strict-types-
Expand Down Expand Up @@ -87,21 +94,23 @@ func Scan(root string, cfg config.Config) (types.ScanResult, error) {
relPath: rel,
dir: filepath.Dir(full),
file: file,
src: src,
imports: traceSDKImports(file),
})
}

allUsages := runWholeScanAnalysis(fset, parsed)
allUsages, allMigrationInventory := runWholeScanAnalysis(fset, parsed)

return types.ScanResult{
ScannedAt: time.Now().UTC().Format(time.RFC3339),
ScanRoot: absRoot,
ScannedFiles: len(relFiles),
TotalUsages: len(allUsages),
UniqueFlags: uniqueFlags(allUsages),
Usages: allUsages,
ScanDurationMs: time.Since(start).Milliseconds(),
Warnings: warnings,
ScannedAt: time.Now().UTC().Format(time.RFC3339),
ScanRoot: absRoot,
ScannedFiles: len(relFiles),
TotalUsages: len(allUsages),
UniqueFlags: uniqueFlags(allUsages),
Usages: allUsages,
ScanDurationMs: time.Since(start).Milliseconds(),
Warnings: warnings,
MigrationInventory: allMigrationInventory,
}, nil
}

Expand All @@ -114,7 +123,7 @@ func Scan(root string, cfg config.Config) (types.ScanResult, error) {
// client behind a struct field, a factory/getter function, or a
// multi-level field chain declared in a different file (sometimes a
// different package) than where it's used.
func runWholeScanAnalysis(fset *token.FileSet, parsed []parsedFile) []types.FlagUsage {
func runWholeScanAnalysis(fset *token.FileSet, parsed []parsedFile) ([]types.FlagUsage, []types.MigrationInventoryItem) {
// Pre-pass 1: package identity. ownPkgKey identifies each file's own
// package (for same-package bare factory calls); importPathToPkgKey and
// importPathToPkgName let OTHER files resolve a qualified
Expand Down Expand Up @@ -305,10 +314,11 @@ func runWholeScanAnalysis(fset *token.FileSet, parsed []parsedFile) []types.Flag
// literal in one file/function can make a field binding visible to a
// different file processed earlier in this loop.
var allUsages []types.FlagUsage
var allMigrationInventory []types.MigrationInventoryItem
for _, pf := range parsed {
ctx := ctxs[pf.file]
pkg := pkgBindings(base, ctx.ownPkgKey)
d := &fileDetector{fset: fset, relPath: pf.relPath}
d := &fileDetector{fset: fset, relPath: pf.relPath, src: pf.src}
for _, s := range fileScopes[pf.file] {
// packageVarTypes merged in first (mergeBindings's later
// argument wins on collision) so a local parameter of the
Expand All @@ -318,12 +328,16 @@ func runWholeScanAnalysis(fset *token.FileSet, parsed []parsedFile) []types.Flag
d.detect(s.body, mergeBindings(pkg, s.paramBindings), declared, ctx.structFieldTypes, ctx)
}
allUsages = append(allUsages, d.usages...)
allMigrationInventory = append(allMigrationInventory, d.migrationInventory...)
}

if allUsages == nil {
allUsages = []types.FlagUsage{}
}
return allUsages
if allMigrationInventory == nil {
allMigrationInventory = []types.MigrationInventoryItem{}
}
return allUsages, allMigrationInventory
}

// pkgBindings returns byPkg's inner map for pkgKey, creating it on first
Expand Down Expand Up @@ -406,6 +420,14 @@ type fileDetector struct {
relPath string
dynamicIndex int
usages []types.FlagUsage

// src is this file's raw source bytes (parsedFile.src), used to build
// each usage's corresponding MigrationInventoryItem — see migration.go.
// May be nil (e.g. a read failure in strict.go's own re-read for the
// --strict-types pass): buildMigrationInventoryItem's exprText calls
// degrade safely to missing expression text in that case, never a panic.
src []byte
migrationInventory []types.MigrationInventoryItem
}

// bindings is the scope's initial bindings (whole-scan package bindings
Expand Down Expand Up @@ -490,6 +512,7 @@ func (d *fileDetector) detect(scope ast.Node, bindings, declared, structFieldTyp
SDK: sdk,
Risk: spec.risk,
})
d.migrationInventory = append(d.migrationInventory, buildMigrationInventoryItem(d.fset, d.src, d.relPath, call, spec, callTypeName, pos, "*", false))
return
}

Expand Down Expand Up @@ -519,6 +542,7 @@ func (d *fileDetector) detect(scope ast.Node, bindings, declared, structFieldTyp
SDK: sdk,
Risk: riskFor(spec, isDynamic),
})
d.migrationInventory = append(d.migrationInventory, buildMigrationInventoryItem(d.fset, d.src, d.relPath, call, spec, callTypeName, pos, flagKey, isDynamic))
})
}

Expand Down
Loading
Loading