language-servers/roc
1
0
Fork 0
mirror of https://github.com/roc-lang/roc.git synced 2025-12-15 21:23:57 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

Allow unsuffixed statements in parser

Browse source 
Moves the "STATEMENT AFTER EXPRESSION" error from the parser to canonicalization.
We'll later use this to allow this case in effectful functions.
This commit is contained in:
Agus Zubiaga 2024-10-14 20:05:33 -03:00
parent f677592f97
commit 2cce5ad023
No known key found for this signature in database
23 changed files with 280 additions and 107 deletions
Download patch file Download diff file Expand all files Collapse all files

3
crates/compiler/can/src/def.rs
View file

@ -1182,6 +1182,7 @@ fn canonicalize_value_defs<'a>(
}
PendingValue::InvalidIngestedFile => { /* skip */ }
PendingValue::ImportNameConflict => { /* skip */ }
PendingValue::StmtAfterExpr => { /* skip */ }
}
}
@ -3019,6 +3020,7 @@ enum PendingValue<'a> {
SignatureDefMismatch,
InvalidIngestedFile,
ImportNameConflict,
StmtAfterExpr,
}
struct PendingExpectOrDbg<'a> {
@ -3293,6 +3295,7 @@ fn to_pending_value_def<'a>(
PendingValue::Def(PendingValueDef::IngestedFile(loc_pattern, ingested_file.annotation.map(|ann| ann.annotation), ingested_file.path))
}
StmtAfterExpr => PendingValue::StmtAfterExpr,
Stmt(_) => internal_error!("a Stmt was not desugared correctly, should have been converted to a Body(...) in desguar"),
}
}

15
crates/compiler/can/src/desugar.rs
View file

