Additionally test associated constants (#6349)

## Description

This PR strengthens existing and adds new tests for associated
constants. This extended test coverage revealed the following existing
issues: #6310, #6348, #6345, #6346, #6343, #6344.

It also pointed out that we are missing rules for a contract
implementing several ABIs with overlapping interface surfaces, where
constants can also be overlapping: #6306.

All the issues are linked to tests via TODOs in code that is commented
out. We will address the issues in separate PRs. With this PR, we want
to first have tests in place.

Additionally, the PR:
- cleans up dead code that become obsolete when `TyConfigurableDecl` was
introduced in #6058.
- deletes some redundant tests for constants.
- groups some tests to reduce test compilation and execution time and
provides guidelines for such groupings in the testing README.md.

## 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.
- [ ] 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.

sway-core/src/language/parsed/declaration/constant.rs

@@ -14,7 +14,6 @@ pub struct ConstantDeclaration {
     pub type_ascription: TypeArgument,
     pub value: Option<Expression>,
     pub visibility: Visibility,
-    pub is_configurable: bool,
     pub span: Span,
 }
 
@@ -26,7 +25,6 @@ impl PartialEqWithEngines for ConstantDeclaration {
             && self.type_ascription.eq(&other.type_ascription, ctx)
             && self.value.eq(&other.value, ctx)
             && self.visibility == other.visibility
-            && self.is_configurable == other.is_configurable
             && self.span == other.span
     }
 }

sway-core/src/semantic_analysis/ast_node/declaration/constant.rs

@@ -28,7 +28,6 @@ impl ty::TyConstantDecl {
             span,
             mut type_ascription,
             value,
-            is_configurable,
             attributes,
             visibility,
         } = decl;
@@ -53,48 +52,28 @@ impl ty::TyConstantDecl {
             })
         }
 
-        // Configurables will be encoded and must be type_checked into "slice"
-        let (value, return_type) = if is_configurable && ctx.experimental.new_encoding {
-            let mut ctx = ctx
-                .by_ref()
-                .with_type_annotation(type_engine.insert(engines, TypeInfo::RawUntypedSlice, None))
-                .with_help_text("Configurables must evaluate to slices.");
-
-            let value = value.map(|value| {
-                ty::TyExpression::type_check(handler, ctx.by_ref(), &value)
-                    .unwrap_or_else(|err| ty::TyExpression::error(err, name.span(), engines))
-            });
-
-            (
-                value,
-                type_engine.insert(engines, TypeInfo::RawUntypedSlice, None),
-            )
-        } else {
-            let mut ctx = ctx
-                .by_ref()
-                .with_type_annotation(type_ascription.type_id)
-                .with_help_text(
-                    "This declaration's type annotation does not match up with the assigned \
-            expression's type.",
-                );
-
-            let value = value.map(|value| {
-                ty::TyExpression::type_check(handler, ctx.by_ref(), &value)
-                    .unwrap_or_else(|err| ty::TyExpression::error(err, name.span(), engines))
-            });
-            // Integers are special in the sense that we can't only rely on the type of `expression`
-            // to get the type of the variable. The type of the variable *has* to follow
-            // `type_ascription` if `type_ascription` is a concrete integer type that does not
-            // conflict with the type of `expression` (i.e. passes the type checking above).
-            let return_type = match &*type_engine.get(type_ascription.type_id) {
-                TypeInfo::UnsignedInteger(_) => type_ascription.type_id,
-                _ => match &value {
-                    Some(value) => value.return_type,
-                    None => type_ascription.type_id,
-                },
-            };
-
-            (value, return_type)
+        let mut ctx = ctx
+            .by_ref()
+            .with_type_annotation(type_ascription.type_id)
+            .with_help_text(
+                "This declaration's type annotation does not match up with the assigned \
+        expression's type.",
+            );
+
+        let value = value.map(|value| {
+            ty::TyExpression::type_check(handler, ctx.by_ref(), &value)
+                .unwrap_or_else(|err| ty::TyExpression::error(err, name.span(), engines))
+        });
+        // Integers are special in the sense that we can't only rely on the type of `expression`
+        // to get the type of the variable. The type of the variable *has* to follow
+        // `type_ascription` if `type_ascription` is a concrete integer type that does not
+        // conflict with the type of `expression` (i.e. passes the type checking above).
+        let return_type = match &*type_engine.get(type_ascription.type_id) {
+            TypeInfo::UnsignedInteger(_) => type_ascription.type_id,
+            _ => match &value {
+                Some(value) => value.return_type,
+                None => type_ascription.type_id,
+            },
         };
 
         let mut call_path: CallPath = name.into();

sway-core/src/semantic_analysis/ast_node/expression/intrinsic_function.rs

@@ -323,7 +323,6 @@ fn type_check_not(
                 received: engines.help_out(return_type).to_string(),
                 help_text: "".into(),
                 span,
-                internal: "8".into(),
             },
         ))),
     }
