From 50ab59ea4b35efb61126fcaeab6699d71911e2fd Mon Sep 17 00:00:00 2001 From: Marcos Henrich Date: Thu, 2 Mar 2023 16:45:17 +0000 Subject: [PATCH] Implements #[allow(dead_code)] (#4170) ## Description Added support for new attribute #[allow(dead_code)] ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers. --- .../analyze_return_paths.rs | 14 +- .../dead_code_analysis.rs | 228 ++++++++++++++---- .../control_flow_analysis/flow_graph/mod.rs | 47 ++-- sway-core/src/transform/attribute.rs | 32 ++- .../to_parsed_lang/convert_parse_tree.rs | 48 +++- sway-error/src/warning.rs | 29 +++ sway-types/src/constants.rs | 5 + .../should_pass/dca/allow_dead_code/Forc.lock | 3 + .../should_pass/dca/allow_dead_code/Forc.toml | 6 + .../dca/allow_dead_code/src/main.sw | 46 ++++ .../should_pass/dca/allow_dead_code/test.toml | 3 + .../language/attributes_warnings/Forc.lock | 13 + .../language/attributes_warnings/Forc.toml | 8 + .../attributes_warnings/json_abi_oracle.json | 25 ++ .../language/attributes_warnings/src/main.sw | 16 ++ .../language/attributes_warnings/test.toml | 11 + 16 files changed, 459 insertions(+), 75 deletions(-) create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/Forc.lock create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/Forc.toml create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/src/main.sw create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/test.toml create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/Forc.lock create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/Forc.toml create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/json_abi_oracle.json create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/src/main.sw create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/test.toml diff --git a/sway-core/src/control_flow_analysis/analyze_return_paths.rs b/sway-core/src/control_flow_analysis/analyze_return_paths.rs index fb797c96e0b..63f86437f84 100644 --- a/sway-core/src/control_flow_analysis/analyze_return_paths.rs +++ b/sway-core/src/control_flow_analysis/analyze_return_paths.rs @@ -78,7 +78,7 @@ impl<'cfg> ControlFlowGraph<'cfg> { let mut last_discovered_span; for rover in rovers { last_discovered_span = match &self.graph[rover] { - ControlFlowGraphNode::ProgramNode(node) => Some(node.span.clone()), + ControlFlowGraphNode::ProgramNode { node, .. } => Some(node.span.clone()), ControlFlowGraphNode::MethodDeclaration { span, .. } => Some(span.clone()), _ => None, }; @@ -137,7 +137,7 @@ fn connect_node<'eng: 'cfg, 'cfg>( .. }) | ty::TyAstNodeContent::ImplicitReturnExpression(_) => { - let this_index = graph.add_node(node.into()); + let this_index = graph.add_node(ControlFlowGraphNode::from_node(node)); for leaf_ix in leaves { graph.add_edge(*leaf_ix, this_index, "".into()); } @@ -150,14 +150,14 @@ fn connect_node<'eng: 'cfg, 'cfg>( // An abridged version of the dead code analysis for a while loop // since we don't really care about what the loop body contains when detecting // divergent paths - let node = graph.add_node(node.into()); + let node = graph.add_node(ControlFlowGraphNode::from_node(node)); for leaf in leaves { graph.add_edge(*leaf, node, "while loop entry".into()); } Ok(NodeConnection::NextStep(vec![node])) } ty::TyAstNodeContent::Expression(ty::TyExpression { .. }) => { - let entry = graph.add_node(node.into()); + let entry = graph.add_node(ControlFlowGraphNode::from_node(node)); // insert organizational dominator node // connected to all current leaves for leaf in leaves { @@ -189,7 +189,7 @@ fn connect_declaration<'eng: 'cfg, 'cfg>( | StorageDeclaration { .. } | GenericTypeForFunctionScope { .. } => Ok(leaves.to_vec()), VariableDeclaration(_) | ConstantDeclaration { .. } => { - let entry_node = graph.add_node(node.into()); + let entry_node = graph.add_node(ControlFlowGraphNode::from_node(node)); for leaf in leaves { graph.add_edge(*leaf, entry_node, "".into()); } @@ -197,7 +197,7 @@ fn connect_declaration<'eng: 'cfg, 'cfg>( } FunctionDeclaration { decl_id, .. } => { let fn_decl = decl_engine.get_function(decl_id); - let entry_node = graph.add_node(node.into()); + let entry_node = graph.add_node(ControlFlowGraphNode::from_node(node)); for leaf in leaves { graph.add_edge(*leaf, entry_node, "".into()); } @@ -208,7 +208,7 @@ fn connect_declaration<'eng: 'cfg, 'cfg>( let ty::TyImplTrait { trait_name, items, .. } = decl_engine.get_impl_trait(decl_id); - let entry_node = graph.add_node(node.into()); + let entry_node = graph.add_node(ControlFlowGraphNode::from_node(node)); for leaf in leaves { graph.add_edge(*leaf, entry_node, "".into()); } diff --git a/sway-core/src/control_flow_analysis/dead_code_analysis.rs b/sway-core/src/control_flow_analysis/dead_code_analysis.rs index ba96b8a8084..74e27d22120 100644 --- a/sway-core/src/control_flow_analysis/dead_code_analysis.rs +++ b/sway-core/src/control_flow_analysis/dead_code_analysis.rs @@ -6,6 +6,7 @@ use crate::{ ty::{self, TyImplItem}, CallPath, Visibility, }, + transform::{self, AttributesMap}, type_system::TypeInfo, Engines, TypeEngine, TypeId, }; @@ -13,7 +14,7 @@ use petgraph::{prelude::NodeIndex, visit::Dfs}; use std::collections::BTreeSet; use sway_error::warning::{CompileWarning, Warning}; use sway_error::{error::CompileError, type_error::TypeError}; -use sway_types::{span::Span, Ident, Spanned}; +use sway_types::{constants::ALLOW_DEAD_CODE_NAME, span::Span, Ident, Spanned}; impl<'cfg> ControlFlowGraph<'cfg> { pub(crate) fn find_dead_code(&self, decl_engine: &DeclEngine) -> Vec { @@ -36,13 +37,17 @@ impl<'cfg> ControlFlowGraph<'cfg> { let dead_function_contains_span = |span: &Span| -> bool { dead_nodes.iter().any(|x| { - if let ControlFlowGraphNode::ProgramNode(ty::TyAstNode { - span: function_span, - content: - ty::TyAstNodeContent::Declaration(ty::TyDeclaration::FunctionDeclaration { - .. - }), - }) = &self.graph[*x] + if let ControlFlowGraphNode::ProgramNode { + node: + ty::TyAstNode { + span: function_span, + content: + ty::TyAstNodeContent::Declaration( + ty::TyDeclaration::FunctionDeclaration { .. }, + ), + }, + .. + } = &self.graph[*x] { function_span.end() >= span.end() && function_span.start() <= span.start() } else { @@ -59,41 +64,61 @@ impl<'cfg> ControlFlowGraph<'cfg> { }; let dead_enum_variant_warnings = dead_nodes .iter() - .filter_map(|x| match &self.graph[*x] { - ControlFlowGraphNode::EnumVariant { - variant_name, - is_public, - } if !is_public => Some(priv_enum_var_warn(variant_name)), - _ => None, + .filter_map(|x| { + // If dead code is allowed return immediately no warning. + if allow_dead_code_node(decl_engine, &self.graph, &self.graph[*x]) { + None + } else { + match &self.graph[*x] { + ControlFlowGraphNode::EnumVariant { + variant_name, + is_public, + .. + } if !is_public => Some(priv_enum_var_warn(variant_name)), + _ => None, + } + } }) .collect::>(); let dead_ast_node_warnings = dead_nodes .iter() - .filter_map(|x| match &self.graph[*x] { - ControlFlowGraphNode::ProgramNode(node) => { - construct_dead_code_warning_from_node(decl_engine, node) + .filter_map(|x| { + // If dead code is allowed return immediately no warning. + if allow_dead_code_node(decl_engine, &self.graph, &self.graph[*x]) { + None + } else { + match &self.graph[*x] { + ControlFlowGraphNode::ProgramNode { node, .. } => { + construct_dead_code_warning_from_node(decl_engine, node) + } + ControlFlowGraphNode::EnumVariant { + variant_name, + is_public, + .. + } if !is_public => Some(priv_enum_var_warn(variant_name)), + ControlFlowGraphNode::EnumVariant { .. } => None, + ControlFlowGraphNode::MethodDeclaration { span, .. } => { + Some(CompileWarning { + span: span.clone(), + warning_content: Warning::DeadMethod, + }) + } + ControlFlowGraphNode::StructField { + struct_field_name, .. + } => Some(CompileWarning { + span: struct_field_name.span(), + warning_content: Warning::StructFieldNeverRead, + }), + ControlFlowGraphNode::StorageField { field_name, .. } => { + Some(CompileWarning { + span: field_name.span(), + warning_content: Warning::DeadStorageDeclaration, + }) + } + ControlFlowGraphNode::OrganizationalDominator(..) => None, + } } - ControlFlowGraphNode::EnumVariant { - variant_name, - is_public, - } if !is_public => Some(priv_enum_var_warn(variant_name)), - ControlFlowGraphNode::EnumVariant { .. } => None, - ControlFlowGraphNode::MethodDeclaration { span, .. } => Some(CompileWarning { - span: span.clone(), - warning_content: Warning::DeadMethod, - }), - ControlFlowGraphNode::StructField { - struct_field_name, .. - } => Some(CompileWarning { - span: struct_field_name.span(), - warning_content: Warning::StructFieldNeverRead, - }), - ControlFlowGraphNode::StorageField { field_name, .. } => Some(CompileWarning { - span: field_name.span(), - warning_content: Warning::DeadStorageDeclaration, - }), - ControlFlowGraphNode::OrganizationalDominator(..) => None, }) .collect::>(); @@ -177,7 +202,7 @@ fn collect_entry_points( let mut entry_points = vec![]; for i in graph.node_indices() { let is_entry = match &graph[i] { - ControlFlowGraphNode::ProgramNode(node) => { + ControlFlowGraphNode::ProgramNode { node, .. } => { node.is_entry_point(decl_engine, tree_type)? } _ => false, @@ -197,6 +222,7 @@ struct NodeConnectionOptions { /// When this is enabled, connect struct fields to the struct itself, /// thus making all struct fields considered as being used in the graph. force_struct_fields_connection: bool, + parent_node: Option, } fn connect_node<'eng: 'cfg, 'cfg>( @@ -212,7 +238,10 @@ fn connect_node<'eng: 'cfg, 'cfg>( let span = node.span.clone(); Ok(match &node.content { ty::TyAstNodeContent::ImplicitReturnExpression(expr) => { - let this_index = graph.add_node(node.into()); + let this_index = graph.add_node(ControlFlowGraphNode::from_node_with_parent( + node, + options.parent_node, + )); for leaf_ix in leaves { graph.add_edge(*leaf_ix, this_index, "".into()); } @@ -241,7 +270,10 @@ fn connect_node<'eng: 'cfg, 'cfg>( span, .. }) => { - let entry = graph.add_node(node.into()); + let entry = graph.add_node(ControlFlowGraphNode::from_node_with_parent( + node, + options.parent_node, + )); // insert organizational dominator node // connected to all current leaves for leaf in leaves { @@ -266,7 +298,8 @@ fn connect_node<'eng: 'cfg, 'cfg>( ty::TyAstNodeContent::SideEffect(_) => (leaves.to_vec(), exit_node), ty::TyAstNodeContent::Declaration(decl) => { // all leaves connect to this node, then this node is the singular leaf - let cfg_node: ControlFlowGraphNode = node.into(); + let cfg_node: ControlFlowGraphNode = + ControlFlowGraphNode::from_node_with_parent(node, options.parent_node); // check if node for this decl already exists let decl_node = match graph.get_node_from_decl(&cfg_node) { Some(node) => node, @@ -352,12 +385,12 @@ fn connect_declaration<'eng: 'cfg, 'cfg>( } StructDeclaration { decl_id, .. } => { let struct_decl = decl_engine.get_struct(decl_id); - connect_struct_declaration(&struct_decl, graph, entry_node, tree_type); + connect_struct_declaration(&struct_decl, *decl_id, graph, entry_node, tree_type); Ok(leaves.to_vec()) } EnumDeclaration { decl_id, .. } => { let enum_decl = decl_engine.get_enum(decl_id); - connect_enum_declaration(&enum_decl, graph, entry_node); + connect_enum_declaration(&enum_decl, *decl_id, graph, entry_node); Ok(leaves.to_vec()) } ImplTrait { decl_id, .. } => { @@ -389,6 +422,7 @@ fn connect_declaration<'eng: 'cfg, 'cfg>( /// connect that field. fn connect_struct_declaration<'eng: 'cfg, 'cfg>( struct_decl: &ty::TyStructDeclaration, + struct_decl_id: DeclId, graph: &mut ControlFlowGraph<'cfg>, entry_node: NodeIndex, tree_type: &TreeType, @@ -401,7 +435,17 @@ fn connect_struct_declaration<'eng: 'cfg, 'cfg>( } = struct_decl; let field_nodes = fields .iter() - .map(|field| (field.name.clone(), graph.add_node(field.into()))) + .map(|field| { + ( + field.name.clone(), + graph.add_node(ControlFlowGraphNode::StructField { + struct_decl_id, + struct_field_name: field.name.clone(), + span: field.span.clone(), + attributes: field.attributes.clone(), + }), + ) + }) .collect::>(); // If this is a library or smart contract, and if this is public, then we want to connect the // declaration node itself to the individual fields. @@ -624,6 +668,7 @@ fn get_struct_type_info_from_type_id( /// we can see clearly, and thusly warn, when individual variants are not ever constructed. fn connect_enum_declaration<'eng: 'cfg, 'cfg>( enum_decl: &ty::TyEnumDeclaration, + enum_decl_id: DeclId, graph: &mut ControlFlowGraph<'cfg>, entry_node: NodeIndex, ) { @@ -634,6 +679,7 @@ fn connect_enum_declaration<'eng: 'cfg, 'cfg>( // keep a mapping of each variant for variant in enum_decl.variants.iter() { let variant_index = graph.add_node(ControlFlowGraphNode::from_enum_variant( + enum_decl_id, variant.name.clone(), enum_decl.visibility != Visibility::Private, )); @@ -670,7 +716,10 @@ fn connect_typed_fn_decl<'eng: 'cfg, 'cfg>( &[entry_node], Some(fn_exit_node), tree_type, - options, + NodeConnectionOptions { + force_struct_fields_connection: options.force_struct_fields_connection, + parent_node: Some(entry_node), + }, )?; if let Some(exit_node) = exit_node { graph.add_edge(fn_exit_node, exit_node, "".into()); @@ -1487,6 +1536,7 @@ fn connect_intrinsic_function<'eng: 'cfg, 'cfg>( exp.span.clone(), NodeConnectionOptions { force_struct_fields_connection: true, + parent_node: Some(node), }, )?; accum.append(&mut res); @@ -1730,3 +1780,91 @@ fn connect_storage_declaration<'eng: 'cfg, 'cfg>( graph.namespace.insert_storage(field_nodes); } + +/// Checks [AttributesMap] for `#[allow(dead_code)]` usage, if so returns true +/// otherwise returns false. +fn allow_dead_code(attributes: AttributesMap) -> bool { + fn allow_dead_code_helper(attributes: AttributesMap) -> Option { + Some( + attributes + .get(&transform::AttributeKind::Allow)? + .last()? + .args + .first()? + .as_str() + == ALLOW_DEAD_CODE_NAME, + ) + } + + allow_dead_code_helper(attributes).unwrap_or_default() +} + +/// Returns true when the given `node` contains the attribute `#[allow(dead_code)]` +fn allow_dead_code_ast_node(decl_engine: &DeclEngine, node: &ty::TyAstNode) -> bool { + match &node.content { + ty::TyAstNodeContent::Declaration(decl) => match &decl { + ty::TyDeclaration::VariableDeclaration(_) => false, + ty::TyDeclaration::ConstantDeclaration { decl_id, .. } => { + allow_dead_code(decl_engine.get_constant(decl_id).attributes) + } + ty::TyDeclaration::FunctionDeclaration { decl_id, .. } => { + allow_dead_code(decl_engine.get_function(decl_id).attributes) + } + ty::TyDeclaration::TraitDeclaration { decl_id, .. } => { + allow_dead_code(decl_engine.get_trait(decl_id).attributes) + } + ty::TyDeclaration::StructDeclaration { decl_id, .. } => { + allow_dead_code(decl_engine.get_struct(decl_id).attributes) + } + ty::TyDeclaration::EnumDeclaration { decl_id, .. } => { + allow_dead_code(decl_engine.get_enum(decl_id).attributes) + } + ty::TyDeclaration::ImplTrait { .. } => false, + ty::TyDeclaration::AbiDeclaration { .. } => false, + ty::TyDeclaration::GenericTypeForFunctionScope { .. } => false, + ty::TyDeclaration::ErrorRecovery(_) => false, + ty::TyDeclaration::StorageDeclaration { .. } => false, + }, + ty::TyAstNodeContent::Expression(_) => false, + ty::TyAstNodeContent::ImplicitReturnExpression(_) => false, + ty::TyAstNodeContent::SideEffect(_) => false, + } +} + +/// Returns true when the given `node` or its parent contains the attribute `#[allow(dead_code)]` +fn allow_dead_code_node( + decl_engine: &DeclEngine, + graph: &Graph, + node: &ControlFlowGraphNode, +) -> bool { + match node { + ControlFlowGraphNode::ProgramNode { node, parent_node } => { + if let Some(parent_node) = parent_node { + let parent_node = &graph[*parent_node]; + if allow_dead_code_node(decl_engine, graph, parent_node) { + return true; + } + } + allow_dead_code_ast_node(decl_engine, node) + } + ControlFlowGraphNode::EnumVariant { enum_decl_id, .. } => { + allow_dead_code(decl_engine.get_enum(enum_decl_id).attributes) + } + ControlFlowGraphNode::MethodDeclaration { + method_decl_ref, .. + } => allow_dead_code(decl_engine.get_function(method_decl_ref).attributes), + ControlFlowGraphNode::StructField { + struct_decl_id, + attributes, + .. + } => { + if allow_dead_code(attributes.clone()) { + true + } else { + allow_dead_code(decl_engine.get_struct(struct_decl_id).attributes) + } + } + ControlFlowGraphNode::StorageField { .. } => false, + ControlFlowGraphNode::OrganizationalDominator(..) => false, + } +} diff --git a/sway-core/src/control_flow_analysis/flow_graph/mod.rs b/sway-core/src/control_flow_analysis/flow_graph/mod.rs index c2efa83bffe..0d54d7dfc19 100644 --- a/sway-core/src/control_flow_analysis/flow_graph/mod.rs +++ b/sway-core/src/control_flow_analysis/flow_graph/mod.rs @@ -6,7 +6,7 @@ use std::collections::HashMap; use crate::{ decl_engine::*, language::ty::{self, GetDeclIdent}, - Engines, Ident, + transform, Engines, Ident, }; use sway_types::{span::Span, BaseIdent, IdentUnique}; @@ -54,8 +54,12 @@ impl std::convert::From<&str> for ControlFlowGraphEdge { pub enum ControlFlowGraphNode<'cfg> { OrganizationalDominator(String), #[allow(clippy::large_enum_variant)] - ProgramNode(ty::TyAstNode), + ProgramNode { + node: ty::TyAstNode, + parent_node: Option, + }, EnumVariant { + enum_decl_id: DeclId, variant_name: Ident, is_public: bool, }, @@ -66,7 +70,9 @@ pub enum ControlFlowGraphNode<'cfg> { engines: Engines<'cfg>, }, StructField { + struct_decl_id: DeclId, struct_field_name: Ident, + attributes: transform::AttributesMap, span: Span, }, StorageField { @@ -78,7 +84,7 @@ impl<'cfg> GetDeclIdent for ControlFlowGraphNode<'cfg> { fn get_decl_ident(&self) -> Option { match self { ControlFlowGraphNode::OrganizationalDominator(_) => None, - ControlFlowGraphNode::ProgramNode(node) => node.get_decl_ident(), + ControlFlowGraphNode::ProgramNode { node, .. } => node.get_decl_ident(), ControlFlowGraphNode::EnumVariant { variant_name, .. } => Some(variant_name.clone()), ControlFlowGraphNode::MethodDeclaration { method_name, .. } => { Some(method_name.clone()) @@ -99,20 +105,6 @@ impl<'cfg> std::convert::From<&ty::TyStorageField> for ControlFlowGraphNode<'cfg } } -impl<'cfg> std::convert::From<&ty::TyAstNode> for ControlFlowGraphNode<'cfg> { - fn from(other: &ty::TyAstNode) -> Self { - ControlFlowGraphNode::ProgramNode(other.clone()) - } -} - -impl<'cfg> std::convert::From<&ty::TyStructField> for ControlFlowGraphNode<'cfg> { - fn from(other: &ty::TyStructField) -> Self { - ControlFlowGraphNode::StructField { - struct_field_name: other.name.clone(), - span: other.span.clone(), - } - } -} impl<'cfg> std::convert::From for ControlFlowGraphNode<'cfg> { fn from(other: String) -> Self { ControlFlowGraphNode::OrganizationalDominator(other) @@ -129,7 +121,7 @@ impl<'cfg> std::fmt::Debug for ControlFlowGraphNode<'cfg> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let text = match self { ControlFlowGraphNode::OrganizationalDominator(s) => s.to_string(), - ControlFlowGraphNode::ProgramNode(node) => format!("{node:?}"), + ControlFlowGraphNode::ProgramNode { node, .. } => format!("{node:?}"), ControlFlowGraphNode::EnumVariant { variant_name, .. } => { format!("Enum variant {variant_name}") } @@ -214,12 +206,31 @@ impl<'cfg> ControlFlowGraph<'cfg> { impl<'cfg> ControlFlowGraphNode<'cfg> { pub(crate) fn from_enum_variant( + enum_decl_id: DeclId, other_name: BaseIdent, is_public: bool, ) -> ControlFlowGraphNode<'cfg> { ControlFlowGraphNode::EnumVariant { + enum_decl_id, variant_name: other_name, is_public, } } + + pub(crate) fn from_node_with_parent( + node: &ty::TyAstNode, + parent_node: Option, + ) -> ControlFlowGraphNode<'cfg> { + ControlFlowGraphNode::ProgramNode { + node: node.clone(), + parent_node, + } + } + + pub(crate) fn from_node(node: &ty::TyAstNode) -> ControlFlowGraphNode<'cfg> { + ControlFlowGraphNode::ProgramNode { + node: node.clone(), + parent_node: None, + } + } } diff --git a/sway-core/src/transform/attribute.rs b/sway-core/src/transform/attribute.rs index 829fa798020..4438acb9902 100644 --- a/sway-core/src/transform/attribute.rs +++ b/sway-core/src/transform/attribute.rs @@ -20,7 +20,7 @@ //! //! #[foo(bar, bar)] -use sway_types::{Ident, Span}; +use sway_types::{constants::ALLOW_DEAD_CODE_NAME, Ident, Span}; use std::{collections::HashMap, hash::Hash, sync::Arc}; @@ -43,6 +43,36 @@ pub enum AttributeKind { Inline, Test, Payable, + Allow, +} + +impl AttributeKind { + // Returns tuple with the mininum and maximum number of expected args + // None can be returned in the second position of the tuple if there is no maximum + pub fn expected_args_len_min_max(self) -> (usize, Option) { + match self { + AttributeKind::Doc => (0, None), + AttributeKind::DocComment => (0, None), + AttributeKind::Storage => (0, None), + AttributeKind::Inline => (0, None), + AttributeKind::Test => (0, None), + AttributeKind::Payable => (0, None), + AttributeKind::Allow => (1, Some(1)), + } + } + + // Returns the expected values for an attribute argument + pub fn expected_args_values(self, _arg_index: usize) -> Option> { + match self { + AttributeKind::Doc => None, + AttributeKind::DocComment => None, + AttributeKind::Storage => None, + AttributeKind::Inline => None, + AttributeKind::Test => None, + AttributeKind::Payable => None, + AttributeKind::Allow => Some(vec![ALLOW_DEAD_CODE_NAME.to_string()]), + } + } } /// Stores the attributes associated with the type. diff --git a/sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs b/sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs index 5c6e6cdbaf2..f3725a09f51 100644 --- a/sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs +++ b/sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs @@ -23,10 +23,10 @@ use sway_error::handler::{ErrorEmitted, Handler}; use sway_error::warning::{CompileWarning, Warning}; use sway_types::{ constants::{ - DESTRUCTURE_PREFIX, DOC_ATTRIBUTE_NAME, DOC_COMMENT_ATTRIBUTE_NAME, INLINE_ATTRIBUTE_NAME, - MATCH_RETURN_VAR_NAME_PREFIX, PAYABLE_ATTRIBUTE_NAME, STORAGE_PURITY_ATTRIBUTE_NAME, - STORAGE_PURITY_READ_NAME, STORAGE_PURITY_WRITE_NAME, TEST_ATTRIBUTE_NAME, - TUPLE_NAME_PREFIX, VALID_ATTRIBUTE_NAMES, + ALLOW_ATTRIBUTE_NAME, DESTRUCTURE_PREFIX, DOC_ATTRIBUTE_NAME, DOC_COMMENT_ATTRIBUTE_NAME, + INLINE_ATTRIBUTE_NAME, MATCH_RETURN_VAR_NAME_PREFIX, PAYABLE_ATTRIBUTE_NAME, + STORAGE_PURITY_ATTRIBUTE_NAME, STORAGE_PURITY_READ_NAME, STORAGE_PURITY_WRITE_NAME, + TEST_ATTRIBUTE_NAME, TUPLE_NAME_PREFIX, VALID_ATTRIBUTE_NAMES, }, integer_bits::IntegerBits, }; @@ -3726,6 +3726,7 @@ fn item_attrs_to_map( INLINE_ATTRIBUTE_NAME => Some(AttributeKind::Inline), TEST_ATTRIBUTE_NAME => Some(AttributeKind::Test), PAYABLE_ATTRIBUTE_NAME => Some(AttributeKind::Payable), + ALLOW_ATTRIBUTE_NAME => Some(AttributeKind::Allow), _ => None, } { match attrs_map.get_mut(&attr_kind) { @@ -3739,6 +3740,45 @@ fn item_attrs_to_map( } } } + + // Check attribute arguments + for (attribute_kind, attributes) in &attrs_map { + for attribute in attributes { + // check attribute arguments length + let (expected_min_len, expected_max_len) = + attribute_kind.clone().expected_args_len_min_max(); + if attribute.args.len() < expected_min_len + || attribute.args.len() > expected_max_len.unwrap_or(usize::MAX) + { + handler.emit_warn(CompileWarning { + span: attribute.name.span().clone(), + warning_content: Warning::AttributeExpectedNumberOfArguments { + attrib_name: attribute.name.clone(), + received_args: attribute.args.len(), + expected_min_len, + expected_max_len, + }, + }) + } + + // check attribute argument value + for (index, arg) in attribute.args.iter().enumerate() { + let possible_values = attribute_kind.clone().expected_args_values(index); + if let Some(possible_values) = possible_values { + if !possible_values.iter().any(|v| v == arg.as_str()) { + handler.emit_warn(CompileWarning { + span: attribute.name.span().clone(), + warning_content: Warning::UnexpectedAttributeArgumentValue { + attrib_name: attribute.name.clone(), + received_value: arg.as_str().to_string(), + expected_values: possible_values, + }, + }) + } + } + } + } + } Ok(AttributesMap::new(Arc::new(attrs_map))) } diff --git a/sway-error/src/warning.rs b/sway-error/src/warning.rs index 93da602bd33..c68215f8b81 100644 --- a/sway-error/src/warning.rs +++ b/sway-error/src/warning.rs @@ -92,6 +92,17 @@ pub enum Warning { UnrecognizedAttribute { attrib_name: Ident, }, + AttributeExpectedNumberOfArguments { + attrib_name: Ident, + received_args: usize, + expected_min_len: usize, + expected_max_len: Option, + }, + UnexpectedAttributeArgumentValue { + attrib_name: Ident, + received_value: String, + expected_values: Vec, + }, EffectAfterInteraction { effect: String, effect_in_suggestion: String, @@ -217,6 +228,24 @@ impl fmt::Display for Warning { ), MatchExpressionUnreachableArm => write!(f, "This match arm is unreachable."), UnrecognizedAttribute {attrib_name} => write!(f, "Unknown attribute: \"{attrib_name}\"."), + AttributeExpectedNumberOfArguments {attrib_name, received_args, expected_min_len, expected_max_len } => write!( + f, + "Attribute: \"{attrib_name}\" expected {} argument(s) received {received_args}.", + if let Some(expected_max_len) = expected_max_len { + if expected_min_len == expected_max_len { + format!("exactly {expected_min_len}") + } else { + format!("between {expected_min_len} and {expected_max_len}") + } + } else { + format!("at least {expected_min_len}") + } + ), + UnexpectedAttributeArgumentValue {attrib_name, received_value, expected_values } => write!( + f, + "Unexpected attribute value: \"{received_value}\" for attribute: \"{attrib_name}\" expected value {}", + expected_values.iter().map(|v| format!("\"{v}\"")).collect::>().join(" or ") + ), EffectAfterInteraction {effect, effect_in_suggestion, block_name} => write!(f, "{effect} after external contract interaction in function or method \"{block_name}\". \ Consider {effect_in_suggestion} before calling another contract"), diff --git a/sway-types/src/constants.rs b/sway-types/src/constants.rs index e20280c9fde..ff73cd9e4fd 100644 --- a/sway-types/src/constants.rs +++ b/sway-types/src/constants.rs @@ -49,6 +49,10 @@ pub const TEST_ATTRIBUTE_NAME: &str = "test"; /// The valid attribute string used for payable functions. pub const PAYABLE_ATTRIBUTE_NAME: &str = "payable"; +/// The valid attribute strings related to allow. +pub const ALLOW_ATTRIBUTE_NAME: &str = "allow"; +pub const ALLOW_DEAD_CODE_NAME: &str = "dead_code"; + /// The list of valid attributes. pub const VALID_ATTRIBUTE_NAMES: &[&str] = &[ STORAGE_PURITY_ATTRIBUTE_NAME, @@ -57,4 +61,5 @@ pub const VALID_ATTRIBUTE_NAMES: &[&str] = &[ TEST_ATTRIBUTE_NAME, INLINE_ATTRIBUTE_NAME, PAYABLE_ATTRIBUTE_NAME, + ALLOW_ATTRIBUTE_NAME, ]; diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/Forc.lock new file mode 100644 index 00000000000..a21b6a1d74c --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/Forc.lock @@ -0,0 +1,3 @@ +[[package]] +name = 'allow_dead_code' +source = 'member' diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/Forc.toml new file mode 100644 index 00000000000..b7198dc4ccb --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/Forc.toml @@ -0,0 +1,6 @@ +[project] +authors = ["Fuel Labs "] +entry = "main.sw" +implicit-std = false +license = "Apache-2.0" +name = "allow_dead_code" diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/src/main.sw new file mode 100644 index 00000000000..f438ccca7a8 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/src/main.sw @@ -0,0 +1,46 @@ +script; + +#[allow(dead_code)] +const A = 1; + +#[allow(dead_code)] +struct A { + i: u64, +} + +#[allow(dead_code)] +enum E { + A: () +} + +#[allow(dead_code)] +fn f() -> u64 { + return 1; +} + +#[allow(dead_code)] +trait Trait { + fn m(self) -> bool; +} + +struct B { + i: u64, + #[allow(dead_code)] + u: u64, +} + +impl B { + fn a(self) -> u64 { + return self.i; + } + + #[allow(dead_code)] + fn b(self) -> u64 { + return 1; + } +} + +fn main() { + let b = B { i: 43, u: 43 }; + let i = b.a(); +} diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/test.toml b/test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/test.toml new file mode 100644 index 00000000000..07dfa07de3f --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/dca/allow_dead_code/test.toml @@ -0,0 +1,3 @@ +category = "compile" + +# not: $()warning \ No newline at end of file diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/Forc.lock new file mode 100644 index 00000000000..8f23cc6f1a5 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/Forc.lock @@ -0,0 +1,13 @@ +[[package]] +name = 'attributes_warnings' +source = 'member' +dependencies = ['std'] + +[[package]] +name = 'core' +source = 'path+from-root-8B6385B3B4229CC8' + +[[package]] +name = 'std' +source = 'path+from-root-8B6385B3B4229CC8' +dependencies = ['core'] diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/Forc.toml new file mode 100644 index 00000000000..d6d4cfe718c --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/Forc.toml @@ -0,0 +1,8 @@ +[project] +authors = ["Fuel Labs "] +entry = "main.sw" +license = "Apache-2.0" +name = "attributes_warnings" + +[dependencies] +std = { path = "../../../../../../../sway-lib-std" } diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/json_abi_oracle.json b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/json_abi_oracle.json new file mode 100644 index 00000000000..fad72a08f17 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/json_abi_oracle.json @@ -0,0 +1,25 @@ +{ + "configurables": [], + "functions": [ + { + "attributes": null, + "inputs": [], + "name": "main", + "output": { + "name": "", + "type": 0, + "typeArguments": null + } + } + ], + "loggedTypes": [], + "messagesTypes": [], + "types": [ + { + "components": [], + "type": "()", + "typeId": 0, + "typeParameters": null + } + ] +} \ No newline at end of file diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/src/main.sw new file mode 100644 index 00000000000..2783d7cdd4a --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/src/main.sw @@ -0,0 +1,16 @@ +script; + +#[allow(foo)] +fn f1() {} + +#[allow] +fn f2() {} + +#[allow(dead_code, dead_code)] +fn f3() {} + +fn main() { + f1(); + f2(); + f3(); +} diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/test.toml b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/test.toml new file mode 100644 index 00000000000..d574a6e7130 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_warnings/test.toml @@ -0,0 +1,11 @@ +category = "compile" +expected_warnings = 3 + +# check: #[allow(foo)] +# nextln: $()Unexpected attribute value: "foo" for attribute: "allow" expected value "dead_code" + +# check: #[allow] +# nextln: $()Attribute: "allow" expected exactly 1 argument(s) received 0. + +# check: #[allow(dead_code, dead_code)] +# nextln: $()Attribute: "allow" expected exactly 1 argument(s) received 2.