From 75bfe134457788f912da5173c395d7fbe0bb731a Mon Sep 17 00:00:00 2001
From: Ary Borenszweig <asterite@gmail.com>
Date: Tue, 23 Jul 2024 16:20:10 -0300
Subject: [PATCH] fix: allow trailing comma when parsing where clauses (#5594)

# Description

## Problem

Resolves #4809

## Summary

Also removes some duplicated parsing functions (I checked and their code
was exactly the same).

## Additional Context

None.

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
---
 compiler/noirc_frontend/src/parser/parser.rs  | 42 ++-----------------
 .../src/parser/parser/traits.rs               |  7 ++--
 2 files changed, 7 insertions(+), 42 deletions(-)

diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs
index 7f3e0e68bbc..ca2f4b69aa5 100644
--- a/compiler/noirc_frontend/src/parser/parser.rs
+++ b/compiler/noirc_frontend/src/parser/parser.rs
@@ -37,7 +37,7 @@ use crate::ast::{
     BinaryOp, BinaryOpKind, BlockExpression, ForLoopStatement, ForRange, Ident, IfExpression,
     InfixExpression, LValue, Literal, ModuleDeclaration, NoirTypeAlias, Param, Path, Pattern,
     Recoverable, Statement, TraitBound, TypeImpl, UnaryRhsMemberAccess, UnaryRhsMethodCall,
-    UnresolvedTraitConstraint, UseTree, UseTreeKind, Visibility,
+    UseTree, UseTreeKind, Visibility,
 };
 use crate::ast::{
     Expression, ExpressionKind, LetStatement, StatementKind, UnresolvedType, UnresolvedTypeData,
@@ -71,6 +71,7 @@ mod test_helpers;
 use literals::literal;
 use path::{maybe_empty_path, path};
 use primitives::{dereference, ident, negation, not, nothing, right_shift_operator, token_kind};
+use traits::where_clause;
 
 /// Entry function for the parser - also handles lexing internally.
 ///
@@ -365,45 +366,8 @@ fn function_declaration_parameters() -> impl NoirParser<Vec<(Ident, UnresolvedTy
         .labelled(ParsingRuleLabel::Parameter)
 }
 
-fn where_clause() -> impl NoirParser<Vec<UnresolvedTraitConstraint>> {
-    struct MultiTraitConstraint {
-        typ: UnresolvedType,
-        trait_bounds: Vec<TraitBound>,
-    }
-
-    let constraints = parse_type()
-        .then_ignore(just(Token::Colon))
-        .then(trait_bounds())
-        .map(|(typ, trait_bounds)| MultiTraitConstraint { typ, trait_bounds });
-
-    keyword(Keyword::Where)
-        .ignore_then(constraints.separated_by(just(Token::Comma)))
-        .or_not()
-        .map(|option| option.unwrap_or_default())
-        .map(|x: Vec<MultiTraitConstraint>| {
-            let mut result: Vec<UnresolvedTraitConstraint> = Vec::new();
-            for constraint in x {
-                for bound in constraint.trait_bounds {
-                    result.push(UnresolvedTraitConstraint {
-                        typ: constraint.typ.clone(),
-                        trait_bound: bound,
-                    });
-                }
-            }
-            result
-        })
-}
-
-fn trait_bounds() -> impl NoirParser<Vec<TraitBound>> {
-    trait_bound().separated_by(just(Token::Plus)).at_least(1).allow_trailing()
-}
-
 pub fn trait_bound() -> impl NoirParser<TraitBound> {
-    path().then(generic_type_args(parse_type())).map(|(trait_path, trait_generics)| TraitBound {
-        trait_path,
-        trait_generics,
-        trait_id: None,
-    })
+    traits::trait_bound()
 }
 
 fn block_expr<'a>(
diff --git a/compiler/noirc_frontend/src/parser/parser/traits.rs b/compiler/noirc_frontend/src/parser/parser/traits.rs
index 4e4c9d5c0db..2c64ea31b75 100644
--- a/compiler/noirc_frontend/src/parser/parser/traits.rs
+++ b/compiler/noirc_frontend/src/parser/parser/traits.rs
@@ -151,7 +151,7 @@ fn trait_implementation_body() -> impl NoirParser<Vec<TraitImplItem>> {
     function.or(alias).repeated()
 }
 
-fn where_clause() -> impl NoirParser<Vec<UnresolvedTraitConstraint>> {
+pub(super) fn where_clause() -> impl NoirParser<Vec<UnresolvedTraitConstraint>> {
     struct MultiTraitConstraint {
         typ: UnresolvedType,
         trait_bounds: Vec<TraitBound>,
@@ -163,7 +163,7 @@ fn where_clause() -> impl NoirParser<Vec<UnresolvedTraitConstraint>> {
         .map(|(typ, trait_bounds)| MultiTraitConstraint { typ, trait_bounds });
 
     keyword(Keyword::Where)
-        .ignore_then(constraints.separated_by(just(Token::Comma)))
+        .ignore_then(constraints.separated_by(just(Token::Comma)).allow_trailing())
         .or_not()
         .map(|option| option.unwrap_or_default())
         .map(|x: Vec<MultiTraitConstraint>| {
@@ -184,7 +184,7 @@ fn trait_bounds() -> impl NoirParser<Vec<TraitBound>> {
     trait_bound().separated_by(just(Token::Plus)).at_least(1).allow_trailing()
 }
 
-fn trait_bound() -> impl NoirParser<TraitBound> {
+pub(super) fn trait_bound() -> impl NoirParser<TraitBound> {
     path().then(generic_type_args(parse_type())).map(|(trait_path, trait_generics)| TraitBound {
         trait_path,
         trait_generics,
@@ -215,6 +215,7 @@ mod test {
                 "trait GenericTrait<T> { fn elem(&mut self, index: Field) -> T; }",
                 "trait GenericTraitWithConstraints<T> where T: SomeTrait { fn elem(self, index: Field) -> T; }",
                 "trait TraitWithMultipleGenericParams<A, B, C> where A: SomeTrait, B: AnotherTrait<C> { let Size: Field; fn zero() -> Self; }",
+                "trait TraitWithMultipleGenericParams<A, B, C> where A: SomeTrait, B: AnotherTrait<C>, { let Size: Field; fn zero() -> Self; }",
             ],
         );