@ -11,8 +11,8 @@ use roc_module::called_via::{BinOp, CalledVia};
use roc_module::ident::ModuleName;
use roc_parse::ast::Expr::{self, *};
use roc_parse::ast::{
AssignedField, Collection, Defs, ModuleImportParams, Pattern, StrLiteral, StrSegment,
TypeAnnotation, ValueDef, WhenBranch,
is_expr_suffixed, AssignedField, Collection, Defs, ModuleImportParams, Pattern, StrLiteral,
StrSegment, TypeAnnotation, ValueDef, WhenBranch,
};
use roc_problem::can::Problem;
use roc_region::all::{Loc, Region};
@ -200,11 +200,20 @@ fn desugar_value_def<'a>(
}
IngestedFileImport(_) => *def,
StmtAfterExpr => internal_error!("unexpected StmtAfterExpr"),
Stmt(stmt_expr) => {
// desugar `stmt_expr!` to
// _ : {}
// _ = stmt_expr!
if !is_expr_suffixed(&stmt_expr.value) {
// [purity-inference] TODO: this is ok in purity inference mode
env.problems.push(Problem::StmtAfterExpr(stmt_expr.region));
return ValueDef::StmtAfterExpr;
}
let region = stmt_expr.region;
let new_pat = env
.arena
@ -364,7 +373,7 @@ pub fn desugar_value_def_suffixed<'a>(arena: &'a Bump, value_def: ValueDef<'a>)
// TODO support desugaring of Dbg and ExpectFx
Dbg { .. } | ExpectFx { .. } => value_def,
ModuleImport { .. } | IngestedFileImport(_) => value_def,
ModuleImport { .. } | IngestedFileImport(_) | StmtAfterExpr => value_def,
Stmt(..) => {
internal_error!(

1
crates/compiler/can/src/effect_module.rs
View file

@ -141,6 +141,7 @@ pub fn build_host_exposed_def(
closure_type: var_store.fresh(),
return_type: var_store.fresh(),
fx_type: var_store.fresh(),
early_returns: vec![],
name: symbol,
captured_symbols: std::vec::Vec::new(),
recursive: Recursive::NotRecursive,

1
crates/compiler/can/src/suffixed.rs
View file

@ -683,6 +683,7 @@ pub fn unwrap_suffixed_expression_defs_help<'a>(
Annotation(..) | Dbg{..} | Expect{..} | ExpectFx{..} | Stmt(..) | ModuleImport{..} | IngestedFileImport(_) => None,
AnnotatedBody { body_pattern, body_expr, ann_type, ann_pattern, .. } => Some((body_pattern, body_expr, Some((ann_pattern, ann_type)))),
Body (def_pattern, def_expr) => Some((def_pattern, def_expr, None)),
StmtAfterExpr => None,
};
match maybe_suffixed_value_def {

1
crates/compiler/can/tests/snapshots/test_suffixed__suffixed_tests__closure_with_annotations.snap
View file

@ -53,6 +53,7 @@ Defs {
[],
),
],
Pure,
@22-30 Apply(
"",
"Task",

1
crates/compiler/can/tests/snapshots/test_suffixed__suffixed_tests__closure_with_defs.snap
View file

@ -62,6 +62,7 @@ Defs {
[],
),
],
Pure,
@34-45 Apply(
"",
"Task",

3
crates/compiler/fmt/src/def.rs
View file

@ -4,6 +4,7 @@ use crate::expr::fmt_str_literal;
use crate::pattern::fmt_pattern;
use crate::spaces::{fmt_default_newline, fmt_default_spaces, fmt_spaces, INDENT};
use crate::Buf;
use roc_error_macros::internal_error;
use roc_parse::ast::{
AbilityMember, Defs, Expr, ExtractSpaces, ImportAlias, ImportAsKeyword, ImportExposingKeyword,
ImportedModuleName, IngestedFileAnnotation, IngestedFileImport, ModuleImport,
@ -423,6 +424,7 @@ impl<'a> Formattable for ValueDef<'a> {
ModuleImport(module_import) => module_import.is_multiline(),
IngestedFileImport(ingested_file_import) => ingested_file_import.is_multiline(),
Stmt(loc_expr) => loc_expr.is_multiline(),
StmtAfterExpr => internal_error!("shouldn't exist before can"),
}
}
@ -464,6 +466,7 @@ impl<'a> Formattable for ValueDef<'a> {
ModuleImport(module_import) => module_import.format(buf, indent),
IngestedFileImport(ingested_file_import) => ingested_file_import.format(buf, indent),
Stmt(loc_expr) => loc_expr.format_with_options(buf, parens, newlines, indent),
StmtAfterExpr => internal_error!("shouldn't exist before can"),
}
}
}

10
crates/compiler/load/tests/test_reporting.rs
View file

@ -4322,21 +4322,15 @@ mod test_reporting {
"
),
@r###"
── STATEMENT AFTER EXPRESSION in tmp/double_equals_in_def/Test.roc ─────────────
── STATEMENT AFTER EXPRESSION in /code/proj/Main.roc ───────────────────────────
I just finished parsing an expression with a series of definitions,
and this line is indented as if it's intended to be part of that
expression:
1│ app "test" provides [main] to "./platform"
2│
3│ main =
4│ x = 3
5│ y =
6│ x == 5
7│ Num.add 1 2
^
^^^^^^
However, I already saw the final expression in that series of
definitions.

4
crates/compiler/load_internal/src/docs.rs
View file

@ -282,6 +282,10 @@ fn generate_entry_docs(
// Don't generate docs for ingested file imports
}
ValueDef::StmtAfterExpr { .. } => {
// Ignore. Canonicalization will produce an error.
}
ValueDef::Stmt(loc_expr) => {
if let roc_parse::ast::Expr::Var {
ident: identifier, ..

43
crates/compiler/load_internal/tests/test_load.rs
View file

@ -846,49 +846,6 @@ fn platform_does_not_exist() {
}
}
#[test]
fn platform_parse_error() {
let modules = vec![
(
"platform/main.roc",
indoc!(
r#"
platform "hello-c"
requires {} { main : Str }
exposes []
packages {}
imports []
provides [mainForHost]
blah 1 2 3 # causing a parse error on purpose
mainForHost : Str
"#
),
),
(
"main.roc",
indoc!(
r#"
app "hello-world"
packages { pf: "platform/main.roc" }
imports []
provides [main] to pf
main = "Hello, World!\n"
"#
),
),
];
match multiple_modules("platform_parse_error", modules) {
Err(report) => {
assert!(report.contains("STATEMENT AFTER EXPRESSION"));
assert!(report.contains("blah 1 2 3 # causing a parse error on purpose"));
}
Ok(_) => unreachable!("we expect failure here"),
}
}
#[test]
// See https://github.com/roc-lang/roc/issues/2413
fn platform_exposes_main_return_by_pointer_issue() {

7
crates/compiler/parse/src/ast.rs
View file

@ -841,6 +841,8 @@ pub enum ValueDef<'a> {
IngestedFileImport(IngestedFileImport<'a>),
Stmt(&'a Loc<Expr<'a>>),
StmtAfterExpr,
}
impl<'a> ValueDef<'a> {
@ -1093,7 +1095,9 @@ impl<'a, 'b> Iterator for RecursiveValueDefIter<'a, 'b> {
}
}
ValueDef::Stmt(loc_expr) => self.push_pending_from_expr(&loc_expr.value),
ValueDef::Annotation(_, _) | ValueDef::IngestedFileImport(_) => {}
ValueDef::Annotation(_, _)
| ValueDef::IngestedFileImport(_)
| ValueDef::StmtAfterExpr => {}
}
self.index += 1;
@ -2752,6 +2756,7 @@ impl<'a> Malformed for ValueDef<'a> {
annotation,
}) => path.is_malformed() || annotation.is_malformed(),
ValueDef::Stmt(loc_expr) => loc_expr.is_malformed(),
ValueDef::StmtAfterExpr => false,
}
}
}

