From 92e96075cee053053c23a75c6f6aaa541b73fb02 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 13 Oct 2026 16:01:26 +0200 Subject: [PATCH] =?UTF-8?q?refactor(parser):=20parse=20None=20as=20a=20lit?= =?UTF-8?q?eral=20=F0=9F=AB=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `None` was an identifier that the analyser and compiler each recognised by name, so it could never be shadowed and `let None = 5` was silently ignored. Lex it as a keyword and parse it as `Expression::NoneLiteral`, like `true` and `false`, removing both special cases. Binding `None` is now a parse error. Co-Authored-By: Claude Opus 5.5 --- manual/src/reference/types/option.md | 2 ++ ndc_analyser/src/analyser.rs | 4 +-- ndc_bin/src/highlighter.rs | 5 ++-- ndc_lexer/src/token.rs | 3 ++ ndc_lsp/src/features/completion.rs | 2 +- ndc_lsp/src/scope_resolve.rs | 1 + ndc_lsp/src/visitor.rs | 2 ++ ndc_parser/src/expression.rs | 2 ++ ndc_parser/src/parser.rs | 1 + ndc_parser/tests/literals.rs | 28 +++++++++++++++++++ ndc_vm/src/compiler.rs | 13 ++++----- .../068_none_is_not_a_binding_name.ndc | 3 ++ 12 files changed, 52 insertions(+), 14 deletions(-) create mode 100644 ndc_parser/tests/literals.rs create mode 100644 tests/functional/programs/004_basic/068_none_is_not_a_binding_name.ndc diff --git a/manual/src/reference/types/option.md b/manual/src/reference/types/option.md index 50e344cb..7a857186 100644 --- a/manual/src/reference/types/option.md +++ b/manual/src/reference/types/option.md @@ -13,6 +13,8 @@ if value.is_none() { } ``` +`None` is a literal, like `true` and `false`, so it can't be used as a variable or parameter name. + Some functions have variations with a `?` appended to their name that return options instead of throwing errors: ```ndc diff --git a/ndc_analyser/src/analyser.rs b/ndc_analyser/src/analyser.rs index 395a40a5..60b74680 100644 --- a/ndc_analyser/src/analyser.rs +++ b/ndc_analyser/src/analyser.rs @@ -257,6 +257,7 @@ impl Analyser { ) -> Result { match expression { Expression::BoolLiteral(_) => Ok(StaticType::Bool), + Expression::NoneLiteral => Ok(StaticType::Option(Box::new(StaticType::Any))), Expression::StringLiteral(_) => Ok(StaticType::String), Expression::NumericLiteral(NumericLiteral::Int64(_)) => Ok(StaticType::Int), Expression::NumericLiteral(NumericLiteral::Float64(_)) => Ok(StaticType::Float), @@ -271,9 +272,6 @@ impl Analyser { resolved, .. } => { - if ident == "None" { - return Ok(StaticType::Option(Box::new(StaticType::Any))); - } let Some(binding) = self.scope_tree.get_binding_any(ident) else { self.emit(AnalysisError::identifier_not_previously_declared( ident, *span, diff --git a/ndc_bin/src/highlighter.rs b/ndc_bin/src/highlighter.rs index 21834fc1..e0eecfcc 100644 --- a/ndc_bin/src/highlighter.rs +++ b/ndc_bin/src/highlighter.rs @@ -55,8 +55,8 @@ impl AndycppHighlighter { let colored = match &token.token { // Strings — green Token::String(_) => substring.rgb(152, 195, 121), - // Numeric literals and booleans — orange - Token::NumericLiteral(_) | Token::True | Token::False => { + // Numeric literals, booleans and None — orange + Token::NumericLiteral(_) | Token::True | Token::False | Token::None => { substring.rgb(209, 154, 102) } // Keywords — coral red @@ -238,6 +238,7 @@ fn collect_function_spans(expr: &ExpressionLocation, spans: &mut AHashSet } // Leaves — no sub-expressions to recurse into Expression::BoolLiteral(_) + | Expression::NoneLiteral | Expression::StringLiteral(_) | Expression::NumericLiteral(_) | Expression::Identifier { .. } diff --git a/ndc_lexer/src/token.rs b/ndc_lexer/src/token.rs index 92ee577e..e894657a 100644 --- a/ndc_lexer/src/token.rs +++ b/ndc_lexer/src/token.rs @@ -89,6 +89,7 @@ pub enum Token { If, In, Let, + None, Pure, Return, Struct, @@ -159,6 +160,7 @@ impl fmt::Display for Token { Self::If => "if", Self::In => "in", Self::Let => "let", + Self::None => "None", Self::Pure => "pure", Self::Return => "return", Self::Struct => "struct", @@ -335,6 +337,7 @@ impl From for Token { "if" => Self::If, "in" => Self::In, "let" => Self::Let, + "None" => Self::None, "not" => Self::LogicNot, "or" => Self::LogicOr, "pure" => Self::Pure, diff --git a/ndc_lsp/src/features/completion.rs b/ndc_lsp/src/features/completion.rs index 5e3a8d37..8eea4e34 100644 --- a/ndc_lsp/src/features/completion.rs +++ b/ndc_lsp/src/features/completion.rs @@ -213,7 +213,7 @@ fn merged_type_display(candidates: &[&StaticType]) -> String { /// Language keywords offered in general (non-dot) completion. const KEYWORDS: &[&str] = &[ "let", "fn", "struct", "if", "else", "while", "for", "in", "as", "return", "break", "continue", - "true", "false", + "true", "false", "None", ]; fn keyword_completions() -> impl Iterator { diff --git a/ndc_lsp/src/scope_resolve.rs b/ndc_lsp/src/scope_resolve.rs index ec22a058..43f3ed2e 100644 --- a/ndc_lsp/src/scope_resolve.rs +++ b/ndc_lsp/src/scope_resolve.rs @@ -204,6 +204,7 @@ fn collect(expr: &ExpressionLocation, scope: Span, out: &mut Vec) { } Expression::Identifier { .. } | Expression::BoolLiteral(_) + | Expression::NoneLiteral | Expression::StringLiteral(_) | Expression::NumericLiteral(_) | Expression::Break diff --git a/ndc_lsp/src/visitor.rs b/ndc_lsp/src/visitor.rs index 8b9e3afe..7071195e 100644 --- a/ndc_lsp/src/visitor.rs +++ b/ndc_lsp/src/visitor.rs @@ -188,6 +188,7 @@ fn child_expressions(expr: &ExpressionLocation) -> Vec<&ExpressionLocation> { // fields are names + type annotations, not expressions. Expression::Identifier { .. } | Expression::BoolLiteral(_) + | Expression::NoneLiteral | Expression::StringLiteral(_) | Expression::NumericLiteral(_) | Expression::Break @@ -357,6 +358,7 @@ fn walk_expression(visitor: &mut impl AstVisitor, expr: &ExpressionLocation) { // fields are names + type annotations, not expressions. Expression::Identifier { .. } | Expression::BoolLiteral(_) + | Expression::NoneLiteral | Expression::StringLiteral(_) | Expression::NumericLiteral(_) | Expression::Break diff --git a/ndc_parser/src/expression.rs b/ndc_parser/src/expression.rs index 22496511..6eed9c88 100644 --- a/ndc_parser/src/expression.rs +++ b/ndc_parser/src/expression.rs @@ -92,6 +92,8 @@ pub struct ExpressionLocation { pub enum Expression { // Literals BoolLiteral(bool), + /// `None`, the empty option. + NoneLiteral, StringLiteral(String), NumericLiteral(NumericLiteral), Identifier { diff --git a/ndc_parser/src/parser.rs b/ndc_parser/src/parser.rs index 94887dc9..7193084b 100644 --- a/ndc_parser/src/parser.rs +++ b/ndc_parser/src/parser.rs @@ -1075,6 +1075,7 @@ impl Parser { let expression = match token_location.token { Token::False => Expression::BoolLiteral(false), Token::True => Expression::BoolLiteral(true), + Token::None => Expression::NoneLiteral, Token::NumericLiteral(literal) => Expression::NumericLiteral(literal), Token::String(value) => Expression::StringLiteral(value), Token::Identifier(identifier) => Expression::Identifier { diff --git a/ndc_parser/tests/literals.rs b/ndc_parser/tests/literals.rs new file mode 100644 index 00000000..a704c20a --- /dev/null +++ b/ndc_parser/tests/literals.rs @@ -0,0 +1,28 @@ +#![allow(unused_crate_dependencies)] + +use ndc_lexer::{Lexer, SourceId}; +use ndc_parser::{Expression, ExpressionLocation, Parser}; + +fn parse_one(source: &str) -> ExpressionLocation { + let tokens = Lexer::new(source, SourceId::SYNTHETIC) + .collect::, _>>() + .unwrap(); + let mut expressions = Parser::from_tokens(tokens).parse().unwrap(); + assert_eq!(expressions.len(), 1); + expressions.pop().unwrap() +} + +#[test] +fn none_is_a_literal() { + assert_eq!(parse_one("None").expression, Expression::NoneLiteral); +} + +#[test] +fn none_cannot_be_a_binding_name() { + for source in ["let None = 5", "fn f(None) { 1 }"] { + let tokens = Lexer::new(source, SourceId::SYNTHETIC) + .collect::, _>>() + .unwrap(); + assert!(Parser::from_tokens(tokens).parse().is_err(), "{source}"); + } +} diff --git a/ndc_vm/src/compiler.rs b/ndc_vm/src/compiler.rs index dc1c2120..9ac682d8 100644 --- a/ndc_vm/src/compiler.rs +++ b/ndc_vm/src/compiler.rs @@ -209,6 +209,10 @@ impl Compiler { let idx = self.ir.add_constant(Value::Bool(b)); self.ir.write(OpCode::Constant(idx), span); } + Expression::NoneLiteral => { + let idx = self.ir.add_constant(Value::None); + self.ir.write(OpCode::Constant(idx), span); + } Expression::StringLiteral(s) => { let idx = self.ir.add_constant(Value::string(s)); self.ir.write(OpCode::Constant(idx), span); @@ -228,14 +232,7 @@ impl Compiler { let idx = self.ir.add_constant(value); self.ir.write(OpCode::Constant(idx), span); } - Expression::Identifier { name, resolved, .. } => { - if name == "None" { - let idx = self.ir.add_constant(Value::None); - self.ir.write(OpCode::Constant(idx), span); - } else { - self.compile_binding(resolved, span)?; - } - } + Expression::Identifier { resolved, .. } => self.compile_binding(resolved, span)?, Expression::Cast { value, resolved_type, diff --git a/tests/functional/programs/004_basic/068_none_is_not_a_binding_name.ndc b/tests/functional/programs/004_basic/068_none_is_not_a_binding_name.ndc new file mode 100644 index 00000000..818413d1 --- /dev/null +++ b/tests/functional/programs/004_basic/068_none_is_not_a_binding_name.ndc @@ -0,0 +1,3 @@ +// `None` is a literal like `true`, so it can't name a variable. +// expect-error: Invalid assignment target +let None = 5;