fix: avoid panics on Jinja-templated SQL with config()/source() blocks#2571
Open
kimfrie wants to merge 1 commit into
Open
fix: avoid panics on Jinja-templated SQL with config()/source() blocks#2571kimfrie wants to merge 1 commit into
kimfrie wants to merge 1 commit into
Conversation
`ReflowPoint::indent_to` (crates/lib/src/utils/reflow/elements.rs)
walks backwards through segments to find the last *literal* newline to
anchor a whitespace fix on. When every newline came from a Jinja
template block (e.g. `{{ config(...) }}` combined with `{{ ref() }}` /
`{{ source() }}` on a referenced table), no literal newline exists
and the `unwrap()` panicked with `Option::unwrap() on a None value`
at `elements.rs:272`. Return no-fix instead.
Once that panic is gone, `fix` (not `lint`) surfaces a second panic
in `ErasedSegment::iter_patches` (crates/lib-core/src/parser/segments.rs)
with `begin > end (788 > 609) when slicing source_str` — a literal
segment with a malformed source/templated slice. Skip the patch when
the slice is invalid rather than panicking.
Verified against 13 real-world dbt/jinja models (redshift dialect,
`apply_dbt_builtins = True`) that previously panicked 11–60× per file
under sqruff 0.37.3: all now lint cleanly and `fix` completes.
Fixes quarylabs#2464
20485e8 to
30bc3e4
Compare
Author
|
@benfdking would you help me with a review? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Option::unwrap() on a None valuepanic atcrates/lib/src/utils/reflow/elements.rs:272that occurs when every newline in aReflowPointcomes from a Jinja template block (e.g. the source combines{{ config(...) }}with{{ ref() }}/{{ source() }}on a referenced table).indent_tonow emits no fix and returns the point unchanged, rather than crashing the linter run.get_position_marker().unwrap()used while matching the literal newline.Fixes #2464.
Reproduction
Using the minimal example from #2464 (dbt templater, bigquery dialect):
{{ config(materialized='table') }} with raw_data as ( select * from {{ source('conn', 'tbl') }} ) select * from raw_dataBefore:
thread '<unnamed>' panicked at crates/lib/src/utils/reflow/elements.rs:272:22: called Option::unwrap() on a None value.After: linter completes with ordinary LT-rule violations, no panic.
Verified locally against 12 real-world dbt/jinja models (redshift dialect,
apply_dbt_builtins = True) that each reproduced the panic on sqruff 0.37.3 — all now lint cleanly.Test plan
cargo test --release --features python -p sqruff-lib --lib reflow— all 10 reflow tests pass🤖 Generated with Claude Code