18
crates/compiler/parse/src/expr.rs
View file

@ -3120,7 +3120,7 @@ fn stmts_to_defs<'a>(
break;
}
Stmt::Expr(e) => {
if is_expr_suffixed(&e) && i + 1 < stmts.len() {
if i + 1 < stmts.len() {
defs.push_value_def(
ValueDef::Stmt(arena.alloc(Loc::at(sp_stmt.item.region, e))),
sp_stmt.item.region,
@ -3128,10 +3128,6 @@ fn stmts_to_defs<'a>(
&[],
);
} else {
if last_expr.is_some() {
return Err(EExpr::StmtAfterExpr(sp_stmt.item.region.start()));
}
let e = if sp_stmt.before.is_empty() {
e
} else {
@ -3142,10 +3138,6 @@ fn stmts_to_defs<'a>(
}
}
Stmt::Backpassing(pats, call) => {
if last_expr.is_some() {
return Err(EExpr::StmtAfterExpr(sp_stmt.item.region.start()));
}
if i + 1 >= stmts.len() {
return Err(EExpr::BackpassContinue(sp_stmt.item.region.end()));
}
@ -3169,10 +3161,6 @@ fn stmts_to_defs<'a>(
}
Stmt::TypeDef(td) => {
if last_expr.is_some() {
return Err(EExpr::StmtAfterExpr(sp_stmt.item.region.start()));
}
if let (
TypeDef::Alias {
header,
@ -3224,10 +3212,6 @@ fn stmts_to_defs<'a>(
}
}
Stmt::ValueDef(vd) => {
if last_expr.is_some() {
return Err(EExpr::StmtAfterExpr(sp_stmt.item.region.start()));
}
// NOTE: it shouldn't be necessary to convert ValueDef::Dbg into an expr, but
// it turns out that ValueDef::Dbg exposes some bugs in the rest of the compiler.
// In particular, it seems that the solver thinks the dbg expr must be a bool.

2
crates/compiler/parse/src/normalize.rs
View file

@ -439,6 +439,7 @@ impl<'a> Normalize<'a> for ValueDef<'a> {
IngestedFileImport(ingested_file_import.normalize(arena))
}
Stmt(loc_expr) => Stmt(arena.alloc(loc_expr.normalize(arena))),
StmtAfterExpr => StmtAfterExpr,
}
}
}
@ -1072,7 +1073,6 @@ impl<'a> Normalize<'a> for EExpr<'a> {
EExpr::IndentEnd(_pos) => EExpr::IndentEnd(Position::zero()),
EExpr::UnexpectedComma(_pos) => EExpr::UnexpectedComma(Position::zero()),
EExpr::UnexpectedTopLevelExpr(_pos) => EExpr::UnexpectedTopLevelExpr(Position::zero()),
EExpr::StmtAfterExpr(_pos) => EExpr::StmtAfterExpr(Position::zero()),
EExpr::RecordUpdateOldBuilderField(_pos) => {
EExpr::RecordUpdateOldBuilderField(Region::zero())
}

1
crates/compiler/parse/src/parser.rs
View file

@ -303,7 +303,6 @@ pub enum EExpr<'a> {
Start(Position),
End(Position),
BadExprEnd(Position),
StmtAfterExpr(Position),
Space(BadInputError, Position),
Dot(Position),

