Defining, parsing, and checking #[attribute]s (#6986)

## Description

This PR reimplements how `#[attribute]`s are parsed, defined, and
checked. It fixes the following issues we had with attributes:
- attributes were in general not checked for their expected and allowed
usage.
- when checked, checks were fixing particular reported issues and were
scattered through various compilation phases: lexing, parsing, and tree
conversion.
- when checked, reported errors in some cases had bugs and were
overalapping with other errors and warnings.
- attribute handling was not centralized, but rahter scattered throught
the whole code base, and mostly done at use sites.

For concrete examples of these issues, see the list of closed issues
below.

The PR:
- encapsulates the attributes handling within a single abstraction:
`Attributes`.
- assigns the following properties to every attribute:
  - _target_: what kind of elements an attribute can annotate.
- _multiplicity_: if more then one attribute of the same kind can be
applied to an element.
- _arguments multiplicity_: a range defining how many arguments an
attribute can or must have.
- _arguments value expectation_: if attribute arguments are expected to
have values.
- validates those properties in a single place, during the
`convert_parse_tree`. Note that this means that modules with attribute
issues will now consistently not reach the type checking phase. This was
previously the case for some of the issues with attributes where custom
checks where done during the type checking phase. #6987 proposes to move
those checks after the tree conversion phase, allowing the continuation
of compilation.
- adds the `AttributeKind::Unknow` to preserve unknown attributes in the
typed tree.
- removes redundant code related to attributes: specific warnings and
errors, specialized parsing, repetitive and error-prone at use site
checks, etc.
- removes the unused `Doc` attribute.

The PR also introduces a new pattern in emitting errors. The
`attr_decls_to_attributes` function will always return `Attributes` even
if they have errors, because:
- we expect the compilation to continue even in the case of errors.
- we expect those errors to be ignored if a valid `#[cfg]` attribute
among those evaluates to false.
- we expect them to be emitted with eventual other errors, or alone, at
the end of the tree conversion of the annotated element.

For more details, see the comment on `attr_decls_to_attributes` and
`cfg_eval`.

Closes #6880; closes #6913; closes #6914; closes #6915; closes #6916;
closes #6917; closes #6918; closes #6931; closes #6981; closes #6983;
closes #6984; closes #6985.

## Breaking changes

Strictly seen, this PR can cause breaking changes, but only in the case
of invalid existing attribute usage. We treat those breaking changes as
bug fixes in the compiler and, thus, do not have them behind a feature
flag.

