Skip to content

Commit

Permalink
review related changes
Browse files Browse the repository at this point in the history
  • Loading branch information
wawel37 committed Jan 9, 2025
1 parent 223ad06 commit 2fa56b2
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 51 deletions.
16 changes: 7 additions & 9 deletions scarb/src/bin/scarb/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ pub enum Command {
to a registry.
")]
Publish(PublishArgs),
/// Run lint checker.
/// Checks a package to catch common mistakes and improve your Cairo code.
Lint(LintArgs),
/// Run arbitrary package scripts.
Run(ScriptsRunnerArgs),
Expand Down Expand Up @@ -538,20 +538,18 @@ pub struct LintArgs {
/// Name of the package.
#[command(flatten)]
pub packages_filter: PackagesFilter,
/// Path to the file or project to analyze
pub path: Option<String>,
/// Logging verbosity.
#[command(flatten)]
pub verbose: VerbositySpec,
/// Comma separated list of target names to compile.
#[arg(long, value_delimiter = ',', env = "SCARB_TARGET_NAMES")]
pub target_names: Vec<String>,

/// Should lint the tests.
#[arg(short, long, default_value_t = false)]
pub test: bool,

/// Should fix the lint when it can.
#[arg(short, long, default_value_t = false)]
pub fix: bool,

/// Do not error on `cairo-version` mismatch.
#[arg(long)]
pub ignore_cairo_version: bool,
}

/// Git reference specification arguments.
Expand Down
1 change: 1 addition & 0 deletions scarb/src/bin/scarb/commands/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub fn run(args: LintArgs, config: &Config) -> Result<()> {
packages,
test: args.test,
fix: args.fix,
ignore_cairo_version: args.ignore_cairo_version,
},
&ws,
)
Expand Down
17 changes: 4 additions & 13 deletions scarb/src/ops/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ use cairo_lang_filesystem::db::FilesGroup;
use cairo_lang_filesystem::ids::{CrateLongId, FileId};
use cairo_lang_semantic::db::SemanticGroup;
use cairo_lang_semantic::diagnostic::SemanticDiagnosticKind;
use cairo_lang_starknet::starknet_plugin_suite;
use cairo_lang_syntax::node::SyntaxNode;
use cairo_lang_test_plugin::test_plugin_suite;
use cairo_lang_utils::Upcast;
use cairo_lint_core::{
diagnostics::format_diagnostic,
Expand All @@ -36,6 +34,7 @@ pub struct LintOptions {
pub packages: Vec<Package>,
pub test: bool,
pub fix: bool,
pub ignore_cairo_version: bool,
}

#[tracing::instrument(skip_all, level = "debug")]
Expand All @@ -52,8 +51,8 @@ pub fn lint(opts: LintOptions, ws: &Workspace<'_>) -> Result<()> {
&feature_opts,
ws,
CompilationUnitsOpts {
ignore_cairo_version: false,
load_prebuilt_macros: false,
ignore_cairo_version: opts.ignore_cairo_version,
load_prebuilt_macros: true,
},
)?;

Expand Down Expand Up @@ -95,15 +94,7 @@ pub fn lint(opts: LintOptions, ws: &Workspace<'_>) -> Result<()> {
.ui()
.print(Status::new("Checking", &compilation_unit.name()));

let additional_plugins = if opts.test {
vec![
test_plugin_suite(),
cairo_lint_plugin_suite(),
starknet_plugin_suite(),
]
} else {
vec![cairo_lint_plugin_suite(), starknet_plugin_suite()]
};
let additional_plugins = vec![cairo_lint_plugin_suite()];
let ScarbDatabase { db, .. } =
build_scarb_root_database(compilation_unit, ws, additional_plugins)?;

Expand Down
116 changes: 87 additions & 29 deletions scarb/tests/lint.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use assert_fs::fixture::FileWriteStr;
use assert_fs::{prelude::PathChild, TempDir};
use indoc::indoc;
use scarb_test_support::{
Expand All @@ -19,18 +20,20 @@ fn lint_main_package() {
"#})
.build(&t);

Scarb::quick_snapbox()
Scarb::quick_snapbox().env("CLICOLOR", "0")
.arg("lint")
.current_dir(&t)
.assert()
// Current expected values include ANSI color codes because lint has custom renderer.
.stdout_matches(indoc! {r#"
Checking hello v1.0.0 ([..]Scarb.toml)
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
--> [..]
|
3 | if x == false {
| ----------
|
Checking hello v1.0.0 ([..]/Scarb.toml)
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
--> [..]/lib.cairo:3:8
|
3 | if x == false {
| ----------
|
"#})
.success();
}
Expand Down Expand Up @@ -79,27 +82,82 @@ fn lint_workspace() {
.arg("--workspace")
.current_dir(&t)
.assert()
// Current expected values include ANSI color codes because lint has custom renderer.
.stdout_matches(indoc! {r#"
Checking first v1.0.0 ([..]first/Scarb.toml)
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
--> [..]/lib.cairo:3:8
|
3 | if first == false {
| --------------
|
Checking main v1.0.0 ([..]/Scarb.toml)
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
--> [..]/lib.cairo:3:8
|
3 | if _main == false {
| --------------
|
Checking second v1.0.0 ([..]second/Scarb.toml)
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
--> [..]/lib.cairo:3:8
|
3 | if second == false {
| ---------------
|
Checking first v1.0.0 ([..]/first/Scarb.toml)
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
--> [..]/lib.cairo:3:8
|
3 | if first == false {
| --------------
|
Checking main v1.0.0 ([..]/Scarb.toml)
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
--> [..]/lib.cairo:3:8
|
3 | if _main == false {
| --------------
|
Checking second v1.0.0 ([..]/second/Scarb.toml)
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
--> [..]/lib.cairo:3:8
|
3 | if second == false {
| ---------------
|
"#}).success();
}

#[test]
fn lint_integration_tests() {
let t = TempDir::new().unwrap();
ProjectBuilder::start()
.name("hello")
.lib_cairo(indoc! {r#"
pub fn f1() -> u32 {
42
}
fn main() {
// This is a comment
}
"#})
.dep_cairo_test()
.build(&t);
t.child("tests/test1.cairo")
.write_str(indoc! {r#"
use hello::f1;
#[test]
fn it_works() {
let x = true;
if false == x {
println!("x is false");
}
assert_eq!(1, f1());
}
"#})
.unwrap();

Scarb::quick_snapbox()
.arg("lint")
.arg("-t")
.current_dir(&t)
.assert()
// Current expected values include ANSI color codes because lint has custom renderer.
.stdout_matches(indoc! {r#"
Checking hello v1.0.0 ([..]/Scarb.toml)
Checking test(hello_unittest) hello v1.0.0 ([..]/Scarb.toml)
Checking test(hello_integrationtest) hello_integrationtest v1.0.0 ([..]/Scarb.toml)
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
--> [..]/tests/test1.cairo:5:8
|
5 | if false == x {
| ----------
|
"#})
.success();
}

0 comments on commit 2fa56b2

Please sign in to comment.