3
crates/compiler/problem/src/can.rs
View file

@ -251,6 +251,7 @@ pub enum Problem {
ReturnAtEndOfFunction {
region: Region,
},
StmtAfterExpr(Region),
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@ -335,6 +336,7 @@ impl Problem {
Problem::ReturnOutsideOfFunction { .. } => Warning,
Problem::StatementsAfterReturn { .. } => Warning,
Problem::ReturnAtEndOfFunction { .. } => Warning,
Problem::StmtAfterExpr(_) => Fatal,
}
}
@ -505,6 +507,7 @@ impl Problem {
| Problem::BadRecursion(cycle_entries) => {
cycle_entries.first().map(|entry| entry.expr_region)
}
Problem::StmtAfterExpr(region) => Some(*region),
Problem::RuntimeError(RuntimeError::UnresolvedTypeVar)
| Problem::RuntimeError(RuntimeError::ErroneousType)
| Problem::RuntimeError(RuntimeError::NonExhaustivePattern)

11
crates/compiler/test_syntax/tests/snapshots/pass/effectful_closure_statements.expr.formatted.roc Normal file
View file

@ -0,0 +1,11 @@
\{} ->
echo "Welcome to the DMV!"
age = readInt
if age < 16 then
echo "You're too young to drive!"
exit 1
else
{}
echo "Let's get started on your driver's license application."

188
crates/compiler/test_syntax/tests/snapshots/pass/effectful_closure_statements.expr.result-ast Normal file
View file

@ -0,0 +1,188 @@
SpaceAfter(
Closure(
[
@1-3 RecordDestructure(
[],
),
],
@11-222 SpaceBefore(
Defs(
Defs {
tags: [
Index(2147483648),
Index(2147483649),
Index(2147483650),
],
regions: [
@11-37,
@42-55,
@61-154,
],
space_before: [
Slice(start = 0, length = 0),
Slice(start = 0, length = 1),
Slice(start = 1, length = 2),
],
space_after: [
Slice(start = 0, length = 0),
Slice(start = 1, length = 0),
Slice(start = 3, length = 0),
],
spaces: [
Newline,
Newline,
Newline,
],
type_defs: [],
value_defs: [
Stmt(
@11-37 Apply(
@11-15 Var {
module_name: "",
ident: "echo",
},
[
@16-37 Str(
PlainLine(
"Welcome to the DMV!",
),
),
],
Space,
),
),
Body(
@42-45 Identifier {
ident: "age",
},
@48-55 Var {
module_name: "",
ident: "readInt",
},
),
Stmt(
@61-154 If {
if_thens: [
(
@64-72 BinOps(
[
(
@64-67 Var {
module_name: "",
ident: "age",
},
@68-69 LessThan,
),
],
@70-72 Num(
"16",
),
),
@86-134 SpaceBefore(
SpaceAfter(
Defs(
Defs {
tags: [
Index(2147483648),
],
regions: [
@86-119,
],
space_before: [
Slice(start = 0, length = 0),
],
space_after: [
Slice(start = 0, length = 0),
],
spaces: [],
type_defs: [],
value_defs: [
Stmt(
@86-119 Apply(
@86-90 Var {
module_name: "",
ident: "echo",
},
[
@91-119 Str(
PlainLine(
"You're too young to drive!",
),
),
],
Space,
),
),
],
},
@128-134 SpaceBefore(
Apply(
@128-132 Var {
module_name: "",
ident: "exit",
},
[
@133-134 Num(
"1",
),
],
Space,
),
[
Newline,
],
),
),
[
Newline,
],
),
[
Newline,
],
),
),
],
final_else: @152-154 SpaceBefore(
Record(
[],
),
[
Newline,
],
),
indented_else: false,
},
),
],
},
@160-222 SpaceBefore(
Apply(
@160-164 Var {
module_name: "",
ident: "echo",
},
[
@165-222 Str(
PlainLine(
"Let's get started on your driver's license application.",
),
),
],
Space,
),
[
Newline,
Newline,
],
),
),
[
Newline,
],
),
),
[
Newline,
],
)

11
crates/compiler/test_syntax/tests/snapshots/pass/effectful_closure_statements.expr.roc Normal file
View file

@ -0,0 +1,11 @@
\{} ->
echo "Welcome to the DMV!"
age = readInt
if age < 16 then
echo "You're too young to drive!"
exit 1
else
{}
echo "Let's get started on your driver's license application."

1
crates/compiler/test_syntax/tests/test_snapshots.rs
View file

@ -313,6 +313,7 @@ mod test_snapshots {
pass/defs_suffixed_middle_extra_indents.moduledefs,
pass/destructure_tag_assignment.expr,
pass/docs.expr,
pass/effectful_closure_statements.expr,
pass/empty_app_header.header,
pass/empty_hosted_header.header,
pass/empty_list.expr,

1
crates/language_server/src/analysis/tokens.rs
View file

@ -643,6 +643,7 @@ impl IterTokens for ValueDef<'_> {
onetoken(Token::Import, import.name.item.region, arena)
}
ValueDef::Stmt(loc_expr) => loc_expr.iter_tokens(arena),
ValueDef::StmtAfterExpr => BumpVec::new_in(arena),
}
}
}

