From a43458eb74b24612f0357dccb2dc07d9a639bf81 Mon Sep 17 00:00:00 2001 From: Pranav Gaddamadugu <23022326+d0cd@users.noreply.github.com> Date: Sun, 5 Jan 2025 18:50:35 -0800 Subject: [PATCH] Improve validation for test annotations --- compiler/ast/src/functions/annotation.rs | 2 +- compiler/parser/src/parser/file.rs | 2 +- .../passes/src/type_checking/check_program.rs | 7 +- compiler/passes/src/type_checking/checker.rs | 75 ++++++++++++++++++- .../errors/type_checker/type_checker_error.rs | 3 +- .../type_checker/type_checker_warning.rs | 35 +++++++++ .../compiler/function/annotated_function.out | 13 ++++ .../function/annotated_function_fail.out | 49 +++++++----- .../compiler/function/annotated_function.leo | 20 +++++ .../function/annotated_function_fail.leo | 14 +++- 10 files changed, 188 insertions(+), 32 deletions(-) create mode 100644 tests/expectations/compiler/function/annotated_function.out create mode 100644 tests/tests/compiler/function/annotated_function.leo diff --git a/compiler/ast/src/functions/annotation.rs b/compiler/ast/src/functions/annotation.rs index 722f7f6ac4..13819aac35 100644 --- a/compiler/ast/src/functions/annotation.rs +++ b/compiler/ast/src/functions/annotation.rs @@ -50,7 +50,7 @@ impl fmt::Display for Annotation { Some(value) => string.push_str(&format!("{key} = \"{value}\",")), } } - string + format!("({string})") } }; write!(f, "@{}{}", self.identifier, data) diff --git a/compiler/parser/src/parser/file.rs b/compiler/parser/src/parser/file.rs index 75914c5acc..7f49a565f9 100644 --- a/compiler/parser/src/parser/file.rs +++ b/compiler/parser/src/parser/file.rs @@ -328,7 +328,7 @@ impl ParserContext<'_, N> { Token::LeftParen => { let (data, _, span) = self.parse_paren_comma_list(|p| { let key = p.expect_identifier()?; - let value = if p.eat(&Token::Eq) { + let value = if p.eat(&Token::Assign) { match &p.token.token { Token::StaticString(s) => { let value = s.clone(); diff --git a/compiler/passes/src/type_checking/check_program.rs b/compiler/passes/src/type_checking/check_program.rs index 141a3ea85b..f9d2283b65 100644 --- a/compiler/passes/src/type_checking/check_program.rs +++ b/compiler/passes/src/type_checking/check_program.rs @@ -237,13 +237,8 @@ impl<'a, N: Network> ProgramVisitor<'a> for TypeChecker<'a, N> { fn visit_function(&mut self, function: &'a Function) { // Check that the function's annotations are valid. - let valid_annotations = [sym::should_fail, sym::test]; for annotation in function.annotations.iter() { - // All Leo annotations currently apply only to test code. - if !self.build_tests || !valid_annotations.contains(&annotation.identifier.name) { - // TODO: Change to compiler warning. - self.emit_err(TypeCheckerError::unknown_annotation(annotation, annotation.span)); - } + self.check_annotation(annotation); } // `interpret` can only be used for tests. diff --git a/compiler/passes/src/type_checking/checker.rs b/compiler/passes/src/type_checking/checker.rs index ebf51a4de7..fdf05ba30f 100644 --- a/compiler/passes/src/type_checking/checker.rs +++ b/compiler/passes/src/type_checking/checker.rs @@ -26,13 +26,14 @@ use crate::{ use leo_ast::*; use leo_errors::{TypeCheckerError, TypeCheckerWarning, emitter::Handler}; -use leo_span::{Span, Symbol}; +use leo_span::{Span, Symbol, sym}; use snarkvm::console::network::Network; use indexmap::{IndexMap, IndexSet}; use itertools::Itertools; -use std::{cell::RefCell, marker::PhantomData}; +use snarkvm::prelude::PrivateKey; +use std::{cell::RefCell, marker::PhantomData, str::FromStr}; pub struct TypeChecker<'a, N: Network> { /// The symbol table for the program. @@ -1364,6 +1365,76 @@ impl<'a, N: Network> TypeChecker<'a, N> { self.handler.emit_err(TypeCheckerError::invalid_operation_outside_finalize(name, span)) } } + + // Check if the annotation is valid. + pub(crate) fn check_annotation(&mut self, annotation: &Annotation) { + match annotation.identifier.name { + sym::test => { + // Check the annotation body. + for (key, value) in annotation.data.iter() { + // Check that the key and associated value is valid. + if key.name == Symbol::intern("private_key") { + // Attempt to parse the value as a private key. + match value { + None => self.emit_warning(TypeCheckerWarning::missing_annotation_value( + annotation.identifier, + key, + key.span, + )), + Some(string) => { + if let Err(err) = PrivateKey::::from_str(&string) { + self.emit_warning(TypeCheckerWarning::invalid_annotation_value( + annotation.identifier, + key, + string, + err, + key.span, + )); + } + } + } + } else if key.name == Symbol::intern("seed") || key.name == Symbol::intern("batch") { + // Attempt to parse the value as a u64. + match value { + None => self.emit_warning(TypeCheckerWarning::missing_annotation_value( + annotation.identifier, + key, + key.span, + )), + Some(string) => { + if let Err(err) = string.parse::() { + self.emit_warning(TypeCheckerWarning::invalid_annotation_value( + annotation.identifier, + key, + string, + err, + key.span, + )); + } + } + } + } else if key.name == Symbol::intern("should_fail") { + // Check that there is no value associated with the key. + if let Some(string) = value { + self.emit_warning(TypeCheckerWarning::unexpected_annotation_value( + annotation.identifier, + key, + string, + key.span, + )); + } + } else { + self.emit_warning(TypeCheckerWarning::unknown_annotation_key( + annotation.identifier, + key, + key.span, + )) + } + } + } + _ => self.emit_warning(TypeCheckerWarning::unknown_annotation(annotation.identifier, annotation.span)), + } + } } fn types_to_string(types: &[Type]) -> String { diff --git a/errors/src/errors/type_checker/type_checker_error.rs b/errors/src/errors/type_checker/type_checker_error.rs index 17dd923cd8..8093672711 100644 --- a/errors/src/errors/type_checker/type_checker_error.rs +++ b/errors/src/errors/type_checker/type_checker_error.rs @@ -269,7 +269,7 @@ create_messages!( help: Some("Remove the code in the loop body that always returns.".to_string()), } - // TODO: Consider emitting a warning instead of an error. + // TODO This error is unused. Remove it in a future version. @formatted unknown_annotation { args: (annotation: impl Display), @@ -419,7 +419,6 @@ create_messages!( } // TODO: Consider changing this to a warning. - @formatted assign_unit_expression_to_variable { args: (), diff --git a/errors/src/errors/type_checker/type_checker_warning.rs b/errors/src/errors/type_checker/type_checker_warning.rs index 1ac294cba3..e33587cd74 100644 --- a/errors/src/errors/type_checker/type_checker_warning.rs +++ b/errors/src/errors/type_checker/type_checker_warning.rs @@ -52,4 +52,39 @@ create_messages!( msg: format!("The type checker has exceeded the max depth of nested conditional blocks: {max}."), help: Some("Re-run with a larger maximum depth using the `--conditional_block_max_depth` build option. Ex: `leo run main --conditional_block_max_depth 25`.".to_string()), } + + @formatted + unknown_annotation { + args: (annotation: impl Display), + msg: format!("Unknown annotation: `{annotation}`."), + help: None, + } + + @formatted + unknown_annotation_key { + args: (annotation: impl Display, key: impl Display), + msg: format!("Unknown key `{key}` in annotation `{annotation}`."), + help: None, + } + + @formatted + missing_annotation_value { + args: (annotation: impl Display, key: impl Display), + msg: format!("Missing value for key `{key}` in annotation `{annotation}`."), + help: None, + } + + @formatted + invalid_annotation_value { + args: (annotation: impl Display, key: impl Display, value: impl Display, error: impl Display), + msg: format!("Invalid value `{value}` for key `{key}` in annotation `{annotation}`. Error: {error}"), + help: None, + } + + @formatted + unexpected_annotation_value { + args: (annotation: impl Display, key: impl Display, value: impl Display), + msg: format!("Unexpected value `{value}` for key `{key}` in annotation `{annotation}`."), + help: None, + } ); diff --git a/tests/expectations/compiler/function/annotated_function.out b/tests/expectations/compiler/function/annotated_function.out new file mode 100644 index 0000000000..9a2505435f --- /dev/null +++ b/tests/expectations/compiler/function/annotated_function.out @@ -0,0 +1,13 @@ +namespace = "Compile" +expectation = "Pass" +outputs = [[{ compile = [{ initial_symbol_table = "690edb74eb02c5ac9e717bfb51933668a2b530f7e803ba5666880d23f28d5bec", type_checked_symbol_table = "e7e865d4fe1f786bea9fa28c5cff41a8c29ce00da69306b522dd79eac50d4c78", unrolled_symbol_table = "e7e865d4fe1f786bea9fa28c5cff41a8c29ce00da69306b522dd79eac50d4c78", initial_ast = "97b8563a49a209737aa82195d8fe84257cdd16ac2b32133de91b43de7bb557b1", unrolled_ast = "97b8563a49a209737aa82195d8fe84257cdd16ac2b32133de91b43de7bb557b1", ssa_ast = "4e6c70e32a0fd6f3fc362568ba2f365b198be164922c9276b48a54513a9a0501", flattened_ast = "5ecbeaa566e8bd200515944b8f73945bff67eace488df97e8d0f477f2255bcd7", destructured_ast = "f0ae5e19798aaa5777a17d477f6eb0b2d9ccd1067c807c4348e1ba0a0f147dd5", inlined_ast = "f0ae5e19798aaa5777a17d477f6eb0b2d9ccd1067c807c4348e1ba0a0f147dd5", dce_ast = "f0ae5e19798aaa5777a17d477f6eb0b2d9ccd1067c807c4348e1ba0a0f147dd5", bytecode = """ +program test.aleo; + +function test: + is.eq 1u32 1u32 into r0; + assert.eq r0 true; + +function test_other: + is.eq 1u32 1u32 into r0; + assert.eq r0 true; +""", errors = "", warnings = "" }] }]] diff --git a/tests/expectations/compiler/function/annotated_function_fail.out b/tests/expectations/compiler/function/annotated_function_fail.out index 7c9b020e13..967040f3ea 100644 --- a/tests/expectations/compiler/function/annotated_function_fail.out +++ b/tests/expectations/compiler/function/annotated_function_fail.out @@ -1,21 +1,36 @@ namespace = "Compile" -expectation = "Fail" -outputs = [""" -Error [ETYC0372027]: Unknown annotation: `@test`. - --> compiler-test:4:5 +expectation = "Pass" +outputs = [[{ compile = [{ initial_symbol_table = "4886e187c6b46dbeeb507a7f5188c89f415516c2ff7781ad5af48ea13fca665b", type_checked_symbol_table = "214bdcdf17793e3098343e1aa5993244ad520847278f8864ce7ce0bf4e3bf8ad", unrolled_symbol_table = "214bdcdf17793e3098343e1aa5993244ad520847278f8864ce7ce0bf4e3bf8ad", initial_ast = "05ac2357efbe8d0e00d30f8b7ee33cb625070471ab6f513d2613cb8baeaeff39", unrolled_ast = "05ac2357efbe8d0e00d30f8b7ee33cb625070471ab6f513d2613cb8baeaeff39", ssa_ast = "2f8548323d07c917964e711b10e5198796ac601eb3c75aae61009a600a437220", flattened_ast = "67707c544bfd8a551287d9bf6d4d869b4817d767003c44735ec27ff33d75cf8d", destructured_ast = "3c3c87cf1069c8691bfe57d696be55e2716021cb03e11f8345d3bafb5b23c99b", inlined_ast = "3c3c87cf1069c8691bfe57d696be55e2716021cb03e11f8345d3bafb5b23c99b", dce_ast = "3c3c87cf1069c8691bfe57d696be55e2716021cb03e11f8345d3bafb5b23c99b", bytecode = """ +program test.aleo; + +function test: + is.eq 1u32 1u32 into r0; + assert.eq r0 true; + +closure foo: + input r0 as u8; + input r1 as u8; + add r0 r1 into r2; + output r2 as u8; + +closure bar: + input r0 as u8; + input r1 as u8; + mul r0 r1 into r2; + output r2 as u8; +""", errors = "", warnings = """ +Warning [WTYC0372005]: Unknown key `foo` in annotation `test`. + --> compiler-test:16:11 | - 4 | @test - | ^^^^^ -Error [ETYC0372027]: Unknown annotation: `@program`. - --> compiler-test:9:5 + 16 | @test(foo) + | ^^^ +Warning [WTYC0372004]: Unknown annotation: `foo`. + --> compiler-test:6:5 | - 9 | @program - | ^^^^^^^^ -Error [ETYC0372083]: A program must have at least one transition function. - --> compiler-test:1:1 + 6 | @foo + | ^^^^ +Warning [WTYC0372004]: Unknown annotation: `program`. + --> compiler-test:11:5 | - 1 | - 2 | - 3 | program test.aleo { - | ^^^^^^^^^^^^ -"""] + 11 | @program + | ^^^^^^^^""" }] }]] diff --git a/tests/tests/compiler/function/annotated_function.leo b/tests/tests/compiler/function/annotated_function.leo new file mode 100644 index 0000000000..6bddeb9ec4 --- /dev/null +++ b/tests/tests/compiler/function/annotated_function.leo @@ -0,0 +1,20 @@ +/* +namespace = "Compile" +expectation = "Pass" +*/ + +program test.aleo { + @test + transition test() { + assert(1u32 == 1u32); + } + + @test( + private_key = "APrivateKey1zkp9wLdmfcM57QFL3ZEzgfZwCWV52nM24ckmLSmTQcp64FL", + batch = "0", + seed = "1234" + ) + transition test_other() { + assert(1u32 == 1u32); + } +} diff --git a/tests/tests/compiler/function/annotated_function_fail.leo b/tests/tests/compiler/function/annotated_function_fail.leo index 2eb1425e09..5e09b4d8c5 100644 --- a/tests/tests/compiler/function/annotated_function_fail.leo +++ b/tests/tests/compiler/function/annotated_function_fail.leo @@ -1,10 +1,12 @@ /* namespace = "Compile" -expectation = "Fail" +expectation = "Pass" */ +// This test should pass, but produce warnings about unrecognized and malformed annotations. + program test.aleo { - @test + @foo function foo(a: u8, b: u8) -> u8 { return a + b; } @@ -12,4 +14,10 @@ program test.aleo { @program function bar(a: u8, b: u8) -> u8 { return a * b; - }} + } + + @test(foo) + transition test() { + assert(1u32 == 1u32); + } +}