Check on enum variant type matching in patterns

When building a semantic model a check is expected that pattern is legal.
When we match an option, we expect a case `Some` to be illegal, as it ignores the parameter. The expected pattern would be `Some(_)`.
This PR adds that check.

commit-id:8eb351cd

Author: eytan-starkware

crates/cairo-lang-lowering/src/test_data/match

@@ -1126,10 +1126,10 @@ fn foo(a: A, b: A) -> felt252 {
     let x = (@a, b);
     let y = @x;
     match y {
-        (A::Two((t, _)), A::One) => **t + 3,
-        (A::Two, _) => 2,
+        (A::Two((t, _)), A::One(_)) => **t + 3,
+        (A::Two(_), _) => 2,
         (A::One(_), A::One(_)) => 1,
-        (_, A::Three(_)) => 3,
+        (_, A::Three) => 3,
         (_, _) => 6,
     }
 }
@@ -1270,10 +1270,10 @@ test_function_lowering(expect_diagnostics: true)
 //! > function
 fn foo(a: A, b: A) -> felt252 {
     match (a, b) {
-        (A::Two, A::One) => 5,
-        (A::Two, _) => 2,
+        (A::Two(_), A::One(_)) => 5,
+        (A::Two(_), _) => 2,
         (A::One(_), A::One(_)) => 1,
-        (_, A::Three(_)) => 3,
+        (_, A::Three) => 3,
         (_, A::Four) => 4,
     }
 }
@@ -1331,7 +1331,7 @@ test_function_lowering(expect_diagnostics: true)
 //! > function
 fn foo(a: A, b: A) -> felt252 {
     match (a, (a, b)) {
-        (A::Two, (A::One, A::One)) => 8,
+        (A::Two(_), (A::One(_), A::One(_))) => 8,
         _ => 4,
     }
 }
@@ -1369,8 +1369,8 @@ test_function_lowering(expect_diagnostics: false)
 //! > function
 fn foo(a: A) -> felt252 {
     match (a, get_bool()) {
-        (A::Two, true) => 5,
-        (A::Two, _) => 2,
+        (A::Two(_), true) => 5,
+        (A::Two(_), _) => 2,
         (A::One(_), false) => 1,
         (_, _) => 6,
     }

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

@@ -5,7 +5,7 @@ use cairo_lang_defs::diagnostic_utils::StableLocation;
 use cairo_lang_defs::ids::{
     EnumId, FunctionTitleId, GenericKind, ImplDefId, ImplFunctionId, ModuleId, ModuleItemId,
     NamedLanguageElementId, StructId, TopLevelLanguageElementId, TraitFunctionId, TraitId,
-    TraitImplId, TraitItemId, UseId,
+    TraitImplId, TraitItemId, UseId, VariantId,
 };
 use cairo_lang_defs::plugin::PluginDiagnostic;
 use cairo_lang_diagnostics::{
@@ -1011,6 +1011,14 @@ impl DiagnosticEntry for SemanticDiagnostic {
             SemanticDiagnosticKind::TypeConstraintsSyntaxNotEnabled => {
                 "Type constraints syntax is not enabled in the current crate.".into()
             }
+            SemanticDiagnosticKind::PatternMissingArgs(variant_id) => {
+                format!(
+                    "Pattern missing subpattern for the parameter of variant `{}`. Consider using \
+                     `{}(_)`",
+                    variant_id.full_path(db),
+                    variant_id.full_path(db)
+                )
+            }
         }
     }
 
@@ -1439,6 +1447,7 @@ pub enum SemanticDiagnosticKind {
         concrete_trait_type_id: ConcreteTraitTypeId,
     },
     TypeConstraintsSyntaxNotEnabled,
+    PatternMissingArgs(VariantId),
 }
 
 /// The kind of an expression with multiple possible return types.

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

