fix method visibility check, more path res tests

Author: sbillig

crates/hir-analysis/src/name_resolution/path_resolver.rs

@@ -364,9 +364,20 @@ impl<'db> PathRes<'db> {
 
     pub fn is_visible_from(&self, db: &'db dyn HirAnalysisDb, from_scope: ScopeId<'db>) -> bool {
         match self {
-            PathRes::Ty(ty) | PathRes::Func(ty) | PathRes::Const(ty) | PathRes::Method(ty, _) => {
+            PathRes::Ty(ty) | PathRes::Func(ty) | PathRes::Const(ty) => {
                 is_ty_visible_from(db, *ty, from_scope)
             }
+            PathRes::Method(_, cand) => {
+                // Method visibility depends on the method's defining scope
+                // (function or trait method), not the receiver type.
+                let method_scope = match cand {
+                    MethodCandidate::InherentMethod(func_def) => func_def.scope(db),
+                    MethodCandidate::TraitMethod(c) | MethodCandidate::NeedsConfirmation(c) => {
+                        c.method.0.scope(db)
+                    }
+                };
+                is_scope_visible_from(db, method_scope, from_scope)
+            }
             r => is_scope_visible_from(db, r.as_scope(db).unwrap(), from_scope),
         }
     }
@@ -605,7 +616,7 @@ where
             if let Some(enum_) = ty.as_enum(db) {
                 // We need to use the concrete enum scope instead of
                 // parent_scope to resolve the variants in all cases,
-                // eg when parent is `Self`. I'm not really sure why this is.
+                // eg when parent is `Self`
                 let query = make_query(db, path, enum_.scope());
                 let bucket = resolve_query(db, query);
 
@@ -884,7 +895,15 @@ pub fn resolve_name_res<'db>(
                     PathRes::Func(TyId::foldl(db, ty, args))
                 }
                 ItemKind::Const(const_) => {
-                    // TODO err if any args
+                    if !args.is_empty() {
+                        return Err(PathResError::new(
+                            PathResErrorKind::ArgNumMismatch {
+                                expected: 0,
+                                given: args.len(),
+                            },
+                            path,
+                        ));
+                    }
                     let ty = if let Some(ty) = const_.ty(db).to_opt() {
                         lower_hir_ty(db, ty, scope, assumptions)
                     } else {
@@ -895,7 +914,8 @@ pub fn resolve_name_res<'db>(
 
                 ItemKind::TypeAlias(type_alias) => {
                     let alias = lower_type_alias(db, type_alias);
-                    if args.len() < alias.params(db).len() {
+                    let expected = alias.params(db).len();
+                    if args.len() < expected {
                         PathRes::TyAlias(
                             alias.clone(),
                             TyId::invalid(
@@ -906,8 +926,27 @@ pub fn resolve_name_res<'db>(
                                 },
                             ),
                         )
+                    } else if args.len() > expected {
+                        return Err(PathResError::new(
+                            PathResErrorKind::ArgNumMismatch { expected, given: args.len() },
+                            path,
+                        ));
                     } else {
-                        PathRes::TyAlias(alias.clone(), alias.alias_to.instantiate(db, args))
+                        let instantiated = alias.alias_to.instantiate(db, args);
+                        if let TyData::Invalid(InvalidCause::TooManyGenericArgs {
+                            expected,
+                            given,
+                        }) = instantiated.data(db)
+                        {
+                            return Err(PathResError::new(
+                                PathResErrorKind::ArgNumMismatch {
+                                    expected: *expected,
+                                    given: *given,
+                                },
+                                path,
+                            ));
+                        }
+                        PathRes::TyAlias(alias.clone(), instantiated)
                     }
                 }
 

crates/hir-analysis/test_files/early_path_resolution/enum_self_variant.fe

@@ -0,0 +1,12 @@
+enum Option<T> {
+    None,
+    Some(T),
+}
+
+impl Option<i32> {
+    fn make_some(val: i32) -> Option<i32> {
+        // Ensure `Self::Some` resolves via the enum scope, not lexical scope.
+        Self::Some(val)
+    }
+}
+

crates/hir-analysis/test_files/early_path_resolution/enum_self_variant.snap

@@ -0,0 +1,41 @@
+---
+source: crates/hir-analysis/tests/early_path_resolution.rs
+assertion_line: 40
+expression: res
+input_file: test_files/early_path_resolution/enum_self_variant.fe
+---
+note: 
+  ┌─ enum_self_variant.fe:3:10
+  │
+3 │     Some(T),
+  │          ^ enum_self_variant::Option::T
+
+note: 
+  ┌─ enum_self_variant.fe:6:6
+  │
+6 │ impl Option<i32> {
+  │      ^^^^^^ enum_self_variant::Option
+
+note: 
+  ┌─ enum_self_variant.fe:6:13
+  │
+6 │ impl Option<i32> {
+  │             ^^^ i32
+
+note: 
+  ┌─ enum_self_variant.fe:7:23
+  │
+7 │     fn make_some(val: i32) -> Option<i32> {
+  │                       ^^^ i32
+
+note: 
+  ┌─ enum_self_variant.fe:7:31
+  │
+7 │     fn make_some(val: i32) -> Option<i32> {
+  │                               ^^^^^^ enum_self_variant::Option
+
+note: 
+  ┌─ enum_self_variant.fe:7:38
+  │
+7 │     fn make_some(val: i32) -> Option<i32> {
+  │                                      ^^^ i32

crates/uitest/fixtures/name_resolution/const_generic_args.fe

@@ -0,0 +1,9 @@
+mod m {
+    pub const C: i32 = 1
+}
+
+pub fn bad() -> i32 {
+    // Generic arguments on a const should error with an arg count mismatch.
+    m::C<i32>
+}
+

crates/uitest/fixtures/name_resolution/const_generic_args.snap

@@ -0,0 +1,11 @@
+---
+source: crates/hir-analysis/tests/name_resolution.rs
+assertion_line: 48
+expression: diagnostic_output
+input_file: test_files/name_resolution/const_generic_args.fe
+---
+error[2-0011]: incorrect number of generic arguments for `C`; expected 0, given 1
+  ┌─ const_generic_args.fe:7:8
+  │
+7 │     m::C<i32>
+  │        ^^^^^^ expected 0 arguments, but 1 were given

crates/uitest/fixtures/name_resolution/method_visibility.fe

@@ -0,0 +1,25 @@
+mod m {
+    pub struct S {}
+
+    impl S {
+        pub fn new() -> S { S {} }
+
+        // Private inherent method: only callable within `m`.
+        fn secret(self) -> i32 { 1 }
+
+        pub fn public(self) -> i32 { self.secret() }
+    }
+
+    pub fn make() -> S { S::new() }
+}
+
+fn ok() -> i32 {
+    // Outside module: public method is callable
+    m::make().public()
+}
+
+fn bad() -> i32 {
+    // Outside module: calling private method should be invisible
+    let s = m::make()
+    s.secret()
+}

crates/uitest/fixtures/name_resolution/method_visibility.snap

@@ -0,0 +1,20 @@
+---
+source: crates/hir-analysis/tests/visibility.rs
+assertion_line: 49
+expression: diagnostic_output
+input_file: test_files/visibility/method_visibility.fe
+---
+error[1-0001]: unexpected syntax while parsing block
+   ┌─ method_visibility.fe:23:22
+   │
+23 │     let s = m::make();
+   │                      ^ unexpected
+
+error[2-0003]: `secret` is not visible
+   ┌─ method_visibility.fe:24:7
+   │
+ 8 │         fn secret(self) -> i32 { 1 }
+   │            ------ `secret` is defined here
+   ·
+24 │     s.secret()
+   │       ^^^^^^ `secret` is not visible

crates/uitest/fixtures/name_resolution/type_alias_extra_args.fe

@@ -0,0 +1,6 @@
+mod m {
+    pub type Alias<T> = T
+}
+
+pub fn f(x: m::Alias<i32, u32>) {}
+

crates/uitest/fixtures/name_resolution/type_alias_extra_args.snap

@@ -0,0 +1,12 @@
+---
+source: crates/hir-analysis/tests/name_resolution.rs
+assertion_line: 48
+expression: diagnostic_output
+input_file: test_files/name_resolution/type_alias_extra_args.fe
+---
+error[2-0011]: incorrect number of generic arguments for `Alias`; expected 1, given 2
+  ┌─ type_alias_extra_args.fe:5:16
+  │
+5 │ pub fn f(x: m::Alias<i32, u32>) {}
+  │                ^^^^^ expected 1 arguments, but 2 were given
+

crates/uitest/fixtures/ty/def/alias_arg_mismatch.snap

@@ -1,8 +1,15 @@
 ---
 source: crates/uitest/tests/ty.rs
+assertion_line: 27
 expression: diags
-input_file: crates/uitest/fixtures/ty/alias_arg_mismatch.fe
+input_file: fixtures/ty/def/alias_arg_mismatch.fe
 ---
+error[2-0011]: incorrect number of generic arguments for `T2`; expected 1, given 2
+  ┌─ alias_arg_mismatch.fe:4:8
+  │
+4 │     u: T2<i32, u32>,
+  │        ^^ expected 1 arguments, but 2 were given
+
 error[3-0003]: all type parameters of type alias must be given
    ┌─ alias_arg_mismatch.fe:2:8
    │
@@ -19,5 +26,3 @@ error[3-0003]: all type parameters of type alias must be given
    │ ----------------------- type alias defined here
 13 │ type T2<T> = T1<T>
    │              ^^^^^ expected at least 2 arguments here
-
-