From bbf4187570ddfbd3b7179a5191b38e458fdd3027 Mon Sep 17 00:00:00 2001 From: Adam Welc Date: Tue, 27 Jun 2023 09:26:19 -0700 Subject: [PATCH] [move-compiler] Added preliminary support for writing linters (#12650) ## Description Added support for writing linters: - integration with Sui CLI - a simple testing harness - an example linter to signal sharing of potentially owned objects ## Test Plan Additional testing framework being part of this PR can be used to test individual linters. --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [ ] protocol change - [x] 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 Developers may now opt into running linters when building/testing/publishing/upgrading packages via CLI by specifying the `--lint` flag. --- Cargo.lock | 3 + crates/sui-framework/build.rs | 3 + crates/sui-move-build/Cargo.toml | 9 + crates/sui-move-build/src/lib.rs | 25 +- crates/sui-move-build/src/linters/mod.rs | 4 + .../sui-move-build/src/linters/share_owned.rs | 267 ++++++++++++++++++ .../tests/linter/share_owned.exp | 23 ++ .../tests/linter/share_owned.move | 38 +++ crates/sui-move-build/tests/linter_tests.rs | 85 ++++++ crates/sui-move/src/build.rs | 6 + crates/sui-move/src/unit_test.rs | 4 + .../sui-source-validation-service/src/lib.rs | 1 + crates/sui/src/client_commands.rs | 18 ++ crates/sui/tests/cli_tests.rs | 9 + .../src/command_line/compiler.rs | 30 ++ .../tests/move_check_testsuite.rs | 34 +-- 16 files changed, 524 insertions(+), 35 deletions(-) create mode 100644 crates/sui-move-build/src/linters/mod.rs create mode 100644 crates/sui-move-build/src/linters/share_owned.rs create mode 100644 crates/sui-move-build/tests/linter/share_owned.exp create mode 100644 crates/sui-move-build/tests/linter/share_owned.move create mode 100644 crates/sui-move-build/tests/linter_tests.rs diff --git a/Cargo.lock b/Cargo.lock index 9c66ad0fc0bb7..975872d0e00e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10082,12 +10082,15 @@ name = "sui-move-build" version = "0.0.0" dependencies = [ "anyhow", + "datatest-stable", "fastcrypto", "move-binary-format", "move-bytecode-utils", "move-bytecode-verifier", + "move-command-line-common", "move-compiler", "move-core-types", + "move-ir-types", "move-package", "move-symbol-pool", "serde-reflection", diff --git a/crates/sui-framework/build.rs b/crates/sui-framework/build.rs index cca3db23031dc..af5a43a490d34 100644 --- a/crates/sui-framework/build.rs +++ b/crates/sui-framework/build.rs @@ -134,6 +134,7 @@ fn build_packages_with_move_config( config: config.clone(), run_bytecode_verifier: true, print_diags_to_stderr: false, + lint: false, } .build(sui_framework_path) .unwrap(); @@ -141,6 +142,7 @@ fn build_packages_with_move_config( config: config.clone(), run_bytecode_verifier: true, print_diags_to_stderr: false, + lint: false, } .build(sui_system_path) .unwrap(); @@ -148,6 +150,7 @@ fn build_packages_with_move_config( config, run_bytecode_verifier: true, print_diags_to_stderr: false, + lint: false, } .build(deepbook_path) .unwrap(); diff --git a/crates/sui-move-build/Cargo.toml b/crates/sui-move-build/Cargo.toml index d2209bd3a3b56..3fa54e8abbd99 100644 --- a/crates/sui-move-build/Cargo.toml +++ b/crates/sui-move-build/Cargo.toml @@ -22,8 +22,17 @@ sui-protocol-config.workspace = true move-binary-format.workspace = true move-bytecode-utils.workspace = true +move-command-line-common.workspace = true move-compiler.workspace = true move-core-types.workspace = true +move-ir-types.workspace = true move-package.workspace = true move-symbol-pool.workspace = true workspace-hack = { version = "0.1", path = "../workspace-hack" } + +[dev-dependencies] +datatest-stable = "0.1.2" + +[[test]] +name = "linter_tests" +harness = false diff --git a/crates/sui-move-build/src/lib.rs b/crates/sui-move-build/src/lib.rs index 181da60cc36ec..3500cd97ea0c8 100644 --- a/crates/sui-move-build/src/lib.rs +++ b/crates/sui-move-build/src/lib.rs @@ -1,6 +1,9 @@ // Copyright (c) Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 +#[macro_use(sp)] +extern crate move_ir_types; + use std::{ collections::{BTreeMap, BTreeSet, HashSet}, io::Write, @@ -50,10 +53,14 @@ use sui_types::{ }; use sui_verifier::verifier as sui_bytecode_verifier; +use crate::linters::share_owned::ShareOwnedVerifier; + #[cfg(test)] #[path = "unit_tests/build_tests.rs"] mod build_tests; +pub mod linters; + /// Wrapper around the core Move `CompiledPackage` with some Sui-specific traits and info #[derive(Debug)] pub struct CompiledPackage { @@ -74,6 +81,8 @@ pub struct BuildConfig { pub run_bytecode_verifier: bool, /// If true, print build diagnostics to stderr--no printing if false pub print_diags_to_stderr: bool, + /// If true, run linters + pub lint: bool, } impl BuildConfig { @@ -116,12 +125,18 @@ impl BuildConfig { fn compile_package( resolution_graph: ResolvedGraph, + lint: bool, writer: &mut W, ) -> anyhow::Result<(MoveCompiledPackage, FnInfoMap)> { let build_plan = BuildPlan::create(resolution_graph)?; let mut fn_info = None; let compiled_pkg = build_plan.compile_with_driver(writer, |compiler| { - let (files, units_res) = compiler.build()?; + let (files, units_res) = if lint { + let lint_visitors = vec![ShareOwnedVerifier.into()]; + compiler.add_visitors(lint_visitors).build()? + } else { + compiler.build()? + }; match units_res { Ok((units, warning_diags)) => { report_warnings(&files, warning_diags); @@ -144,6 +159,7 @@ impl BuildConfig { /// Given a `path` and a `build_config`, build the package in that path, including its dependencies. /// If we are building the Sui framework, we skip the check that the addresses should be 0 pub fn build(self, path: PathBuf) -> SuiResult { + let lint = self.lint; let print_diags_to_stderr = self.print_diags_to_stderr; let run_bytecode_verifier = self.run_bytecode_verifier; let resolution_graph = self.resolution_graph(&path)?; @@ -152,6 +168,7 @@ impl BuildConfig { resolution_graph, run_bytecode_verifier, print_diags_to_stderr, + lint, ) } @@ -174,13 +191,14 @@ pub fn build_from_resolution_graph( resolution_graph: ResolvedGraph, run_bytecode_verifier: bool, print_diags_to_stderr: bool, + lint: bool, ) -> SuiResult { let (published_at, dependency_ids) = gather_published_ids(&resolution_graph); let result = if print_diags_to_stderr { - BuildConfig::compile_package(resolution_graph, &mut std::io::stderr()) + BuildConfig::compile_package(resolution_graph, lint, &mut std::io::stderr()) } else { - BuildConfig::compile_package(resolution_graph, &mut std::io::sink()) + BuildConfig::compile_package(resolution_graph, lint, &mut std::io::sink()) }; // write build failure diagnostics to stderr, convert `error` to `String` using `Debug` // format to include anyhow's error context chain. @@ -541,6 +559,7 @@ impl Default for BuildConfig { config: MoveBuildConfig::default(), run_bytecode_verifier: true, print_diags_to_stderr: false, + lint: false, } } } diff --git a/crates/sui-move-build/src/linters/mod.rs b/crates/sui-move-build/src/linters/mod.rs new file mode 100644 index 0000000000000..1385f10e0c8ff --- /dev/null +++ b/crates/sui-move-build/src/linters/mod.rs @@ -0,0 +1,4 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +pub mod share_owned; diff --git a/crates/sui-move-build/src/linters/share_owned.rs b/crates/sui-move-build/src/linters/share_owned.rs new file mode 100644 index 0000000000000..230c88e91cf35 --- /dev/null +++ b/crates/sui-move-build/src/linters/share_owned.rs @@ -0,0 +1,267 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//! This analysis flags making objects passed as function parameters or resulting from unpacking +//! (likely already owned) shareable which would lead to an abort. A typical patterns is to create a +//! fresh object and share it within the same function + +use move_ir_types::location::*; + +use move_compiler::{ + cfgir::{ + absint::JoinResult, + ast::Program, + visitor::{ + LocalState, SimpleAbsInt, SimpleAbsIntConstructor, SimpleDomain, SimpleExecutionContext, + }, + CFGContext, + }, + diag, + diagnostics::{ + codes::{custom, DiagnosticInfo, Severity}, + Diagnostic, Diagnostics, + }, + hlir::ast::{ + BaseType_, Command, Exp, LValue, LValue_, Label, ModuleCall, SingleType, SingleType_, Type, + Type_, Var, + }, + parser::ast::Ability_, + shared::Identifier, +}; +use std::collections::BTreeMap; + +const SHARE_FUNCTIONS: &[(&str, &str, &str)] = &[ + ("sui", "transfer", "public_share_object"), + ("sui", "transfer", "share_object"), +]; + +const SHARE_OWNED_DIAG: DiagnosticInfo = custom( + "Lint ", + Severity::Warning, + /* category */ 1, + /* code */ 1, + "possible owned object share", +); + +//************************************************************************************************** +// types +//************************************************************************************************** + +pub struct ShareOwnedVerifier; +pub struct ShareOwnedVerifierAI; + +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Default)] +pub enum Value { + /// a fresh object resulting from packing + FreshObj, + /// a most likely non-fresh object coming from unpacking or a function argument + NotFreshObj(Loc), + #[default] + Other, +} + +pub struct ExecutionContext { + diags: Diagnostics, +} + +#[derive(Clone, Debug)] +pub struct State { + locals: BTreeMap>, +} + +//************************************************************************************************** +// impls +//************************************************************************************************** + +impl SimpleAbsIntConstructor for ShareOwnedVerifier { + type AI<'a> = ShareOwnedVerifierAI; + + fn new<'a>( + _program: &'a Program, + context: &'a CFGContext<'a>, + _init_state: &mut as SimpleAbsInt>::State, + ) -> Option> { + let Some(_) = &context.module else { + return None + }; + Some(ShareOwnedVerifierAI) + } +} + +impl SimpleAbsInt for ShareOwnedVerifierAI { + type State = State; + type ExecutionContext = ExecutionContext; + + fn finish(&mut self, _final_states: BTreeMap, diags: Diagnostics) -> Diagnostics { + diags + } + + fn start_command(&self, _: &mut State) -> ExecutionContext { + ExecutionContext { + diags: Diagnostics::new(), + } + } + + fn finish_command(&self, context: ExecutionContext, _state: &mut State) -> Diagnostics { + let ExecutionContext { diags } = context; + diags + } + + fn exp_custom( + &self, + context: &mut ExecutionContext, + state: &mut State, + e: &Exp, + ) -> Option> { + use move_compiler::hlir::ast::UnannotatedExp_ as E; + + if let E::Pack(_, _, fields) = &e.exp.value { + for (_, _, inner) in fields.iter() { + self.exp(context, state, inner); + } + return Some(vec![Value::FreshObj]); + }; + + None + } + + fn call_custom( + &self, + context: &mut ExecutionContext, + _state: &mut State, + loc: &Loc, + return_ty: &Type, + f: &ModuleCall, + args: Vec, + ) -> Option> { + if SHARE_FUNCTIONS + .iter() + .any(|(addr, module, fun)| f.is(addr, module, fun)) + && args[0] != Value::FreshObj + { + let msg = "Potential abort from a (potentially) owned object created by a different transaction."; + let uid_msg = "Creating a fresh object and sharing it within the same function will ensure this does not abort."; + let mut d = diag!( + SHARE_OWNED_DIAG, + (*loc, msg), + (f.arguments.exp.loc, uid_msg) + ); + if let Value::NotFreshObj(l) = args[0] { + d.add_secondary_label((l, "A potentially owned object coming from here")) + } + context.add_diag(d) + } + Some(match &return_ty.value { + Type_::Unit => vec![], + Type_::Single(t) => { + let v = if is_obj_type(t) { + Value::NotFreshObj(t.loc) + } else { + Value::Other + }; + vec![v] + } + Type_::Multiple(types) => types + .iter() + .map(|t| { + if is_obj_type(t) { + Value::NotFreshObj(t.loc) + } else { + Value::Other + } + }) + .collect(), + }) + } + + fn command_custom(&self, _: &mut ExecutionContext, _: &mut State, _: &Command) -> bool { + false + } + + fn lvalue_custom( + &self, + context: &mut ExecutionContext, + state: &mut State, + l: &LValue, + _value: &Value, + ) -> bool { + use LValue_ as L; + + let sp!(_, l_) = l; + if let L::Unpack(_, _, fields) = l_ { + for (f, l) in fields { + let v = if is_obj(l) { + Value::NotFreshObj(f.loc()) + } else { + Value::default() + }; + self.lvalue(context, state, l, v) + } + return true; + } + false + } +} + +fn is_obj(l: &LValue) -> bool { + let sp!(_, l_) = l; + if let LValue_::Var(_, st) = l_ { + return is_obj_type(st); + } + false +} + +fn is_obj_type(st: &SingleType) -> bool { + let sp!(_, st_) = st; + let bt = match st_ { + SingleType_::Base(v) => v, + SingleType_::Ref(_, v) => v, + }; + let sp!(_, bt_) = bt; + if let BaseType_::Apply(abilities, _, _) = bt_ { + if abilities.has_ability_(Ability_::Key) { + return true; + } + } + false +} + +impl SimpleDomain for State { + type Value = Value; + + fn new(context: &CFGContext, mut locals: BTreeMap>) -> Self { + for (v, st) in &context.signature.parameters { + if is_obj_type(st) { + let local_state = locals.get_mut(v).unwrap(); + if let LocalState::Available(loc, _) = local_state { + *local_state = LocalState::Available(*loc, Value::NotFreshObj(*loc)); + } + } + } + State { locals } + } + + fn locals_mut(&mut self) -> &mut BTreeMap> { + &mut self.locals + } + + fn locals(&self) -> &BTreeMap> { + &self.locals + } + + fn join_value(v1: &Value, v2: &Value) -> Value { + match (v1, v2) { + (Value::FreshObj, Value::FreshObj) => Value::FreshObj, + (stale @ Value::NotFreshObj(_), _) | (_, stale @ Value::NotFreshObj(_)) => *stale, + (Value::Other, _) | (_, Value::Other) => Value::Other, + } + } + + fn join_impl(&mut self, _: &Self, _: &mut JoinResult) {} +} + +impl SimpleExecutionContext for ExecutionContext { + fn add_diag(&mut self, diag: Diagnostic) { + self.diags.add(diag) + } +} diff --git a/crates/sui-move-build/tests/linter/share_owned.exp b/crates/sui-move-build/tests/linter/share_owned.exp new file mode 100644 index 0000000000000..47c0dd5dbbb48 --- /dev/null +++ b/crates/sui-move-build/tests/linter/share_owned.exp @@ -0,0 +1,23 @@ +warning[Lint W01001]: possible owned object share + ┌─ tests/linter/share_owned.move:14:9 + │ +12 │ public entry fun arg_object(o: Obj) { + │ - A potentially owned object coming from here +13 │ let arg = o; +14 │ transfer::public_share_object(arg); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + │ │ │ + │ │ Creating a fresh object and sharing it within the same function will ensure this does not abort. + │ Potential abort from a (potentially) owned object created by a different transaction. + +warning[Lint W01001]: possible owned object share + ┌─ tests/linter/share_owned.move:35:9 + │ +34 │ let Wrapper { id, i: _, o } = w; + │ - A potentially owned object coming from here +35 │ transfer::public_share_object(o); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + │ │ │ + │ │ Creating a fresh object and sharing it within the same function will ensure this does not abort. + │ Potential abort from a (potentially) owned object created by a different transaction. + diff --git a/crates/sui-move-build/tests/linter/share_owned.move b/crates/sui-move-build/tests/linter/share_owned.move new file mode 100644 index 0000000000000..6fd645c7b29a6 --- /dev/null +++ b/crates/sui-move-build/tests/linter/share_owned.move @@ -0,0 +1,38 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +module 0x42::test1 { + use sui::transfer; + use sui::object::UID; + + struct Obj has key, store { + id: UID + } + + public entry fun arg_object(o: Obj) { + let arg = o; + transfer::public_share_object(arg); + } +} + + +module 0x42::test2 { + use sui::transfer; + use sui::object::{Self, UID}; + + struct Obj has key, store { + id: UID + } + + struct Wrapper has key, store { + id: UID, + i: u32, + o: Obj, + } + + public entry fun unpack_obj(w: Wrapper) { + let Wrapper { id, i: _, o } = w; + transfer::public_share_object(o); + object::delete(id); + } +} diff --git a/crates/sui-move-build/tests/linter_tests.rs b/crates/sui-move-build/tests/linter_tests.rs new file mode 100644 index 0000000000000..2544c87ef2bf9 --- /dev/null +++ b/crates/sui-move-build/tests/linter_tests.rs @@ -0,0 +1,85 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +use std::{collections::BTreeMap, fs, path::Path}; + +use move_command_line_common::{ + testing::EXP_EXT, + testing::{add_update_baseline_fix, format_diff, read_env_update_baseline}, +}; +use move_compiler::{ + command_line::compiler::move_check_for_errors, shared::NumericalAddress, Compiler, PASS_PARSER, +}; + +use sui_move_build::linters::share_owned::ShareOwnedVerifier; + +const SUI_FRAMEWORK_PATH: &str = "../sui-framework/packages/sui-framework"; +const MOVE_STDLIB_PATH: &str = "../sui-framework/packages/move-stdlib"; + +fn default_testing_addresses() -> BTreeMap { + let mapping = [("std", "0x1"), ("sui", "0x2")]; + mapping + .iter() + .map(|(name, addr)| (name.to_string(), NumericalAddress::parse_str(addr).unwrap())) + .collect() +} + +fn linter_tests(path: &Path) -> datatest_stable::Result<()> { + run_tests(path)?; + Ok(()) +} + +fn run_tests(path: &Path) -> anyhow::Result<()> { + let exp_path = path.with_extension(EXP_EXT); + + let targets: Vec = vec![path.to_str().unwrap().to_owned()]; + let lint_visitors = vec![ShareOwnedVerifier.into()]; + let (files, comments_and_compiler_res) = Compiler::from_files( + targets, + vec![MOVE_STDLIB_PATH.to_string(), SUI_FRAMEWORK_PATH.to_string()], + default_testing_addresses(), + ) + .add_visitors(lint_visitors) + .run::()?; + + let diags = move_check_for_errors(comments_and_compiler_res); + + let has_diags = !diags.is_empty(); + let diag_buffer = if has_diags { + move_compiler::diagnostics::report_diagnostics_to_buffer(&files, diags) + } else { + vec![] + }; + + let update_baseline = read_env_update_baseline(); + + let rendered_diags = std::str::from_utf8(&diag_buffer)?; + + if update_baseline { + if has_diags { + fs::write(exp_path, rendered_diags)?; + } else if exp_path.is_file() { + fs::remove_file(exp_path)?; + } + return Ok(()); + } + + let exp_exists = exp_path.is_file(); + if exp_exists { + let expected_diags = fs::read_to_string(exp_path)?; + if rendered_diags != expected_diags { + let msg = format!( + "Expected output differ from the actual one:\n{}", + format_diff(expected_diags, rendered_diags), + ); + anyhow::bail!(add_update_baseline_fix(msg)); + } + } else { + let msg = format!("Unexpected output :\n{}", rendered_diags); + anyhow::bail!(add_update_baseline_fix(msg)); + } + + Ok(()) +} + +datatest_stable::harness!(linter_tests, "tests/linter", r".move$"); diff --git a/crates/sui-move/src/build.rs b/crates/sui-move/src/build.rs index ce403e1de3243..f8ebbdd86d296 100644 --- a/crates/sui-move/src/build.rs +++ b/crates/sui-move/src/build.rs @@ -32,6 +32,9 @@ pub struct Build { /// and events. #[clap(long, global = true)] pub generate_struct_layouts: bool, + /// If `true`, enable linters + #[clap(long, global = true)] + pub lint: bool, } impl Build { @@ -49,6 +52,7 @@ impl Build { self.legacy_digest, self.dump_bytecode_as_base64, self.generate_struct_layouts, + self.lint, ) } @@ -59,11 +63,13 @@ impl Build { legacy_digest: bool, dump_bytecode_as_base64: bool, generate_struct_layouts: bool, + lint: bool, ) -> anyhow::Result<()> { let pkg = BuildConfig { config, run_bytecode_verifier: true, print_diags_to_stderr: true, + lint, } .build(rerooted_path)?; if dump_bytecode_as_base64 { diff --git a/crates/sui-move/src/unit_test.rs b/crates/sui-move/src/unit_test.rs index 58741d3c79203..1dabdf9f3da2a 100644 --- a/crates/sui-move/src/unit_test.rs +++ b/crates/sui-move/src/unit_test.rs @@ -28,6 +28,9 @@ const MAX_UNIT_TEST_INSTRUCTIONS: u64 = 1_000_000; pub struct Test { #[clap(flatten)] pub test: test::Test, + /// If `true`, enable linters + #[clap(long, global = true)] + pub lint: bool, } impl Test { @@ -54,6 +57,7 @@ impl Test { legacy_digest, dump_bytecode_as_base64, generate_struct_layouts, + self.lint, )?; run_move_unit_tests( rerooted_path, diff --git a/crates/sui-source-validation-service/src/lib.rs b/crates/sui-source-validation-service/src/lib.rs index 3f3550705dc16..4bb190c6822fe 100644 --- a/crates/sui-source-validation-service/src/lib.rs +++ b/crates/sui-source-validation-service/src/lib.rs @@ -43,6 +43,7 @@ pub async fn verify_package( config, run_bytecode_verifier: false, /* no need to run verifier if code is on-chain */ print_diags_to_stderr: false, + lint: false, }; let compiled_package = build_config .build(package_path.as_ref().to_path_buf()) diff --git a/crates/sui/src/client_commands.rs b/crates/sui/src/client_commands.rs index 28bc3cafa2c6a..d07233940d739 100644 --- a/crates/sui/src/client_commands.rs +++ b/crates/sui/src/client_commands.rs @@ -199,6 +199,10 @@ pub enum SuiClientCommands { /// (SenderSignedData) using base64 encoding, and print out the string. #[clap(long, required = false)] serialize_signed_transaction: bool, + + /// If `true`, enable linters + #[clap(long, global = true)] + lint: bool, }, /// Run the bytecode verifier on the package @@ -269,6 +273,10 @@ pub enum SuiClientCommands { /// (SenderSignedData) using base64 encoding, and print out the string. #[clap(long, required = false)] serialize_signed_transaction: bool, + + /// If `true`, enable linters + #[clap(long, global = true)] + lint: bool, }, /// Verify local Move packages against on-chain packages, and optionally their dependencies. @@ -636,6 +644,7 @@ impl SuiClientCommands { legacy_digest, serialize_unsigned_transaction, serialize_signed_transaction, + lint, } => { let sender = context.try_get_object_owner(&gas).await?; let sender = sender.unwrap_or(context.active_address()?); @@ -648,6 +657,7 @@ impl SuiClientCommands { package_path, with_unpublished_dependencies, skip_dependency_verification, + lint, ) .await?; @@ -721,6 +731,7 @@ impl SuiClientCommands { with_unpublished_dependencies, serialize_unsigned_transaction, serialize_signed_transaction, + lint, } => { let sender = context.try_get_object_owner(&gas).await?; let sender = sender.unwrap_or(context.active_address()?); @@ -732,6 +743,7 @@ impl SuiClientCommands { package_path, with_unpublished_dependencies, skip_dependency_verification, + lint, ) .await?; @@ -1233,6 +1245,7 @@ impl SuiClientCommands { config: build_config, run_bytecode_verifier: true, print_diags_to_stderr: true, + lint: false, } .build(package_path)?; @@ -1272,6 +1285,7 @@ fn compile_package_simple( config: resolve_lock_file_path(build_config, Some(package_path.clone()))?, run_bytecode_verifier: false, print_diags_to_stderr: false, + lint: false, }; let resolution_graph = config.resolution_graph(&package_path)?; @@ -1280,6 +1294,7 @@ fn compile_package_simple( resolution_graph, false, false, + false, )?) } @@ -1289,6 +1304,7 @@ async fn compile_package( package_path: PathBuf, with_unpublished_dependencies: bool, skip_dependency_verification: bool, + lint: bool, ) -> Result< ( PackageDependencies, @@ -1305,6 +1321,7 @@ async fn compile_package( config, run_bytecode_verifier, print_diags_to_stderr, + lint, }; let resolution_graph = config.resolution_graph(&package_path)?; let (package_id, dependencies) = gather_published_ids(&resolution_graph); @@ -1317,6 +1334,7 @@ async fn compile_package( resolution_graph, run_bytecode_verifier, print_diags_to_stderr, + lint, )?; if !compiled_package.is_system_package() { if let Some(already_published) = compiled_package.published_root_module() { diff --git a/crates/sui/tests/cli_tests.rs b/crates/sui/tests/cli_tests.rs index 49fb17f302718..45a00777b3d9b 100644 --- a/crates/sui/tests/cli_tests.rs +++ b/crates/sui/tests/cli_tests.rs @@ -439,6 +439,7 @@ async fn test_move_call_args_linter_command() -> Result<(), anyhow::Error> { with_unpublished_dependencies: false, serialize_unsigned_transaction: false, serialize_signed_transaction: false, + lint: false, } .execute(context) .await?; @@ -660,6 +661,7 @@ async fn test_package_publish_command() -> Result<(), anyhow::Error> { with_unpublished_dependencies: false, serialize_unsigned_transaction: false, serialize_signed_transaction: false, + lint: false, } .execute(context) .await?; @@ -729,6 +731,7 @@ async fn test_package_publish_command_with_unpublished_dependency_succeeds( with_unpublished_dependencies, serialize_unsigned_transaction: false, serialize_signed_transaction: false, + lint: false, } .execute(context) .await?; @@ -797,6 +800,7 @@ async fn test_package_publish_command_with_unpublished_dependency_fails( with_unpublished_dependencies, serialize_unsigned_transaction: false, serialize_signed_transaction: false, + lint: false, } .execute(context) .await; @@ -843,6 +847,7 @@ async fn test_package_publish_command_non_zero_unpublished_dep_fails() -> Result with_unpublished_dependencies, serialize_unsigned_transaction: false, serialize_signed_transaction: false, + lint: false, } .execute(context) .await; @@ -898,6 +903,7 @@ async fn test_package_publish_command_failure_invalid() -> Result<(), anyhow::Er with_unpublished_dependencies, serialize_unsigned_transaction: false, serialize_signed_transaction: false, + lint: false, } .execute(context) .await; @@ -940,6 +946,7 @@ async fn test_package_publish_nonexistent_dependency() -> Result<(), anyhow::Err with_unpublished_dependencies: false, serialize_unsigned_transaction: false, serialize_signed_transaction: false, + lint: false, } .execute(context) .await; @@ -996,6 +1003,7 @@ async fn test_package_upgrade_command() -> Result<(), anyhow::Error> { with_unpublished_dependencies: false, serialize_unsigned_transaction: false, serialize_signed_transaction: false, + lint: false, } .execute(context) .await?; @@ -1069,6 +1077,7 @@ async fn test_package_upgrade_command() -> Result<(), anyhow::Error> { legacy_digest: false, serialize_unsigned_transaction: false, serialize_signed_transaction: false, + lint: false, } .execute(context) .await?; diff --git a/external-crates/move/move-compiler/src/command_line/compiler.rs b/external-crates/move/move-compiler/src/command_line/compiler.rs index d7899dc79ae8c..52f4800379a0d 100644 --- a/external-crates/move/move-compiler/src/command_line/compiler.rs +++ b/external-crates/move/move-compiler/src/command_line/compiler.rs @@ -743,6 +743,36 @@ fn has_compiled_module_magic_number(path: &str) -> bool { num_bytes_read == BinaryConstants::MOVE_MAGIC_SIZE && magic == BinaryConstants::MOVE_MAGIC } +pub fn move_check_for_errors( + comments_and_compiler_res: Result<(CommentMap, SteppedCompiler<'_, PASS_PARSER>), Diagnostics>, +) -> Diagnostics { + fn try_impl( + comments_and_compiler_res: Result< + (CommentMap, SteppedCompiler<'_, PASS_PARSER>), + Diagnostics, + >, + ) -> Result<(Vec, Diagnostics), Diagnostics> { + let (_, compiler) = comments_and_compiler_res?; + + let (mut compiler, cfgir) = compiler.run::()?.into_ast(); + let compilation_env = compiler.compilation_env(); + if compilation_env.flags().is_testing() { + unit_test::plan_builder::construct_test_plan(compilation_env, None, &cfgir); + } + + let (units, diags) = compiler.at_cfgir(cfgir).build()?; + Ok((units, diags)) + } + + let (units, inner_diags) = match try_impl(comments_and_compiler_res) { + Ok((units, inner_diags)) => (units, inner_diags), + Err(inner_diags) => return inner_diags, + }; + let mut diags = compiled_unit::verify_units(&units); + diags.extend(inner_diags); + diags +} + //************************************************************************************************** // Translations //************************************************************************************************** diff --git a/external-crates/move/move-compiler/tests/move_check_testsuite.rs b/external-crates/move/move-compiler/tests/move_check_testsuite.rs index 6a693b35f7122..ba261700bcaa0 100644 --- a/external-crates/move/move-compiler/tests/move_check_testsuite.rs +++ b/external-crates/move/move-compiler/tests/move_check_testsuite.rs @@ -9,10 +9,10 @@ use move_command_line_common::{ testing::{add_update_baseline_fix, format_diff, read_env_update_baseline, EXP_EXT, OUT_EXT}, }; use move_compiler::{ - compiled_unit::AnnotatedCompiledUnit, + command_line::compiler::move_check_for_errors, diagnostics::*, shared::{Flags, NumericalAddress}, - unit_test, CommentMap, Compiler, SteppedCompiler, PASS_CFGIR, PASS_PARSER, + Compiler, PASS_PARSER, }; /// Shared flag to keep any temporary results of the test @@ -210,34 +210,4 @@ fn run_test( } } -fn move_check_for_errors( - comments_and_compiler_res: Result<(CommentMap, SteppedCompiler<'_, PASS_PARSER>), Diagnostics>, -) -> Diagnostics { - fn try_impl( - comments_and_compiler_res: Result< - (CommentMap, SteppedCompiler<'_, PASS_PARSER>), - Diagnostics, - >, - ) -> Result<(Vec, Diagnostics), Diagnostics> { - let (_, compiler) = comments_and_compiler_res?; - - let (mut compiler, cfgir) = compiler.run::()?.into_ast(); - let compilation_env = compiler.compilation_env(); - if compilation_env.flags().is_testing() { - unit_test::plan_builder::construct_test_plan(compilation_env, None, &cfgir); - } - - let (units, diags) = compiler.at_cfgir(cfgir).build()?; - Ok((units, diags)) - } - - let (units, inner_diags) = match try_impl(comments_and_compiler_res) { - Ok((units, inner_diags)) => (units, inner_diags), - Err(inner_diags) => return inner_diags, - }; - let mut diags = move_compiler::compiled_unit::verify_units(&units); - diags.extend(inner_diags); - diags -} - datatest_stable::harness!(move_check_testsuite, "tests/move_check", r".*\.move$");