@@ -83,8 +83,8 @@ use crate::types::{
 };
 use crate::usage::Usages;
 use crate::{
-    ConcreteEnumId, GenericArgumentId, GenericParam, LocalItem, Member, Mutability, Parameter,
-    PatternStringLiteral, PatternStruct, Signature, StatementItemKind,
+    ConcreteEnumId, ConcreteVariant, GenericArgumentId, GenericParam, LocalItem, Member,
+    Mutability, Parameter, PatternStringLiteral, PatternStruct, Signature, StatementItemKind,
 };
 
 /// Expression with its id.
@@ -2219,18 +2219,13 @@ fn maybe_compute_pattern_semantic(
             let generic_variant = try_extract_matches!(item, ResolvedGenericItem::Variant)
                 .ok_or_else(|| ctx.diagnostics.report(path.stable_ptr(db), NotAVariant))?;
 
-            let (concrete_enum, n_snapshots) = extract_concrete_enum_from_pattern_and_validate(
-                ctx,
-                pattern_syntax,
-                ty,
-                generic_variant.enum_id,
-            )?;
-
-            // TODO(lior): Should we report a diagnostic here?
-            let concrete_variant = ctx
-                .db
-                .concrete_enum_variant(concrete_enum, &generic_variant)
-                .map_err(|_| ctx.diagnostics.report(path.stable_ptr(db), UnknownEnum))?;
+            let (concrete_variant, n_snapshots) =
+                extract_concrete_variant_from_pattern_and_validate(
+                    ctx,
+                    pattern_syntax,
+                    ty,
+                    generic_variant,
+                )?;
 
             // Compute inner pattern.
             let inner_ty = wrap_in_snapshots(ctx.db, concrete_variant.ty, n_snapshots);
@@ -2266,17 +2261,13 @@ fn maybe_compute_pattern_semantic(
                 if let Some(generic_variant) =
                     try_extract_matches!(item, ResolvedGenericItem::Variant)
                 {
-                    let (concrete_enum, _n_snapshots) =
-                        extract_concrete_enum_from_pattern_and_validate(
+                    let (concrete_variant, _n_snapshots) =
+                        extract_concrete_variant_from_pattern_and_validate(
                             ctx,
                             pattern_syntax,
                             ty,
-                            generic_variant.enum_id,
+                            generic_variant,
                         )?;
-                    let concrete_variant = ctx
-                        .db
-                        .concrete_enum_variant(concrete_enum, &generic_variant)
-                        .map_err(|_| ctx.diagnostics.report(path.stable_ptr(db), UnknownEnum))?;
                     return Ok(Pattern::EnumVariant(PatternEnumVariant {
                         variant: concrete_variant,
                         inner_pattern: None,
@@ -2625,6 +2616,43 @@ fn extract_concrete_enum_from_pattern_and_validate(
     Ok((concrete_enum, n_snapshots))
 }
 
+/// Validates that the semantic type of an enum pattern is an enum.
+/// After that finds the concrete variant in the patter, and verifies it has args if needed.
+/// Returns the concrete variant and the number of snapshots.
+fn extract_concrete_variant_from_pattern_and_validate(
+    ctx: &mut ComputationContext<'_>,
+    pattern: &ast::Pattern,
+    ty: TypeId,
+    generic_variant: semantic::Variant,
+) -> Maybe<(ConcreteVariant, usize)> {
+    let (concrete_enum, n_snapshots) =
+        extract_concrete_enum_from_pattern_and_validate(ctx, pattern, ty, generic_variant.enum_id)?;
+    let db = ctx.db;
+
+    // TODO(lior): Should we report a diagnostic here?
+    let needs_args = generic_variant.ty != unit_ty(db);
+    let has_args = matches!(
+        pattern,
+        ast::Pattern::Enum(inner)
+            if matches!(
+                inner.pattern(db),
+                ast::OptionPatternEnumInnerPattern::PatternEnumInnerPattern(_)
+            )
+    );
+
+    if needs_args && !has_args {
+        return Err(ctx
+            .diagnostics
+            .report(pattern.stable_ptr(db), PatternMissingArgs(generic_variant.id)));
+    }
+
+    let concrete_variant = db
+        .concrete_enum_variant(concrete_enum, &generic_variant)
+        .map_err(|_| ctx.diagnostics.report(pattern.stable_ptr(db), UnknownEnum))?;
+
+    Ok((concrete_variant, n_snapshots))
+}
+
 /// Creates a local variable pattern.
 fn create_variable_pattern(
     ctx: &mut ComputationContext<'_>,

crates/cairo-lang-semantic/src/expr/test_data/match

@@ -254,3 +254,173 @@ error: Unexpected argument type. Expected: "test::MyEnum::<core::integer::u32>",
  --> lib.cairo:11:9
     bar(a);
         ^
+
+//! > ==========================================================================
+
+//! > Test match on variant without args.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: true)
+
+//! > function
+fn foo(a: A, b: A) -> felt252 {
+    match (a, b) {
+        (A::One, _) => 2,
+        (_, _) => 6,
+    }
+}
+
+//! > function_name
+foo
+
+//! > module_code
+#[derive(Copy, Drop)]
+enum A {
+    One: felt252,
+}
+
+//! > expected_diagnostics
+error: Pattern missing subpattern for the parameter of variant `test::A::One`. Consider using `test::A::One(_)`
+ --> lib.cairo:7:10
+        (A::One, _) => 2,
+         ^^^^^^
+
+//! > ==========================================================================
+
+//! > Test match on unit variant without args.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: false)
+
+//! > function
+fn foo(a: A, b: A) -> felt252 {
+    match (a, b) {
+        (A::One(_), _) => 2,
+        (A::Two, _) => 6,
+    }
+}
+
+//! > function_name
+foo
+
+//! > module_code
+#[derive(Copy, Drop)]
+enum A {
+    One: felt252,
+    Two: (),
+}
+
+//! > expected_diagnostics
+
+//! > ==========================================================================
+
+//! > Test match on unit variant with args.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: false)
+
+//! > function
+fn foo(a: A, b: A) -> felt252 {
+    match (a, b) {
+        (A::One(_), _) => 2,
+        (A::Two(_), _) => 6,
+    }
+}
+
+//! > function_name
+foo
+
+//! > module_code
+#[derive(Copy, Drop)]
+enum A {
+    One: felt252,
+    Two: (),
+}
+
+//! > expected_diagnostics
+
+//! > ==========================================================================
+
+//! > Test match on Some without args.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: true)
+
+//! > function
+fn foo<T>(a: Option<T>) -> felt252 {
+    match a {
+        Some => 2,
+        None => 6,
+    }
+}
+fn bar() -> felt252 {
+    let a = Some(());
+    foo(a)
+}
+
+//! > function_name
+bar
+
+//! > module_code
+
+//! > expected_diagnostics
+error: Pattern missing subpattern for the parameter of variant `core::option::Option::Some`. Consider using `core::option::Option::Some(_)`
+ --> lib.cairo:3:9
+        Some => 2,
+        ^^^^
+
+//! > ==========================================================================
+
+//! > Test match on Path no args.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: false)
+
+//! > function
+fn foo(a: A, b: A) -> felt252 {
+    match (a, b) {
+        (A::One(_), _) => 2,
+        (Two, _) => 6,
+    }
+}
+
+//! > function_name
+foo
+
+//! > module_code
+#[derive(Copy, Drop)]
+enum A {
+    One: felt252,
+    Two: (),
+}
+use A::Two;
+
+//! > expected_diagnostics
+
+//! > ==========================================================================
+
+//! > Test match on infered unit Some without args.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: true)
+
+//! > function
+fn fb() -> Option<()> {
+    let x = None;
+    match x {
+        Some => 2,
+        None => 6,
+    }
+    return x;
+}
+
+//! > function_name
+fb
+
+//! > module_code
+
+//! > expected_diagnostics
+error: Pattern missing subpattern for the parameter of variant `core::option::Option::Some`. Consider using `core::option::Option::Some(_)`
+ --> lib.cairo:4:9
+        Some => 2,
+        ^^^^