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 and raises a warning when needed.

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,
     }
@@ -1715,4 +1715,72 @@ error: Unreachable pattern arm.
         ^^^^^^^^^
 
 //! > lowering_flat
-<Failed lowering function - run with RUST_LOG=warn (or less) to see diagnostics>
+<Failed lowering function - run with RUST_LOG=warn (or less) to see diagnostics>
+
+//! > ==========================================================================
+
+//! > Test lower match missing args.
+
+//! > test_runner_name
+test_function_lowering(expect_diagnostics: true)
+
+//! > function
+fn foo(a: felt252) -> felt252 {
+    match Some(a) {
+        Some => 2,
+        _ => 0,
+    }
+}
+
+//! > function_name
+foo
+
+//! > module_code
+
+//! > semantic_diagnostics
+warning: Pattern missing subpattern for the payload of variant. Consider using `Some(_)`
+ --> lib.cairo:3:9
+        Some => 2,
+        ^^^^
+
+//! > lowering_diagnostics
+
+//! > lowering_flat
+Parameters: v0: core::felt252
+blk0 (root):
+Statements:
+  (v1: core::felt252) <- 2
+End:
+  Return(v1)
+
+//! > ==========================================================================
+
+//! > Test lower match with _ args.
+
+//! > test_runner_name
+test_function_lowering(expect_diagnostics: false)
+
+//! > function
+fn foo(a: felt252) -> felt252 {
+    match Some(a) {
+        Some(_) => 2,
+        _ => 0,
+    }
+}
+
+//! > function_name
+foo
+
+//! > module_code
+
+//! > semantic_diagnostics
+
+//! > lowering_diagnostics
+
+//! > lowering_flat
+Parameters: v0: core::felt252
+blk0 (root):
+Statements:
+  (v1: core::felt252) <- 2
+End:
+  Return(v1)

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

@@ -13,6 +13,8 @@ use cairo_lang_diagnostics::{
     ErrorCode, Severity, error_code,
 };
 use cairo_lang_filesystem::db::Edition;
+use cairo_lang_syntax::node::ast;
+use cairo_lang_syntax::node::helpers::GetIdentifier;
 use cairo_lang_syntax::{self as syntax};
 use itertools::Itertools;
 use smol_str::SmolStr;
@@ -1011,6 +1013,12 @@ impl DiagnosticEntry for SemanticDiagnostic {
             SemanticDiagnosticKind::TypeConstraintsSyntaxNotEnabled => {
                 "Type constraints syntax is not enabled in the current crate.".into()
             }
+            SemanticDiagnosticKind::PatternMissingArgs(path) => {
+                format!(
+                    "Pattern missing subpattern for the payload of variant. Consider using `{}(_)`",
+                    path.elements(db).into_iter().map(|seg| seg.identifier(db)).join("::")
+                )
+            }
         }
     }
 
@@ -1048,6 +1056,7 @@ impl DiagnosticEntry for SemanticDiagnostic {
             | SemanticDiagnosticKind::CallingShadowedFunction { .. }
             | SemanticDiagnosticKind::UnusedConstant
             | SemanticDiagnosticKind::UnusedUse
+            | SemanticDiagnosticKind::PatternMissingArgs(_)
             | SemanticDiagnosticKind::UnsupportedAllowAttrArguments => Severity::Warning,
             SemanticDiagnosticKind::PluginDiagnostic(diag) => diag.severity,
             _ => Severity::Error,
@@ -1439,6 +1448,7 @@ pub enum SemanticDiagnosticKind {
         concrete_trait_type_id: ConcreteTraitTypeId,
     },
     TypeConstraintsSyntaxNotEnabled,
+    PatternMissingArgs(ast::ExprPath),
 }
 
 /// 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,46 @@ 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 {
+        let path = match pattern {
+            ast::Pattern::Enum(pattern) => pattern.path(db),
+            ast::Pattern::Path(p) => p.clone(),
+            _ => unreachable!(),
+        };
+        ctx.diagnostics.report(pattern.stable_ptr(db), PatternMissingArgs(path));
+    }
+
+    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/if

@@ -137,3 +137,58 @@ error[E0006]: Identifier not found.
  --> lib.cairo:4:16
         return y == 9;
                ^
+
+//! > ==========================================================================
+
+//! > if_let some no args.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: true)
+
+//! > function
+fn foo() -> bool {
+    let x = Some(7_u64);
+    if let Some = x {
+        return true;
+    } else {
+        return false;
+    }
+}
+
+//! > function_name
+foo
+
+//! > module_code
+
+//! > expected_diagnostics
+warning: Pattern missing subpattern for the payload of variant. Consider using `Some(_)`
+ --> lib.cairo:3:12
+    if let Some = x {
+           ^^^^
+
+//! > ==========================================================================
+
+//! > if_let enum unit no args.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: false)
+
+//! > function
+fn foo() -> bool {
+    let a = A::B(());
+    if let A::B = a {
+        return true;
+    } else {
+        return false;
+    }
+}
+
+//! > function_name
+foo
+
+//! > module_code
+enum A {
+    B: (),
+}
+
+//! > expected_diagnostics