@@ -1241,7 +1240,6 @@ fn type_check_bitwise_binary_op(
                 received: engines.help_out(return_type).to_string(),
                 help_text: "".into(),
                 span,
-                internal: "7".into(),
             },
         ))),
     }
@@ -1316,7 +1314,6 @@ fn type_check_shift_binary_op(
                 received: engines.help_out(return_type).to_string(),
                 help_text: "Incorrect argument type".into(),
                 span: lhs.span,
-                internal: "6".into(),
             },
         ))),
     }

sway-core/src/semantic_analysis/ast_node/expression/typed_expression/if_expression.rs

@@ -80,7 +80,6 @@ pub(crate) fn instantiate_if_expression(
             help_text: "The two branches of an if expression must return the same type."
                 .to_string(),
             span: span.clone(),
-            internal: "".into(),
         }));
     }
 

sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs

@@ -979,7 +979,6 @@ pub(crate) fn item_const_to_constant_declaration(
         type_ascription,
         value: expr,
         visibility: pub_token_opt_to_visibility(item_const.visibility),
-        is_configurable: false,
         attributes,
         span,
     };

sway-core/src/type_system/engine.rs

@@ -224,11 +224,6 @@ impl TypeEngine {
                         received: engines.help_out(received).to_string(),
                         help_text: help_text.to_string(),
                         span: span.clone(),
-                        internal: format!(
-                            "expected:[{:?}]; received:[{:?}]",
-                            engines.help_out(expected),
-                            engines.help_out(received),
-                        ),
                     }));
                 }
             }

sway-core/src/type_system/unify/unifier.rs

@@ -324,7 +324,6 @@ impl<'a> Unifier<'a> {
                         received,
                         help_text: self.help_text.clone(),
                         span: span.clone(),
-                        internal: "4".into(),
                     }
                     .into(),
                 );
@@ -353,7 +352,6 @@ impl<'a> Unifier<'a> {
                     received,
                     help_text: self.help_text.clone(),
                     span: span.clone(),
-                    internal: "3".into(),
                 }
                 .into(),
             );
@@ -397,7 +395,6 @@ impl<'a> Unifier<'a> {
                     received,
                     help_text: self.help_text.clone(),
                     span: span.clone(),
-                    internal: "2".into(),
                 }
                 .into(),
             );
@@ -428,15 +425,13 @@ impl<'a> Unifier<'a> {
                 self.unify(handler, rtp.type_id, etp.type_id, span);
             });
         } else {
-            let internal = format!("[{received:?}] versus [{expected:?}]");
             let (received, expected) = self.assign_args(received, expected);
             handler.emit_err(
                 TypeError::MismatchedType {
                     expected,
                     received,
                     help_text: self.help_text.clone(),
                     span: span.clone(),
-                    internal,
                 }
                 .into(),
             );
@@ -475,7 +470,6 @@ impl<'a> Unifier<'a> {
                     received,
                     help_text: self.help_text.clone(),
                     span: span.clone(),
-                    internal: "1".into(),
                 }
                 .into(),
             );

sway-error/src/type_error.rs

@@ -15,7 +15,6 @@ pub enum TypeError {
         received: String,
         help_text: String,
         span: Span,
-        internal: String,
     },
     #[error("This type is not known. Try annotating it with a type annotation.")]
     UnknownType { span: Span },

test/src/e2e_vm_tests/README.md

@@ -4,10 +4,13 @@ In order to minimize compilation time of individual tests, strive to reduce depe
 
 To achieve that, follow these guidelines:
 - Use `implicit-std = false` if dependency on `core` is not needed. This is often possible when testing `should_pass/language` features.
+- If the dependency on `core` is not needed, instead of using the project type `script`, that will, because of the encoding, depend on `core`, try using `library` instead.
 - Do not use `std` just to conveniently get an arbitrary type or trait. E.g., if a test requires an arbitrary type or trait, go with `struct Dummy {}` or `trait Trait {}` instead of importing `Option` or `Hash`.
 - If `std` functionality is needed, import the minimal [reduced `std` library](reduced_std_libs/README.md) that provides the functionality.
 - Import the full `std` only if the provided [reduced `std` libraries](reduced_std_libs/README.md) do not provide required types.
 
+Additionally, try to meaningfully group tests with high cohesion, rather then splitting them into several tests. Having only one test project reduces compilation time. E.g., instead of having two tests `test_some_feature_for_option_a` and `test_some_feature_for_option_b`, try having just `test_some_feature` and the two cases tested in, e.g., modules in that one test. Makes sure, though, that such test groupings are meaningful. We don't want to end up having artificially combined tests.
+
 # Running end-to-end VM tests
 
 This document assumes you have `fuel-core` running on the default port.

