Skip to content

Commit

Permalink
Implement partial equivalence and extend forc migrate tool (#6900)
Browse files Browse the repository at this point in the history
## Description

This PR:
- implements partial equivalence in the `core::ops`, as explained in
detail in #6883. The implementation is opt-in and behind the
`partial_eq` experimental feature flag.
- implements `forc migrate` migration steps for automatic migration to
`partial_eq` feature.
- extends the infrastructure for writing `forc migrate` migrations.
- preserves `Annotation`s on `LexedModule`s in the `LexedProgram`.
(Those were before stripped off and not available in the compiled
`Programs`. This resulted in loss off `//!` doc-comments that are
represented as `doc-comment` annotations.)

Extensions in the `forc migrate` infrastructure include:
- possibility to postpone migration steps. This allows writing
multi-step migrations where the first step is usually early adopting an
experimental feature by using `cfg` attributes in code. This possibility
is crucial for migrating our own codebase where we want to early-adopt
and test and stabilize experimental features.
- possibility to match elements on both mutable and immutable parsed
trees. The initial assumption that immutable matching on typed trees
will always be sufficient does not hold. Experimental `cfg` attributes
can remove parts of typed trees that might be needed in analysis.
- `LexedLocate(As)Annotated` for easier location and retrieval of
`Annotated` elements.
- additional implementations of existing traits.
- additional `Modifier`s for modifying existing elements in the lexed
tree.
- `New` struct for convenient creation of often needed lexed tree
elements. Note that we do not expect migrations to generate large new
parts of lexed trees, but mostly to modify or copy and modify existing
ones.

Almost all of the changes in the Sway files are done automatically by
the migration steps. The only manual change was in the `core` library
(`std` library is also automatically migrated) and in slight extension
of two of the tests.

Note that some of the tests have `Eq` traits in trait constraints. As
explained in the #6883, it is not necessary to change those to
`PartialEq`. We might consider changing some of them, for testing
purposes, and such a change is done on one of the tests that was testing
the behavior of the `Eq` trait. But for the majority of the tests, there
is no strict need for switching from `Eq` to `PartialEq`.

Rolling `PartialEq` out in the tests provoked three existing issues that
will be fixed in separate PRs: #6897, #6898, #6899.

## 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).
- [ ] 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.
  • Loading branch information
ironcev authored Feb 11, 2025
1 parent 70c84ca commit bb7c99b
Show file tree
Hide file tree
Showing 171 changed files with 6,608 additions and 1,163 deletions.
10 changes: 10 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,10 @@ jobs:
run: cargo run --locked --release -p forc -- build --experimental error_type --release --locked --path ./test/src/sdk-harness
- name: Cargo Test sway-lib-std - Experimental Feature 'error_type'
run: cargo test --locked --release --manifest-path ./test/src/sdk-harness/Cargo.toml -- --nocapture
- name: Build All Tests - Experimental Feature 'partial_eq'
run: cargo run --locked --release -p forc -- build --experimental partial_eq --release --locked --path ./test/src/sdk-harness
- name: Cargo Test sway-lib-std - Experimental Feature 'partial_eq'
run: cargo test --locked --release --manifest-path ./test/src/sdk-harness/Cargo.toml -- --nocapture

forc-run-benchmarks:
runs-on: buildjet-4vcpu-ubuntu-2204
Expand Down Expand Up @@ -547,18 +551,24 @@ jobs:
run: forc build --experimental storage_domains --path sway-lib-core && forc test --experimental storage_domains --path sway-lib-core
- name: Run Core Unit Tests - Experimental feature 'error_type'
run: forc build --experimental error_type --path sway-lib-core && forc test --experimental error_type --path sway-lib-core
- name: Run Core Unit Tests - Experimental feature 'partial_eq'
run: forc build --experimental partial_eq --path sway-lib-core && forc test --experimental partial_eq --path sway-lib-core
- name: Run Std Unit Tests
run: forc build --path sway-lib-std && forc test --path sway-lib-std
- name: Run Std Unit Tests - Experimental feature 'storage_domains'
run: forc build --experimental storage_domains --path sway-lib-std && forc test --experimental storage_domains --path sway-lib-std
- name: Run Std Unit Tests - Experimental feature 'error_type'
run: forc build --experimental error_type --path sway-lib-std && forc test --experimental error_type --path sway-lib-std
- name: Run Std Unit Tests - Experimental feature 'partial_eq'
run: forc build --experimental partial_eq --path sway-lib-std && forc test --experimental partial_eq --path sway-lib-std
- name: Run In Language Unit Tests
run: forc build --path test/src/in_language_tests && forc test --path test/src/in_language_tests
- name: Run In Language Unit Tests - Experimental feature 'storage_domains'
run: forc build --experimental storage_domains --path test/src/in_language_tests && forc test --experimental storage_domains --path test/src/in_language_tests
- name: Run In Language Unit Tests - Experimental feature 'error_type'
run: forc build --experimental error_type --path test/src/in_language_tests && forc test --experimental error_type --path test/src/in_language_tests
- name: Run In Language Unit Tests - Experimental feature 'partial_eq'
run: forc build --experimental partial_eq --path test/src/in_language_tests && forc test --experimental partial_eq --path test/src/in_language_tests

forc-pkg-fuels-deps-check:
runs-on: buildjet-4vcpu-ubuntu-2204
Expand Down
12 changes: 12 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ devault = "0.2"
dialoguer = "0.11"
dirs = "5.0"
downcast-rs = "1.2"
duplicate = "2.0"
either = "1.9"
ethabi = { package = "fuel-ethabi", version = "18.0" }
etk-asm = { package = "fuel-etk-asm", version = "0.3.1-dev" }
Expand Down
1 change: 1 addition & 0 deletions forc-plugins/forc-migrate/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ repository.workspace = true
[dependencies]
anyhow.workspace = true
clap = { workspace = true, features = ["derive"] }
duplicate.workspace = true
forc-pkg.workspace = true
forc-tracing.workspace = true
forc-util.workspace = true
Expand Down
4 changes: 2 additions & 2 deletions forc-plugins/forc-migrate/src/cli/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ pub(crate) fn exec(command: Command) -> Result<()> {
for migration_step in migration_steps.iter() {
let migration_point_spans = match migration_step.kind {
MigrationStepKind::Instruction(instruction) => instruction(&program_info)?,
MigrationStepKind::CodeModification(modification, _) => {
MigrationStepKind::CodeModification(modification, ..) => {
modification(&mut program_info.as_mut(), DryRun::Yes)?
}
MigrationStepKind::Interaction(instruction, _, _) => instruction(&program_info)?,
MigrationStepKind::Interaction(instruction, ..) => instruction(&program_info)?,
};

check_result.push((feature, migration_step, migration_point_spans));
Expand Down
117 changes: 81 additions & 36 deletions forc-plugins/forc-migrate/src/cli/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use clap::Parser;
use forc_tracing::{println_action_green, println_action_yellow, println_yellow_bold};
use forc_util::{format_diagnostic, fs_locking::is_file_dirty};
use itertools::Itertools;
use sway_ast::{attribute::Annotated, Module};
use sway_ast::Module;
use sway_core::{
language::lexed::{LexedModule, LexedProgram},
Engines,
Expand All @@ -27,7 +27,10 @@ use crate::{
},
},
get_migration_steps_or_return, instructive_error,
migrations::{DryRun, MigrationStep, MigrationStepKind, MigrationSteps, ProgramInfo},
migrations::{
ContinueMigrationProcess, DryRun, InteractionResponse, MigrationStep, MigrationStepKind,
MigrationSteps, ProgramInfo,
},
};

forc_util::cli_examples! {
Expand Down Expand Up @@ -127,6 +130,7 @@ pub(crate) fn exec(command: Command) -> Result<()> {
)
.0;
let mut current_feature_migration_has_code_changes = false;
let mut num_of_postponed_steps = 0;
for (feature, migration_steps) in migration_steps.iter() {
for migration_step in migration_steps.iter() {
match migration_step.kind {
Expand All @@ -145,7 +149,11 @@ pub(crate) fn exec(command: Command) -> Result<()> {
println_yellow_bold("If you've already reviewed the above points, you can ignore this info.");
}
}
MigrationStepKind::CodeModification(modification, manual_migration_actions) => {
MigrationStepKind::CodeModification(
modification,
manual_migration_actions,
continue_migration_process,
) => {
let occurrences_spans = modification(&mut program_info.as_mut(), DryRun::No)?;

output_modified_modules(
Expand All @@ -159,7 +167,9 @@ pub(crate) fn exec(command: Command) -> Result<()> {
feature,
migration_step,
manual_migration_actions,
continue_migration_process,
&occurrences_spans,
InteractionResponse::None,
&mut current_feature_migration_has_code_changes,
);
if stop_migration_process == StopMigrationProcess::Yes {
Expand All @@ -170,6 +180,7 @@ pub(crate) fn exec(command: Command) -> Result<()> {
instruction,
interaction,
manual_migration_actions,
continue_migration_process,
) => {
let instruction_occurrences_spans = instruction(&program_info)?;

Expand All @@ -183,9 +194,13 @@ pub(crate) fn exec(command: Command) -> Result<()> {

// We have occurrences, let's continue with the interaction.
if !instruction_occurrences_spans.is_empty() {
let interaction_occurrences_spans =
let (interaction_response, interaction_occurrences_spans) =
interaction(&mut program_info.as_mut())?;

if interaction_response == InteractionResponse::PostponeStep {
num_of_postponed_steps += 1;
}

output_modified_modules(
&build_instructions.manifest_dir()?,
&program_info,
Expand All @@ -197,7 +212,9 @@ pub(crate) fn exec(command: Command) -> Result<()> {
feature,
migration_step,
manual_migration_actions,
continue_migration_process,
&interaction_occurrences_spans,
interaction_response,
&mut current_feature_migration_has_code_changes,
);
if stop_migration_process == StopMigrationProcess::Yes {
Expand All @@ -212,7 +229,7 @@ pub(crate) fn exec(command: Command) -> Result<()> {
// stop for a review before continuing with the next feature.
if current_feature_migration_has_code_changes {
if *feature == last_migration_feature {
print_migration_finished_action();
print_migration_finished_action(num_of_postponed_steps);
} else {
print_continue_migration_action("Review the changed code");
}
Expand All @@ -224,7 +241,7 @@ pub(crate) fn exec(command: Command) -> Result<()> {
// We've run through all the migration steps.
// Print the confirmation message, even if there were maybe infos
// displayed for manual reviews.
print_migration_finished_action();
print_migration_finished_action(num_of_postponed_steps);

Ok(())
}
Expand All @@ -235,16 +252,23 @@ enum StopMigrationProcess {
No,
}

#[allow(clippy::too_many_arguments)]
fn print_modification_result(
max_len: usize,
feature: &Feature,
migration_step: &MigrationStep,
manual_migration_actions: &[&str],
continue_migration_process: ContinueMigrationProcess,
occurrences_spans: &[Span],
interaction_response: InteractionResponse,
current_feature_migration_has_code_changes: &mut bool,
) -> StopMigrationProcess {
if occurrences_spans.is_empty() {
print_checked_action(max_len, feature, migration_step);
if interaction_response == InteractionResponse::PostponeStep {
print_postponed_action(max_len, feature, migration_step);
} else {
print_checked_action(max_len, feature, migration_step);
}
StopMigrationProcess::No
} else {
print_changing_code_action(max_len, feature, migration_step);
Expand All @@ -256,25 +280,35 @@ fn print_modification_result(
plural_s(occurrences_spans.len())
);

// Check if we can proceed with the next migration step or break for manual action.
if !migration_step.has_manual_actions() {
// Mark the feature as having made code changes in the migration, and proceed with the
// next migration step *within the same feature*, if any.
*current_feature_migration_has_code_changes = true;

StopMigrationProcess::No
} else {
// Display the manual migration actions and stop the further execution of the migration steps.
println!();
println!("You still need to manually:");
manual_migration_actions
.iter()
.for_each(|help| println!("- {help}"));
println!();
println!("{}", detailed_migration_guide_msg(feature));
print_continue_migration_action("Do the above manual changes");
// Check if we can proceed with the next migration step,
// or we have a mandatory stop, or a stop for completing manual actions.
match continue_migration_process {
ContinueMigrationProcess::Never => {
print_continue_migration_action("Review the changed code");

StopMigrationProcess::Yes
StopMigrationProcess::Yes
}
ContinueMigrationProcess::IfNoManualMigrationActionsNeeded => {
if !migration_step.has_manual_actions() {
// Mark the feature as having made code changes in the migration, and proceed with the
// next migration step *within the same feature*, if any.
*current_feature_migration_has_code_changes = true;

StopMigrationProcess::No
} else {
// Display the manual migration actions and stop the further execution of the migration steps.
println!();
println!("You still need to manually:");
manual_migration_actions
.iter()
.for_each(|help| println!("- {help}"));
println!();
println!("{}", detailed_migration_guide_msg(feature));
print_continue_migration_action("Do the above manual changes");

StopMigrationProcess::Yes
}
}
}
}
}
Expand Down Expand Up @@ -347,17 +381,10 @@ fn output_changed_lexed_program(
modified_modules: &ModifiedModules,
lexed_module: &LexedModule,
) -> Result<()> {
if let Some(path) = modified_modules.get_path_if_modified(&lexed_module.tree) {
if let Some(path) = modified_modules.get_path_if_modified(&lexed_module.tree.value) {
let mut formatter = Formatter::from_dir(manifest_dir)?;

let annotated_module = Annotated {
// TODO: Handle annotations instead of stripping them.
// See: https://github.com/FuelLabs/sway/issues/6802
attribute_list: vec![],
value: lexed_module.tree.clone(),
};

let code = formatter.format_module(&annotated_module)?;
let code = formatter.format_module(&lexed_module.tree.clone())?;

std::fs::write(path, code)?;
}
Expand Down Expand Up @@ -411,8 +438,26 @@ fn print_review_action(max_len: usize, feature: &Feature, migration_step: &Migra
);
}

fn print_migration_finished_action() {
println_action_green("Finished", PROJECT_IS_COMPATIBLE);
fn print_postponed_action(max_len: usize, feature: &Feature, migration_step: &MigrationStep) {
println_action_yellow(
"Postponed",
&full_migration_step_title(max_len, feature, migration_step),
);
}

fn print_migration_finished_action(num_of_postponed_steps: usize) {
if num_of_postponed_steps > 0 {
println_action_green(
"Finished",
&format!(
"Run `forc migrate` at a later point to resolve {} postponed migration step{}",
number_to_str(num_of_postponed_steps),
plural_s(num_of_postponed_steps),
),
)
} else {
println_action_green("Finished", PROJECT_IS_COMPATIBLE);
}
}

fn print_continue_migration_action(txt: &str) {
Expand Down
8 changes: 4 additions & 4 deletions forc-plugins/forc-migrate/src/cli/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,16 +248,16 @@ pub(crate) fn create_migration_diagnostic(
})
.chain(match migration_step.kind {
MigrationStepKind::Instruction(_) => vec![],
MigrationStepKind::CodeModification(_, []) => vec![],
MigrationStepKind::CodeModification(_, manual_migration_actions) => {
MigrationStepKind::CodeModification(_, [], _) => vec![],
MigrationStepKind::CodeModification(_, manual_migration_actions, _) => {
get_manual_migration_actions_help(manual_migration_actions)
}
MigrationStepKind::Interaction(_, _, []) => vec![
MigrationStepKind::Interaction(_, _, [], _) => vec![
"This migration step will interactively modify the code, based on your input."
.to_string(),
Diagnostic::help_empty_line(),
],
MigrationStepKind::Interaction(_, _, manual_migration_actions) => vec![
MigrationStepKind::Interaction(_, _, manual_migration_actions, _) => vec![
"This migration step will interactively modify the code, based on your input."
.to_string(),
Diagnostic::help_empty_line(),
Expand Down
Loading

0 comments on commit bb7c99b

Please sign in to comment.