Skip to content

Commit

Permalink
chore: format function signature (#3495)
Browse files Browse the repository at this point in the history
  • Loading branch information
kek kek kek authored Nov 16, 2023
1 parent 484e032 commit 52dbcf1
Show file tree
Hide file tree
Showing 16 changed files with 196 additions and 53 deletions.
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::{
};
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 @@ fn check_for_compute_note_hash_and_nullifier_definition(module: &SortedModule) -
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 generate_selector_impl(structure: &NoirStruct) -> TypeImpl {
/// 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 @@ pub(crate) fn create_inputs(ty: &str) -> (Pattern, UnresolvedType, Visibility) {
/// 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 @@ fn create_context(ty: &str, params: &[(Pattern, UnresolvedType, Visibility)]) ->
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
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::{
};
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 @@ -760,7 +760,7 @@ impl<'a> Resolver<'a> {
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

0 comments on commit 52dbcf1

Please sign in to comment.