[compiler-v2] More concise visibility declarations (aptos-labs#14417)

The syntax `public(friend) fun` and `public(package) fun` is unnecessary verbose. Taken over from Rust, perhaps in Rust `pub(crate) fn` is OK, but Move does add to much noise.

This PR suggests to introduce the syntax `package fun` and `friend fun`. Those read naturally and are also in this in other languages. (E.g. C#).

third_party/move/move-compiler-v2/tests/checking-lang-v1/direct_visibility.exp

@@ -0,0 +1,19 @@
+
+Diagnostics:
+error: unsupported language construct
+  ┌─ tests/checking-lang-v1/direct_visibility.move:2:5
+  │
+2 │     package fun f() {}
+  │     ^^^^^^^ Move 2 language construct is not enabled: direct `package` declaration
+
+error: unsupported language construct
+  ┌─ tests/checking-lang-v1/direct_visibility.move:7:5
+  │
+7 │     friend fun f() {}
+  │     ^^^^^^ Move 2 language construct is not enabled: direct `friend` declaration
+
+error: unsupported language construct
+   ┌─ tests/checking-lang-v1/direct_visibility.move:11:5
+   │
+11 │     friend fun f() {
+   │     ^^^^^^ Move 2 language construct is not enabled: direct `friend` declaration

third_party/move/move-compiler-v2/tests/checking-lang-v1/direct_visibility.move

@@ -0,0 +1,15 @@
+module 0x815::a {
+    package fun f() {}
+}
+
+module 0x815::b {
+    friend 0x815::c;
+    friend fun f() {}
+}
+
+module 0x815::c {
+    friend fun f() {
+        0x815::a::f();
+        0x815::b::f();
+    }
+}

third_party/move/move-compiler-v2/tests/checking/visibility-checker/direct_visibility.exp

@@ -0,0 +1,22 @@
+// -- Model dump before bytecode pipeline
+module 0x815::b {
+    friend fun f() {
+        Tuple()
+    }
+} // end 0x815::b
+module 0x815::a {
+    friend fun f() {
+        Tuple()
+    }
+    friend fun g() {
+        Tuple()
+    }
+} // end 0x815::a
+module 0x815::c {
+    public fun f() {
+        a::f();
+        a::g();
+        b::f();
+        Tuple()
+    }
+} // end 0x815::c

third_party/move/move-compiler-v2/tests/checking/visibility-checker/direct_visibility.move

@@ -0,0 +1,17 @@
+module 0x815::a {
+    package fun f() {}
+    public(package) fun g(){}
+}
+
+module 0x815::b {
+    friend 0x815::c;
+    friend fun f() {}
+}
+
+module 0x815::c {
+    public fun f() {
+        0x815::a::f();
+        0x815::a::g();
+        0x815::b::f();
+    }
+}

third_party/move/move-compiler-v2/tests/checking/visibility-checker/direct_visibility_err.exp

@@ -0,0 +1,10 @@
+
+Diagnostics:
+error: function `0x815::b::f` cannot be called from function `0x815::c::f` because module `0x815::c` is not a `friend` of `0x815::b`
+  ┌─ tests/checking/visibility-checker/direct_visibility_err.move:2:16
+  │
+2 │     friend fun f() {}
+  │                ^ callee
+  ·
+7 │         0x815::b::f();
+  │         ------------- called here

third_party/move/move-compiler-v2/tests/checking/visibility-checker/direct_visibility_err.move

@@ -0,0 +1,9 @@
+module 0x815::b {
+    friend fun f() {}
+}
+
+module 0x815::c {
+    public fun f() {
+        0x815::b::f();
+    }
+}

third_party/move/move-compiler-v2/tests/checking/visibility-checker/direct_visibility_err2.exp

@@ -0,0 +1,9 @@
+
+Diagnostics:
+error: duplicate declaration, item, or annotation
+  ┌─ tests/checking/visibility-checker/direct_visibility_err2.move:2:13
+  │
+2 │     package friend fun f() {}
+  │     ------- ^^^^^^ Duplicate visibility modifier
+  │     │
+  │     Visibility modifier previously given here

third_party/move/move-compiler-v2/tests/checking/visibility-checker/direct_visibility_err2.move

@@ -0,0 +1,3 @@
+module 0x815::a {
+    package friend fun f() {}
+}

third_party/move/move-compiler-v2/tests/checking/visibility-checker/direct_visibility_err3.exp

@@ -0,0 +1,10 @@
+
+Diagnostics:
+error: unexpected token
+  ┌─ tests/checking/visibility-checker/direct_visibility_err3.move:2:13
+  │
+2 │     package 0x815::a;
+  │             ^^^^^
+  │             │
+  │             Unexpected '0x815'
+  │             Expected a module member: 'spec', 'use', 'friend', 'const', 'fun', 'inline', or 'struct'

third_party/move/move-compiler-v2/tests/checking/visibility-checker/direct_visibility_err3.move

@@ -0,0 +1,3 @@
+module 0x815::b {
+    package 0x815::a;
+}

third_party/move/move-compiler/src/parser/syntax.rs

@@ -553,20 +553,39 @@ impl Modifiers {
 // ModuleMemberModifiers checks for uniqueness, meaning each individual ModuleMemberModifier can
 // appear only once
 fn parse_module_member_modifiers(context: &mut Context) -> Result<Modifiers, Box<Diagnostic>> {
+    let check_previous_vis = |context: &mut Context, mods: &mut Modifiers, vis: &Visibility| {
+        if let Some(prev_vis) = &mods.visibility {
+            let msg = "Duplicate visibility modifier".to_string();
+            let prev_msg = "Visibility modifier previously given here".to_string();
+            context.env.add_diag(diag!(
+                Declarations::DuplicateItem,
+                (vis.loc().unwrap(), msg),
+                (prev_vis.loc().unwrap(), prev_msg),
+            ));
+        }
+    };
     let mut mods = Modifiers::empty();
     loop {
         match context.tokens.peek() {
             Tok::Public => {
                 let vis = parse_visibility(context)?;
-                if let Some(prev_vis) = mods.visibility {
-                    let msg = "Duplicate visibility modifier".to_string();
-                    let prev_msg = "Visibility modifier previously given here".to_string();
-                    context.env.add_diag(diag!(
-                        Declarations::DuplicateItem,
-                        (vis.loc().unwrap(), msg),
-                        (prev_vis.loc().unwrap(), prev_msg),
-                    ));
-                }
+                check_previous_vis(context, &mut mods, &vis);
+                mods.visibility = Some(vis)
+            },
+            Tok::Friend => {
+                let loc = current_token_loc(context.tokens);
+                context.tokens.advance()?;
+                require_move_2(context, loc, "direct `friend` declaration");
+                let vis = Visibility::Friend(loc);
+                check_previous_vis(context, &mut mods, &vis);
+                mods.visibility = Some(vis)
+            },
+            Tok::Identifier if context.tokens.content() == "package" => {
+                let loc = current_token_loc(context.tokens);
+                context.tokens.advance()?;
+                require_move_2(context, loc, "direct `package` declaration");
+                let vis = Visibility::Package(loc);
+                check_previous_vis(context, &mut mods, &vis);
                 mods.visibility = Some(vis)
             },
             Tok::Native => {
@@ -605,6 +624,8 @@ fn parse_module_member_modifiers(context: &mut Context) -> Result<Modifiers, Box
 
 // Parse a function visibility modifier:
 //      Visibility = "public" ( "(" "script" | "friend" | "package" ")" )?
+// Notice that "package" and "friend" visibility can also directly be provided
+// without "public" in declarations, but this is not handled by this function
 fn parse_visibility(context: &mut Context) -> Result<Visibility, Box<Diagnostic>> {
     let start_loc = context.tokens.start_loc();
     consume_token(context.tokens, Tok::Public)?;
@@ -3266,7 +3287,12 @@ fn parse_module(
                 },
                 // Regular move constructs
                 Tok::Use => ModuleMember::Use(parse_use_decl(attributes, context)?),
-                Tok::Friend => ModuleMember::Friend(parse_friend_decl(attributes, context)?),
+                Tok::Friend if context.tokens.lookahead()? != Tok::Fun => {
+                    // Only interpret as module friend declaration if not directly
+                    // followed by fun keyword. This is invalid syntax in v1, so
+                    // we can re-interpret it for Move 2.
+                    ModuleMember::Friend(parse_friend_decl(attributes, context)?)
+                },
                 _ => {
                     context.tokens.match_doc_comments();
                     let start_loc = context.tokens.start_loc();