Skip to content
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

Disallow a confusing constructor syntax #11856

Merged
merged 4 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@
#### Enso Language & Runtime

- [Intersection types & type checks][11600]
- A constructor or type definition with a single inline argument definition was
previously allowed to use spaces in the argument definition without
parentheses. [This is now a syntax error.][11856]

[11600]: https://github.com/enso-org/enso/pull/11600
[11856]: https://github.com/enso-org/enso/pull/11856

# Next Release

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void testConstructor() throws Exception {
type Foo
@a foo
@b bar
Cons a b = a b
Cons a b
""");

var typeDefinition = (Definition.SugaredType) ir.bindings().apply(0);
Expand Down
9 changes: 8 additions & 1 deletion lib/rust/parser/debug/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,11 @@ where T: serde::Serialize + Reflect {
let text_escape_token = rust_to_meta[&TextEscape::reflect().id];
let token_to_str = move |token: Value| {
let range = token_code_range(&token, base);
code[range].to_owned().into_boxed_str()
if range.is_empty() {
"".into()
} else {
code[range].to_owned().into_boxed_str()
}
};
for token in identish_tokens {
let token_to_str_ = token_to_str.clone();
Expand Down Expand Up @@ -158,6 +162,9 @@ fn token_code_range(token: &Value, base: usize) -> std::ops::Range<usize> {
|field| fields(token).find(|(name, _)| *name == field).unwrap().1.as_u64().unwrap() as u32;
let begin = get_u32(":codeReprBegin");
let len = get_u32(":codeReprLen");
if len == 0 {
return 0..0;
}
let begin = (begin as u64) | (base as u64 & !(u32::MAX as u64));
let begin = if begin < (base as u64) { begin + 1 << 32 } else { begin };
let begin = begin as usize - base;
Expand Down
8 changes: 5 additions & 3 deletions lib/rust/parser/debug/tests/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,12 +346,14 @@ fn type_def_full() {

#[test]
fn type_def_defaults() {
let code = ["type Result error ok=Nothing", " Ok value:ok = Nothing"];
test!(code.join("\n"),
test!("type Result error ok=Nothing\n Ok value:ok=Nothing\n Error (value:e = Nothing)",
(TypeDef Result #((() (Ident error) () ())
(() (Ident ok) () ((Ident Nothing))))
#(,(Constructor::new("Ok")
.with_arg(sexp![(() (Ident value) (":" (Ident ok)) ((Ident Nothing)))])))));
.with_arg(sexp![(() (Ident value) (":" (Ident ok)) ((Ident Nothing)))]))
,(Constructor::new("Error")
.with_arg(sexp![(() (Ident value) (":" (Ident e)) ((Ident Nothing)))])))));
expect_invalid_node("type Result\n Ok value:ok = Nothing");
}

#[test]
Expand Down
3 changes: 2 additions & 1 deletion lib/rust/parser/src/macros/built_in.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@ fn lambda_body<'s>(
let (body, mut rest) = segments.pop();
let arguments = rest.pop().unwrap();
let backslash = arguments.header.with_variant(token::variant::LambdaOperator());
let arguments = syntax::parse_args(&mut arguments.result.tokens(), 0, precedence);
let arguments =
syntax::parse_args(&mut arguments.result.tokens(), 0, precedence, &mut default());
let arrow = body.header.with_variant(token::variant::ArrowOperator());
let body_expression = precedence.resolve(&mut body.result.tokens());
let body_expression =
Expand Down
53 changes: 8 additions & 45 deletions lib/rust/parser/src/syntax/statement/function_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,20 +182,19 @@ pub fn parse_args<'s>(
items: &mut Vec<Item<'s>>,
start: usize,
precedence: &mut Precedence<'s>,
args_buffer: &mut Vec<ArgumentDefinition<'s>>,
) -> Vec<ArgumentDefinition<'s>> {
let mut arg_starts = vec![];
for (i, item) in items.iter().enumerate().skip(start) {
if i == start || Spacing::of_item(item) == Spacing::Spaced {
arg_starts.push(i);
}
}
let mut defs: Vec<_> = arg_starts
.drain(..)
.rev()
.map(|arg_start| parse_arg_def(items, arg_start, precedence))
.collect();
defs.reverse();
defs
args_buffer.extend(
arg_starts.drain(..).rev().map(|arg_start| parse_arg_def(items, arg_start, precedence)),
);
debug_assert_eq!(items.len(), start);
args_buffer.drain(..).rev().collect()
}

pub fn parse_constructor_definition<'s>(
Expand Down Expand Up @@ -270,43 +269,12 @@ fn parse_constructor_decl<'s>(
precedence: &mut Precedence<'s>,
args_buffer: &mut Vec<ArgumentDefinition<'s>>,
) -> (token::Ident<'s>, Vec<ArgumentDefinition<'s>>) {
let args = parse_type_args(items, start + 1, precedence, args_buffer);
let args = parse_args(items, start + 1, precedence, args_buffer);
let name = items.pop().unwrap().into_token().unwrap().try_into().unwrap();
debug_assert_eq!(items.len(), start);
(name, args)
}

pub fn parse_type_args<'s>(
items: &mut Vec<Item<'s>>,
start: usize,
precedence: &mut Precedence<'s>,
args_buffer: &mut Vec<ArgumentDefinition<'s>>,
) -> Vec<ArgumentDefinition<'s>> {
if start == items.len() {
return default();
}
let mut arg_starts = vec![start];
let mut expecting_rhs = false;
for (i, item) in items.iter().enumerate().skip(start + 1) {
if expecting_rhs {
expecting_rhs = false;
continue;
}
if let Item::Token(Token { variant: token::Variant::AssignmentOperator(_), .. }) = item {
expecting_rhs = true;
continue;
}
if Spacing::of_item(item) == Spacing::Spaced {
arg_starts.push(i);
}
}
args_buffer.extend(
arg_starts.drain(..).rev().map(|arg_start| parse_arg_def(items, arg_start, precedence)),
);
debug_assert_eq!(items.len(), start);
args_buffer.drain(..).rev().collect()
}

pub fn try_parse_foreign_function<'s>(
items: &mut Vec<Item<'s>>,
start: usize,
Expand Down Expand Up @@ -493,12 +461,7 @@ fn parse_arg_def<'s>(
.or_else(|| open2.as_ref().map(|t| t.code.position_after()))
.or_else(|| open1.as_ref().map(|t| t.code.position_after()))
.or_else(|| type_.as_ref().map(|t| t.operator.left_offset.code.position_before()))
// Why does this one need a type annotation???
.or_else(|| {
close2
.as_ref()
.map(|t: &token::CloseSymbol| t.left_offset.code.position_before())
})
.or_else(|| close2.as_ref().map(|t| t.left_offset.code.position_before()))
.or_else(|| default.as_ref().map(|t| t.equals.left_offset.code.position_before()))
.or_else(|| close1.as_ref().map(|t| t.left_offset.code.position_before()))
.unwrap(),
Expand Down
4 changes: 2 additions & 2 deletions lib/rust/parser/src/syntax/statement/type_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use crate::syntax::maybe_with_error;
use crate::syntax::operator::Precedence;
use crate::syntax::statement::apply_excess_private_keywords;
use crate::syntax::statement::compound_lines;
use crate::syntax::statement::function_def::parse_args;
use crate::syntax::statement::function_def::parse_constructor_definition;
use crate::syntax::statement::function_def::parse_type_args;
use crate::syntax::statement::parse_statement;
use crate::syntax::statement::scan_private_keywords;
use crate::syntax::statement::EvaluationContext;
Expand Down Expand Up @@ -66,7 +66,7 @@ pub fn try_parse_type_def<'s>(
default()
};

let params = parse_type_args(items, start + 2, precedence, args_buffer);
let params = parse_args(items, start + 2, precedence, args_buffer);

let name = items.pop().unwrap().into_token().unwrap().try_into().unwrap();

Expand Down
Loading