Check that override makes sense

Signed-off-by: Sean Young <sean@mess.org>
KiChjang · Sep 17, 2020 · 987126d · 987126d
1 parent 737403c
commit 987126d
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 0 deletions.
diff --git a/src/sema/contracts.rs b/src/sema/contracts.rs
@@ -467,6 +467,55 @@ fn layout_contract(contract_no: usize, ns: &mut ast::Namespace) {
                         continue;
                     }
 
+                    if func_prev.ty != cur.ty {
+                        ns.diagnostics.push(ast::Diagnostic::error_with_note(
+                            cur.loc,
+                            format!("{} ‘{}’ overrides {}", cur.ty, cur.name, func_prev.ty,),
+                            func_prev.loc,
+                            format!("previous definition of ‘{}’", func_prev.name),
+                        ));
+
+                        continue;
+                    }
+
+                    if func_prev
+                        .params
+                        .iter()
+                        .zip(cur.params.iter())
+                        .any(|(a, b)| a.ty != b.ty)
+                    {
+                        ns.diagnostics.push(ast::Diagnostic::error_with_note(
+                            cur.loc,
+                            format!(
+                                "{} ‘{}’ overrides {} with different argument types",
+                                cur.ty, cur.name, func_prev.ty,
+                            ),
+                            func_prev.loc,
+                            format!("previous definition of ‘{}’", func_prev.name),
+                        ));
+
+                        continue;
+                    }
+
+                    if func_prev
+                        .returns
+                        .iter()
+                        .zip(cur.returns.iter())
+                        .any(|(a, b)| a.ty != b.ty)
+                    {
+                        ns.diagnostics.push(ast::Diagnostic::error_with_note(
+                            cur.loc,
+                            format!(
+                                "{} ‘{}’ overrides {} with different return types",
+                                cur.ty, cur.name, func_prev.ty,
+                            ),
+                            func_prev.loc,
+                            format!("previous definition of ‘{}’", func_prev.name),
+                        ));
+
+                        continue;
+                    }
+
                     if let Some((loc, override_list)) = &cur.is_override {
                         if !func_prev.is_virtual {
                             ns.diagnostics.push(ast::Diagnostic::error_with_note(

diff --git a/tests/substrate_modifier/mod.rs b/tests/substrate_modifier/mod.rs
@@ -501,4 +501,23 @@ fn mutability() {
         first_error(ns.diagnostics),
         "function declared ‘pure’ but this expression reads from state"
     );
+
+    let ns = parse_and_resolve(
+        r#"
+        contract base {
+            modifier foo() virtual {
+                _;
+            }
+        }
+
+        contract apex is base {
+            function foo() public override {}
+        }"#,
+        Target::Substrate,
+    );
+
+    assert_eq!(
+        first_error(ns.diagnostics),
+        "function ‘foo’ overrides modifier"
+    );
 }