fix a bug when using an impl with missing constant/impl (#5879)

crates/cairo-lang-semantic/src/db.rs

@@ -720,7 +720,7 @@ pub trait SemanticGroup:
         &self,
         impl_def_id: ImplDefId,
         trait_type_id: TraitTypeId,
-    ) -> Maybe<Option<ImplTypeDefId>>;
+    ) -> Maybe<ImplTypeDefId>;
 
     /// Returns the constant items in the impl.
     #[salsa::invoke(items::imp::impl_constants)]
@@ -838,7 +838,7 @@ pub trait SemanticGroup:
     /// type with a concrete impl.
     #[salsa::invoke(items::imp::impl_type_concrete_implized)]
     #[salsa::cycle(items::imp::impl_type_concrete_implized_cycle)]
-    fn impl_type_concrete_implized(&self, impl_type_def_id: ImplTypeId) -> Maybe<Option<TypeId>>;
+    fn impl_type_concrete_implized(&self, impl_type_def_id: ImplTypeId) -> Maybe<TypeId>;
 
     // Impl constant def.
     // ================
@@ -1055,7 +1055,7 @@ pub trait SemanticGroup:
         &self,
         trait_type_def_id: TraitTypeId,
         impl_def_id: ImplDefId,
-    ) -> Maybe<Option<TypeId>>;
+    ) -> Maybe<TypeId>;
 
     // Free function.
     // ==============

crates/cairo-lang-semantic/src/expr/inference.rs

