Rework and fix associated const visibility for impl items

sway-ast/src/item/item_const.rs

@@ -2,7 +2,7 @@ use crate::priv_prelude::*;
 
 #[derive(Clone, Debug, Serialize)]
 pub struct ItemConst {
-    pub visibility: Option<PubToken>,
+    pub pub_token: Option<PubToken>,
     pub const_token: ConstToken,
     pub name: Ident,
     pub ty_opt: Option<(ColonToken, Ty)>,
@@ -13,7 +13,7 @@ pub struct ItemConst {
 
 impl Spanned for ItemConst {
     fn span(&self) -> Span {
-        let start = match &self.visibility {
+        let start = match &self.pub_token {
             Some(pub_token) => pub_token.span(),
             None => self.const_token.span(),
         };

sway-core/src/semantic_analysis/namespace/contract_helpers.rs

@@ -10,6 +10,7 @@ use crate::{
     language::{
         parsed::{AstNode, AstNodeContent, Declaration, ExpressionKind},
         ty::{TyAstNode, TyAstNodeContent},
+        Visibility,
     },
     semantic_analysis::{
         namespace::Root, symbol_collection_context::SymbolCollectionContext, TypeCheckContext,
@@ -69,6 +70,7 @@ fn bind_contract_id_in_root_module(
         handler,
         engines,
         const_item,
+        Visibility::Private,
         attributes,
         true,
     )?;

sway-core/src/semantic_analysis/type_resolve.rs

@@ -7,7 +7,7 @@ use sway_types::{Ident, Span, Spanned};
 use crate::{
     language::{
         ty::{self, TyTraitItem},
-        CallPath, QualifiedCallPath,
+        CallPath, QualifiedCallPath, Visibility,
     },
     monomorphization::type_decl_opt_to_type_id,
     namespace::{Module, ModulePath, ResolvedDeclaration, ResolvedTraitImplItem, Root},
@@ -253,6 +253,7 @@ pub fn resolve_qualified_call_path(
             &qualified_call_path.call_path,
             self_type,
         )
+        .map(|(d, _)| d)
     } else {
         resolve_call_path(
             handler,
@@ -274,6 +275,7 @@ pub fn resolve_qualified_call_path(
 /// The `mod_path` is significant here as we assume the resolution is done within the
 /// context of the module pointed to by `mod_path` and will only check the call path prefixes
 /// and the symbol's own visibility.
+#[allow(clippy::too_many_arguments)]
 pub fn resolve_call_path(
     handler: &Handler,
     engines: &Engines,
@@ -283,9 +285,10 @@ pub fn resolve_call_path(
     self_type: Option<TypeId>,
     check_visibility: VisibilityCheck,
 ) -> Result<ResolvedDeclaration, ErrorEmitted> {
+    let mut visibility_level = Visibility::Public;
     let full_path = call_path.to_fullpath_from_mod_path(engines, namespace, &mod_path.to_vec());
 
-    let (decl, decl_mod_path) = resolve_symbol_and_mod_path(
+    let (decl, is_self_type, decl_mod_path) = resolve_symbol_and_mod_path(
         handler,
         engines,
         namespace.root_ref(),
@@ -294,6 +297,10 @@ pub fn resolve_call_path(
         self_type,
     )?;
 
+    if is_self_type == IsSelfType::Yes {
+        visibility_level = Visibility::Private;
+    }
+
     if check_visibility == VisibilityCheck::No {
         return Ok(decl);
     }
@@ -317,7 +324,7 @@ pub fn resolve_call_path(
     }
 
     // Otherwise, check the visibility modifier
-    if !decl.visibility(engines).is_public() {
+    if !decl.visibility(engines).is_public() && visibility_level.is_public() {
         handler.emit_err(CompileError::ImportPrivateSymbol {
             name: call_path.suffix.clone(),
             span: call_path.suffix.span(),
@@ -336,7 +343,7 @@ pub(super) fn resolve_symbol_and_mod_path(
     mod_path: &ModulePath,
     symbol: &Ident,
     self_type: Option<TypeId>,
-) -> Result<(ResolvedDeclaration, Vec<Ident>), ErrorEmitted> {
+) -> Result<(ResolvedDeclaration, IsSelfType, Vec<Ident>), ErrorEmitted> {
     assert!(!mod_path.is_empty());
     if mod_path[0] == *root.current_package_name() {
         resolve_symbol_and_mod_path_inner(handler, engines, root, mod_path, symbol, self_type)
@@ -374,40 +381,48 @@ fn resolve_symbol_and_mod_path_inner(
     mod_path: &ModulePath,
     symbol: &Ident,
     self_type: Option<TypeId>,
-) -> Result<(ResolvedDeclaration, Vec<Ident>), ErrorEmitted> {
+) -> Result<(ResolvedDeclaration, IsSelfType, Vec<Ident>), ErrorEmitted> {
     assert!(!mod_path.is_empty());
     assert!(mod_path[0] == *root.current_package_name());
 
     // This block tries to resolve associated types
     let mut current_module = root.current_package_root_module();
     let mut current_mod_path = vec![mod_path[0].clone()];
     let mut decl_opt = None;
+    let mut is_self_type = IsSelfType::No;
     for ident in mod_path.iter().skip(1) {
         if let Some(decl) = decl_opt {
-            decl_opt = Some(resolve_associated_type_or_item(
+            let (decl, ret_is_self_type) = resolve_associated_type_or_item(
                 handler,
                 engines,
                 current_module,
                 ident,
                 decl,
                 None,
                 self_type,
-            )?);
+            )?;
+            decl_opt = Some(decl);
+            if ret_is_self_type == IsSelfType::Yes {
+                is_self_type = IsSelfType::Yes;
+            }
         } else {
             match current_module.submodule(&[ident.clone()]) {
                 Some(ns) => {
                     current_module = ns;
                     current_mod_path.push(ident.clone());
                 }
                 None => {
+                    if ident.as_str() == "Self" {
+                        is_self_type = IsSelfType::Yes;
+                    }
                     let (decl, _) = current_module.resolve_symbol(handler, engines, ident)?;
                     decl_opt = Some(decl);
                 }
             }
         }
     }
     if let Some(decl) = decl_opt {
-        let decl = resolve_associated_type_or_item(
+        let (decl, ret_is_self_type) = resolve_associated_type_or_item(
             handler,
             engines,
             current_module,
@@ -416,13 +431,16 @@ fn resolve_symbol_and_mod_path_inner(
             None,
             self_type,
         )?;
-        return Ok((decl, current_mod_path));
+        if ret_is_self_type == IsSelfType::Yes {
+            is_self_type = IsSelfType::Yes;
+        }
+        return Ok((decl, is_self_type, current_mod_path));
     }
 
     root.require_module(handler, &mod_path.to_vec())
         .and_then(|module| {
             let (decl, decl_path) = module.resolve_symbol(handler, engines, symbol)?;
-            Ok((decl, decl_path))
+            Ok((decl, is_self_type, decl_path))
         })
 }
 
@@ -460,6 +478,12 @@ fn decl_to_type_info(
     }
 }
 
+#[derive(Debug, PartialEq, Eq)]
+pub(crate) enum IsSelfType {
+    Yes,
+    No,
+}
+
 #[allow(clippy::too_many_arguments)]
 fn resolve_associated_item_from_type_id(
     handler: &Handler,
@@ -469,9 +493,11 @@ fn resolve_associated_item_from_type_id(
     type_id: TypeId,
     as_trait: Option<CallPath>,
     self_type: Option<TypeId>,
-) -> Result<ResolvedDeclaration, ErrorEmitted> {
+) -> Result<(ResolvedDeclaration, IsSelfType), ErrorEmitted> {
+    let mut is_self_type = IsSelfType::No;
     let type_id = if engines.te().get(type_id).is_self_type() {
         if let Some(self_type) = self_type {
+            is_self_type = IsSelfType::Yes;
             self_type
         } else {
             return Err(handler.emit_err(CompileError::Internal(
@@ -484,14 +510,15 @@ fn resolve_associated_item_from_type_id(
     };
     let item_ref =
         TraitMap::get_trait_item_for_type(module, handler, engines, symbol, type_id, as_trait)?;
-    match item_ref {
+    let resolved = match item_ref {
         ResolvedTraitImplItem::Parsed(_item) => todo!(),
         ResolvedTraitImplItem::Typed(item) => match item {
-            TyTraitItem::Fn(fn_ref) => Ok(ResolvedDeclaration::Typed(fn_ref.into())),
-            TyTraitItem::Constant(const_ref) => Ok(ResolvedDeclaration::Typed(const_ref.into())),
-            TyTraitItem::Type(type_ref) => Ok(ResolvedDeclaration::Typed(type_ref.into())),
+            TyTraitItem::Fn(fn_ref) => ResolvedDeclaration::Typed(fn_ref.into()),
+            TyTraitItem::Constant(const_ref) => ResolvedDeclaration::Typed(const_ref.into()),
+            TyTraitItem::Type(type_ref) => ResolvedDeclaration::Typed(type_ref.into()),
         },
-    }
+    };
+    Ok((resolved, is_self_type))
 }
 
 #[allow(clippy::too_many_arguments)]
@@ -503,7 +530,7 @@ fn resolve_associated_type_or_item(
     decl: ResolvedDeclaration,
     as_trait: Option<CallPath>,
     self_type: Option<TypeId>,
-) -> Result<ResolvedDeclaration, ErrorEmitted> {
+) -> Result<(ResolvedDeclaration, IsSelfType), ErrorEmitted> {
     let type_info = decl_to_type_info(handler, engines, symbol, decl)?;
     let type_id = engines
         .te()
@@ -523,10 +550,11 @@ fn resolve_call_path_and_root_type_id(
     mut as_trait: Option<CallPath>,
     call_path: &CallPath,
     self_type: Option<TypeId>,
-) -> Result<ResolvedDeclaration, ErrorEmitted> {
+) -> Result<(ResolvedDeclaration, IsSelfType), ErrorEmitted> {
     // This block tries to resolve associated types
     let mut decl_opt = None;
     let mut type_id_opt = Some(root_type_id);
+    let mut is_self_type = IsSelfType::No;
     for ident in call_path.prefixes.iter() {
         if let Some(type_id) = type_id_opt {
             type_id_opt = None;
@@ -540,7 +568,10 @@ fn resolve_call_path_and_root_type_id(
                 self_type,
             )?);
             as_trait = None;
-        } else if let Some(decl) = decl_opt {
+        } else if let Some((decl, ret_is_self_type)) = decl_opt {
+            if ret_is_self_type == IsSelfType::Yes {
+                is_self_type = IsSelfType::Yes;
+            }
             decl_opt = Some(resolve_associated_type_or_item(
                 handler,
                 engines,
@@ -554,7 +585,7 @@ fn resolve_call_path_and_root_type_id(
         }
     }
     if let Some(type_id) = type_id_opt {
-        let decl = resolve_associated_item_from_type_id(
+        let (decl, ret_is_self_type) = resolve_associated_item_from_type_id(
             handler,
             engines,
             module,
@@ -563,10 +594,16 @@ fn resolve_call_path_and_root_type_id(
             as_trait,
             self_type,
         )?;
-        return Ok(decl);
+        if ret_is_self_type == IsSelfType::Yes {
+            is_self_type = IsSelfType::Yes;
+        }
+        return Ok((decl, is_self_type));
     }
-    if let Some(decl) = decl_opt {
-        let decl = resolve_associated_type_or_item(
+    if let Some((decl, ret_is_self_type)) = decl_opt {
+        if ret_is_self_type == IsSelfType::Yes {
+            is_self_type = IsSelfType::Yes;
+        }
+        let (decl, ret_is_self_type) = resolve_associated_type_or_item(
             handler,
             engines,
             module,
@@ -575,7 +612,10 @@ fn resolve_call_path_and_root_type_id(
             as_trait,
             self_type,
         )?;
-        Ok(decl)
+        if ret_is_self_type == IsSelfType::Yes {
+            is_self_type = IsSelfType::Yes;
+        }
+        Ok((decl, is_self_type))
     } else {
         Err(handler.emit_err(CompileError::Internal("Unexpected error", call_path.span())))
     }

sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs

@@ -232,7 +232,13 @@ pub fn item_to_ast_nodes(
         }
         ItemKind::Const(item_const) => decl(Declaration::ConstantDeclaration({
             item_const_to_constant_declaration(
-                context, handler, engines, item_const, attributes, true,
+                context,
+                handler,
+                engines,
+                item_const,
+                Visibility::Private,
+                attributes,
+                true,
             )?
         })),
         ItemKind::Storage(item_storage) => decl(Declaration::StorageDeclaration(
@@ -688,7 +694,13 @@ fn item_trait_to_trait_declaration(
                         .map(TraitItem::TraitFn)
                 }
                 ItemTraitItem::Const(const_decl, _) => item_const_to_constant_declaration(
-                    context, handler, engines, const_decl, attributes, false,
+                    context,
+                    handler,
+                    engines,
+                    const_decl,
+                    Visibility::Public,
+                    attributes,
+                    false,
                 )
                 .map(TraitItem::Constant),
                 ItemTraitItem::Type(trait_type, _) => trait_type_to_trait_type_declaration(
@@ -772,7 +784,13 @@ pub fn item_impl_to_declaration(
                 )
                 .map(ImplItem::Fn),
                 sway_ast::ItemImplItem::Const(const_item) => item_const_to_constant_declaration(
-                    context, handler, engines, const_item, attributes, false,
+                    context,
+                    handler,
+                    engines,
+                    const_item,
+                    Visibility::Private,
+                    attributes,
+                    false,
                 )
                 .map(ImplItem::Constant),
                 sway_ast::ItemImplItem::Type(type_item) => trait_type_to_trait_type_declaration(
@@ -911,7 +929,13 @@ fn item_abi_to_abi_declaration(
                             Ok(TraitItem::TraitFn(trait_fn))
                         }
                         ItemTraitItem::Const(const_decl, _) => item_const_to_constant_declaration(
-                            context, handler, engines, const_decl, attributes, false,
+                            context,
+                            handler,
+                            engines,
+                            const_decl,
+                            Visibility::Public,
+                            attributes,
+                            false,
                         )
                         .map(TraitItem::Constant),
                         ItemTraitItem::Type(type_decl, _) => trait_type_to_trait_type_declaration(
@@ -973,6 +997,7 @@ pub(crate) fn item_const_to_constant_declaration(
     handler: &Handler,
     engines: &Engines,
     item_const: ItemConst,
+    default_visibility: Visibility,
     attributes: AttributesMap,
     require_expression: bool,
 ) -> Result<ParsedDeclId<ConstantDeclaration>, ErrorEmitted> {
@@ -1005,11 +1030,16 @@ pub(crate) fn item_const_to_constant_declaration(
         }
     };
 
+    let visibility = match item_const.pub_token {
+        Some(pub_token) => pub_token_opt_to_visibility(Some(pub_token)),
+        None => default_visibility,
+    };
+
     let const_decl = ConstantDeclaration {
         name: item_const.name,
         type_ascription,
         value: expr,
-        visibility: pub_token_opt_to_visibility(item_const.visibility),
+        visibility,
         attributes,
         span,
     };

sway-lsp/src/traverse/lexed_tree.rs

@@ -383,7 +383,7 @@ impl Parse for ItemAbi {
 
 impl Parse for ItemConst {
     fn parse(&self, ctx: &ParseContext) {
-        if let Some(visibility) = &self.visibility {
+        if let Some(visibility) = &self.pub_token {
             insert_keyword(ctx, visibility.span());
         }
         insert_keyword(ctx, self.const_token.span());