-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[bugfix] literals in template strings #1132
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Reviewed everything up to a2180b3 in 38 seconds
More details
- Looked at
155
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_VKsK1T5XIpb7RRJN
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
minijinja::value::ValueKind::Bool => Type::Bool, | ||
minijinja::value::ValueKind::String => Type::String, | ||
minijinja::value::ValueKind::Bool => { | ||
match bool::from_str(&v.to_string()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using bool::from_str
on the stringified value of minijinja::value::Value
is not ideal. Consider directly converting the Value
to a boolean without string conversion.
match bool::from_str(&v.to_string()) { | |
match v.as_bool().unwrap_or(false) { |
@@ -410,7 +417,12 @@ | |||
} | |||
minijinja::value::ValueKind::Map => Type::Unknown, | |||
// We don't handle these types | |||
minijinja::value::ValueKind::Number => Type::Number, | |||
minijinja::value::ValueKind::Number => { | |||
match i64::from_str(&v.to_string()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using i64::from_str
on the stringified value of minijinja::value::Value
is not ideal. Consider directly converting the Value
to an integer without string conversion.
match i64::from_str(&v.to_string()) { | |
match v.as_i64() { |
Important
Fixes handling of literals in template strings by updating type inference and test cases to correctly identify and evaluate literal values.
infer_const_type
inexpr.rs
to returnType::Literal
forBool
,String
, andNumber
kinds.test_expr.rs
andtest_stmt.rs
to reflect literal types in function argument errors.test_ifexpr
,test_call_function
, andtest_output_format
intest_expr.rs
to check forType::Literal
.if_else
intest_stmt.rs
to handle literal types in conditional expressions.This description was created by
for a2180b3. It will automatically update as commits are pushed.