Skip to content

Commit

Permalink
feat(zk_toolbox): Run formatters and linterrs (matter-labs#2675)
Browse files Browse the repository at this point in the history
## What ❔

Adding an ability to run linters and formatters for zk supervisor

## Why ❔

<!-- Why are these changes done? What goal do they contribute to? What
are the principles behind them? -->
<!-- Example: PR templates ensure PR reviewers, observers, and future
iterators are in context about the evolution of repos. -->

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [ ] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [ ] Tests for the changes have been added / updated.
- [ ] Documentation comments have been added / updated.
- [ ] Code has been formatted via `zk fmt` and `zk lint`.

---------

Signed-off-by: Danil <[email protected]>
Co-authored-by: Alexander Melnikov <[email protected]>
  • Loading branch information
Deniallugo and sanekmelnikov authored Aug 20, 2024
1 parent fa866cd commit caedd1c
Show file tree
Hide file tree
Showing 11 changed files with 274 additions and 80 deletions.
26 changes: 26 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,32 @@
bellman-cuda
sdk/zksync-rs/CHANGELOG.md
CHANGELOG.md
core/lib/dal/.sqlx
prover/lib/dal/.sqlx
node_modules

# Ignore contract submodules
contracts

**/target/**
**/node_modules
volumes
**/build/**
dist
.git
generated
grafonnet-lib
prettier-config
lint-config
**/cache
**/artifacts
**/typechain
binaryen
system-contracts
artifacts-zk
cache-zk
// Ignore directories with OZ and forge submodules.
contracts/l1-contracts/lib

**/.git
**/node_modules
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,15 @@ contract HeapBenchmark {
mstore(add(array, sub(n, 1)), 4242)

let j := 0
for {} lt(j, n) {} {
for {

} lt(j, n) {

} {
v1 := mload(add(array, mod(mul(j, j), n)))
v2 := mload(add(array, j))
mstore(add(array, j), add(add(v1, v2), 42))
j := add(j, 1)
j := add(j, 1)
if gt(j, sub(n, 1)) {
j := 0
}
Expand Down
6 changes: 3 additions & 3 deletions etc/contracts-test-data/counter/counter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ contract Counter {
value += x;
}

function incrementWithRevertPayable(uint256 x, bool shouldRevert) payable public returns (uint256) {
function incrementWithRevertPayable(uint256 x, bool shouldRevert) public payable returns (uint256) {
return incrementWithRevert(x, shouldRevert);
}

function incrementWithRevert(uint256 x, bool shouldRevert) public returns (uint256) {
value += x;
if(shouldRevert) {
if (shouldRevert) {
revert("This method always reverts");
}
return value;
Expand All @@ -24,4 +24,4 @@ contract Counter {
function get() public view returns (uint256) {
return value;
}
}
}
1 change: 1 addition & 0 deletions zk_toolbox/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 zk_toolbox/crates/zk_supervisor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ url.workspace = true
xshell.workspace = true
serde.workspace = true
clap-markdown.workspace = true
futures.workspace = true
127 changes: 127 additions & 0 deletions zk_toolbox/crates/zk_supervisor/src/commands/fmt.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
use std::path::PathBuf;

use clap::Parser;
use common::{cmd::Cmd, logger, spinner::Spinner};
use config::EcosystemConfig;
use xshell::{cmd, Shell};

use crate::{
commands::lint_utils::{get_unignored_files, Extension},
messages::{
msg_running_fmt_for_extension_spinner, msg_running_fmt_for_extensions_spinner,
msg_running_rustfmt_for_dir_spinner, MSG_RUNNING_CONTRACTS_FMT_SPINNER,
},
};

async fn prettier(shell: Shell, extension: Extension, check: bool) -> anyhow::Result<()> {
let spinner = Spinner::new(&msg_running_fmt_for_extension_spinner(extension));
let files = get_unignored_files(&shell, &extension)?;

if files.is_empty() {
return Ok(());
}

spinner.freeze();
let mode = if check { "--check" } else { "--write" };
let config = format!("etc/prettier-config/{extension}.js");
Ok(
Cmd::new(cmd!(shell, "yarn --silent prettier {mode} --config {config}").args(files))
.run()?,
)
}

async fn prettier_contracts(shell: Shell, check: bool) -> anyhow::Result<()> {
let spinner = Spinner::new(MSG_RUNNING_CONTRACTS_FMT_SPINNER);
spinner.freeze();
let prettier_command = cmd!(shell, "yarn --silent --cwd contracts")
.arg(format!("prettier:{}", if check { "check" } else { "fix" }));

Ok(Cmd::new(prettier_command).run()?)
}

async fn rustfmt(shell: Shell, check: bool, link_to_code: PathBuf) -> anyhow::Result<()> {
for dir in [".", "prover", "zk_toolbox"] {
let spinner = Spinner::new(&msg_running_rustfmt_for_dir_spinner(dir));
let _dir = shell.push_dir(link_to_code.join(dir));
let mut cmd = cmd!(shell, "cargo fmt -- --config imports_granularity=Crate --config group_imports=StdExternalCrate");
if check {
cmd = cmd.arg("--check");
}
spinner.freeze();
Cmd::new(cmd).run()?;
}
Ok(())
}

async fn run_all_rust_formatters(
shell: Shell,
check: bool,
link_to_code: PathBuf,
) -> anyhow::Result<()> {
rustfmt(shell.clone(), check, link_to_code).await?;
Ok(())
}

#[derive(Debug, Parser)]
pub enum Formatter {
Rustfmt,
Contract,
Prettier {
#[arg(short, long)]
extensions: Vec<Extension>,
},
}

#[derive(Debug, Parser)]
pub struct FmtArgs {
#[clap(long, short = 'c')]
pub check: bool,
#[clap(subcommand)]
pub formatter: Option<Formatter>,
}

pub async fn run(shell: Shell, args: FmtArgs) -> anyhow::Result<()> {
let ecosystem = EcosystemConfig::from_file(&shell)?;
match args.formatter {
None => {
let mut tasks = vec![];
let extensions: Vec<_> =
vec![Extension::Js, Extension::Ts, Extension::Md, Extension::Sol];
let spinner = Spinner::new(&msg_running_fmt_for_extensions_spinner(&extensions));
spinner.freeze();
for ext in extensions {
tasks.push(tokio::spawn(prettier(shell.clone(), ext, args.check)));
}
tasks.push(tokio::spawn(rustfmt(
shell.clone(),
args.check,
ecosystem.link_to_code,
)));
tasks.push(tokio::spawn(prettier_contracts(shell.clone(), args.check)));

futures::future::join_all(tasks)
.await
.iter()
.for_each(|res| {
if let Err(err) = res {
logger::error(err)
}
});
}
Some(Formatter::Prettier { mut extensions }) => {
if extensions.is_empty() {
extensions = vec![Extension::Js, Extension::Ts, Extension::Md, Extension::Sol];
}
let spinner = Spinner::new(&msg_running_fmt_for_extensions_spinner(&extensions));
for ext in extensions {
prettier(shell.clone(), ext, args.check).await?
}
spinner.finish()
}
Some(Formatter::Rustfmt) => {
run_all_rust_formatters(shell.clone(), args.check, ".".into()).await?
}
Some(Formatter::Contract) => prettier_contracts(shell.clone(), args.check).await?,
}
Ok(())
}
95 changes: 24 additions & 71 deletions zk_toolbox/crates/zk_supervisor/src/commands/lint.rs
Original file line number Diff line number Diff line change
@@ -1,43 +1,16 @@
use clap::{Parser, ValueEnum};
use clap::Parser;
use common::{cmd::Cmd, logger, spinner::Spinner};
use config::EcosystemConfig;
use strum::EnumIter;
use xshell::{cmd, Shell};

use crate::messages::{
msg_running_linter_for_extension_spinner, msg_running_linters_for_files,
MSG_LINT_CONFIG_PATH_ERR, MSG_RUNNING_CONTRACTS_LINTER_SPINNER,
use crate::{
commands::lint_utils::{get_unignored_files, Extension},
messages::{
msg_running_linter_for_extension_spinner, msg_running_linters_for_files,
MSG_LINT_CONFIG_PATH_ERR, MSG_RUNNING_CONTRACTS_LINTER_SPINNER,
},
};

const IGNORED_DIRS: [&str; 18] = [
"target",
"node_modules",
"volumes",
"build",
"dist",
".git",
"generated",
"grafonnet-lib",
"prettier-config",
"lint-config",
"cache",
"artifacts",
"typechain",
"binaryen",
"system-contracts",
"artifacts-zk",
"cache-zk",
// Ignore directories with OZ and forge submodules.
"contracts/l1-contracts/lib",
];

const IGNORED_FILES: [&str; 4] = [
"KeysWithPlonkVerifier.sol",
"TokenInit.sol",
".tslintrc.js",
".prettierrc.js",
];

const CONFIG_PATH: &str = "etc/lint-config";

#[derive(Debug, Parser)]
Expand All @@ -48,16 +21,6 @@ pub struct LintArgs {
pub extensions: Vec<Extension>,
}

#[derive(Debug, ValueEnum, EnumIter, strum::Display, PartialEq, Eq, Clone)]
#[strum(serialize_all = "lowercase")]
pub enum Extension {
Rs,
Md,
Sol,
Js,
Ts,
}

pub fn run(shell: &Shell, args: LintArgs) -> anyhow::Result<()> {
let extensions = if args.extensions.is_empty() {
vec![
Expand All @@ -77,7 +40,7 @@ pub fn run(shell: &Shell, args: LintArgs) -> anyhow::Result<()> {

for extension in extensions {
match extension {
Extension::Rs => lint_rs(shell, &ecosystem)?,
Extension::Rs => lint_rs(shell, &ecosystem, args.check)?,
Extension::Sol => lint_contracts(shell, &ecosystem, args.check)?,
ext => lint(shell, &ecosystem, &ext, args.check)?,
}
Expand All @@ -86,25 +49,33 @@ pub fn run(shell: &Shell, args: LintArgs) -> anyhow::Result<()> {
Ok(())
}

fn lint_rs(shell: &Shell, ecosystem: &EcosystemConfig) -> anyhow::Result<()> {
fn lint_rs(shell: &Shell, ecosystem: &EcosystemConfig, check: bool) -> anyhow::Result<()> {
let spinner = Spinner::new(&msg_running_linter_for_extension_spinner(&Extension::Rs));

let link_to_code = &ecosystem.link_to_code;
let lint_to_prover = &ecosystem.link_to_code.join("prover");
let link_to_toolbox = &ecosystem.link_to_code.join("zk_toolbox");
let paths = vec![link_to_code, lint_to_prover, link_to_toolbox];

spinner.freeze();
for path in paths {
let _dir_guard = shell.push_dir(path);
Cmd::new(cmd!(
shell,
"cargo clippy --locked -- -D warnings -D unstable_features"
))
.run()?;
let mut cmd = cmd!(shell, "cargo clippy");
let common_args = &[
"--locked",
"--",
"-D",
"warnings",
"-D",
"unstable_features",
];
if !check {
cmd = cmd.args(&["--fix", "--allow-dirty"]);
}
cmd = cmd.args(common_args);
Cmd::new(cmd).with_force_run().run()?;
}

spinner.finish();

Ok(())
}

Expand All @@ -127,7 +98,6 @@ fn lint(
let spinner = Spinner::new(&msg_running_linter_for_extension_spinner(extension));
let _dir_guard = shell.push_dir(&ecosystem.link_to_code);
let files = get_unignored_files(shell, extension)?;

let cmd = cmd!(shell, "yarn");
let config_path = ecosystem.link_to_code.join(CONFIG_PATH);
let config_path = config_path.join(format!("{}.js", extension));
Expand Down Expand Up @@ -170,20 +140,3 @@ fn lint_contracts(shell: &Shell, ecosystem: &EcosystemConfig, check: bool) -> an

Ok(())
}

fn get_unignored_files(shell: &Shell, extension: &Extension) -> anyhow::Result<Vec<String>> {
let mut files = Vec::new();
let output = cmd!(shell, "git ls-files").read()?;

for line in output.lines() {
let path = line.to_string();
if !IGNORED_DIRS.iter().any(|dir| path.contains(dir))
&& !IGNORED_FILES.contains(&path.as_str())
&& path.ends_with(&format!(".{}", extension))
{
files.push(path);
}
}

Ok(files)
}
Loading

0 comments on commit caedd1c

Please sign in to comment.