1
crates/repl_ui/src/repl_state.rs
View file

@ -246,6 +246,7 @@ impl ReplState {
return ReplAction::Nothing;
}
ValueDef::Stmt(_) => todo!(),
ValueDef::StmtAfterExpr => todo!("effects in repl"),
}
}
}

28
crates/reporting/src/error/canonicalize.rs
View file

@ -64,6 +64,7 @@ const ABILITY_IMPLEMENTATION_NOT_IDENTIFIER: &str = "ABILITY IMPLEMENTATION NOT
const DUPLICATE_IMPLEMENTATION: &str = "DUPLICATE IMPLEMENTATION";
const UNNECESSARY_IMPLEMENTATIONS: &str = "UNNECESSARY IMPLEMENTATIONS";
const INCOMPLETE_ABILITY_IMPLEMENTATION: &str = "INCOMPLETE ABILITY IMPLEMENTATION";
const STATEMENT_AFTER_EXPRESSION: &str = "STATEMENT AFTER EXPRESSION";
pub fn can_problem<'b>(
alloc: &'b RocDocAllocator<'b>,
@ -1400,6 +1401,33 @@ pub fn can_problem<'b>(
title = "UNNECESSARY RETURN".to_string();
}
Problem::StmtAfterExpr(region) => {
// TODO: Update when [purity-inference] is fully implemented
doc = alloc.stack([
alloc
.reflow(r"I just finished parsing an expression with a series of definitions,"),
alloc.reflow(
r"and this line is indented as if it's intended to be part of that expression:",
),
alloc.region(lines.convert_region(region), severity),
alloc.concat([alloc.reflow(
"However, I already saw the final expression in that series of definitions.",
)]),
alloc.tip().append(
alloc.reflow(
"An expression like `4`, `\"hello\"`, or `functionCall MyThing` is like `return 4` in other programming languages. To me, it seems like you did `return 4` followed by more code in the lines after, that code would never be executed!"
)
),
alloc.tip().append(
alloc.reflow(
"If you are working with `Task`, this error can happen if you forgot a `!` somewhere."
)
)
]);
title = STATEMENT_AFTER_EXPRESSION.to_string();
}
};
Report {

33
crates/reporting/src/error/parse.rs
View file

@ -669,39 +669,6 @@ fn to_expr_report<'a>(
severity,
}
}
EExpr::StmtAfterExpr(pos) => {
let surroundings = Region::new(start, *pos);
let region = LineColumnRegion::from_pos(lines.convert_pos(*pos));
let doc = alloc.stack([
alloc
.reflow(r"I just finished parsing an expression with a series of definitions,"),
alloc.reflow(
r"and this line is indented as if it's intended to be part of that expression:",
),
alloc.region_with_subregion(lines.convert_region(surroundings), region, severity),
alloc.reflow(
"However, I already saw the final expression in that series of definitions."
),
alloc.tip().append(
alloc.reflow(
"An expression like `4`, `\"hello\"`, or `functionCall MyThing` is like `return 4` in other programming languages. To me, it seems like you did `return 4` followed by more code in the lines after, that code would never be executed!"
)
),
alloc.tip().append(
alloc.reflow(
"If you are working with `Task`, this error can happen if you forgot a `!` somewhere."
)
)
]);
Report {
filename,
doc,
title: "STATEMENT AFTER EXPRESSION".to_string(),
severity,
}
}
EExpr::Return(EReturn::Return(pos) | EReturn::IndentReturnValue(pos), start) => {
to_expr_report(
alloc,