Skip to content

Commit

Permalink
[move-compiler] Added preliminary support for writing linters (Mysten…
Browse files Browse the repository at this point in the history
…Labs#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.
  • Loading branch information
awelc authored Jun 27, 2023
1 parent 78ebb2f commit bbf4187
Show file tree
Hide file tree
Showing 16 changed files with 524 additions and 35 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock

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

3 changes: 3 additions & 0 deletions crates/sui-framework/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,20 +134,23 @@ 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();
let system_pkg = BuildConfig {
config: config.clone(),
run_bytecode_verifier: true,
print_diags_to_stderr: false,
lint: false,
}
.build(sui_system_path)
.unwrap();
let deepbook_pkg = BuildConfig {
config,
run_bytecode_verifier: true,
print_diags_to_stderr: false,
lint: false,
}
.build(deepbook_path)
.unwrap();
Expand Down
9 changes: 9 additions & 0 deletions crates/sui-move-build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
25 changes: 22 additions & 3 deletions crates/sui-move-build/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -116,12 +125,18 @@ impl BuildConfig {

fn compile_package<W: Write>(
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);
Expand All @@ -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<CompiledPackage> {
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)?;
Expand All @@ -152,6 +168,7 @@ impl BuildConfig {
resolution_graph,
run_bytecode_verifier,
print_diags_to_stderr,
lint,
)
}

Expand All @@ -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<CompiledPackage> {
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.
Expand Down Expand Up @@ -541,6 +559,7 @@ impl Default for BuildConfig {
config: MoveBuildConfig::default(),
run_bytecode_verifier: true,
print_diags_to_stderr: false,
lint: false,
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions crates/sui-move-build/src/linters/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Copyright (c) Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

pub mod share_owned;
Loading

0 comments on commit bbf4187

Please sign in to comment.