Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: format function signature #3495

Merged
merged 12 commits into from Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ pub struct FunctionDefinition {
pub visibility: FunctionVisibility,

pub generics: UnresolvedGenerics,
pub parameters: Vec<(Pattern, UnresolvedType, Visibility)>,
pub parameters: Vec<Param>,
pub body: BlockExpression,
pub span: Span,
pub where_clause: Vec<UnresolvedTraitConstraint>,
Expand All @@ -379,6 +379,14 @@ pub struct FunctionDefinition {
pub return_distinctness: Distinctness,
}

#[derive(Debug, PartialEq, Eq, Clone)]
pub struct Param {
pub visibility: Visibility,
pub pattern: Pattern,
pub typ: UnresolvedType,
pub span: Span,
}

#[derive(Debug, PartialEq, Eq, Clone)]
pub enum FunctionReturnType {
/// Returns type is not specified.
Expand Down Expand Up @@ -634,8 +642,11 @@ impl FunctionDefinition {
) -> FunctionDefinition {
let p = parameters
.iter()
.map(|(ident, unresolved_type)| {
(Pattern::Identifier(ident.clone()), unresolved_type.clone(), Visibility::Private)
.map(|(ident, unresolved_type)| Param {
visibility: Visibility::Private,
pattern: Pattern::Identifier(ident.clone()),
typ: unresolved_type.clone(),
span: ident.span().merge(unresolved_type.span.unwrap()),
})
.collect();
FunctionDefinition {
Expand All @@ -661,8 +672,8 @@ impl Display for FunctionDefinition {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
writeln!(f, "{:?}", self.attributes)?;

let parameters = vecmap(&self.parameters, |(name, r#type, visibility)| {
format!("{name}: {visibility} {type}")
let parameters = vecmap(&self.parameters, |Param { visibility, pattern, typ, span: _ }| {
format!("{pattern}: {visibility} {typ}")
});

let where_clause = vecmap(&self.where_clause, ToString::to_string);
Expand Down
8 changes: 6 additions & 2 deletions compiler/noirc_frontend/src/ast/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use noirc_errors::Span;

use crate::{
token::{Attributes, FunctionAttribute, SecondaryAttribute},
FunctionReturnType, Ident, Pattern, Visibility,
FunctionReturnType, Ident, Param, Visibility,
};

use super::{FunctionDefinition, UnresolvedType, UnresolvedTypeData};
Expand Down Expand Up @@ -45,6 +45,10 @@ impl NoirFunction {
NoirFunction { kind: FunctionKind::Oracle, def }
}

pub fn return_visibility(&self) -> Visibility {
self.def.return_visibility
}

pub fn return_type(&self) -> UnresolvedType {
match &self.def.return_type {
FunctionReturnType::Default(_) => {
Expand All @@ -59,7 +63,7 @@ impl NoirFunction {
pub fn name_ident(&self) -> &Ident {
&self.def.name
}
pub fn parameters(&self) -> &Vec<(Pattern, UnresolvedType, Visibility)> {
pub fn parameters(&self) -> &[Param] {
&self.def.parameters
}
pub fn attributes(&self) -> &Attributes {
Expand Down
8 changes: 8 additions & 0 deletions compiler/noirc_frontend/src/ast/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,14 @@ pub enum Pattern {
}

impl Pattern {
pub fn span(&self) -> Span {
match self {
Pattern::Identifier(ident) => ident.span(),
Pattern::Mutable(_, span) | Pattern::Tuple(_, span) | Pattern::Struct(_, _, span) => {
*span
}
}
}
pub fn name_ident(&self) -> &Ident {
match self {
Pattern::Identifier(name_ident) => name_ident,
Expand Down
19 changes: 10 additions & 9 deletions compiler/noirc_frontend/src/hir/aztec_library.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
};
use crate::{
ForLoopStatement, ForRange, FunctionDefinition, FunctionVisibility, ImportStatement,
NoirStruct, PrefixExpression, Signedness, StatementKind, StructType, Type, TypeImpl, UnaryOp,
NoirStruct, Param, PrefixExpression, Signedness, StatementKind, StructType, Type, TypeImpl,
UnaryOp,
};
use fm::FileId;

Expand Down Expand Up @@ -226,12 +227,12 @@
module.functions.iter().any(|func| {
func.def.name.0.contents == "compute_note_hash_and_nullifier"
&& func.def.parameters.len() == 4
&& func.def.parameters[0].1.typ == UnresolvedTypeData::FieldElement
&& func.def.parameters[1].1.typ == UnresolvedTypeData::FieldElement
&& func.def.parameters[2].1.typ == UnresolvedTypeData::FieldElement
&& func.def.parameters[0].typ.typ == UnresolvedTypeData::FieldElement
&& func.def.parameters[1].typ.typ == UnresolvedTypeData::FieldElement
&& func.def.parameters[2].typ.typ == UnresolvedTypeData::FieldElement
// checks if the 4th parameter is an array and the Box<UnresolvedType> in
// Array(Option<UnresolvedTypeExpression>, Box<UnresolvedType>) contains only fields
&& match &func.def.parameters[3].1.typ {
&& match &func.def.parameters[3].typ.typ {
UnresolvedTypeData::Array(_, inner_type) => {
match inner_type.typ {
UnresolvedTypeData::FieldElement => true,
Expand Down Expand Up @@ -513,14 +514,14 @@
/// fn foo() {
/// // ...
/// }
pub(crate) fn create_inputs(ty: &str) -> (Pattern, UnresolvedType, Visibility) {
pub(crate) fn create_inputs(ty: &str) -> Param {
let context_ident = ident("inputs");
let context_pattern = Pattern::Identifier(context_ident);
let type_path = chained_path!("aztec", "abi", ty);
let context_type = make_type(UnresolvedTypeData::Named(type_path, vec![]));
let visibility = Visibility::Private;

(context_pattern, context_type, visibility)
Param { pattern: context_pattern, typ: context_type, visibility, span: Span::default() }
}

/// Creates the private context object to be accessed within the function, the parameters need to be extracted to be
Expand Down Expand Up @@ -548,7 +549,7 @@
/// let mut context = PrivateContext::new(inputs, hasher.hash());
/// }
/// ```
fn create_context(ty: &str, params: &[(Pattern, UnresolvedType, Visibility)]) -> Vec<Statement> {
fn create_context(ty: &str, params: &[Param]) -> Vec<Statement> {
let mut injected_expressions: Vec<Statement> = vec![];

// `let mut hasher = Hasher::new();`
Expand All @@ -564,7 +565,7 @@
injected_expressions.push(let_hasher);

// Iterate over each of the function parameters, adding to them to the hasher
params.iter().for_each(|(pattern, typ, _vis)| {
params.iter().for_each(|Param { pattern, typ, span: _, visibility: _ }| {
match pattern {
Pattern::Identifier(identifier) => {
// Match the type to determine the padding to do
Expand Down Expand Up @@ -913,7 +914,7 @@
UnresolvedTypeData::Named(..) => {
let hasher_method_name = "add_multiple".to_owned();
let call = method_call(
// All serialise on each element

Check warning on line 917 in compiler/noirc_frontend/src/hir/aztec_library.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (serialise)
arr_index, // variable
"serialize", // method name
vec![], // args
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
};
use crate::{
ArrayLiteral, ContractFunctionType, Distinctness, ForRange, FunctionVisibility, Generics,
LValue, NoirStruct, NoirTypeAlias, Path, PathKind, Pattern, Shared, StructType, Type,
LValue, NoirStruct, NoirTypeAlias, Param, Path, PathKind, Pattern, Shared, StructType, Type,
TypeAliasType, TypeBinding, TypeVariable, UnaryOp, UnresolvedGenerics,
UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, UnresolvedTypeExpression,
Visibility, ERROR_IDENT,
Expand Down Expand Up @@ -95,7 +95,7 @@

/// True if the current module is a contract.
/// This is usually determined by self.path_resolver.module_id(), but it can
/// be overriden for impls. Impls are an odd case since the methods within resolve

Check warning on line 98 in compiler/noirc_frontend/src/hir/resolution/resolver.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (overriden)
/// as if they're in the parent module, but should be placed in a child module.
/// Since they should be within a child module, in_contract is manually set to false
/// for these so we can still resolve them in the parent module without them being in a contract.
Expand Down Expand Up @@ -760,7 +760,7 @@
let mut parameters = vec![];
let mut parameter_types = vec![];

for (pattern, typ, visibility) in func.parameters().iter().cloned() {
for Param { visibility, pattern, typ, span: _ } in func.parameters().iter().cloned() {
if visibility == Visibility::Public && !self.pub_allowed(func) {
self.push_err(ResolverError::UnnecessaryPub {
ident: func.name_ident().clone(),
Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ impl ParserError {
pub fn reason(&self) -> Option<&ParserErrorReason> {
self.reason.as_ref()
}

pub fn is_warning(&self) -> bool {
matches!(self.reason(), Some(ParserErrorReason::ExperimentalFeature(_)))
}
}

impl std::fmt::Display for ParserError {
Expand Down
25 changes: 14 additions & 11 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ use crate::{
BinaryOp, BinaryOpKind, BlockExpression, ConstrainKind, ConstrainStatement, Distinctness,
ForLoopStatement, ForRange, FunctionDefinition, FunctionReturnType, FunctionVisibility, Ident,
IfExpression, InfixExpression, LValue, Lambda, Literal, NoirFunction, NoirStruct, NoirTrait,
NoirTraitImpl, NoirTypeAlias, Path, PathKind, Pattern, Recoverable, Statement, TraitBound,
TraitImplItem, TraitItem, TypeImpl, UnaryOp, UnresolvedTraitConstraint,
NoirTraitImpl, NoirTypeAlias, Param, Path, PathKind, Pattern, Recoverable, Statement,
TraitBound, TraitImplItem, TraitItem, TypeImpl, UnaryOp, UnresolvedTraitConstraint,
UnresolvedTypeExpression, UseTree, UseTreeKind, Visibility,
};

Expand Down Expand Up @@ -342,17 +342,20 @@ fn lambda_parameters() -> impl NoirParser<Vec<(Pattern, UnresolvedType)>> {
.labelled(ParsingRuleLabel::Parameter)
}

fn function_parameters<'a>(
allow_self: bool,
) -> impl NoirParser<Vec<(Pattern, UnresolvedType, Visibility)>> + 'a {
fn function_parameters<'a>(allow_self: bool) -> impl NoirParser<Vec<Param>> + 'a {
let typ = parse_type().recover_via(parameter_recovery());

let full_parameter = pattern()
.recover_via(parameter_name_recovery())
.then_ignore(just(Token::Colon))
.then(optional_visibility())
.then(typ)
.map(|((name, visibility), typ)| (name, typ, visibility));
.map_with_span(|((pattern, visibility), typ), span| Param {
visibility,
pattern,
typ,
span,
});

let self_parameter = if allow_self { self_parameter().boxed() } else { nothing().boxed() };

Expand All @@ -369,7 +372,7 @@ fn nothing<T>() -> impl NoirParser<T> {
one_of([]).map(|_| unreachable!())
}

fn self_parameter() -> impl NoirParser<(Pattern, UnresolvedType, Visibility)> {
fn self_parameter() -> impl NoirParser<Param> {
let mut_ref_pattern = just(Token::Ampersand).then_ignore(keyword(Keyword::Mut));
let mut_pattern = keyword(Keyword::Mut);

Expand Down Expand Up @@ -398,7 +401,7 @@ fn self_parameter() -> impl NoirParser<(Pattern, UnresolvedType, Visibility)> {
_ => (),
}

(pattern, self_type, Visibility::Private)
Param { pattern, typ: self_type, visibility: Visibility::Private, span }
})
}

Expand Down Expand Up @@ -517,16 +520,16 @@ fn function_declaration_parameters() -> impl NoirParser<Vec<(Ident, UnresolvedTy

let full_parameter = ident().recover_via(parameter_name_recovery()).then(typ);
let self_parameter = self_parameter().validate(|param, span, emit| {
match param.0 {
Pattern::Identifier(ident) => (ident, param.1),
match param.pattern {
Pattern::Identifier(ident) => (ident, param.typ),
other => {
emit(ParserError::with_reason(
ParserErrorReason::PatternInTraitFunctionParameter,
span,
));
// into_ident panics on tuple or struct patterns but should be fine to call here
// since the `self` parser can only parse `self`, `mut self` or `&mut self`.
(other.into_ident(), param.1)
(other.into_ident(), param.typ)
}
}
});
Expand Down
5 changes: 3 additions & 2 deletions tooling/nargo_cli/src/cli/fmt_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use fm::FileManager;
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::NOIR_ARTIFACT_VERSION_STRING;
use noirc_errors::CustomDiagnostic;
use noirc_frontend::hir::def_map::parse_file;
use noirc_frontend::{hir::def_map::parse_file, parser::ParserError};

use crate::errors::CliError;

Expand Down Expand Up @@ -33,7 +33,8 @@ pub(crate) fn run(_args: FormatCommand, config: NargoConfig) -> Result<(), CliEr
let file_id = file_manager.add_file(&entry.path()).expect("file exists");
let (parsed_module, errors) = parse_file(&file_manager, file_id);

if !errors.is_empty() {
let is_all_warnings = errors.iter().all(ParserError::is_warning);
if !is_all_warnings {
let errors = errors
.into_iter()
.map(|error| {
Expand Down
1 change: 0 additions & 1 deletion tooling/nargo_fmt/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ fn format_{test_name}() {{


let (parsed_module, errors) = noirc_frontend::parse_program(&input);
assert!(errors.is_empty());

let config = nargo_fmt::Config::of("{config}").unwrap();
let fmt_text = nargo_fmt::format(&input, parsed_module, &config);
Expand Down
29 changes: 28 additions & 1 deletion tooling/nargo_fmt/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::visitor::FmtVisitor;
use noirc_frontend::hir::resolution::errors::Span;
use noirc_frontend::lexer::Lexer;
use noirc_frontend::token::Token;
use noirc_frontend::{Expression, Ident};
use noirc_frontend::{Expression, Ident, Param, Visibility};

pub(crate) fn changed_comment_content(original: &str, new: &str) -> bool {
comments(original).ne(comments(new))
Expand Down Expand Up @@ -237,6 +237,33 @@ impl Item for (Ident, Expression) {
}
}

impl Item for Param {
fn span(&self) -> Span {
self.span
}

fn format(self, visitor: &FmtVisitor) -> String {
let visibility = match self.visibility {
Visibility::Public => "pub ",
Visibility::Private => "",
};
let pattern = visitor.slice(self.pattern.span());
let ty = visitor.slice(self.typ.span.unwrap());

format!("{pattern}: {visibility}{ty}")
}
}

impl Item for Ident {
fn span(&self) -> Span {
self.span()
}

fn format(self, visitor: &FmtVisitor) -> String {
visitor.slice(self.span()).into()
}
}

pub(crate) fn first_line_width(exprs: &str) -> usize {
exprs.lines().next().map_or(0, |line: &str| line.chars().count())
}
Expand Down
Loading
Loading