Skip to content

Commit

Permalink
[move-2024] Add migration support for globally qualifying names (Myst…
Browse files Browse the repository at this point in the history
…enLabs#16632)

## Description 

In legacy move, we allowed three-place names to use an address in the
first position, even if there was also a module in scope with that name.
Move 2024's resolution rules now require `::` proceeding that name to do
this. This updates migration to support.

Also, this moves writing the edition flag to the `Move.toml` file
_after_ successful migration, so that we aren't updating the it in the
error case.

## Test Plan 

New tests plus more local testing against the sui framework code.
 
---
If your changes are not user-facing and do not break anything, you can
skip the following section. Otherwise, please briefly describe what has
changed under the Release Notes section.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
cgswords authored Mar 13, 2024
1 parent 48efbe8 commit 73a7696
Show file tree
Hide file tree
Showing 23 changed files with 262 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Please select one of the following editions:
1) 2024.beta
2) legacy

Selection (default=1): Recorded edition in 'Move.toml'
Selection (default=1):

Would you like the Move compiler to migrate your code to Move 2024? (Y/n)
Generated changes . . .
Expand Down Expand Up @@ -276,6 +276,7 @@ Updating "tests/test0.move" . . .
Changes complete
Wrote patchfile out to: ./migration.patch

Recorded edition in 'Move.toml'
External Command `diff -r -s sources migration_sources`:
Files sources/mod0.move and migration_sources/mod0.move are identical
Files sources/mod1.move and migration_sources/mod1.move are identical
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Please select one of the following editions:
1) 2024.beta
2) legacy

Selection (default=1): Recorded edition in 'Move.toml'
Selection (default=1):

Would you like the Move compiler to migrate your code to Move 2024? (Y/n)
Generated changes . . .
Expand Down Expand Up @@ -108,6 +108,7 @@ Updating "sources/mod_let_errors.move" . . .
Changes complete
Wrote patchfile out to: ./migration.patch

Recorded edition in 'Move.toml'
External Command `diff -r -s sources migration_sources`:
Files sources/mod0.move and migration_sources/mod0.move are identical
Files sources/mod_intermodule_errors.move and migration_sources/mod_intermodule_errors.move are identical
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Please select one of the following editions:
1) 2024.beta
2) legacy

Selection (default=1): Recorded edition in 'Move.toml'
Selection (default=1):

Would you like the Move compiler to migrate your code to Move 2024? (Y/n)
Generated changes . . .
Expand Down Expand Up @@ -36,6 +36,7 @@ Updating "sources/mut_name.move" . . .
Changes complete
Wrote patchfile out to: ./migration.patch

Recorded edition in 'Move.toml'
External Command `diff -r -s sources migration_sources`:
Files sources/mut_name.move and migration_sources/mut_name.move are identical
External Command `diff -s Move.toml Move.toml.expected`:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

# TOML FILE

[package]
name = "migration"

# this is a comment
[addresses]
migration = "0x3"
std = "0x1"

[dependencies]
MoveStdlib = { local = "../../../../move-stdlib" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@

# TOML FILE

[package]
name = "migration"
edition = "2024.beta"

# this is a comment
[addresses]
migration = "0x3"
std = "0x1"

[dependencies]
MoveStdlib = { local = "../../../../move-stdlib" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
Command `migrate`:
Package toml does not specify an edition. As of 2024, Move requires all packages to define a language edition.

Please select one of the following editions:

1) 2024.beta
2) legacy

Selection (default=1):

Would you like the Move compiler to migrate your code to Move 2024? (Y/n)
Generated changes . . .
INCLUDING DEPENDENCY MoveStdlib
BUILDING migration

The following changes will be made.
============================================================

--- sources/other.move
+++ sources/other.move
@@ -4,1 +4,1 @@
- public fun t() { migration::migration::t() }
+ public fun t() { ::migration::migration::t() }
--- tests/test0.move
+++ tests/test0.move
@@ -6,1 +6,1 @@
- #[expected_failure(abort_code = migration::validate::ErrorCode)]
+ #[expected_failure(abort_code = ::migration::validate::ErrorCode)]


============================================================
Apply changes? (Y/n)
Updating "sources/other.move" . . .
Updating "tests/test0.move" . . .

Changes complete
Wrote patchfile out to: ./migration.patch