E.g., this kind of code was possible before, but will now emit and
error:
```sway
#[storage(read, write, read, write)]
struct Struct {}
```

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [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.

Author: ironcev

.typos.toml

@@ -13,5 +13,7 @@ extend-ignore-identifiers-re = [
 extend-exclude = [
     "docs/book/theme/highlight.js",
     "docs/reference/theme/highlight.js",
-    "forc-plugins/forc-doc/src/static.files/highlight.js"
+    "forc-plugins/forc-doc/src/static.files/highlight.js",
+    "test/src/e2e_vm_tests/test_programs/should_fail/attributes_invalid_args/**",
+    "test/src/e2e_vm_tests/test_programs/should_pass/language/attributes_unknown/**",
 ]

docs/book/src/blockchain-development/calling_contracts.md

@@ -171,7 +171,7 @@ When a contract is compiled, a special section called "contract selection" is al
 1 - if no fallback function was specified, the contract will revert;
 2 - otherwise, the fallback function will be called.
 
-For all intents and purposes the fallback function is considered a contract method, which means that it has all the limitations that other contract methods have. As the fallback function signature, the function cannot have arguments, but they can return anything.
+For all intents and purposes the fallback function is considered a contract method, which means that it has all the limitations that other contract methods have. As for the fallback function signature, the function cannot have arguments, but it can return anything.
 
 If for some reason the fallback function needs to returns different types, the intrinsic `__contract_ret` can be used.
 
@@ -182,7 +182,7 @@ abi MyContract {
     fn some_method();
 }
 
-impl ContractB for Contract {
+impl MyContract for Contract {
     fn some_method() {
     }
 }
@@ -192,8 +192,8 @@ fn fallback() {
 }
 ```
 
-You may still access the method selector and arguments to the call in the fallback.
-For instance, let's assume a function `fn foobar(bool, u64) {}` gets called on a contract that doesn't have it with arguments `true` and `42`.
+You may still access the method selector and call arguments in the fallback function.
+For instance, let's assume a function `fn foobar(bool, u64) {}` gets called on a contract that doesn't have it, with arguments `true` and `42`.
 It can execute the following fallback:
 
 ```sway

docs/reference/src/code/language/annotations/src/main.sw

@@ -90,7 +90,7 @@ fn bar() {}
 
 
 // ANCHOR: allow_deprecated_annotation
-#[deprecated(note = "this is deprecated")]
+#[deprecated(note = "This is deprecated.")]
 struct DeprecatedStruct {}
 
 #[allow(deprecated)]

forc-pkg/src/pkg.rs

@@ -29,6 +29,7 @@ use std::{
     sync::{atomic::AtomicBool, Arc},
 };
 use sway_core::namespace::Package;
+use sway_core::transform::AttributeArg;
 pub use sway_core::Programs;
 use sway_core::{
     abi_generation::{
@@ -44,7 +45,6 @@ use sway_core::{
     language::parsed::TreeType,
     semantic_analysis::namespace,
     source_map::SourceMap,
-    transform::AttributeKind,
     write_dwarf, BuildTarget, Engines, FinalizedEntry, LspConfig,
 };
 use sway_core::{set_bytecode_configurables_offset, PrintAsm, PrintIr};
@@ -2003,49 +2003,55 @@ impl PkgEntryKind {
 
 impl PkgTestEntry {
     fn from_decl(decl_ref: &DeclRefFunction, engines: &Engines) -> Result<Self> {
+        fn get_invalid_revert_code_error_msg(
+            test_function_name: &Ident,
+            should_revert_arg: &AttributeArg,
+        ) -> String {
+            format!("Invalid revert code for test \"{}\".\nA revert code must be a string containing a \"u64\", e.g.: \"42\".\nThe invalid revert code was: {}.",
+                test_function_name,
+                should_revert_arg.value.as_ref().expect("`get_string_opt` returned either a value or an error, which means that the invalid value must exist").span().as_str(),
+            )
+        }
+
         let span = decl_ref.span();
         let test_function_decl = engines.de().get_function(decl_ref);
 
-        const FAILING_TEST_KEYWORD: &str = "should_revert";
+        let Some(test_attr) = test_function_decl.attributes.test() else {
+            unreachable!("`test_function_decl` is guaranteed to be a test function and it must have a `#[test]` attribute");
+        };
 
-        let test_args: HashMap<String, Option<String>> = test_function_decl
-            .attributes
-            .get(&AttributeKind::Test)
-            .expect("test declaration is missing test attribute")
+        let pass_condition = match test_attr
+            .args
             .iter()
-            .flat_map(|attr| attr.args.iter())
-            .map(|arg| {
-                (
-                    arg.name.to_string(),
-                    arg.value
-                        .as_ref()
-                        .map(|val| val.span().as_str().to_string()),
-                )
-            })
-            .collect();
+            // Last "should_revert" argument wins ;-)
+            .rfind(|arg| arg.is_test_should_revert())
+        {
+            Some(should_revert_arg) => {
+                match should_revert_arg.get_string_opt(&Handler::default()) {
+                    Ok(should_revert_arg_value) => TestPassCondition::ShouldRevert(
+                        should_revert_arg_value
+                            .map(|val| val.parse::<u64>())
+                            .transpose()
+                            .map_err(|_| {
+                                anyhow!(get_invalid_revert_code_error_msg(
+                                    &test_function_decl.name,
+                                    should_revert_arg
+                                ))
+                            })?,
+                    ),
+                    Err(_) => bail!(get_invalid_revert_code_error_msg(
+                        &test_function_decl.name,
+                        should_revert_arg
+                    )),
+                }
+            }
+            None => TestPassCondition::ShouldNotRevert,
+        };
 
-        let pass_condition = if test_args.is_empty() {
-            anyhow::Ok(TestPassCondition::ShouldNotRevert)
-        } else if let Some(args) = test_args.get(FAILING_TEST_KEYWORD) {
-            let expected_revert_code = args
-                .as_ref()
-                .map(|arg| {
-                    let arg_str = arg.replace('"', "");
-                    arg_str.parse::<u64>()
-                })
-                .transpose()?;
-            anyhow::Ok(TestPassCondition::ShouldRevert(expected_revert_code))
-        } else {
-            let test_name = &test_function_decl.name;
-            bail!("Invalid test argument(s) for test: {test_name}.")
-        }?;
-
-        let file_path = Arc::new(
-            engines.se().get_path(
-                span.source_id()
-                    .ok_or_else(|| anyhow::anyhow!("Missing span for test function"))?,
-            ),
-        );
+        let file_path =
+            Arc::new(engines.se().get_path(span.source_id().ok_or_else(|| {
+                anyhow!("Missing span for test \"{}\".", test_function_decl.name)
+            })?));
         Ok(Self {
             pass_condition,
             span,

forc-plugins/forc-doc/src/render/mod.rs

@@ -19,7 +19,7 @@ use std::{
     collections::BTreeMap,
     ops::{Deref, DerefMut},
 };
-use sway_core::{language::ty::TyProgramKind, transform::AttributesMap};
+use sway_core::{language::ty::TyProgramKind, transform::Attributes};
 use sway_types::BaseIdent;
 
 pub mod constant;
@@ -71,7 +71,7 @@ impl RenderedDocumentation {
     pub fn from_raw_docs(
         raw_docs: Documentation,
         render_plan: RenderPlan,
-        root_attributes: Option<AttributesMap>,
+        root_attributes: Option<Attributes>,
         program_kind: &TyProgramKind,
         forc_version: Option<String>,
     ) -> Result<RenderedDocumentation> {

forc-plugins/forc-doc/src/render/util/format/docstring.rs

@@ -2,15 +2,15 @@
 use crate::render::util::format::constant::*;
 use comrak::{markdown_to_html, ComrakOptions};
 use std::fmt::Write;
-use sway_core::transform::{AttributeKind, AttributesMap};
+use sway_core::transform::{AttributeKind, Attributes};
 use sway_lsp::utils::markdown::format_docs;
 
 pub(crate) trait DocStrings {
     fn to_html_string(&self) -> String;
     fn to_raw_string(&self) -> String;
 }
-/// Creates an HTML String from an [AttributesMap]
-impl DocStrings for AttributesMap {
+/// Creates an HTML String from [Attributes].
+impl DocStrings for Attributes {
     fn to_html_string(&self) -> String {
         let docs = self.to_raw_string();
 
@@ -26,15 +26,14 @@ impl DocStrings for AttributesMap {
         markdown_to_html(&format_docs(&docs), &options)
     }
     fn to_raw_string(&self) -> String {
-        let attributes = self.get(&AttributeKind::DocComment);
         let mut docs = String::new();
-
-        if let Some(vec_attrs) = attributes {
-            for arg in vec_attrs.iter().flat_map(|attribute| &attribute.args) {
-                writeln!(docs, "{}", arg.name.as_str()).expect(
-                    "problem appending `arg.name.as_str()` to `docs` with `writeln` macro.",
-                );
-            }
+        // TODO: Change this logic once https://github.com/FuelLabs/sway/issues/6938 gets implemented.
+        for arg in self
+            .of_kind(AttributeKind::DocComment)
+            .flat_map(|attribute| &attribute.args)
+        {
+            writeln!(docs, "{}", arg.name.as_str())
+                .expect("problem appending `arg.name.as_str()` to `docs` with `writeln` macro.");
         }
         docs
     }

forc-plugins/forc-migrate/src/cli/commands/run.rs

@@ -451,7 +451,7 @@ fn print_migration_finished_action(num_of_postponed_steps: usize) {
             "Finished",
             &format!(
                 "Run `forc migrate` at a later point to resolve {} postponed migration step{}",
-                number_to_str(num_of_postponed_steps),
+                num_to_str(num_of_postponed_steps),
                 plural_s(num_of_postponed_steps),
             ),
         )

forc-plugins/forc-migrate/src/matching/lexed_tree.rs

@@ -227,10 +227,7 @@ pub mod __mod_name {
             self.items
                 .__iter()
                 .filter_map(|annotated| match __ref([annotated.value]) {
-                    ItemKind::Impl(item_impl) => {
-                        Some((__ref([annotated.attribute_list]), item_impl))
-                    }
-                    // ItemKind::Impl(__ref_mut([item_impl])) => Some((__ref([annotated.attribute_list]), item_impl)),
+                    ItemKind::Impl(item_impl) => Some((__ref([annotated.attributes]), item_impl)),
                     _ => None,
                 })
                 .find(|(_attributes, item_impl)| item_impl.span() == ty_element.span)
@@ -255,7 +252,6 @@ pub mod __mod_name {
         attribute::{Attribute, AttributeArg},
         AttributeDecl, CommaToken, Parens, Punctuated,
     };
-    use sway_types::constants::CFG_ATTRIBUTE_NAME;
 
     pub(crate) fn storage_decl<'a, P>(parent: __ref_type([P])) -> Option<__ref_type([ItemStorage])>
     where
@@ -306,7 +302,7 @@ pub mod __mod_name {
         attributes
             .__iter()
             .flat_map(|attr| attr.attribute.inner.__iter())
-            .filter(|attr| attr.name.as_str() == CFG_ATTRIBUTE_NAME)
+            .filter(|attr| attr.is_cfg())
     }
 
     /// Returns all `cfg` attributes that act as only attribute within
@@ -323,7 +319,7 @@ pub mod __mod_name {
             .__iter()
             .filter(|attr| attr.attribute.inner.iter().count() == 1)
             .flat_map(|attr| attr.attribute.inner.__iter())
-            .filter(|attr| attr.name.as_str() == CFG_ATTRIBUTE_NAME)
+            .filter(|attr| attr.is_cfg())
             .filter(|attr| {
                 attr.args
                     .as_ref()

forc-plugins/forc-migrate/src/migrations/partial_eq.rs

@@ -277,7 +277,7 @@ fn remove_deprecated_eq_trait_implementations_interaction(
         for annotated_eq_impl in annotated_eq_impls {
             // Check if the `#[cfg(experimental_partial_eq = false)]` attribute exists.
             if lexed_match::cfg_attribute_standalone_single_arg(
-                &annotated_eq_impl.attribute_list,
+                &annotated_eq_impl.attributes,
                 EXPERIMENTAL_PARTIAL_EQ_ATTRIBUTE,
                 |arg| arg.as_ref().is_some_and(literal::is_bool_false),
             )
@@ -330,7 +330,7 @@ fn remove_deprecated_eq_trait_implementations_interaction(
             // Check if the `#[cfg(experimental_partial_eq = true)]` attribute exists.
             let Some(cfg_experimental_partial_eq_attr) =
                 lexed_match::cfg_attribute_standalone_single_arg(
-                    &annotated_trait_impl.attribute_list,
+                    &annotated_trait_impl.attributes,
                     EXPERIMENTAL_PARTIAL_EQ_ATTRIBUTE,
                     |arg| arg.as_ref().is_some_and(literal::is_bool_true),
                 )
@@ -386,7 +386,7 @@ fn find_trait_impl(
     let attributed_eq_trait_impls = lexed_match::impl_self_or_trait_decls_annotated(module)
         .filter_map(|annotated| match &annotated.value {
             ItemKind::Impl(item_impl) if item_impl::implements_trait(trait_name)(&item_impl) => {
-                Some((&annotated.attribute_list, item_impl))
+                Some((&annotated.attributes, item_impl))
             }
             _ => None,
         });
@@ -523,7 +523,7 @@ fn implement_experimental_partial_eq_and_eq_traits(
 
             // Set the `experimental_partial_eq` attribute to true.
             let Some(experimental_partial_eq_arg) = lexed_match_mut::cfg_attribute_arg(
-                &mut annotated_impl_partial_eq_trait.attribute_list,
+                &mut annotated_impl_partial_eq_trait.attributes,
                 with_name_mut(EXPERIMENTAL_PARTIAL_EQ_ATTRIBUTE),
             ) else {
                 bail!(internal_error(

forc-plugins/forc-migrate/src/modifying/annotated.rs

@@ -18,7 +18,7 @@ impl<T> Modifier<'_, Annotated<T>> {
         &mut self,
         attribute_span: &Span,
     ) -> &mut Self {
-        self.element.attribute_list.retain(|attribute_decl| {
+        self.element.attributes.retain(|attribute_decl| {
             attribute_decl
                 .attribute
                 .inner

forc-plugins/forc-migrate/src/modifying/attribute.rs

@@ -1,13 +1,10 @@
 use sway_ast::{
-    attribute::{Attribute, AttributeArg},
+    attribute::{Attribute, AttributeArg, CFG_ATTRIBUTE_NAME, DOC_COMMENT_ATTRIBUTE_NAME},
     brackets::SquareBrackets,
     keywords::{HashBangToken, HashToken, Token},
     AttributeDecl, Literal, Parens, Punctuated,
 };
-use sway_types::{
-    constants::{CFG_ATTRIBUTE_NAME, DOC_COMMENT_ATTRIBUTE_NAME},
-    Ident, Span, Spanned,
-};
+use sway_types::{Ident, Span, Spanned};
 
 use crate::assert_insert_span;
 

forc-plugins/forc-migrate/src/modifying/module.rs

@@ -50,7 +50,7 @@ impl Modifier<'_, Module> {
 
     pub(crate) fn append_function(&mut self, function: ItemFn) -> &mut Self {
         let function = Annotated {
-            attribute_list: vec![],
+            attributes: vec![],
             value: ItemKind::Fn(function),
         };
         self.append_annotated_item(function)