Uh oh!
There was an error while loading. Please reload this page.
fix: make std.text and std.date reachable as types via _self - #6151
fix: make std.text and std.date reachable as types via _self#6151prql-bot wants to merge 4 commits into
std.text and std.date reachable as types via _self#6151Conversation
`Module::lookup` redirects a bare module name to its `_self` member, which
until now was always a `DeclKind::InstanceOf` (a table instance). Adding
`type _self` to `std.text` / `std.date` made that redirect fire for types
too, so a bare `date` or `text` in expression position resolved to the type
instead of inferring a column: `from t | filter date > @2020-01-01` failed
with "expected a value, but found a type".
Restrict the redirect to non-type `_self` decls, and resolve `module m
{ type _self }` in type position instead, where `<std.text>` is actually
used. `text.*` is filtered out of wildcard resolution for the same reason —
it was expanding to an empty tuple (`SELECT NULL`) rather than erroring.
With the redirect no longer returning `_self` for types, the `_self`
stripping in `ambiguous_error` is unreachable, so it's reverted.The expression-position test claimed a bare `date` resolves to the column; `prqlc debug lineage` shows it resolves to the `std.date` module (as it did before this PR). The point the test guards — that it doesn't resolve to the type — is unchanged.
prql-bot
left a comment
There was a problem hiding this comment.
Reviewed as the PR author's own reviewer, so this is a COMMENT rather than an approval.
The three guards are each scoped so they only fire for a module whose _self is a DeclKind::Ty, i.e. only for the two decls this PR adds — Module::lookup, wildcard resolution and expression resolution all behave exactly as before for this/that/table instances, whose _self stays an InstanceOf. <text> and <date> never reach any of this: type_expr's basic parser turns them into TyKind::Primitive before the ident branch is tried, so removing the two root-level type text = text / type date = date lines can only affect the std.-qualified path. test_ambiguous in error_messages.rs still pins could be any of: std.date, this.date, which is what guards the reverted _self stripping in ambiguous_error.
Two observations inline, neither blocking. One further follow-up, outside this diff: the changelog cites "an internal dump of the module" as the symptom, and that error is still what any other module produces in type position — <std.math> hits the same ok_or_else in fold_type with found: decl.to_string(), which renders Module { names: {...} } via the Display for DeclKind arm Self::Module(arg0) => f.debug_tuple("Module")…. Worth a separate PR to give that arm a short description; happy to push one if it's wanted.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Adds `Module::self_ty`, so `Module::lookup`, `fold_type` and `resolve_ident_wildcard` all express "a module whose `_self` is a type" the same way, and keep the pre-redirect ident name for the type-name fallback in `fold_type`.
Implements the
_selfoption @kgutwin proposed in #6146, plus fixes for the two places_selfleaked into user-facing messages.std.prqldeclaredtype text = text/type date = dateamong the type primitives and thenmodule text { … }/module date { … }under the same names. The module def came later and replaced the type decl, so bothtypelines were inert andstd.text/std.datewere unreachable as types. This moves each type alias inside its module astype _self, so the name carries both meanings:Bare
<text>andtext.lowerare unaffected.The
_selfleaks@kgutwin flagged that the ambiguous-name hint became
could be any of: std.date._self, this.date. There was a second leak too — a type mismatch against<std.text>reportedexpected type `_self`withHelp: Type `_self` expands to `text`, wherestd.int(an ordinarytype int = intalias) saysexpected type `int`.Both come from
_selfbeing used as a display name, and both are fixed at the point of display:fold_type_def_stmtnames atype _selfafter its containing module, so the type prints astext/date.ambiguous_errordrops a trailing_selfpart from each candidate.With those,
test_ambiguousinerror_messages.rsis unchanged — the hint iscould be any of: std.date, this.dateas before. No user-visible message changes in this PR.Unblocking the duplicate check
The point of #6146 was that std.prql's reliance on module-over-type replacement blocks the non-module half of the duplicate check in
fold_module_def_stmt. I verified locally that it's now unblocked: with a temporary check rejecting a module def over a non-module decl, the standard library loads clean andlet x = 1followed bymodule x { … }reportsduplicate declarations of xinstead of silently droppingx. That check isn't in this PR — it belongs on top of #6147.Tests
Three inline tests in
stmt.rscoverstd.textas a type, the mismatch message under the module's own name, and the module still working as a function namespace. Remaining snapshot churn is byte-offset shifts in std.prql spans only.Verification
cargo insta test -p prqlc -p prqlc-parser -p compile-files -p prqlc-macros -p mdbook-prql— all green (475 + 94 + 80 + 27 + 5 + 3 + 3 passed).cargo clippy -p prqlc --all-targetsclean,cargo fmt --checkclean.The only snapshot diffs outside the new tests are span byte offsets in file 0 (std.prql), which shift on any edit to that file:
cargo-instaisn't on the tend sandbox PATH (#6144), so I installed it withcargo install cargo-instafor this run.