Skip to content

Commit 80c282c

Browse files
A name narrowed out of null may still be set to null
After `if x == null { return }` the checker held an assignment to `x` to the narrowed type, so `x = null` was refused though `x` is declared optional. An assignment is held to the type the name had before any condition narrowed it; what that type does not hold is refused as before.
1 parent 54f85ba commit 80c282c

2 files changed

Lines changed: 44 additions & 1 deletion

File tree

‎src/sema/types.rs‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -788,6 +788,10 @@ struct Checker<'a, 'p> {
788788
/// The names in each scope nothing may assign to, and what each is: a
789789
/// derived value, a prop, a loop variable. Parallel to `scopes`.
790790
fixed: Vec<HashMap<String, &'static str>>,
791+
/// Each name narrowed in a scope, and its type before: what it may be
792+
/// assigned, whatever a condition proved it holds now. Parallel to
793+
/// `scopes`.
794+
widened: Vec<HashMap<String, Type>>,
791795
/// The component being checked, for `emit`.
792796
component: Option<&'p ComponentDecl>,
793797
/// The `return` types met in the action being checked.
@@ -847,6 +851,7 @@ impl<'a, 'p> Checker<'a, 'p> {
847851
info,
848852
scopes: Vec::new(),
849853
fixed: Vec::new(),
854+
widened: Vec::new(),
850855
component: None,
851856
returns: Vec::new(),
852857
current_span: Span::dummy(),
@@ -868,11 +873,23 @@ impl<'a, 'p> Checker<'a, 'p> {
868873
fn push_scope(&mut self) {
869874
self.scopes.push(HashMap::new());
870875
self.fixed.push(HashMap::new());
876+
self.widened.push(HashMap::new());
871877
}
872878

873879
fn pop_scope(&mut self) {
874880
self.scopes.pop();
875881
self.fixed.pop();
882+
self.widened.pop();
883+
}
884+
885+
/// What `name` may be assigned: its type before any condition narrowed
886+
/// it. `if x == null { return }` proves `x` holds a value from there on;
887+
/// `x = null` after it is still a value of its type.
888+
fn assignable(&self, name: &str) -> Option<Type> {
889+
self.widened
890+
.iter()
891+
.find_map(|w| w.get(name).cloned())
892+
.or_else(|| self.lookup(name))
876893
}
877894

878895
/// Bind a name nothing may assign to, saying what it is.
@@ -953,6 +970,11 @@ impl<'a, 'p> Checker<'a, 'p> {
953970
// A narrowed name is still what it was: a derived value checked
954971
// for null is still not assignable.
955972
let what = self.fixed_kind(name);
973+
if let Some(before) = self.lookup(name)
974+
&& let Some(widened) = self.widened.last_mut()
975+
{
976+
widened.entry(name.to_string()).or_insert(before);
977+
}
956978
if let Some(scope) = self.scopes.last_mut() {
957979
scope.insert(name.to_string(), ty);
958980
}
@@ -1666,7 +1688,7 @@ impl<'a, 'p> Checker<'a, 'p> {
16661688
let target = match &a.target {
16671689
// A name nothing declares compiles to a signal's
16681690
// `.set`, `_name.set(…)`, which throws: `T13`.
1669-
Expr::Identifier(name) => match self.lookup(name) {
1691+
Expr::Identifier(name) => match self.assignable(name) {
16701692
Some(ty) => ty,
16711693
None => {
16721694
if !self.is_known_name(name) {

‎tests/checks.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,27 @@ fn an_else_and_an_early_return_narrow_out_null() {
340340
clean(&src);
341341
}
342342

343+
#[test]
344+
fn a_name_narrowed_out_of_null_may_still_be_set_to_null() {
345+
// `if sel == null { return }` proves `sel` holds a value from there on;
346+
// it is still a `Sel?`, and `null` is still one of its values.
347+
let src = format!(
348+
"type Sel {{ title: String }}\n{}",
349+
page(
350+
" state sel: Sel? = null\n action clear() {\n if sel == null { return }\n log(sel.title)\n sel = null\n }\n Button(\"x\") { on click { clear() } }"
351+
)
352+
);
353+
clean(&src);
354+
// What its type does not hold is refused as before.
355+
let wrong = format!(
356+
"type Sel {{ title: String }}\n{}",
357+
page(
358+
" state sel: Sel? = null\n action clear() {\n if sel == null { return }\n sel = 5\n }\n Button(\"x\") { on click { clear() } }"
359+
)
360+
);
361+
assert_eq!(with(&wrong, "T01").len(), 1, "{:?}", with(&wrong, "T01"));
362+
}
363+
343364
#[test]
344365
fn t19_a_value_that_may_be_null_shown_as_text_draws_a_warning() {
345366
let src = format!(

0 commit comments

Comments
 (0)