Make linting opt in with --lint

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 - Removed requirement to install binaryen. The `wasm-opt` tool is now compiled into `cargo-contract`.
+- Make linting opt in with `--lint` - [#799](https://github.com/paritytech/cargo-contract/pull/799)
 
 ## [2.0.0-alpha.4] - 2022-10-03
 

README.md

@@ -39,13 +39,13 @@ Modern releases of gcc and clang, as well as Visual Studio 2019+ should work.
 
 * Step 1: `rustup component add rust-src`.
 
-* Step 2: Install `dylint`
+* Step 2: `cargo install --force --locked cargo-contract`.
+
+* Step 3: (**Optional**) Install `dylint` for linting.
   * (MacOS) `brew install openssl`
   * `cargo install cargo-dylint dylint-link`.
 
-* Step 3: `cargo install --force --locked cargo-contract`.
-
-You can always update the `cargo-contract` binary to the latest version by running the Step 3.
+You can always update the `cargo-contract` binary to the latest version by running the Step 2.
 
 ### Installation using Docker Image
 

crates/cargo-contract/src/cmd/build/mod.rs

@@ -82,7 +82,7 @@ pub(crate) struct ExecuteArgs {
     pub unstable_flags: UnstableFlags,
     pub optimization_passes: OptimizationPasses,
     pub keep_debug_symbols: bool,
-    pub skip_linting: bool,
+    pub lint: bool,
     pub output_type: OutputType,
 }
 
@@ -106,9 +106,9 @@ pub struct BuildCommand {
     /// Build offline
     #[clap(long = "offline")]
     build_offline: bool,
-    /// Skips linting checks during the build process
+    /// Performs linting checks during the build process
     #[clap(long)]
-    skip_linting: bool,
+    lint: bool,
     /// Which build artifacts to generate.
     ///
     /// - `all`: Generate the Wasm, the metadata and a bundled `<name>.contract` file.
@@ -191,9 +191,11 @@ impl BuildCommand {
             false => Network::Online,
         };
 
-        // The invocation of `cargo dylint` requires network access, so in offline mode the linting
-        // step must be skipped otherwise the build can fail.
-        let skip_linting = self.skip_linting || matches!(network, Network::Offline);
+        if self.lint && matches!(network, Network::Offline) {
+            anyhow::bail!(
+                "Linting requires network access in order to download available lints"
+            )
+        }
 
         let output_type = match self.output_json {
             true => OutputType::Json,
@@ -214,7 +216,7 @@ impl BuildCommand {
             unstable_flags,
             optimization_passes,
             keep_debug_symbols: self.keep_debug_symbols,
-            skip_linting,
+            lint: self.lint,
             output_type,
         };
 
@@ -250,7 +252,7 @@ impl CheckCommand {
             unstable_flags,
             optimization_passes: OptimizationPasses::Zero,
             keep_debug_symbols: false,
-            skip_linting: false,
+            lint: false,
             output_type: OutputType::default(),
         };
 
@@ -591,7 +593,7 @@ pub(crate) fn execute(args: ExecuteArgs) -> Result<BuildResult> {
         unstable_flags,
         optimization_passes,
         keep_debug_symbols,
-        skip_linting,
+        lint,
         output_type,
     } = args;
 
@@ -602,30 +604,30 @@ pub(crate) fn execute(args: ExecuteArgs) -> Result<BuildResult> {
         assert_debug_mode_supported(&crate_metadata.ink_version)?;
     }
 
-    let build = || -> Result<(OptimizationResult, BuildInfo)> {
-        use crate::cmd::metadata::WasmOptSettings;
-
-        if skip_linting {
-            maybe_println!(
-                verbosity,
-                " {} {}",
-                format!("[1/{}]", build_artifact.steps()).bold(),
-                "Skip ink! linting rules".bright_yellow().bold()
-            );
-        } else {
+    let maybe_lint = || -> Result<(usize, usize)> {
+        if lint {
             maybe_println!(
                 verbosity,
                 " {} {}",
                 format!("[1/{}]", build_artifact.steps()).bold(),
                 "Checking ink! linting rules".bright_green().bold()
             );
             exec_cargo_dylint(&crate_metadata, verbosity)?;
+            Ok((2, build_artifact.steps() + 1))
+        } else {
+            Ok((1, build_artifact.steps()))
         }
+    };
+
+    let build = || -> Result<(OptimizationResult, BuildInfo)> {
+        use crate::cmd::metadata::WasmOptSettings;
+
+        let (next_step, total_steps) = maybe_lint()?;
 
         maybe_println!(
             verbosity,
             " {} {}",
-            format!("[2/{}]", build_artifact.steps()).bold(),
+            format!("[{}/{}]", next_step, total_steps).bold(),
             "Building cargo project".bright_green().bold()
         );
         exec_cargo_for_wasm_target(
@@ -640,15 +642,15 @@ pub(crate) fn execute(args: ExecuteArgs) -> Result<BuildResult> {
         maybe_println!(
             verbosity,
             " {} {}",
-            format!("[3/{}]", build_artifact.steps()).bold(),
+            format!("[{}/{}]", next_step + 1, total_steps).bold(),
             "Post processing wasm file".bright_green().bold()
         );
         post_process_wasm(&crate_metadata)?;
 
         maybe_println!(
             verbosity,
             " {} {}",
-            format!("[4/{}]", build_artifact.steps()).bold(),
+            format!("[{}/{}]", next_step + 2, total_steps).bold(),
             "Optimizing wasm file".bright_green().bold()
         );
 
@@ -682,27 +684,12 @@ pub(crate) fn execute(args: ExecuteArgs) -> Result<BuildResult> {
 
     let (opt_result, metadata_result) = match build_artifact {
         BuildArtifacts::CheckOnly => {
-            if skip_linting {
-                maybe_println!(
-                    verbosity,
-                    " {} {}",
-                    format!("[1/{}]", build_artifact.steps()).bold(),
-                    "Skip ink! linting rules".bright_yellow().bold()
-                );
-            } else {
-                maybe_println!(
-                    verbosity,
-                    " {} {}",
-                    format!("[1/{}]", build_artifact.steps()).bold(),
-                    "Checking ink! linting rules".bright_green().bold()
-                );
-                exec_cargo_dylint(&crate_metadata, verbosity)?;
-            }
+            let (next_step, total_steps) = maybe_lint()?;
 
             maybe_println!(
                 verbosity,
                 " {} {}",
-                format!("[2/{}]", build_artifact.steps()).bold(),
+                format!("[{}/{}]", next_step, total_steps).bold(),
                 "Executing `cargo check`".bright_green().bold()
             );
             exec_cargo_for_wasm_target(

crates/cargo-contract/src/cmd/build/tests.rs

@@ -80,7 +80,7 @@ fn build_code_only(manifest_path: &ManifestPath) -> Result<()> {
         manifest_path: manifest_path.clone(),
         build_mode: BuildMode::Release,
         build_artifact: BuildArtifacts::CodeOnly,
-        skip_linting: true,
+        lint: false,
         ..Default::default()
     };
 
@@ -120,7 +120,7 @@ fn check_must_not_output_contract_artifacts_in_project_dir(
     let args = crate::cmd::build::ExecuteArgs {
         manifest_path: manifest_path.clone(),
         build_artifact: BuildArtifacts::CheckOnly,
-        skip_linting: true,
+        lint: false,
         ..Default::default()
     };
 
@@ -158,7 +158,7 @@ fn optimization_passes_from_cli_must_take_precedence_over_profile(
         // we choose zero optimization passes as the "cli" parameter
         optimization_passes: Some(OptimizationPasses::Zero),
         keep_debug_symbols: false,
-        skip_linting: true,
+        lint: false,
         output_json: false,
     };
 
@@ -199,7 +199,7 @@ fn optimization_passes_from_profile_must_be_used(
         // we choose no optimization passes as the "cli" parameter
         optimization_passes: None,
         keep_debug_symbols: false,
-        skip_linting: true,
+        lint: false,
         output_json: false,
     };
 
@@ -241,7 +241,7 @@ fn contract_lib_name_different_from_package_name_must_build(
         unstable_options: UnstableOptions::default(),
         optimization_passes: None,
         keep_debug_symbols: false,
-        skip_linting: true,
+        lint: false,
         output_json: false,
     };
     let res = cmd.exec().expect("build failed");
@@ -262,7 +262,7 @@ fn building_template_in_debug_mode_must_work(manifest_path: &ManifestPath) -> Re
     let args = crate::cmd::build::ExecuteArgs {
         manifest_path: manifest_path.clone(),
         build_mode: BuildMode::Debug,
-        skip_linting: true,
+        lint: false,
         ..Default::default()
     };
 
@@ -281,7 +281,7 @@ fn building_template_in_release_mode_must_work(
     let args = crate::cmd::build::ExecuteArgs {
         manifest_path: manifest_path.clone(),
         build_mode: BuildMode::Release,
-        skip_linting: true,
+        lint: false,
         ..Default::default()
     };
 
@@ -311,7 +311,7 @@ fn building_contract_with_source_file_in_subfolder_must_work(
     let args = crate::cmd::build::ExecuteArgs {
         manifest_path: manifest_path.clone(),
         build_artifact: BuildArtifacts::CheckOnly,
-        skip_linting: true,
+        lint: false,
         ..Default::default()
     };
 
@@ -329,7 +329,7 @@ fn keep_debug_symbols_in_debug_mode(manifest_path: &ManifestPath) -> Result<()>
         build_mode: BuildMode::Debug,
         build_artifact: BuildArtifacts::CodeOnly,
         keep_debug_symbols: true,
-        skip_linting: true,
+        lint: false,
         ..Default::default()
     };
 
@@ -347,7 +347,7 @@ fn keep_debug_symbols_in_release_mode(manifest_path: &ManifestPath) -> Result<()
         build_mode: BuildMode::Release,
         build_artifact: BuildArtifacts::CodeOnly,
         keep_debug_symbols: true,
-        skip_linting: true,
+        lint: false,
         ..Default::default()
     };
 
@@ -364,7 +364,7 @@ fn build_with_json_output_works(manifest_path: &ManifestPath) -> Result<()> {
     let args = crate::cmd::build::ExecuteArgs {
         manifest_path: manifest_path.clone(),
         output_type: OutputType::Json,
-        skip_linting: true,
+        lint: false,
         ..Default::default()
     };
 
@@ -436,7 +436,7 @@ fn generates_metadata(manifest_path: &ManifestPath) -> Result<()> {
     fs::write(final_contract_wasm_path, "TEST FINAL WASM BLOB").unwrap();
 
     let mut args = crate::cmd::build::ExecuteArgs {
-        skip_linting: true,
+        lint: false,
         ..Default::default()
     };
     args.manifest_path = manifest_path.clone();