Recorded edition in 'Move.toml'
External Command `diff -r -s sources migration_sources`:
Files sources/migration.move and migration_sources/migration.move are identical
Files sources/other.move and migration_sources/other.move are identical
Files sources/validate.move and migration_sources/validate.move are identical
External Command `diff -r -s tests migration_tests`:
Files tests/test0.move and migration_tests/test0.move are identical
External Command `diff -s Move.toml Move.toml.expected`:
Files Move.toml and Move.toml.expected are identical
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
migrate
> diff -r -s sources migration_sources
> diff -r -s tests migration_tests
> diff -s Move.toml Move.toml.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module migration::migration {

public fun t() { abort migration::validate::make_error_code() }

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module migration::other {
use migration::migration;

public fun t() { ::migration::migration::t() }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module migration::validate {

const ErrorCode: u64= 0;

public fun make_error_code(): u64 { ErrorCode }

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#[test_only]
module migration::migration_tests {
use migration::migration;

#[test]
#[expected_failure(abort_code = ::migration::validate::ErrorCode)]
fun test_t() {
migration::t()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module migration::migration {

public fun t() { abort migration::validate::make_error_code() }

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module migration::other {
use migration::migration;

public fun t() { migration::migration::t() }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module migration::validate {

const ErrorCode: u64= 0;

public fun make_error_code(): u64 { ErrorCode }

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#[test_only]
module migration::migration_tests {
use migration::migration;

#[test]
#[expected_failure(abort_code = migration::validate::ErrorCode)]
fun test_t() {
migration::t()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

# TOML FILE

[package]
name = "A"

# this is a comment
[addresses]
A = "0x1"
std = "0x1"

[dependencies]
MoveStdlib = { local = "../../../../move-stdlib" }
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,20 @@ Please select one of the following editions:
1) 2024.beta
2) legacy

Selection (default=1): Recorded edition in 'Move.toml'
Selection (default=1):

Would you like the Move compiler to migrate your code to Move 2024? (Y/n)
Generated changes . . .
INCLUDING DEPENDENCY MoveStdlib
BUILDING A
Unable to generate migration patch due to compilation errors.
Please fix the errors in your current edition before attempting to migrate.
error[E03006]: unexpected name in this position
┌─ sources/mod1.move:5:9
5 │ mod0::mod0::f();
│ ^^^^ Expected an address in this position, not a module

error[E04007]: incompatible types
┌─ sources/mod.move:12:13
Expand All @@ -26,3 +32,5 @@ error[E04007]: incompatible types
│ Given: '(A=0x1)::mod0::S'

Error: Compilation error
External Command `diff -s Move.toml Move.toml.expected`:
Files Move.toml and Move.toml.expected are identical
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
migrate
> diff -s Move.toml Move.toml.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module A::mod1 {
use A::mod0;

public fun t0(x: u64): u64 {
mod0::mod0::f();
x
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ codes!(
NeedsPublic: { msg: "move 2024 migration: public struct", severity: NonblockingError },
NeedsLetMut: { msg: "move 2024 migration: let mut", severity: NonblockingError },
NeedsRestrictedIdentifier: { msg: "move 2024 migration: restricted identifier", severity: NonblockingError },
NeedsGlobalQualification: { msg: "move 2024 migration: global qualification", severity: NonblockingError },
]
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ enum MigrationChange {
AddMut,
AddPublic,
Backquote(String),
AddGlobalQual,
}

// All of the migration changes
Expand Down Expand Up @@ -804,6 +805,7 @@ impl Migration {
const NEEDS_MUT: u8 = codes::Migration::NeedsLetMut as u8;
const NEEDS_PUBLIC: u8 = codes::Migration::NeedsPublic as u8;
const NEEDS_BACKTICKS: u8 = codes::Migration::NeedsRestrictedIdentifier as u8;
const NEEDS_GLOBAL_QUAL: u8 = codes::Migration::NeedsGlobalQualification as u8;

let (file_id, line, col) = self.find_file_location(&diag);
let file_change_entry = self.changes.entry(file_id).or_default();
Expand All @@ -815,6 +817,9 @@ impl Migration {
let old_name = diag.primary_msg().to_string();
line_change_entry.push((col, MigrationChange::Backquote(old_name)))
}
(CAT, NEEDS_GLOBAL_QUAL) => {
line_change_entry.push((col, MigrationChange::AddGlobalQual))
}
_ => unreachable!(),
}
}
Expand Down Expand Up @@ -854,6 +859,10 @@ impl Migration {
output = format!("`{}`{}{}", old_name, &rest[old_name.len()..], output);
line_prefix = &line_prefix[..*col];
}
MigrationChange::AddGlobalQual => {
output = format!("::{}{}", rest, output);
line_prefix = &line_prefix[..*col];
}
}
}
}
Expand Down
Loading

0 comments on commit 73a7696

Please sign in to comment.