Repository navigation
feat(lang): remove legacy type_builder/dynamic/@@dynamic syntax; E0098 removed-feature diagnostics (BEP-066 s1, PR 2) - #4328
Conversation
…PR 2)
BEP-066 replaces the old TypeBuilder feature; the compiler still parsed
its syntax but dropped it at CST->AST. Remove it fully and make every
legacy form surface a targeted E0098 removed-feature error:
- lexer: drop TokenKind::{TypeBuilder,Dynamic} (both lex as Word now)
- parser: drop KW_TYPE_BUILDER/KW_DYNAMIC mappings, TypeBuilder/Dynamic
recovery+lookahead sets, parse_type_builder_block/parse_dynamic_type_def;
add cheap word-text recoveries (try_recover_removed_type_builder /
try_recover_removed_dynamic_def) at top level, in config items, and in
LLM function bodies: emit ParseError::RemovedFeature (E0098) and swallow
the construct in a balanced-brace ERROR node
- syntax: drop KW_TYPE_BUILDER/KW_DYNAMIC/TYPE_BUILDER_BLOCK/
DYNAMIC_TYPE_DEF and the TypeBuilderBlock/DynamicTypeDef ast nodes
- hir: reject the legacy @@dynamic block attribute on classes/enums
(previously a silent no-op) with E0098 via RemovedDynamicAttribute
- diagnostics: drop never-emitted E0040-E0043 ids; rename
InstanceofRemoved -> RemovedFeature (shared E0098 id); add
ParseError::RemovedFeature; bump bex_cache FORMAT_VERSION 3 -> 4 for
the DiagnosticId discriminant shift
- fmt: drop TypeBuilderBlock/TypeBuilderItem/ConfigBlockMember::TypeBuilder
and the LlmFunctionBody type_builder field
- corpora: convert compiles/type_builder_test into
broken_syntax/removed_type_builder (asserts one E0098 per legacy form),
delete redundant compiles/type_builder_errors, add
diagnostic_errors/removed_dynamic_attribute, strip type_builder sections
from format_checks/config_decls; regenerate LSP fixture expectations
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe compiler removes active support for legacy ChangesRemoved legacy syntax
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
Binary size checks passed✅ 7 passed
Generated by |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
baml_language/crates/baml_lsp2_actions/src/check.rs (1)
492-493: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct unit coverage for
ParseError::RemovedFeature.Add a Rust unit test in
check.rsthat uses legacytype_builder ...inside a function body and asserts that the function scope and descendants are tainted. Runcargo test --libforbaml_language/crates/baml_lsp2_actionsto cover this code path.🤖 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 `@baml_language/crates/baml_lsp2_actions/src/check.rs` around lines 492 - 493, Add a Rust unit test in check.rs targeting ParseError::RemovedFeature by placing legacy type_builder syntax inside a function body, then assert the function scope and its descendants are tainted. Run cargo test --lib for the baml_lsp2_actions crate to verify the coverage.Source: Coding guidelines
baml_language/crates/baml_compiler_parser/src/parser.rs (1)
1331-1388: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate duplicated brace-skipping logic.
skip_balanced_braces(Lines 1390-1406) duplicates the inline brace-depth-tracking loop already present intry_recover_invalid_block(Lines 1367-1384). Both trackbrace_depth, bump onLBrace/RBrace, and stop at end of input. Reuse the new helper intry_recover_invalid_blockinstead of keeping two copies of the same recovery logic. A future fix to one brace-skipping loop (for example, handling an edge case) would otherwise need to be applied to both copies separately.♻️ Proposed refactor to reuse `skip_balanced_braces`
// Skip to matching closing brace - if self.at(TokenKind::LBrace) { - self.bump(); // consume '{' - let mut brace_depth = 1; - - while !self.at_end() && brace_depth > 0 { - match self.current().map(|t| t.kind) { - Some(TokenKind::LBrace) => { - brace_depth += 1; - self.bump(); - } - Some(TokenKind::RBrace) => { - brace_depth -= 1; - self.bump(); - } - _ => self.bump(), - } - } - } + self.skip_balanced_braces();Also applies to: 1390-1406
🤖 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 `@baml_language/crates/baml_compiler_parser/src/parser.rs` around lines 1331 - 1388, Update try_recover_invalid_block to call the existing skip_balanced_braces helper after consuming the opening LBrace, removing its inline brace_depth loop. Preserve the current recovery behavior while centralizing balanced-brace skipping in skip_balanced_braces.
🤖 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 `@baml_language/crates/baml_compiler_parser/src/parser.rs`:
- Around line 1408-1482: Add unit tests in the parser crate’s existing mod tests
covering both try_recover_removed_type_builder and
try_recover_removed_dynamic_def, including brace and colon-brace type_builder
forms plus dynamic class and dynamic enum definitions. Assert recovery produces
the removed-feature diagnostic and consumes the legacy construct without parser
failure, then run cargo test --lib -p baml_compiler_parser.
---
Nitpick comments:
In `@baml_language/crates/baml_compiler_parser/src/parser.rs`:
- Around line 1331-1388: Update try_recover_invalid_block to call the existing
skip_balanced_braces helper after consuming the opening LBrace, removing its
inline brace_depth loop. Preserve the current recovery behavior while
centralizing balanced-brace skipping in skip_balanced_braces.
In `@baml_language/crates/baml_lsp2_actions/src/check.rs`:
- Around line 492-493: Add a Rust unit test in check.rs targeting
ParseError::RemovedFeature by placing legacy type_builder syntax inside a
function body, then assert the function scope and its descendants are tainted.
Run cargo test --lib for the baml_lsp2_actions crate to verify the coverage.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2fc55973-fb9d-4a8b-bf0a-2b03e70f6c6a
⛔ Files ignored due to path filters (21)
baml_language/crates/baml_tests/snapshots/broken_syntax/removed_type_builder/baml_tests__broken_syntax__removed_type_builder__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_errors/baml_tests__compiles__type_builder_errors__03_ppir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_errors/baml_tests__compiles__type_builder_errors__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_errors/baml_tests__compiles__type_builder_errors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_errors/baml_tests__compiles__type_builder_errors__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_errors/baml_tests__compiles__type_builder_errors__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_errors/baml_tests__compiles__type_builder_errors__10_formatter__test.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_test/baml_tests__compiles__type_builder_test__03_ppir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_test/baml_tests__compiles__type_builder_test__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_test/baml_tests__compiles__type_builder_test__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_test/baml_tests__compiles__type_builder_test__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_test/baml_tests__compiles__type_builder_test__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_test/baml_tests__compiles__type_builder_test__10_formatter__test.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/basic_types/baml_tests__diagnostic_errors__basic_types__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/format_checks/baml_tests__diagnostic_errors__format_checks__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/format_checks/baml_tests__diagnostic_errors__format_checks__10_formatter__config_decls.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/removed_dynamic_attribute/baml_tests__diagnostic_errors__removed_dynamic_attribute__03_ppir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/removed_dynamic_attribute/baml_tests__diagnostic_errors__removed_dynamic_attribute__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/removed_dynamic_attribute/baml_tests__diagnostic_errors__removed_dynamic_attribute__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/removed_dynamic_attribute/baml_tests__diagnostic_errors__removed_dynamic_attribute__10_formatter__removed_dynamic_attribute.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/stream_types/baml_tests__diagnostic_errors__stream_types__05_diagnostics.snapis excluded by!**/*.snap
📒 Files selected for processing (26)
baml_language/crates/baml_compiler2_ast/src/lowering_diagnostic.rsbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_compiler2_hir/src/diagnostic.rsbaml_language/crates/baml_compiler_diagnostics/src/diagnostic.rsbaml_language/crates/baml_compiler_diagnostics/src/errors/parse_error.rsbaml_language/crates/baml_compiler_diagnostics/src/to_diagnostic.rsbaml_language/crates/baml_compiler_lexer/src/tokens.rsbaml_language/crates/baml_compiler_parser/src/parser.rsbaml_language/crates/baml_compiler_syntax/src/ast.rsbaml_language/crates/baml_compiler_syntax/src/syntax_kind.rsbaml_language/crates/baml_fmt/src/ast/declarations.rsbaml_language/crates/baml_fmt/src/ast/tokens.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/dynamic_type_builder.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/enum_decls.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/misc/dynamic_types.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/misc/dynamic_types_external_cycle_errors.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/misc/dynamic_types_internal_cycle_errors.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/misc/dynamic_types_parser_errors.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/misc/dynamic_types_validation_errors.bamlbaml_language/crates/baml_tests/projects/broken_syntax/removed_type_builder/removed_type_builder.bamlbaml_language/crates/baml_tests/projects/compiles/type_builder_errors/test.bamlbaml_language/crates/baml_tests/projects/compiles/type_builder_test/test.bamlbaml_language/crates/baml_tests/projects/diagnostic_errors/format_checks/config_decls.bamlbaml_language/crates/baml_tests/projects/diagnostic_errors/removed_dynamic_attribute/removed_dynamic_attribute.bamlbaml_language/crates/bex_cache/src/lib.rs
💤 Files with no reviewable changes (6)
- baml_language/crates/baml_tests/projects/compiles/type_builder_test/test.baml
- baml_language/crates/baml_tests/projects/compiles/type_builder_errors/test.baml
- baml_language/crates/baml_compiler_syntax/src/syntax_kind.rs
- baml_language/crates/baml_fmt/src/ast/tokens.rs
- baml_language/crates/baml_compiler_lexer/src/tokens.rs
- baml_language/crates/baml_tests/projects/diagnostic_errors/format_checks/config_decls.baml
|
@coderabbitai review |
✅ Action performedReview finished.
|
…nd; reflect.type_of removed (BEP-066 s1, PR 3) (#4330) **BEP-066 slice-1 stack, PR 3 of 5** — chained on #4328. ## What - **The move**: `reflect` stops being a root package and becomes the `baml.reflect` namespace (manifest, deps, emit allowlist, native wiring folded into the baml package dispatch, `ARG_FQN` updates). SDK routing needed nothing — no special cases existed; symbols route under `baml/` automatically. - **The shorthand**: bare `reflect.…` and `type.…` in expression position re-resolve as `baml.…` on full miss — one uniform rule mirrored through TIR, hover/semantic tokens, and completions. User names always shadow the shorthand (pinned by a corpus project including a user *class* named `reflect`). - **`type.of<T>()` / `type.of_value(v)`** via a carrier namespace (`baml/ns_type/type.baml`): `of` is the existing compiler intrinsic re-keyed (`"baml.type.of"`), `of_value` a new native over `value_concrete_ty` — fail-open to the `unknown` type value for definition objects, `throws never`-honest; K-12 pinned (`of_value(5)` equals `type.of<int>()`). **Zero parser changes** — expression-position `type.of<…>()` already parses; alias declarations are top-level-only, so the feared ambiguity is vacuous (pinned by parser tests). - **I-9 removal**: `reflect.type_of` (bare or qualified) hits a targeted funnel — `` `reflect.type_of` was renamed to `type.of` (BEP-066) `` — with no DiagnosticId discriminant shift. Every internal call site migrated in this commit (stdlib ×5 files, 16 baml_src suites, 4 projects, inline-Rust BAML in baml_tests + bex_engine, sdk fixtures, LSP fixtures). ## Gates 68 baml_tests binaries 0 failed · bex_vm 21/0 · tir 324/0 · LSP 468/0 (2 reviewed expectation updates) · project 82/0 · parser 154/0 · surface 17/0 · bytecode_cache 43/0 · workspace check clean. Snapshot accepts grouped by five causes in the commit message (stub-listing filter, new namespaces, FQN rendering, spelling migration, new projects). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `type.of<T>()` to obtain runtime type information. * Added `type.of_value(...)` to derive runtime types from values, returning `unknown` when reconstruction is unavailable. * Unified reflection and type utilities under the `baml` namespace. * **Bug Fixes** * Improved namespace resolution, editor completions, and Python routing for the new type namespace. * **Diagnostics** * Added guidance to replace removed `reflect.type_of<T>()` usage with `type.of<T>()`. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
BEP-066 slice-1 stack, PR 2 of 5 — chained on #4325. 47 files, +514/−747.
What
Deletes the inert legacy dynamic-type syntax end to end — lexer tokens (
type_builder,dynamic), parser rules, syntax kinds, AST nodes, ~185 lines of formatter support, and the four never-emitted diagnostic ids (E0040–E0043;bex_cache::FORMAT_VERSIONbumped 3→4 per the DiagnosticId-discriminant convention).Replacement diagnostics (E0098)
InstanceofRemovedgeneralizes toRemovedFeature(same code/discriminant). Old syntax now gets targeted errors instead of silence or E0010 noise:type_builder { … }(all forms) anddynamic class|enumswallow into a balanced ERROR node with one E0098 — "removed; runtime type construction isbaml.reflect(BEP-066)".@@dynamicon classes/enums — previously a silent no-op — now E0098 (dotted@@stream.*untouched; unknown future attributes still pass through).Notes for review
compiles/type_builder_test→broken_syntax/removed_type_builder(8 E0098 shapes);type_builder_errorsdeleted as redundant; newdiagnostic_errors/removed_dynamic_attribute.compiles/testset_dynamicinspected: unrelated (dynamic test generation), untouched.looks_like_test_expr_bodytype_builder check was dead code (tested Word-kind against a keyword token) — the token removal activates it, which is exactly what routes old-style test blocks to the targeted error.Gates
baml_tests 1476 ✓ (+aux) · fmt 102 ✓ · LSP 468 ✓ (reviewed before UPDATE_EXPECT) · clippy clean on diff. Snapshot accepts verified to contain only intended deltas (listed in the commit message).
Summary by CodeRabbit
type_buildersyntax and@@dynamicattributes from the language.baml.reflect.