better error message for unexpected constructors · nandi.uk/gleam@0f80af7

+254 -190

compiler-core/src/parse.rs

··· 2567 2567 let documentation = self.take_documentation(start); 2568 2568 let (name_start, name, parameters, end, name_end) = self.expect_type_name()?; 2569 2569 let name_location = SrcSpan::new(name_start, name_end); 2570 - let (constructors, end_position) = if self.maybe_one(&Token::LeftBrace).is_some() { 2571 - // Custom Type 2572 - let constructors = Parser::series_of( 2573 - self, 2574 - &|p| { 2575 - // The only attribute supported on constructors is @deprecated 2576 - let mut attributes = Attributes::default(); 2577 - let attr_loc = Parser::parse_attributes(p, &mut attributes)?; 2578 2570 2579 - if let Some(attr_span) = attr_loc { 2580 - // Expecting all but the deprecated atterbutes to be default 2581 - if attributes.external_erlang.is_some() 2582 - || attributes.external_javascript.is_some() 2583 - || attributes.target.is_some() 2584 - || attributes.internal != InternalAttribute::Missing 2585 - { 2586 - return parse_error( 2587 - ParseErrorType::UnknownAttributeRecordVariant, 2588 - attr_span, 2589 - ); 2590 - } 2591 - } 2571 + let (constructors, end_position) = match self.tok0.take() { 2572 + // If we see `type Wibble {`, then we know we're parsing a custom type. 2573 + Some((_, Token::LeftBrace, _)) => { 2574 + self.advance(); 2592 2575 2593 - match Parser::maybe_upname(p) { 2594 - Some((c_s, c_n, c_e)) => { 2595 - let documentation = p.take_documentation(c_s); 2596 - let (arguments, arguments_e) = 2597 - Parser::parse_type_constructor_arguments(p)?; 2598 - let end = arguments_e.max(c_e); 2599 - Ok(Some(RecordConstructor { 2600 - location: SrcSpan { start: c_s, end }, 2601 - name_location: SrcSpan { 2602 - start: c_s, 2603 - end: c_e, 2604 - }, 2605 - name: c_n, 2606 - arguments, 2607 - documentation, 2608 - deprecation: attributes.deprecated, 2609 - })) 2610 - } 2611 - _ => Ok(None), 2612 - } 2613 - }, 2614 - // No separator 2615 - None, 2616 - )?; 2617 - let (_, close_end) = self.expect_custom_type_close(&name, public, opaque)?; 2618 - (constructors, close_end) 2619 - } else { 2620 - match self.maybe_one(&Token::Equal) { 2621 - Some((eq_s, eq_e)) => { 2622 - // Type Alias 2623 - if opaque { 2624 - return parse_error( 2625 - ParseErrorType::OpaqueTypeAlias, 2626 - SrcSpan { start, end }, 2627 - ); 2628 - } 2576 + // If we see a lowercase name, rather than an uppercase one. We 2577 + // know there's a syntax error! So now we can try and provide a 2578 + // nice error message, based on what that wrong code looks like. 2579 + if let Some((name_start, Token::Name { .. }, name_end)) = &self.tok0 { 2580 + return Err(self.invalid_record_constructor_error( 2581 + name, 2582 + public, 2583 + opaque, 2584 + *name_start, 2585 + *name_end, 2586 + )); 2587 + } 2629 2588 2630 - match self.parse_type()? { 2631 - Some(t) => { 2632 - let type_end = t.location().end; 2633 - return Ok(Some(Definition::TypeAlias(TypeAlias { 2634 - documentation, 2635 - location: SrcSpan::new(start, type_end), 2636 - publicity: self.publicity(public, attributes.internal)?, 2637 - alias: name, 2638 - name_location, 2639 - parameters, 2640 - type_ast: t, 2641 - type_: (), 2642 - deprecation: std::mem::take(&mut attributes.deprecated), 2643 - }))); 2644 - } 2645 - _ => { 2646 - return parse_error( 2647 - ParseErrorType::ExpectedType, 2648 - SrcSpan::new(eq_s, eq_e), 2649 - ); 2650 - } 2651 - } 2589 + let constructors = self.series_of( 2590 + &|parser| parser.parse_record_constructor(), 2591 + // No separator 2592 + None, 2593 + )?; 2594 + let close_end = self.expect_custom_type_close()?; 2595 + (constructors, close_end) 2596 + } 2597 + 2598 + // If we see `type Wibble =` then we know we're parsing a type alias. 2599 + Some((equal_start, Token::Equal, equal_end)) => { 2600 + self.advance(); 2601 + 2602 + if opaque { 2603 + return parse_error(ParseErrorType::OpaqueTypeAlias, SrcSpan { start, end }); 2652 2604 } 2653 - _ => (vec![], end), 2605 + 2606 + if let Some(type_) = self.parse_type()? { 2607 + return Ok(Some(Definition::TypeAlias(TypeAlias { 2608 + documentation, 2609 + location: SrcSpan::new(start, type_.location().end), 2610 + publicity: self.publicity(public, attributes.internal)?, 2611 + alias: name, 2612 + name_location, 2613 + parameters, 2614 + type_ast: type_, 2615 + type_: (), 2616 + deprecation: std::mem::take(&mut attributes.deprecated), 2617 + }))); 2618 + } else { 2619 + return parse_error( 2620 + ParseErrorType::ExpectedType, 2621 + SrcSpan::new(equal_start, equal_end), 2622 + ); 2623 + } 2624 + } 2625 + 2626 + token @ (Some(_) | None) => { 2627 + self.tok0 = token; 2628 + (vec![], end) 2654 2629 } 2655 2630 }; 2656 2631 ··· 2671 2646 }))) 2672 2647 } 2673 2648 2649 + fn parse_record_constructor(&mut self) -> Result<Option<RecordConstructor<()>>, ParseError> { 2650 + // The only attribute supported on constructors is @deprecated 2651 + let mut attributes = Attributes::default(); 2652 + let attr_loc = self.parse_attributes(&mut attributes)?; 2653 + 2654 + if let Some(attr_span) = attr_loc { 2655 + // Expecting all but the deprecated atterbutes to be default 2656 + if attributes.external_erlang.is_some() 2657 + || attributes.external_javascript.is_some() 2658 + || attributes.target.is_some() 2659 + || attributes.internal != InternalAttribute::Missing 2660 + { 2661 + return parse_error(ParseErrorType::UnknownAttributeRecordVariant, attr_span); 2662 + } 2663 + } 2664 + 2665 + match self.maybe_upname() { 2666 + Some((name_start, constructor_name, name_end)) => { 2667 + let documentation = self.take_documentation(name_start); 2668 + let (arguments, arguments_end) = self.parse_record_constructor_arguments()?; 2669 + 2670 + Ok(Some(RecordConstructor { 2671 + location: SrcSpan { 2672 + start: name_start, 2673 + end: arguments_end.max(name_end), 2674 + }, 2675 + name_location: SrcSpan { 2676 + start: name_start, 2677 + end: name_end, 2678 + }, 2679 + name: constructor_name, 2680 + arguments, 2681 + documentation, 2682 + deprecation: attributes.deprecated, 2683 + })) 2684 + } 2685 + _ => Ok(None), 2686 + } 2687 + } 2688 + 2689 + /// This takes place when we find a lowercase name as a record constructor 2690 + /// variant (that name is passed as an argument here). 2691 + /// We want to look at the following tokens to produce a nice error message: 2692 + /// 2693 + /// ```gleam 2694 + /// pub type Wibble { 2695 + /// wibble 2696 + /// //^^^^^^ Error, this should be uppercase! 2697 + /// } 2698 + /// ``` 2699 + /// 2700 + /// But if the thing looks like a record definition, we want a specialised 2701 + /// error message: 2702 + /// 2703 + /// ```gleam 2704 + /// pub type Wibble { 2705 + /// wibble: Int, 2706 + /// wobble: String 2707 + /// } 2708 + /// // Suggest wrapping this in a constructor. 2709 + /// ``` 2710 + /// 2711 + fn invalid_record_constructor_error( 2712 + &mut self, 2713 + type_name: EcoString, 2714 + public: bool, 2715 + opaque: bool, 2716 + name_start: u32, 2717 + name_end: u32, 2718 + ) -> ParseError { 2719 + let fields = self.series_of( 2720 + &|parser| parser.parse_record_constructor_field(), 2721 + Some(&Token::Comma), 2722 + ); 2723 + 2724 + match fields { 2725 + // If there's a list of fields right inside the type that means the 2726 + // developer might have forgotten to wrap the thing in a constructor. 2727 + // Basically writing something like this: 2728 + // 2729 + // ```gleam 2730 + // pub type Wibble { 2731 + // String, 2732 + // wibble: Int, 2733 + // } 2734 + // ``` 2735 + // 2736 + // So we want to produce a specialised error message pointing them 2737 + // in the right direction. 2738 + Ok(fields) if let Some((_, Token::RightBrace, _)) = self.tok0 => ParseError { 2739 + location: SrcSpan { 2740 + start: fields 2741 + .first() 2742 + .map_or(name_start, |field| field.location.start), 2743 + end: fields.last().map_or(name_end, |field| field.location.end), 2744 + }, 2745 + error: ParseErrorType::ExpectedRecordConstructor { 2746 + type_name: type_name.clone(), 2747 + public, 2748 + opaque, 2749 + fields, 2750 + }, 2751 + }, 2752 + 2753 + // Otherwise we fall back to telling them the lowercase name should 2754 + // be uppercased! 2755 + Ok(_) | Err(_) => ParseError { 2756 + error: ParseErrorType::IncorrectUpName, 2757 + location: SrcSpan { 2758 + start: name_start, 2759 + end: name_end, 2760 + }, 2761 + }, 2762 + } 2763 + } 2764 + 2765 + // examples: 2766 + // *no args* 2767 + // () 2768 + // (a, b) 2769 + fn parse_record_constructor_arguments( 2770 + &mut self, 2771 + ) -> Result<(Vec<RecordConstructorArg<()>>, u32), ParseError> { 2772 + if self.maybe_one(&Token::LeftParen).is_some() { 2773 + let arguments = Parser::series_of( 2774 + self, 2775 + &|parser| parser.parse_record_constructor_field(), 2776 + Some(&Token::Comma), 2777 + )?; 2778 + let (_, end) = self 2779 + .expect_one_following_series(&Token::RightParen, "a constructor argument name")?; 2780 + Ok((arguments, end)) 2781 + } else { 2782 + Ok((vec![], 0)) 2783 + } 2784 + } 2785 + 2786 + fn parse_record_constructor_field( 2787 + &mut self, 2788 + ) -> Result<Option<RecordConstructorArg<()>>, ParseError> { 2789 + match (self.tok0.take(), self.tok1.take()) { 2790 + (Some((start, Token::Name { name }, name_end)), Some((_, Token::Colon, end))) => { 2791 + let _ = Parser::next_tok(self); 2792 + let _ = Parser::next_tok(self); 2793 + let doc = self.take_documentation(start); 2794 + match Parser::parse_type(self)? { 2795 + Some(type_ast) => { 2796 + let end = type_ast.location().end; 2797 + Ok(Some(RecordConstructorArg { 2798 + label: Some((SrcSpan::new(start, name_end), name)), 2799 + ast: type_ast, 2800 + location: SrcSpan { start, end }, 2801 + type_: (), 2802 + doc, 2803 + })) 2804 + } 2805 + None => parse_error(ParseErrorType::ExpectedType, SrcSpan { start, end }), 2806 + } 2807 + } 2808 + (t0, t1) => { 2809 + self.tok0 = t0; 2810 + self.tok1 = t1; 2811 + match Parser::parse_type(self)? { 2812 + Some(type_ast) => { 2813 + let doc = match &self.tok0 { 2814 + Some((start, _, _)) => self.take_documentation(*start), 2815 + None => None, 2816 + }; 2817 + let type_location = type_ast.location(); 2818 + Ok(Some(RecordConstructorArg { 2819 + label: None, 2820 + ast: type_ast, 2821 + location: type_location, 2822 + type_: (), 2823 + doc, 2824 + })) 2825 + } 2826 + None => Ok(None), 2827 + } 2828 + } 2829 + } 2830 + } 2831 + 2674 2832 // examples: 2675 2833 // A 2676 2834 // A(one, two) ··· 2724 2882 }) 2725 2883 } else { 2726 2884 Ok((start, upname, vec![], end, end)) 2727 - } 2728 - } 2729 - 2730 - // examples: 2731 - // *no args* 2732 - // () 2733 - // (a, b) 2734 - fn parse_type_constructor_arguments( 2735 - &mut self, 2736 - ) -> Result<(Vec<RecordConstructorArg<()>>, u32), ParseError> { 2737 - if self.maybe_one(&Token::LeftParen).is_some() { 2738 - let arguments = Parser::series_of( 2739 - self, 2740 - &|p| match (p.tok0.take(), p.tok1.take()) { 2741 - ( 2742 - Some((start, Token::Name { name }, name_end)), 2743 - Some((_, Token::Colon, end)), 2744 - ) => { 2745 - let _ = Parser::next_tok(p); 2746 - let _ = Parser::next_tok(p); 2747 - let doc = p.take_documentation(start); 2748 - match Parser::parse_type(p)? { 2749 - Some(type_ast) => { 2750 - let end = type_ast.location().end; 2751 - Ok(Some(RecordConstructorArg { 2752 - label: Some((SrcSpan::new(start, name_end), name)), 2753 - ast: type_ast, 2754 - location: SrcSpan { start, end }, 2755 - type_: (), 2756 - doc, 2757 - })) 2758 - } 2759 - None => { 2760 - parse_error(ParseErrorType::ExpectedType, SrcSpan { start, end }) 2761 - } 2762 - } 2763 - } 2764 - (t0, t1) => { 2765 - p.tok0 = t0; 2766 - p.tok1 = t1; 2767 - match Parser::parse_type(p)? { 2768 - Some(type_ast) => { 2769 - let doc = match &p.tok0 { 2770 - Some((start, _, _)) => p.take_documentation(*start), 2771 - None => None, 2772 - }; 2773 - let type_location = type_ast.location(); 2774 - Ok(Some(RecordConstructorArg { 2775 - label: None, 2776 - ast: type_ast, 2777 - location: type_location, 2778 - type_: (), 2779 - doc, 2780 - })) 2781 - } 2782 - None => Ok(None), 2783 - } 2784 - } 2785 - }, 2786 - Some(&Token::Comma), 2787 - )?; 2788 - let (_, end) = self 2789 - .expect_one_following_series(&Token::RightParen, "a constructor argument name")?; 2790 - Ok((arguments, end)) 2791 - } else { 2792 - Ok((vec![], 0)) 2793 2885 } 2794 2886 } 2795 2887 ··· 4046 4138 4047 4139 /// Expect the end to a custom type definiton or handle an incorrect 4048 4140 /// record constructor definition. 4049 - /// 4050 - /// Used for mapping to a more specific error type and message. 4051 - fn expect_custom_type_close( 4052 - &mut self, 4053 - name: &EcoString, 4054 - public: bool, 4055 - opaque: bool, 4056 - ) -> Result<(u32, u32), ParseError> { 4141 + fn expect_custom_type_close(&mut self) -> Result<u32, ParseError> { 4057 4142 match self.maybe_one(&Token::RightBrace) { 4058 - Some((start, end)) => Ok((start, end)), 4143 + Some((_, end)) => Ok(end), 4059 4144 None => match self.next_tok() { 4060 4145 None => parse_error(ParseErrorType::UnexpectedEof, SrcSpan { start: 0, end: 0 }), 4061 4146 Some((start, token, end)) => { 4062 - // If provided a Name, map to a more detailed error 4063 - // message to nudge the user. 4064 - // Else, handle as an unexpected token. 4065 - let field = if let Token::Name { name } = token { 4066 - name 4067 - } else { 4068 - let hint = match (&token, self.tok0.take()) { 4069 - (&Token::Fn, _) | (&Token::Pub, Some((_, Token::Fn, _))) => { 4070 - let text = "Gleam is not an object oriented programming language so 4147 + let hint = match (&token, self.tok0.take()) { 4148 + (&Token::Fn, _) | (&Token::Pub, Some((_, Token::Fn, _))) => { 4149 + let text = "Gleam is not an object oriented programming language so 4071 4150 functions are declared separately from types."; 4072 - Some(wrap(text).into()) 4073 - } 4074 - (_, _) => None, 4075 - }; 4076 - 4077 - return parse_error( 4078 - ParseErrorType::UnexpectedToken { 4079 - token, 4080 - expected: vec![ 4081 - Token::RightBrace.to_string().into(), 4082 - "a record constructor".into(), 4083 - ], 4084 - hint, 4085 - }, 4086 - SrcSpan { start, end }, 4087 - ); 4088 - }; 4089 - let field_type = match self.parse_type_annotation(&Token::Colon) { 4090 - Ok(Some(annotation)) => Some(Box::new(annotation)), 4091 - _ => None, 4151 + Some(wrap(text).into()) 4152 + } 4153 + (_, _) => None, 4092 4154 }; 4155 + 4093 4156 parse_error( 4094 - ParseErrorType::ExpectedRecordConstructor { 4095 - name: name.clone(), 4096 - public, 4097 - opaque, 4098 - field, 4099 - field_type, 4157 + ParseErrorType::UnexpectedToken { 4158 + token, 4159 + expected: vec![ 4160 + Token::RightBrace.to_string().into(), 4161 + "a record constructor".into(), 4162 + ], 4163 + hint, 4100 4164 }, 4101 4165 SrcSpan { start, end }, 4102 4166 )

+38 -21

compiler-core/src/parse/error.rs

View file

··· 1 1 // SPDX-License-Identifier: Apache-2.0 2 2 // SPDX-FileCopyrightText: 2020 The Gleam contributors 3 3 4 - use crate::ast::{SrcSpan, TypeAst}; 4 + use crate::ast::{RecordConstructorArg, SrcSpan, TypeAst}; 5 5 use crate::diagnostic::{ExtraLabel, Label}; 6 6 use crate::error::{wrap, wrap_format}; 7 7 use crate::parse::Token; ··· 145 145 RedundantInternalAttribute, // for a private definition marked as internal 146 146 InvalidModuleTypePattern, // for patterns that have a dot like: `name.thing` 147 147 ListPatternSpreadFollowedByElements, // When there is a pattern after a spread [..rest, pattern] 148 + 149 + /// This happens when someone forgets to write the type constructor around 150 + /// its fields in a custom type definition. For example: 151 + /// 152 + /// ```gleam 153 + /// pub type Wibble { 154 + /// String, 155 + /// field: Int, 156 + /// field_2: a, 157 + /// } 158 + /// ``` 148 159 ExpectedRecordConstructor { 149 - name: EcoString, 160 + type_name: EcoString, 150 161 public: bool, 151 162 opaque: bool, 152 - field: EcoString, 153 - field_type: Option<Box<TypeAst>>, 163 + /// Those are the fields that have been written. 164 + fields: Vec<RecordConstructorArg<()>>, 154 165 }, 155 166 CallInClauseGuard, // case x { _ if f() -> 1 } 156 167 IfExpression, ··· 684 695 }, 685 696 686 697 ParseErrorType::ExpectedRecordConstructor { 687 - name, 698 + type_name, 688 699 public, 689 700 opaque, 690 - field, 691 - field_type, 701 + fields, 692 702 } => { 693 703 let (accessor, opaque) = match *public { 694 704 true if *opaque => ("pub ", "opaque "), ··· 696 706 false => ("", ""), 697 707 }; 698 708 699 - let mut annotation = EcoString::new(); 700 - match field_type { 701 - Some(t) => t.print(&mut annotation), 702 - None => annotation.push_str("Type"), 703 - }; 709 + let fields = fields 710 + .iter() 711 + .map(|field| { 712 + let mut type_ = EcoString::new(); 713 + field.ast.print(&mut type_); 714 + 715 + match field.label.as_ref() { 716 + Some((_, label)) => format!(" {label}: {type_},"), 717 + None => format!(" {type_},"), 718 + } 719 + }) 720 + .join("\n"); 704 721 705 722 ParseErrorDetails { 706 - text: [ 707 - "Each custom type variant must have a constructor:\n".into(), 708 - format!("{accessor}{opaque}type {name} {{"), 709 - format!(" {name}("), 710 - format!(" {field}: {annotation},"), 711 - " )".into(), 712 - "}".into(), 713 - ] 714 - .join("\n"), 723 + text: format!( 724 + "Each custom type variant must have a constructor: 725 + 726 + {accessor}{opaque}type {type_name} {{ 727 + {type_name}( 728 + {fields} 729 + ) 730 + }}" 731 + ), 715 732 hint: None, 716 733 label_text: "I was not expecting this".into(), 717 734 extra_labels: vec![],

+20

compiler-core/src/parse/snapshots/gleam_core__parse__tests__lowercase_constructor_name_in_custom_type.snap

View file

··· 1 + --- 2 + source: compiler-core/src/parse/tests.rs 3 + expression: "\npub type Wibble {\n wibble(Int, String)\n}\n" 4 + --- 5 + ----- SOURCE CODE 6 + 7 + pub type Wibble { 8 + wibble(Int, String) 9 + } 10 + 11 + 12 + ----- ERROR 13 + error: Syntax error 14 + ┌─ /src/parse/error.gleam:3:3 15 + │ 16 + 3 │ wibble(Int, String) 17 + │ ^^^^^^ I'm expecting a type name here 18 + 19 + Hint: Type names start with a uppercase letter, and can contain a-z, A-Z, 20 + or 0-9.

+28

compiler-core/src/parse/snapshots/gleam_core__parse__tests__missing_constructor_name_with_multiple_fields.snap

View file

··· 1 + --- 2 + source: compiler-core/src/parse/tests.rs 3 + expression: "\npub type Wibble(a) {\n field: Int,\n other: a\n}\n" 4 + --- 5 + ----- SOURCE CODE 6 + 7 + pub type Wibble(a) { 8 + field: Int, 9 + other: a 10 + } 11 + 12 + 13 + ----- ERROR 14 + error: Syntax error 15 + ┌─ /src/parse/error.gleam:3:3 16 + │ 17 + 3 │ ╭ field: Int, 18 + 4 │ │ other: a 19 + │ ╰──────────^ I was not expecting this 20 + 21 + Each custom type variant must have a constructor: 22 + 23 + pub type Wibble { 24 + Wibble( 25 + field: Int, 26 + other: a, 27 + ) 28 + }

+1 -1

compiler-core/src/parse/snapshots/gleam_core__parse__tests__type_invalid_record_constructor.snap

View file

··· 14 14 ┌─ /src/parse/error.gleam:3:5 15 15 │ 16 16 3 │ name: String, 17 - │ ^^^^ I was not expecting this 17 + │ ^^^^^^^^^^^^ I was not expecting this 18 18 19 19 Each custom type variant must have a constructor: 20 20

+3 -8

compiler-core/src/parse/snapshots/gleam_core__parse__tests__type_invalid_record_constructor_invalid_field_type.snap

View file

··· 14 14 ┌─ /src/parse/error.gleam:3:5 15 15 │ 16 16 3 │ name: "Test User", 17 - │ ^^^^ I was not expecting this 18 - 19 - Each custom type variant must have a constructor: 17 + │ ^^^^ I'm expecting a type name here 20 18 21 - type User { 22 - User( 23 - name: Type, 24 - ) 25 - } 19 + Hint: Type names start with a uppercase letter, and can contain a-z, A-Z, 20 + or 0-9.

+1 -1

compiler-core/src/parse/snapshots/gleam_core__parse__tests__type_invalid_record_constructor_without_field_type.snap

View file

··· 20 20 21 21 pub opaque type User { 22 22 User( 23 - name: Type, 23 + name, 24 24 ) 25 25 }

+23

compiler-core/src/parse/tests.rs

View file

··· 2510 2510 }" 2511 2511 ); 2512 2512 } 2513 + 2514 + #[test] 2515 + fn missing_constructor_name_with_multiple_fields() { 2516 + assert_module_error!( 2517 + r#" 2518 + pub type Wibble(a) { 2519 + field: Int, 2520 + other: a 2521 + } 2522 + "# 2523 + ); 2524 + } 2525 + 2526 + #[test] 2527 + fn lowercase_constructor_name_in_custom_type() { 2528 + assert_module_error!( 2529 + r#" 2530 + pub type Wibble { 2531 + wibble(Int, String) 2532 + } 2533 + "# 2534 + ); 2535 + }

Configure Feed

Configure Feed