validate num. variants in enum declaration (#120)

* validate num. variants in enum declaration * add ERROR annotations to test * ignore solc parser error that is caught by solar's AST validation
paradigmxyz · Nov 18, 2024 · cea63c1 · cea63c1
1 parent 18eba2a
commit cea63c1
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 1 deletion.
diff --git a/crates/parse/src/parser/item.rs b/crates/parse/src/parser/item.rs
@@ -330,7 +330,7 @@ impl<'sess, 'ast> Parser<'sess, 'ast> {
     /// Parses an enum definition.
     fn parse_enum(&mut self) -> PResult<'sess, ItemEnum<'ast>> {
         let name = self.parse_ident()?;
-        let variants = self.parse_delim_comma_seq(Delimiter::Brace, false, |this| {
+        let variants = self.parse_delim_comma_seq(Delimiter::Brace, true, |this| {
             this.ignore_doc_comments();
             this.parse_ident()
         })?;

diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs
@@ -64,6 +64,20 @@ impl<'ast> Visit<'ast> for AstValidator<'_> {
         ControlFlow::Continue(())
     }
 
+    fn visit_item_enum(
+        &mut self,
+        enum_: &'ast ast::ItemEnum<'ast>,
+    ) -> ControlFlow<Self::BreakValue> {
+        let ast::ItemEnum { name, variants } = enum_;
+        if variants.is_empty() {
+            self.dcx().err("enum must have at least one variant").span(name.span).emit();
+        }
+        if variants.len() > 256 {
+            self.dcx().err("enum cannot have more than 256 variants").span(name.span).emit();
+        }
+        ControlFlow::Continue(())
+    }
+
     fn visit_pragma_directive(
         &mut self,
         pragma: &'ast ast::PragmaDirective<'ast>,

diff --git a/tests/ui/typeck/enum_variants.sol b/tests/ui/typeck/enum_variants.sol
@@ -0,0 +1,71 @@
+contract EmptyEnum {
+    enum NeedsAtLeastOneVariant { } //~ ERROR: enum must have at least one variant
+}
+
+contract ValidEnum {
+    enum HasOneVariant {
+        One
+    }
+}
+
+contract MaxEnum {
+    enum Has256Variants {
+        V0,  V1,  V2,  V3,  V4,  V5,  V6,  V7,  V8,  V9,
+        V10, V11, V12, V13, V14, V15, V16, V17, V18, V19,
+        V20, V21, V22, V23, V24, V25, V26, V27, V28, V29,
+        V30, V31, V32, V33, V34, V35, V36, V37, V38, V39,
+        V40, V41, V42, V43, V44, V45, V46, V47, V48, V49,
+        V50, V51, V52, V53, V54, V55, V56, V57, V58, V59,
+        V60, V61, V62, V63, V64, V65, V66, V67, V68, V69,
+        V70, V71, V72, V73, V74, V75, V76, V77, V78, V79,
+        V80, V81, V82, V83, V84, V85, V86, V87, V88, V89,
+        V90, V91, V92, V93, V94, V95, V96, V97, V98, V99,
+        V100, V101, V102, V103, V104, V105, V106, V107, V108, V109,
+        V110, V111, V112, V113, V114, V115, V116, V117, V118, V119,
+        V120, V121, V122, V123, V124, V125, V126, V127, V128, V129,
+        V130, V131, V132, V133, V134, V135, V136, V137, V138, V139,
+        V140, V141, V142, V143, V144, V145, V146, V147, V148, V149,
+        V150, V151, V152, V153, V154, V155, V156, V157, V158, V159,
+        V160, V161, V162, V163, V164, V165, V166, V167, V168, V169,
+        V170, V171, V172, V173, V174, V175, V176, V177, V178, V179,
+        V180, V181, V182, V183, V184, V185, V186, V187, V188, V189,
+        V190, V191, V192, V193, V194, V195, V196, V197, V198, V199,
+        V200, V201, V202, V203, V204, V205, V206, V207, V208, V209,
+        V210, V211, V212, V213, V214, V215, V216, V217, V218, V219,
+        V220, V221, V222, V223, V224, V225, V226, V227, V228, V229,
+        V230, V231, V232, V233, V234, V235, V236, V237, V238, V239,
+        V240, V241, V242, V243, V244, V245, V246, V247, V248, V249,
+        V250, V251, V252, V253, V254, V255
+    }
+}
+
+contract InvalidEnum {
+    enum ExceedsMaxVariants { //~ ERROR: enum cannot have more than 256 variants
+        V0,  V1,  V2,  V3,  V4,  V5,  V6,  V7,  V8,  V9,
+        V10, V11, V12, V13, V14, V15, V16, V17, V18, V19,
+        V20, V21, V22, V23, V24, V25, V26, V27, V28, V29,
+        V30, V31, V32, V33, V34, V35, V36, V37, V38, V39,
+        V40, V41, V42, V43, V44, V45, V46, V47, V48, V49,
+        V50, V51, V52, V53, V54, V55, V56, V57, V58, V59,
+        V60, V61, V62, V63, V64, V65, V66, V67, V68, V69,
+        V70, V71, V72, V73, V74, V75, V76, V77, V78, V79,
+        V80, V81, V82, V83, V84, V85, V86, V87, V88, V89,
+        V90, V91, V92, V93, V94, V95, V96, V97, V98, V99,
+        V100, V101, V102, V103, V104, V105, V106, V107, V108, V109,
+        V110, V111, V112, V113, V114, V115, V116, V117, V118, V119,
+        V120, V121, V122, V123, V124, V125, V126, V127, V128, V129,
+        V130, V131, V132, V133, V134, V135, V136, V137, V138, V139,
+        V140, V141, V142, V143, V144, V145, V146, V147, V148, V149,
+        V150, V151, V152, V153, V154, V155, V156, V157, V158, V159,
+        V160, V161, V162, V163, V164, V165, V166, V167, V168, V169,
+        V170, V171, V172, V173, V174, V175, V176, V177, V178, V179,
+        V180, V181, V182, V183, V184, V185, V186, V187, V188, V189,
+        V190, V191, V192, V193, V194, V195, V196, V197, V198, V199,
+        V200, V201, V202, V203, V204, V205, V206, V207, V208, V209,
+        V210, V211, V212, V213, V214, V215, V216, V217, V218, V219,
+        V220, V221, V222, V223, V224, V225, V226, V227, V228, V229,
+        V230, V231, V232, V233, V234, V235, V236, V237, V238, V239,
+        V240, V241, V242, V243, V244, V245, V246, V247, V248, V249,
+        V250, V251, V252, V253, V254, V255, V256
+    }
+}
diff --git a/tests/ui/typeck/enum_variants.stderr b/tests/ui/typeck/enum_variants.stderr
@@ -0,0 +1,16 @@
+error: enum must have at least one variant
+  --> ROOT/tests/ui/typeck/enum_variants.sol:LL:CC
+   |
+LL |     enum NeedsAtLeastOneVariant { }
+   |          ^^^^^^^^^^^^^^^^^^^^^^
+   |
+
+error: enum cannot have more than 256 variants
+  --> ROOT/tests/ui/typeck/enum_variants.sol:LL:CC
+   |
+LL |     enum ExceedsMaxVariants {
+   |          ^^^^^^^^^^^^^^^^^^
+   |
+
+error: aborting due to 2 previous errors
+
diff --git a/tools/tester/src/solc/solidity.rs b/tools/tester/src/solc/solidity.rs
@@ -91,6 +91,8 @@ pub(crate) fn should_skip(path: &Path) -> Option<&'static str> {
         | "boost_filesystem_bug"
         // Invalid UTF-8 is not supported.
         | "invalid_utf8_sequence"
+        // Validation is in solar's AST stage (https://github.com/paradigmxyz/solar/pull/120).
+        | "empty_enum"
     ) {
         return Some("manually skipped");
     };