fix: parenthesized pattern, and correct 1-element tuple printing

compiler/noirc_frontend/src/ast/statement.rs

@@ -567,6 +567,7 @@ pub enum Pattern {
     Mutable(Box<Pattern>, Location, /*is_synthesized*/ bool),
     Tuple(Vec<Pattern>, Location),
     Struct(Path, Vec<(Ident, Pattern)>, Location),
+    Parenthesized(Box<Pattern>, Location),
     Interned(InternedPattern, Location),
 }
 
@@ -580,6 +581,7 @@ impl Pattern {
             Pattern::Mutable(_, location, _)
             | Pattern::Tuple(_, location)
             | Pattern::Struct(_, _, location)
+            | Pattern::Parenthesized(_, location)
             | Pattern::Interned(_, location) => *location,
         }
     }
@@ -624,6 +626,7 @@ impl Pattern {
                     location: *location,
                 })
             }
+            Pattern::Parenthesized(pattern, _) => pattern.try_as_expression(interner),
             Pattern::Interned(id, _) => interner.get_pattern(*id).try_as_expression(interner),
         }
     }
@@ -990,6 +993,9 @@ impl Display for Pattern {
                 let fields = vecmap(fields, |(name, pattern)| format!("{name}: {pattern}"));
                 write!(f, "{} {{ {} }}", typename, fields.join(", "))
             }
+            Pattern::Parenthesized(pattern, _) => {
+                write!(f, "({})", pattern)
+            }
             Pattern::Interned(_, _) => {
                 write!(f, "?Interned")
             }

compiler/noirc_frontend/src/ast/visitor.rs

@@ -519,6 +519,10 @@
         true
     }
 
+    fn visit_parenthesized_pattern(&mut self, _: &Pattern, _: Span) -> bool {
+        true
+    }
+
     fn visit_interned_pattern(&mut self, _: &InternedPattern, _: Span) {}
 
     fn visit_secondary_attribute(
@@ -1482,8 +1486,8 @@
             UnresolvedTypeData::FieldElement => {
                 visitor.visit_field_element_type(self.location.span);
             }
             UnresolvedTypeData::Integer(signdness, size) => {
                 visitor.visit_integer_type(*signdness, *size, self.location.span);
             }
             UnresolvedTypeData::Bool => visitor.visit_bool_type(self.location.span),
             UnresolvedTypeData::Unit => visitor.visit_unit_type(self.location.span),
@@ -1624,6 +1628,11 @@
                     }
                 }
             }
+            Pattern::Parenthesized(pattern, location) => {
+                if visitor.visit_parenthesized_pattern(pattern, location.span) {
+                    pattern.accept(visitor);
+                }
+            }
             Pattern::Interned(id, location) => {
                 visitor.visit_interned_pattern(id, location.span);
             }

compiler/noirc_frontend/src/debug/mod.rs

@@ -322,7 +322,7 @@
             }
             ast::LValue::Dereference(_lv, location) => {
                 // TODO: this is a dummy statement for now, but we should
                 // somehow track the derefence and update the pointed to
                 // variable
                 ast::Statement {
                     kind: ast::StatementKind::Expression(uint_expr(0, *location)),
@@ -726,10 +726,13 @@
             ast::Pattern::Tuple(patterns, _) => {
                 stack.extend(patterns.iter().map(|pattern| (pattern, false)));
             }
             ast::Pattern::Struct(_, pids, _) => {
                 stack.extend(pids.iter().map(|(_, pattern)| (pattern, is_mut)));
                 vars.extend(pids.iter().map(|(id, _)| (id.clone(), false)));
             }
+            ast::Pattern::Parenthesized(pattern, _) => {
+                stack.push_back((pattern, false));
+            }
             ast::Pattern::Interned(_, _) => (),
         }
     }
@@ -739,7 +742,7 @@
 fn pattern_to_string(pattern: &ast::Pattern) -> String {
     match pattern {
         ast::Pattern::Identifier(id) => id.to_string(),
         ast::Pattern::Mutable(mpat, _, _) => format!("mut {}", pattern_to_string(mpat.as_ref())),
         ast::Pattern::Tuple(elements, _) => format!(
             "({})",
             elements.iter().map(pattern_to_string).collect::<Vec<String>>().join(", ")
@@ -757,6 +760,9 @@
                     .join(", "),
             )
         }
+        ast::Pattern::Parenthesized(pattern, _) => {
+            format!("({})", pattern_to_string(pattern.as_ref()))
+        }
         ast::Pattern::Interned(_, _) => "?Interned".to_string(),
     }
 }

