chore: Per-element precision for literal destructuring - #86
Conversation
📝 WalkthroughWalkthroughThe TypeScript parser now attributes destructured symbols to individual initializer elements or object properties when static mapping is safe. Nested bindings map recursively. Unsupported or ambiguous cases use the full initializer span. The project version is 0.25.0. ChangesDestructured taint attribution
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DeclarationHandling as Destructured declaration handling
participant DestructuredBindings as destructuredBindings
participant MappingHelpers as Initializer mapping helpers
DeclarationHandling->>DestructuredBindings: Extract destructured bindings
DestructuredBindings->>MappingHelpers: Map bindings to initializer elements or properties
MappingHelpers-->>DestructuredBindings: Return specific spans or full initializer fallback
DestructuredBindings-->>DeclarationHandling: Create attributed symbols
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/tsparse/tsparse.go`:
- Around line 528-543: Update the source-span calculation in the
binding-resolution logic around be.Initializer so mapped bindings with default
initializers extend end through the initializer expression. Preserve the
existing source-expression mapping for object and array bindings, while ensuring
initializer expressions are included in the span used by
findTaintedSymbolsByUsage and taint detection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 42deaa20-cd1e-41c9-a78a-2f96f199e084
📒 Files selected for processing (3)
CHANGELOG.mdVERSIONinternal/tsparse/tsparse.go
| // Resolve the source expression + span for this binding. | ||
| var src *ast.Node | ||
| start, end := fbStart, fbEnd | ||
| if be.DotDotDotToken == nil && mappable { | ||
| if isArray { | ||
| if idx < len(arrElems) { | ||
| src = arrElems[idx] | ||
| } | ||
| } else if key := bindingSourceKey(be); key != "" { | ||
| src = objVals[key] | ||
| } | ||
| if src != nil && !ast.IsOmittedExpression(src) { | ||
| start = posToLine(scanner.SkipTrivia(text, src.Pos()), lineMap) | ||
| end = posToLine(src.End(), lineMap) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the BindingElement definition in the vendored tsgo AST.
fd -e go -g '*ast*' --exec rg -nP -A 12 'type\s+BindingElement\s+struct'Repository: gooddata/gooddata-goodchanges
Length of output: 167
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- module metadata ---'
find . -maxdepth 3 -type f \( -name 'go.mod' -o -name 'go.sum' \) -print
printf '%s\n' '--- relevant files ---'
git ls-files | rg '(^|/)(internal/tsparse/tsparse\.go|internal/analyzer/analyzer\.go|.*ts.*ast.*|go\.mod|go\.sum)$' || true
printf '%s\n' '--- symbols ---'
rg -n -S 'BindingElement|collectBindings|findTaintedSymbolsByUsage' . --glob '*.go' --glob '!vendor/**' || trueRepository: gooddata/gooddata-goodchanges
Length of output: 2499
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- go.mod ---'
cat go.mod
printf '%s\n' '--- collectBindings context ---'
sed -n '470,570p' internal/tsparse/tsparse.go
printf '%s\n' '--- binding helper context ---'
sed -n '590,625p' internal/tsparse/tsparse.go
printf '%s\n' '--- taint usage function ---'
sed -n '980,1035p' internal/analyzer/analyzer.go
printf '%s\n' '--- dependency references ---'
rg -n -S 'tsgo|typescript-go|github.com/.*/go-ts' go.mod go.sum internal --glob '*.go' || true
printf '%s\n' '--- Go module cache candidates ---'
go env GOPATH GOMODCACHE 2>/dev/null || trueRepository: gooddata/gooddata-goodchanges
Length of output: 7710
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- vendor root ---'
find _vendor/typescript-go -maxdepth 3 -type f | head -80
printf '%s\n' '--- BindingElement definitions and references ---'
rg -n -S -A 18 -B 5 'type[[:space:]]+BindingElement[[:space:]]+struct|BindingElement struct|Initializer.*BindingElement|func .*BindingElement' _vendor/typescript-go --glob '*.go' || true
printf '%s\n' '--- AST initializer field references ---'
rg -n -S 'BindingElement|\.Initializer' _vendor/typescript-go/pkg/ast --glob '*.go' | head -120 || trueRepository: gooddata/gooddata-goodchanges
Length of output: 581
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- repository top-level entries ---'
find . -maxdepth 2 -type d -print | sort
printf '%s\n' '--- all local BindingElement matches ---'
rg -n -S 'BindingElement|type[[:space:]]+BindingElement' . /home/jailuser/go/pkg/mod /home/jailuser/go/pkg/mod/cache 2>/dev/null | head -200 || true
printf '%s\n' '--- module cache directories matching typescript or tsgo ---'
find /home/jailuser/go/pkg/mod -maxdepth 5 -type d \( -iname '*typescript*' -o -iname '*tsgo*' \) -print 2>/dev/null | head -100
printf '%s\n' '--- git tracked replacement metadata ---'
git ls-files | rg '(^|/)(_vendor|vendor|typescript-go|go\.mod|go\.sum)' || trueRepository: gooddata/gooddata-goodchanges
Length of output: 1184
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- vendor-tsgo.sh ---'
cat vendor-tsgo.sh
printf '%s\n' '--- concise Git metadata ---'
git status --short
git log -1 --format='%h %s'
printf '%s\n' '--- dependency strings in repository metadata ---'
rg -n -S 'typescript-go|tsgo|goodchanges/tsgo-vendor|github.com' .github vendor-tsgo.sh go.mod go.sum .git/config 2>/dev/null || trueRepository: gooddata/gooddata-goodchanges
Length of output: 7742
🌐 Web query:
microsoft typescript-go AST BindingElement struct Initializer Node source
💡 Result:
In the microsoft/typescript-go project, a BindingElement is represented as a struct within the ast package [1][2][3]. This node type is part of the AST (Abstract Syntax Tree) used to represent binding elements within object or array binding patterns [2]. A BindingElement struct typically includes an Initializer field, which is an optional node (or pointer to a Node) representing the default value assigned to the binding element if the property or array element is missing or undefined during destructuring [1]. Key technical aspects include: - The Initializer field allows for default value assignment, similar to the TypeScript compiler's BindingElement interface [4][5]. - Transformer utilities in the project frequently check for the presence of this Initializer to convert binding patterns into standard assignment expressions during code generation [1]. - The node structure is designed to support both object and array destructuring, with methods like convertBindingElementToArrayAssignmentElement and convertBindingElementToObjectAssignmentElement handling the transformation of these elements [1]. - The AST node also holds other essential properties such as the Name (the binding name) and the optional PropertyName (used in object binding patterns) [1][2][3]. This implementation mirrors the TypeScript compiler's underlying AST, where BindingElement is defined as an interface that includes a name, an optional property name, an optional initializer, and a dotDotDotToken for rest elements [4][5].
Citations:
- 1: https://github.com/microsoft/typescript-go/blob/582e066c/internal/transformers/utilities.go
- 2: https://github.com/microsoft/typescript-go/blob/2c251880/internal/ast/utilities.go
- 3: https://github.com/microsoft/typescript-go/blob/48e73d6a2d582b28137fc4bd125b2f0b9521095d/internal/ast/utilities.go
- 4: https://github.com/microsoft/TypeScript/blob/a9c89289000a74a39601dba586aa4c5bb2f2890e/src/compiler/types.ts
- 5: https://thejohnfreeman.github.io/TypeScript/interfaces/export_.nodefactory.html
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
examples = {
"object": "const { a = compute() } = { a: p }\n",
"array": "const [a = fallbackVal] = [p]\n",
}
for kind, text in examples.items():
lines = text.splitlines()
source = lines[0]
if kind == "object":
mapped = "p"
default = "compute()"
else:
mapped = "p"
default = "fallbackVal"
mapped_start = source.index(mapped)
mapped_end = mapped_start + len(mapped)
default_start = source.index(default)
default_end = default_start + len(default)
mapped_text = source[mapped_start:mapped_end]
print(f"{kind}: mapped span text={mapped_text!r}, default={source[default_start:default_end]!r}, "
f"default_in_mapped_span={default_start >= mapped_start and default_end <= mapped_end}, "
f"tainted_default_detected={default in mapped_text}")
PYRepository: gooddata/gooddata-goodchanges
Length of output: 392
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
examples = {
"object": "const { a = compute() } = { a: p }\n",
"array": "const [a = fallbackVal] = [p]\n",
}
for kind, text in examples.items():
source = text.splitlines()[0]
mapped = "p"
default = "compute()" if kind == "object" else "fallbackVal"
mapped_start = source.index(mapped)
mapped_end = mapped_start + len(mapped)
default_start = source.index(default)
default_end = default_start + len(default)
mapped_text = source[mapped_start:mapped_end]
print(
f"{kind}: mapped span text={mapped_text!r}, "
f"default={source[default_start:default_end]!r}, "
f"default_in_mapped_span="
f"{default_start >= mapped_start and default_end <= mapped_end}, "
f"tainted_default_detected={default in mapped_text}"
)
PYRepository: gooddata/gooddata-goodchanges
Length of output: 392
Include binding default initializers in the recorded span.
When be.Initializer is present, extend the mapped span to include that expression. Otherwise, compute() in const { a = compute() } = { a: p } and fallbackVal in const [a = fallbackVal] = [p] are excluded from findTaintedSymbolsByUsage and taint detection misses them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/tsparse/tsparse.go` around lines 528 - 543, Update the source-span
calculation in the binding-resolution logic around be.Initializer so mapped
bindings with default initializers extend end through the initializer
expression. Preserve the existing source-expression mapping for object and array
bindings, while ensuring initializer expressions are included in the span used
by findTaintedSymbolsByUsage and taint detection.
2 participants
Risk: low
Summary by CodeRabbit
New Features
Documentation