test/src/e2e_vm_tests/test_programs/should_fail/abi_associated_const_access_with_impl_in_contract/Forc.lock

@@ -0,0 +1,3 @@
+[[package]]
+name = "abi_associated_const_access_with_impl_in_contract"
+source = "member"

test/src/e2e_vm_tests/test_programs/should_pass/language/associated_const_abi_default/Forc.toml → test/src/e2e_vm_tests/test_programs/should_fail/abi_associated_const_access_with_impl_in_contract/Forc.toml

@@ -2,8 +2,5 @@
 authors = ["Fuel Labs <[email protected]>"]
 entry = "main.sw"
 license = "Apache-2.0"
-name = "associated_const_abi_default"
+name = "abi_associated_const_access_with_impl_in_contract"
 implicit-std = false
-
-[dependencies]
-core = { path = "../../../../../../../sway-lib-core" }

test/src/e2e_vm_tests/test_programs/should_fail/abi_associated_const_access_with_impl_in_contract/src/main.sw

@@ -0,0 +1,17 @@
+contract;
+
+abi ConstantId {
+    const ID: u32 = 0;
+}
+
+impl ConstantId for Contract {
+    const ID: u32 = 1;
+}
+
+fn main() -> u32 {
+  let _ = ConstantId::ID;
+
+  // Leave enough space to avoid having both `let` lines in both error messages.
+
+  let _ = Contract::ID;
+}

test/src/e2e_vm_tests/test_programs/should_fail/abi_associated_const_access_with_impl_in_contract/test.toml

@@ -0,0 +1,7 @@
+category = "fail"
+
+#check: $()let _ = ConstantId::ID;
+#nextln: $()Could not find symbol "ID" in this scope.
+
+#check: $()let _ = Contract::ID;
+#nextln: $()Could not find symbol "ID" in this scope.

test/src/e2e_vm_tests/test_programs/should_fail/abi_associated_const_missing_impl/src/main.sw

@@ -1,14 +1,7 @@
-script;
+contract;
 
 abi ConstantId {
     const ID: u32;
 }
 
-struct Struct {}
-
-impl ConstantId for Struct {
-}
-
-fn main() -> u32 {
-  0
-}
+impl ConstantId for Contract { }

test/src/e2e_vm_tests/test_programs/should_fail/abi_associated_const_missing_type_ascription/src/main.sw

@@ -1,9 +1,5 @@
-script;
+library;
 
 abi ConstantId {
     const ID;
 }
-
-fn main() -> u32 {
-  0
-}

test/src/e2e_vm_tests/test_programs/should_fail/abi_associated_const_multiple_same_name/src/main.sw

@@ -1,10 +1,6 @@
-script;
+library;
 
 abi ConstantId {
     const ID: u32;
     const ID: u32;
 }
-
-fn main() -> u32 {
-  0
-}

test/src/e2e_vm_tests/test_programs/should_fail/array_oob_global_const_index/Forc.lock

@@ -1,13 +1,8 @@
 [[package]]
-name = 'array_oob_global_const_index'
-source = 'member'
-dependencies = ['std']
+name = "array_oob_global_const_index"
+source = "member"
+dependencies = ["core"]
 
 [[package]]
-name = 'core'
-source = 'path+from-root-FA9005293D2605B5'
-
-[[package]]
-name = 'std'
-source = 'path+from-root-FA9005293D2605B5'
-dependencies = ['core']
+name = "core"
+source = "path+from-root-FA9005293D2605B5"

test/src/e2e_vm_tests/test_programs/should_fail/array_oob_global_const_index/Forc.toml

@@ -3,6 +3,7 @@ authors = ["Fuel Labs <[email protected]>"]
 license = "Apache-2.0"
 name = "array_oob_global_const_index"
 entry = "main.sw"
+implicit-std = false
 
 [dependencies]
-std = { path = "../../../reduced_std_libs/sway-lib-std-assert" }
+core = { path = "../../../../../../sway-lib-core" }

test/src/e2e_vm_tests/test_programs/should_fail/array_oob_global_const_index/src/main.sw

@@ -1,9 +1,9 @@
 script;
 
-const i: u64 = 4;
+const I: u64 = 4;
 
 fn main() -> u64 {
     // index out of bounds: the length is 3 but the index is 4
     let ary = [1, 2, 3];
-    ary[i]
+    ary[I]
 }

test/src/e2e_vm_tests/test_programs/should_fail/const_eval_bad_struct_with_return/Forc.lock

@@ -1,8 +1,3 @@
 [[package]]
 name = "const_eval_bad_struct_with_return"
 source = "member"
-dependencies = ["core"]
-
-[[package]]
-name = "core"
-source = "path+from-root-154BF654D5BA1912"