From f2105fed292fc8d3ee6c3722e9dedb8b3b16453d Mon Sep 17 00:00:00 2001
From: Tom French <15848336+TomAFrench@users.noreply.github.com>
Date: Wed, 28 Feb 2024 20:08:24 +0000
Subject: [PATCH] chore: create parser specifically for function visibility
 (#4425)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

This PR creates a parser for `'pub(crate)'|'pub'|''` which returns the
`FunctionVisibility`. This allows parsing visibility more reusable for
when we add visibility for imports, structs, etc.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** 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.
---
 .../src/parser/parser/function.rs             | 51 ++++++++-----------
 1 file changed, 22 insertions(+), 29 deletions(-)

diff --git a/compiler/noirc_frontend/src/parser/parser/function.rs b/compiler/noirc_frontend/src/parser/parser/function.rs
index 0d34c719061..42ee484bfc9 100644
--- a/compiler/noirc_frontend/src/parser/parser/function.rs
+++ b/compiler/noirc_frontend/src/parser/parser/function.rs
@@ -38,13 +38,7 @@ pub(super) fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunct
                 is_unconstrained: modifiers.0,
                 is_open: modifiers.2,
                 is_internal: modifiers.3,
-                visibility: if modifiers.1 {
-                    FunctionVisibility::PublicCrate
-                } else if modifiers.4 {
-                    FunctionVisibility::Public
-                } else {
-                    FunctionVisibility::Private
-                },
+                visibility: modifiers.1,
                 generics,
                 parameters,
                 body,
@@ -57,36 +51,35 @@ pub(super) fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunct
         })
 }
 
-/// function_modifiers: 'unconstrained'? 'pub(crate)'? 'pub'? 'open'? 'internal'?
+/// visibility_modifier: 'pub(crate)'? 'pub'? ''
+fn visibility_modifier() -> impl NoirParser<FunctionVisibility> {
+    let is_pub_crate = (keyword(Keyword::Pub)
+        .then_ignore(just(Token::LeftParen))
+        .then_ignore(keyword(Keyword::Crate))
+        .then_ignore(just(Token::RightParen)))
+    .map(|_| FunctionVisibility::PublicCrate);
+
+    let is_pub = keyword(Keyword::Pub).map(|_| FunctionVisibility::Public);
+
+    let is_private = empty().map(|_| FunctionVisibility::Private);
+
+    choice((is_pub_crate, is_pub, is_private))
+}
+
+/// function_modifiers: 'unconstrained'? (visibility)? 'open'? 'internal'?
 ///
-/// returns (is_unconstrained, is_pub_crate, is_open, is_internal, is_pub) for whether each keyword was present
-fn function_modifiers() -> impl NoirParser<(bool, bool, bool, bool, bool)> {
+/// returns (is_unconstrained, visibility, is_open, is_internal) for whether each keyword was present
+fn function_modifiers() -> impl NoirParser<(bool, FunctionVisibility, bool, bool)> {
     keyword(Keyword::Unconstrained)
         .or_not()
-        .then(is_pub_crate())
-        .then(keyword(Keyword::Pub).or_not())
+        .then(visibility_modifier())
         .then(keyword(Keyword::Open).or_not())
         .then(keyword(Keyword::Internal).or_not())
-        .map(|((((unconstrained, pub_crate), public), open), internal)| {
-            (
-                unconstrained.is_some(),
-                pub_crate,
-                open.is_some(),
-                internal.is_some(),
-                public.is_some(),
-            )
+        .map(|(((unconstrained, visibility), open), internal)| {
+            (unconstrained.is_some(), visibility, open.is_some(), internal.is_some())
         })
 }
 
-fn is_pub_crate() -> impl NoirParser<bool> {
-    (keyword(Keyword::Pub)
-        .then_ignore(just(Token::LeftParen))
-        .then_ignore(keyword(Keyword::Crate))
-        .then_ignore(just(Token::RightParen)))
-    .or_not()
-    .map(|a| a.is_some())
-}
-
 /// non_empty_ident_list: ident ',' non_empty_ident_list
 ///                     | ident
 ///