Skip to content

[K9VULN-13253] Panic on error node fix#878

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits intomainfrom
jdelgo/panic-on-error-fix
Apr 22, 2026
Merged

[K9VULN-13253] Panic on error node fix#878
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits intomainfrom
jdelgo/panic-on-error-fix

Conversation

@jdelgo
Copy link
Copy Markdown
Contributor

@jdelgo jdelgo commented Apr 15, 2026

What problem are you trying to solve?

The analyzer panics when processing Python files with invalid nodes

What is your solution?

parse_field_child_node now returns Option<MaybeAliased> instead of panicking on unexpected node types. All 3 callers handle None gracefully

Alternatives considered

An alternative was to skip files at the tree level by checking has_error() and has_missing() on the root node in analyze.rs but this was too broad as it would skip an entire file for any parse error, even when most of the file is valid

What the reviewer should know

Added two unit tests that first assert the input has an ERROR node and then verifies that no panic occurs and the correct imports are returned

@datadog-datadog-prod-us1
Copy link
Copy Markdown

datadog-datadog-prod-us1 Bot commented Apr 15, 2026

🎯 Code Coverage (details)
Patch Coverage: 97.56%
Overall Coverage: 85.03% (+0.02%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: b037789 | Docs | Datadog PR Page | Give us feedback!

@jdelgo jdelgo marked this pull request as ready for review April 15, 2026 20:25
@jdelgo jdelgo requested a review from a team as a code owner April 15, 2026 20:25
@jdelgo jdelgo requested review from bahar-shah and Copilot April 15, 2026 20:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a panic in the Python static import analyzer when tree-sitter produces malformed AST nodes (e.g., ERROR nodes without a field name) inside import_statements, ensuring analysis continues and still extracts valid imports.

Changes:

  • Add a runtime guard in parse_import_statement to skip named children that are not in the "name" field (preventing ERROR nodes from reaching parse_field_child_node).
  • Add a unit test covering malformed syntax (import foo, + bar) to ensure parsing does not panic and still returns valid imports.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bahar-shah
bahar-shah previously approved these changes Apr 15, 2026
Copy link
Copy Markdown

@bahar-shah bahar-shah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few small comments

Comment thread crates/static-analysis-kernel/src/analysis/languages/python/imports.rs Outdated
Comment thread crates/static-analysis-kernel/src/analysis/languages/python/imports.rs Outdated
Comment thread crates/static-analysis-kernel/src/analysis/languages/python/imports.rs Outdated
@jdelgo jdelgo force-pushed the jdelgo/panic-on-error-fix branch from 7df1bb1 to 49e09d8 Compare April 16, 2026 22:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jasonforal
jasonforal previously approved these changes Apr 22, 2026
Copy link
Copy Markdown
Collaborator

@jasonforal jasonforal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Small nits, because the language feels a bit Claude-y

Comment thread crates/static-analysis-kernel/src/analysis/languages/python/imports.rs Outdated
Comment thread crates/static-analysis-kernel/src/analysis/languages/python/imports.rs Outdated
Comment thread crates/static-analysis-kernel/src/analysis/languages/python/imports.rs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants