Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format missing module errors in upgrade errors #20541

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
small fixes
  • Loading branch information
jordanjennings-mysten committed Dec 21, 2024
commit 82865223f7b73fb05fed800142b6647e0c63a65e
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@ error[Compatibility E01001]: missing public declaration

error[Compatibility E01007]: module missing
┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v2/Move.toml:1:1
1 │ [package]
│ ^^^^^^^^^ Package is missing module 'missing_module'
= Modules are part of a package and cannot be removed or changed during an upgrade.
1 │ ╭ [package]
2 │ │ name = "upgrades"
3 │ │ edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move
jordanjennings-mysten marked this conversation as resolved.
Show resolved Hide resolved
│ ╰────────────────────────────────────────────────────────────────────────^ Package is missing module 'missing_module'
= Modules which are part package cannot be removed during an upgrade.
= add missing module 'missing_module' back to the package.


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs
expression: output
---
error[Compatibility E01007]: module missing
┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/malformed_move_toml/empty/Move.toml:1:1
┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/missing_module_toml/empty/Move.toml:1:1
1 │
│ ^ Package is missing module 'identifier'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs
expression: output
---
error[Compatibility E01007]: module missing
┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/malformed_move_toml/whitespace/Move.toml:1:1
1 │
│ ^ Package is missing module 'identifier'
┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/missing_module_toml/whitespace/Move.toml:1:1
1 │ ╭
2 │ │
│ ╰──^ Package is missing module 'identifier'
= Modules which are part package cannot be removed during an upgrade.
= add missing module 'identifier' back to the package.
18 changes: 6 additions & 12 deletions crates/sui/src/unit_tests/upgrade_compatibility_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,11 @@ fn test_missing_module_toml() {
// since a Move.toml which is empty will not build
for malformed_pkg in [
"emoji",
// "whitespace",
"addresses_first",
"starts_second_line",
"package_no_name",
"whitespace",
"empty",
] {
let move_pkg_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("src/unit_tests/fixtures/upgrade_errors/missing_module_toml/")
Expand All @@ -108,7 +109,7 @@ fn test_missing_module_toml() {
missing_module_diag(&Identifier::from_str("identifier").unwrap(), &move_pkg_path);

let move_toml: Arc<str> = fs::read_to_string(move_pkg_path.join("Move.toml"))
.unwrap()
.unwrap_or_default()
.into();
let file_hash = FileHash::new(&move_toml);
let mut files = FilesSourceText::new();
Expand All @@ -129,20 +130,13 @@ fn test_missing_module_toml() {
#[test]

fn test_malformed_toml() {
// whitespace example
// no_file example
let move_pkg_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("src/unit_tests/fixtures/upgrade_errors/missing_module_toml/whitespace/");
.join("src/unit_tests/fixtures/upgrade_errors/missing_module_toml/no_file/");

let result = missing_module_diag(&Identifier::from_str("identifier").unwrap(), &move_pkg_path);
assert!(result.is_err());
assert_eq!(result.unwrap_err().to_string(), "Malformed Move.toml");

// empty example
let move_pkg_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("src/unit_tests/fixtures/upgrade_errors/missing_module_toml/empty/");
let result = missing_module_diag(&Identifier::from_str("identifier").unwrap(), &move_pkg_path);
assert!(result.is_err());
assert_eq!(result.unwrap_err().to_string(), "Malformed Move.toml");
assert_eq!(result.unwrap_err().to_string(), "Unable to read Move.toml");
}

fn get_packages(name: &str) -> (Vec<CompiledModule>, CompiledPackage, PathBuf) {
Expand Down
14 changes: 5 additions & 9 deletions crates/sui/src/upgrade_compatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,31 +823,27 @@ fn module_compatibility_error_diag(
Ok(diags)
}

const PACKAGE_TABLE: &str = "[package]";
fn missing_module_diag(
module_name: &Identifier,
package_path: &PathBuf,
) -> Result<Diagnostics, Error> {
const PACKAGE_TABLE: &str = "[package]";
jordanjennings-mysten marked this conversation as resolved.
Show resolved Hide resolved
let mut diags = Diagnostics::new();

// read Move.toml to get the hash and first line start and end
let toml_path = package_path.join("Move.toml");
let toml_str = fs::read_to_string(&toml_path).context("Unable to read Move.toml")?;
jordanjennings-mysten marked this conversation as resolved.
Show resolved Hide resolved
let hash = FileHash::new(&toml_str);

let start: usize = toml_str
.find(PACKAGE_TABLE)
.context("Malformed Move.toml")?;
let start: usize = toml_str.find(PACKAGE_TABLE).unwrap_or_default();
// default to the end of the package table definition
let mut end = start + PACKAGE_TABLE.len();

// get the third newline after the start of the package table declaration if it exists
toml_str
let end = toml_str[start..]
.match_indices('\n')
.filter(|(idx, _)| *idx > start)
.take(3)
.last()
.map(|(idx, _)| end = idx);
.map(|(idx, _)| start + idx)
.unwrap_or(start + PACKAGE_TABLE.len());

let loc = Loc::new(hash, start as ByteIndex, end as ByteIndex);

Expand Down