compiler/noirc_frontend/src/elaborator/patterns.rs

@@ -170,6 +170,14 @@ impl Elaborator<'_> {
                     new_definitions,
                 )
             }
+            Pattern::Parenthesized(pattern, _) => self.elaborate_pattern_mut(
+                *pattern,
+                expected_type,
+                definition,
+                mutable,
+                new_definitions,
+                warn_if_unused,
+            ),
             Pattern::Interned(id, _) => {
                 let pattern = self.interner.get_pattern(id).clone();
                 self.elaborate_pattern_mut(

compiler/noirc_frontend/src/hir/comptime/display.rs

@@ -385,7 +385,11 @@ impl Display for ValuePrinter<'_, '_> {
             Value::Closure(..) => write!(f, "(closure)"),
             Value::Tuple(fields) => {
                 let fields = vecmap(fields, |field| field.display(self.interner).to_string());
-                write!(f, "({})", fields.join(", "))
+                if fields.len() == 1 {
+                    write!(f, "({},)", fields[0])
+                } else {
+                    write!(f, "({})", fields.join(", "))
+                }
             }
             Value::Struct(fields, typ) => {
                 let typename = match typ.follow_bindings() {
@@ -952,21 +956,25 @@ fn remove_interned_in_generic_type_args(
 fn remove_interned_in_pattern(interner: &NodeInterner, pattern: Pattern) -> Pattern {
     match pattern {
         Pattern::Identifier(_) => pattern,
-        Pattern::Mutable(pattern, span, is_synthesized) => Pattern::Mutable(
+        Pattern::Mutable(pattern, location, is_synthesized) => Pattern::Mutable(
             Box::new(remove_interned_in_pattern(interner, *pattern)),
-            span,
+            location,
             is_synthesized,
         ),
-        Pattern::Tuple(patterns, span) => Pattern::Tuple(
+        Pattern::Tuple(patterns, location) => Pattern::Tuple(
             vecmap(patterns, |pattern| remove_interned_in_pattern(interner, pattern)),
-            span,
+            location,
         ),
-        Pattern::Struct(path, patterns, span) => {
+        Pattern::Struct(path, patterns, location) => {
             let patterns = vecmap(patterns, |(name, pattern)| {
                 (name, remove_interned_in_pattern(interner, pattern))
             });
-            Pattern::Struct(path, patterns, span)
+            Pattern::Struct(path, patterns, location)
         }
+        Pattern::Parenthesized(pattern, location) => Pattern::Parenthesized(
+            Box::new(remove_interned_in_pattern(interner, *pattern)),
+            location,
+        ),
         Pattern::Interned(id, _) => interner.get_pattern(id).clone(),
     }
 }

compiler/noirc_frontend/src/hir_def/types.rs

@@ -1021,7 +1021,11 @@
             }
             Type::Tuple(elements) => {
                 let elements = vecmap(elements, ToString::to_string);
-                write!(f, "({})", elements.join(", "))
+                if elements.len() == 1 {
+                    write!(f, "({},)", elements[0])
+                } else {
+                    write!(f, "({})", elements.join(", "))
+                }
             }
             Type::Bool => write!(f, "bool"),
             Type::String(len) => write!(f, "str<{len}>"),
@@ -2397,7 +2401,7 @@
         }
 
         let recur_on_binding = |id, replacement: &Type| {
             // Prevent recuring forever if there's a `T := T` binding
             if replacement.type_variable_id() == Some(id) {
                 replacement.clone()
             } else {
@@ -2483,7 +2487,7 @@
                 Type::Tuple(fields)
             }
             Type::Forall(typevars, typ) => {
                 // Trying to substitute_helper a variable de, substitute_bound_typevarsfined within a nested Forall
                 // is usually impossible and indicative of an error in the type checker somewhere.
                 for var in typevars {
                     assert!(!type_bindings.contains_key(&var.id()));

compiler/noirc_frontend/src/parser/parser/pattern.rs

@@ -109,6 +109,7 @@ impl Parser<'_> {
 
     /// PatternNoMut
     ///     = InternedPattern
+    ///     | ParenthesizedPattern
     ///     | TuplePattern
     ///     | StructPattern
     ///     | IdentifierPattern
@@ -121,7 +122,7 @@ impl Parser<'_> {
             return Some(pattern);
         }
 
-        if let Some(pattern) = self.parse_tuple_pattern() {
+        if let Some(pattern) = self.parse_parenthesized_or_tuple_pattern() {
             return Some(pattern);
         }
 
@@ -162,23 +163,30 @@ impl Parser<'_> {
         }
     }
 
+    /// ParenthesizedPattern = '(' Pattern ')'
     /// TuplePattern = '(' PatternList? ')'
     ///
     /// PatternList = Pattern ( ',' Pattern )* ','?
-    fn parse_tuple_pattern(&mut self) -> Option<Pattern> {
+    fn parse_parenthesized_or_tuple_pattern(&mut self) -> Option<Pattern> {
         let start_location = self.current_token_location;
 
         if !self.eat_left_paren() {
             return None;
         }
 
-        let patterns = self.parse_many(
+        let (mut patterns, has_trailing_comma) = self.parse_many_return_trailing_separator_if_any(
             "tuple elements",
             separated_by_comma_until_right_paren(),
             Self::parse_tuple_pattern_element,
         );
 
-        Some(Pattern::Tuple(patterns, self.location_since(start_location)))
+        let location = self.location_since(start_location);
+
+        Some(if patterns.len() == 1 && !has_trailing_comma {
+            Pattern::Parenthesized(Box::new(patterns.remove(0)), location)
+        } else {
+            Pattern::Tuple(patterns, location)
+        })
     }
 
     fn parse_tuple_pattern_element(&mut self) -> Option<Pattern> {
@@ -289,7 +297,31 @@ mod tests {
     }
 
     #[test]
-    fn parses_tuple_pattern() {
+    fn parses_parenthesized_pattern() {
+        let src = "(foo)";
+        let pattern = parse_pattern_no_errors(src);
+        let Pattern::Parenthesized(pattern, _) = pattern else {
+            panic!("Expected a tuple pattern")
+        };
+
+        let Pattern::Identifier(ident) = *pattern else { panic!("Expected an identifier pattern") };
+        assert_eq!(ident.to_string(), "foo");
+    }
+
+    #[test]
+    fn parses_tuple_pattern_one_element() {
+        let src = "(foo,)";
+        let pattern = parse_pattern_no_errors(src);
+        let Pattern::Tuple(mut patterns, _) = pattern else { panic!("Expected a tuple pattern") };
+        assert_eq!(patterns.len(), 1);
+
+        let pattern = patterns.remove(0);
+        let Pattern::Identifier(ident) = pattern else { panic!("Expected an identifier pattern") };
+        assert_eq!(ident.to_string(), "foo");
+    }
+
+    #[test]
+    fn parses_tuple_pattern_two_elements() {
         let src = "(foo, bar)";
         let pattern = parse_pattern_no_errors(src);
         let Pattern::Tuple(mut patterns, _) = pattern else { panic!("Expected a tuple pattern") };

compiler/noirc_frontend/src/tests.rs

@@ -3027,7 +3027,7 @@ fn do_not_infer_partial_global_types() {
         pub global FORMATTED_VALUE: str<5> = "there";
         pub global FMT_STR: fmtstr<_, _> = f"hi {FORMATTED_VALUE}";
                    ^^^^^^^ Globals must have a specified type
-                                           ~~~~~~~~~~~~~~~~~~~~~~~ Inferred type is `fmtstr<20, (str<5>)>`
+                                           ~~~~~~~~~~~~~~~~~~~~~~~ Inferred type is `fmtstr<20, (str<5>,)>`
         pub global TUPLE_WITH_MULTIPLE: ([str<_>], [[Field; _]; 3]) = 
                    ^^^^^^^^^^^^^^^^^^^ Globals must have a specified type
             (&["hi"], [[]; 3]);

compiler/noirc_printable_type/src/lib.rs

@@ -187,6 +187,9 @@ fn to_string<F: AcirField>(value: &PrintableValue<F>, typ: &PrintableType) -> Op
                     output.push_str(", ");
                 }
             }
+            if types.len() == 1 {
+                output.push(',');
+            }
             output.push(')');
         }
 
@@ -489,6 +492,31 @@ mod tests {
         assert_eq!(display.to_string(), expected);
     }
 
+    #[test]
+    fn one_element_tuple_to_string() {
+        let value = PrintableValue::<FieldElement>::Vec {
+            array_elements: vec![PrintableValue::Field(1_u128.into())],
+            is_slice: false,
+        };
+        let typ = PrintableType::Tuple { types: vec![PrintableType::Field] };
+        let string = to_string(&value, &typ);
+        assert_eq!(string.unwrap(), "(0x01,)");
+    }
+
+    #[test]
+    fn two_elements_tuple_to_string() {
+        let value = PrintableValue::<FieldElement>::Vec {
+            array_elements: vec![
+                PrintableValue::Field(1_u128.into()),
+                PrintableValue::Field(2_u128.into()),
+            ],
+            is_slice: false,
+        };
+        let typ = PrintableType::Tuple { types: vec![PrintableType::Field, PrintableType::Field] };
+        let string = to_string(&value, &typ);
+        assert_eq!(string.unwrap(), "(0x01, 0x02)");
+    }
+
     proptest! {
         #[test]
         fn handles_decoding_u128_values(uint_value: u128) {

test_programs/compile_failure/noirc_frontend_tests_do_not_infer_partial_global_types/stderr.txt

@@ -16,7 +16,7 @@ error: Globals must have a specified type
   ┌─ src/main.nr:8:20
   │
 8 │         pub global FMT_STR: fmtstr<_, _> = f"hi {FORMATTED_VALUE}";
-  │                    -------                 ----------------------- Inferred type is `fmtstr<20, (str<5>)>`
+  │                    -------                 ----------------------- Inferred type is `fmtstr<20, (str<5>,)>`
   │
 
 error: Globals must have a specified type

tooling/lsp/src/requests/completion.rs

@@ -546,6 +546,9 @@ impl<'a> NodeFinder<'a> {
                     self.collect_local_variables(pattern);
                 }
             }
+            Pattern::Parenthesized(pattern, _) => {
+                self.collect_local_variables(pattern);
+            }
             Pattern::Interned(..) => (),
         }
     }
@@ -1100,7 +1103,9 @@ impl<'a> NodeFinder<'a> {
                     }
                 }
             }
-            Pattern::Mutable(pattern, ..) => self.try_set_self_type(pattern),
+            Pattern::Mutable(pattern, ..) | Pattern::Parenthesized(pattern, _) => {
+                self.try_set_self_type(pattern);
+            }
             Pattern::Tuple(..) | Pattern::Struct(..) | Pattern::Interned(..) => (),
         }
     }

tooling/lsp/src/requests/inlay_hint.rs

@@ -465,6 +465,9 @@ fn push_type_parts(typ: &Type, parts: &mut Vec<InlayHintLabelPart>, files: &File
                     parts.push(string_part(", "));
                 }
             }
+            if types.len() == 1 {
+                parts.push(string_part(","));
+            }
             parts.push(string_part(")"));
         }
         Type::DataType(struct_type, generics) => {

tooling/lsp/src/with_file.rs

@@ -126,6 +126,10 @@ fn pattern_with_file(pattern: Pattern, file: FileId) -> Pattern {
             }),
             location_with_file(location, file),
         ),
+        Pattern::Parenthesized(pattern, location) => Pattern::Parenthesized(
+            Box::new(pattern_with_file(*pattern, file)),
+            location_with_file(location, file),
+        ),
         Pattern::Interned(interned_pattern, location) => {
             Pattern::Interned(interned_pattern, location_with_file(location, file))
         }

tooling/nargo_fmt/src/formatter/global.rs

@@ -53,8 +53,11 @@ impl ChunkFormatter<'_, '_> {
 
                 *pattern
             }
-            Pattern::Tuple(..) | Pattern::Struct(..) | Pattern::Interned(..) => {
-                unreachable!("Global pattern cannot be a tuple, struct or interned")
+            Pattern::Tuple(..)
+            | Pattern::Struct(..)
+            | Pattern::Parenthesized(..)
+            | Pattern::Interned(..) => {
+                unreachable!("Global pattern cannot be a tuple, struct, parenthesized or interned")
             }
         };
 

tooling/nargo_fmt/src/formatter/pattern.rs

@@ -94,6 +94,11 @@ impl Formatter<'_> {
 
                 self.format_chunk_group(group);
             }
+            Pattern::Parenthesized(pattern, _) => {
+                self.write_left_paren();
+                self.format_pattern(*pattern);
+                self.write_right_paren();
+            }
             Pattern::Interned(..) => {
                 unreachable!("Should not be present in the AST")
             }
@@ -144,6 +149,13 @@ mod tests {
         assert_format(src, expected);
     }
 
+    #[test]
+    fn format_parenthesized_pattern_one_element() {
+        let src = "fn foo( (  x      ) : i32) {}";
+        let expected = "fn foo((x): i32) {}\n";
+        assert_format(src, expected);
+    }
+
     #[test]
     fn format_struct_pattern_empty() {
         let src = "fn foo( Foo {  } : i32) {}";