@@ -733,8 +733,7 @@ impl<'db> Inference<'db> {
                     ty,
                     self.db
                         .impl_type_concrete_implized(ImplTypeId::new(impl_id, trait_ty, self.db))
-                        .map_err(|_| ErrorSet)?
-                        .unwrap(),
+                        .map_err(|_| ErrorSet)?,
                 )?;
             }
             for (trait_constant, constant_id) in mappings.constants {
@@ -1101,7 +1100,7 @@ impl<'a> SemanticRewriter<TypeLongId, NoError> for Inference<'a> {
                         impl_type_id_rewrite_result
                     }
                     ImplLongId::Concrete(_) => {
-                        if let Ok(Some(ty)) = self.db.impl_type_concrete_implized(ImplTypeId::new(
+                        if let Ok(ty) = self.db.impl_type_concrete_implized(ImplTypeId::new(
                             impl_id, trait_ty, self.db,
                         )) {
                             *value = self.rewrite(ty).no_err().lookup_intern(self.db);

crates/cairo-lang-semantic/src/expr/inference/conform.rs

@@ -475,7 +475,7 @@ impl Inference<'_> {
         let trait_ty = impl_type_id.ty();
         if let ImplLongId::ImplVar(var) = impl_id.lookup_intern(self.db) {
             Ok(self.rewritten_impl_type(var, trait_ty))
-        } else if let Ok(Some(ty)) =
+        } else if let Ok(ty) =
             self.db.impl_type_concrete_implized(ImplTypeId::new(impl_id, trait_ty, self.db))
         {
             Ok(ty)

crates/cairo-lang-semantic/src/items/imp.rs

@@ -131,13 +131,6 @@ impl ConcreteImplId {
     ) -> Maybe<Option<ImplFunctionId>> {
         db.impl_function_by_trait_function(self.impl_def_id(db), function)
     }
-    pub fn get_impl_type_def(
-        &self,
-        db: &dyn SemanticGroup,
-        ty: TraitTypeId,
-    ) -> Maybe<Option<ImplTypeDefId>> {
-        db.impl_type_by_trait_type(self.impl_def_id(db), ty)
-    }
     pub fn name(&self, db: &dyn SemanticGroup) -> SmolStr {
         self.impl_def_id(db).name(db.upcast())
     }
@@ -795,16 +788,20 @@ pub fn impl_type_by_trait_type(
     db: &dyn SemanticGroup,
     impl_def_id: ImplDefId,
     trait_type_id: TraitTypeId,
-) -> Maybe<Option<ImplTypeDefId>> {
+) -> Maybe<ImplTypeDefId> {
     if trait_type_id.trait_id(db.upcast()) != db.impl_def_trait(impl_def_id)? {
-        // The trait type belongs to a trait other than the one the impl implements.
-        return Ok(None);
+        unreachable!(
+            "impl_type_by_trait_type called with a trait type that does not belong to the impl's \
+             trait"
+        )
     }
 
     let defs_db = db.upcast();
     let name = trait_type_id.name(defs_db);
-    db.impl_item_by_name(impl_def_id, name).map(|maybe_item_id| {
-        maybe_item_id.and_then(|item_id| try_extract_matches!(item_id, ImplItemId::Type))
+    // If the trait type's name is not found, then a missing item diagnostic is reported.
+    db.impl_item_by_name(impl_def_id, name).and_then(|maybe_item_id| match maybe_item_id {
+        Some(item_id) => Ok(extract_matches!(item_id, ImplItemId::Type)),
+        None => Err(skip_diagnostic()),
     })
 }
 
@@ -831,8 +828,11 @@ pub fn impl_constant_by_trait_constant(
 
     let defs_db = db.upcast();
     let name = trait_constant_id.name(defs_db);
-    db.impl_item_by_name(impl_def_id, name)
-        .map(|maybe_item_id| extract_matches!(maybe_item_id.unwrap(), ImplItemId::Constant))
+    // If the trait constant's name is not found, then a missing item diagnostic is reported.
+    db.impl_item_by_name(impl_def_id, name).and_then(|maybe_item_id| match maybe_item_id {
+        Some(item_id) => Ok(extract_matches!(item_id, ImplItemId::Constant)),
+        None => Err(skip_diagnostic()),
+    })
 }
 
 /// Query implementation of [crate::db::SemanticGroup::impl_impls].
@@ -875,8 +875,11 @@ pub fn impl_impl_by_trait_impl(
 
     let defs_db = db.upcast();
     let name = trait_impl_id.name(defs_db);
-    db.impl_item_by_name(impl_def_id, name)
-        .map(|maybe_item_id| extract_matches!(maybe_item_id.unwrap(), ImplItemId::Impl))
+    // If the trait impl's name is not found, then a missing item diagnostic is reported.
+    db.impl_item_by_name(impl_def_id, name).and_then(|maybe_item_id| match maybe_item_id {
+        Some(item_id) => Ok(extract_matches!(item_id, ImplItemId::Impl)),
+        None => Err(skip_diagnostic()),
+    })
 }
 
 // --- Computation ---
@@ -1802,40 +1805,38 @@ fn validate_impl_item_type(
 pub fn impl_type_concrete_implized(
     db: &dyn SemanticGroup,
     impl_type_id: ImplTypeId,
-) -> Maybe<Option<TypeId>> {
+) -> Maybe<TypeId> {
     let concrete_impl = match impl_type_id.impl_id().lookup_intern(db) {
         ImplLongId::Concrete(concrete_impl) => concrete_impl,
         ImplLongId::ImplImpl(imp_impl_id) => {
             let ImplLongId::Concrete(concrete_impl) =
                 db.impl_impl_concrete_implized(imp_impl_id)?.lookup_intern(db)
             else {
-                return Ok(Some(TypeLongId::ImplType(impl_type_id).intern(db)));
+                return Ok(TypeLongId::ImplType(impl_type_id).intern(db));
             };
             concrete_impl
         }
-        ImplLongId::GenericParameter(_) | ImplLongId::TraitImpl(_) => {
-            return Ok(Some(TypeLongId::ImplType(impl_type_id).intern(db)));
+        ImplLongId::GenericParameter(_) | ImplLongId::TraitImpl(_) | ImplLongId::ImplVar(_) => {
+            return Ok(TypeLongId::ImplType(impl_type_id).intern(db));
         }
-
-        ImplLongId::ImplVar(_) => return Ok(None),
     };
 
     let impl_def_id = concrete_impl.impl_def_id(db);
     let ty = db.trait_type_implized_by_context(impl_type_id.ty(), impl_def_id);
-    let Ok(Some(ty)) = ty else {
+    let Ok(ty) = ty else {
         return ty;
     };
 
     let substitution = &concrete_impl.substitution(db)?;
-    SubstitutionRewriter { db, substitution }.rewrite(ty).map(Some)
+    SubstitutionRewriter { db, substitution }.rewrite(ty)
 }
 
 /// Cycle handling for [crate::db::SemanticGroup::impl_type_concrete_implized].
 pub fn impl_type_concrete_implized_cycle(
     db: &dyn SemanticGroup,
     _cycle: &[String],
     impl_type_id: &ImplTypeId,
-) -> Maybe<Option<TypeId>> {
+) -> Maybe<TypeId> {
     // Forwarding cycle handling to `priv_impl_type_semantic_data` handler.
     impl_type_concrete_implized(db, *impl_type_id)
 }

crates/cairo-lang-semantic/src/items/implization.rs

@@ -9,12 +9,10 @@ pub fn trait_type_implized_by_context(
     db: &dyn SemanticGroup,
     trait_type_id: TraitTypeId,
     impl_def_id: ImplDefId,
-) -> Maybe<Option<TypeId>> {
-    let Some(impl_type_def_id) = db.impl_type_by_trait_type(impl_def_id, trait_type_id)? else {
-        return Ok(None);
-    };
+) -> Maybe<TypeId> {
+    let impl_type_def_id = db.impl_type_by_trait_type(impl_def_id, trait_type_id)?;
 
-    Ok(Some(db.impl_type_def_resolved_type(impl_type_def_id)?))
+    db.impl_type_def_resolved_type(impl_type_def_id)
 }
 
 /// Cycle handling for [crate::db::SemanticGroup::trait_type_implized_by_context].
@@ -23,7 +21,7 @@ pub fn trait_type_implized_by_context_cycle(
     _cycle: &[String],
     trait_type_id: &TraitTypeId,
     impl_def_id: &ImplDefId,
-) -> Maybe<Option<TypeId>> {
+) -> Maybe<TypeId> {
     // Forwarding cycle handling to `priv_impl_type_semantic_data` handler.
     trait_type_implized_by_context(db, *trait_type_id, *impl_def_id)
 }

crates/cairo-lang-semantic/src/items/tests/trait_impl

@@ -1103,3 +1103,38 @@ impl MyImpl of MyTrait {
 }
 
 //! > expected_diagnostics
+
+//! > ==========================================================================
+
+//! > Use of missing Impl Impl.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: true)
+
+//! > function
+fn foo() {
+    MyImpl::I::f()
+}
+
+//! > function_name
+foo
+
+//! > module_code
+trait MyTrait {
+    impl I: Self;
+    fn f();
+}
+impl MyImpl of MyTrait {
+    fn f() {}
+}
+
+//! > expected_diagnostics
+error: Not all trait items are implemented. Missing: 'I'.
+ --> lib.cairo:5:6
+impl MyImpl of MyTrait {
+     ^****^
+
+error: Trait has no implementation in context: test::MyTrait.
+ --> lib.cairo:8:10
+fn foo() {
+         ^

crates/cairo-lang-semantic/src/items/tests/trait_type

@@ -2248,3 +2248,35 @@ impl ArrayFirst<T> of FirstTrait<Array<T>> {
 }
 
 //! > expected_diagnostics
+
+//! > ==========================================================================
+
+//! > Use of missing Impl type.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: true)
+
+//! > function
+fn foo() {
+    let _: MyImpl::ty = 5;
+}
+
+//! > function_name
+foo
+
+//! > module_code
+trait MyTrait {
+    type ty;
+}
+impl MyImpl of MyTrait {}
+
+//! > expected_diagnostics
+error: Not all trait items are implemented. Missing: 'ty'.
+ --> lib.cairo:4:6
+impl MyImpl of MyTrait {}
+     ^****^
+
+error: Trait has no implementation in context: test::MyTrait.
+ --> lib.cairo:5:10
+fn foo() {
+         ^

crates/cairo-lang-semantic/src/substitution.rs

@@ -434,12 +434,14 @@ impl<'a> SemanticRewriter<TypeLongId, DiagnosticAdded> for SubstitutionRewriter<
             }
             TypeLongId::ImplType(impl_type_id) => {
                 let impl_type_id_rewrite_result = self.internal_rewrite(impl_type_id)?;
-                return Ok(if let Some(ty) = self.db.impl_type_concrete_implized(*impl_type_id)? {
-                    *value = ty.lookup_intern(self.db);
-                    RewriteResult::Modified
+                let new_value =
+                    self.db.impl_type_concrete_implized(*impl_type_id)?.lookup_intern(self.db);
+                if new_value != *value {
+                    *value = new_value;
+                    return Ok(RewriteResult::Modified);
                 } else {
-                    impl_type_id_rewrite_result
-                });
+                    return Ok(impl_type_id_rewrite_result);
+                }
             }
             TypeLongId::TraitType(trait_type_id) => {
                 if let Some(self_impl) = &self.substitution.self_impl {
@@ -448,11 +450,8 @@ impl<'a> SemanticRewriter<TypeLongId, DiagnosticAdded> for SubstitutionRewriter<
                         self_impl.concrete_trait(self.db)?.trait_id(self.db)
                     );
                     let impl_type_id = ImplTypeId::new(*self_impl, *trait_type_id, self.db);
-                    *value = if let Some(ty) = self.db.impl_type_concrete_implized(impl_type_id)? {
-                        ty.lookup_intern(self.db)
-                    } else {
-                        TypeLongId::ImplType(impl_type_id)
-                    };
+                    *value =
+                        self.db.impl_type_concrete_implized(impl_type_id)?.lookup_intern(self.db);
                     return Ok(RewriteResult::Modified);
                 }
             }

crates/cairo-lang-semantic/src/types.rs

@@ -1,7 +1,7 @@
 use cairo_lang_debug::DebugWithDb;
 use cairo_lang_defs::ids::{
-    EnumId, ExternTypeId, GenericParamId, GenericTypeId, ImplTypeDefId, ModuleFileId,
-    NamedLanguageElementId, StructId, TraitTypeId,
+    EnumId, ExternTypeId, GenericParamId, GenericTypeId, ModuleFileId, NamedLanguageElementId,
+    StructId, TraitTypeId,
 };
 use cairo_lang_diagnostics::{DiagnosticAdded, Maybe};
 use cairo_lang_proc_macros::SemanticObject;
@@ -26,7 +26,7 @@ use crate::expr::inference::canonic::ResultNoErrEx;
 use crate::expr::inference::{InferenceData, InferenceError, InferenceId, TypeVar};
 use crate::items::attribute::SemanticQueryAttrs;
 use crate::items::constant::{resolve_const_expr_and_evaluate, ConstValue, ConstValueId};
-use crate::items::imp::{ImplId, ImplLongId, ImplLookupContext};
+use crate::items::imp::{ImplId, ImplLookupContext};
 use crate::resolve::{ResolvedConcreteItem, Resolver};
 use crate::substitution::SemanticRewriter;
 use crate::{semantic, semantic_object_for_id, ConcreteTraitId, FunctionId, GenericArgumentId};
@@ -405,18 +405,6 @@ impl ImplTypeId {
     pub fn ty(&self) -> TraitTypeId {
         self.ty
     }
-    /// Gets the impl type def (language element), if `self.impl_id` is of a concrete impl.
-    pub fn impl_type_def(&self, db: &dyn SemanticGroup) -> Maybe<Option<ImplTypeDefId>> {
-        match self.impl_id.lookup_intern(db) {
-            ImplLongId::Concrete(concrete_impl_id) => {
-                concrete_impl_id.get_impl_type_def(db, self.ty)
-            }
-            ImplLongId::GenericParameter(_)
-            | ImplLongId::ImplVar(_)
-            | ImplLongId::ImplImpl(_)
-            | ImplLongId::TraitImpl(_) => Ok(None),
-        }
-    }
     pub fn format(&self, db: &dyn SemanticGroup) -> SmolStr {
         format!("{}::{}", self.impl_id.name(db.upcast()), self.ty.name(db.upcast())).into()
     }