Skip to content

Commit 2fa56b2

Browse files
committed
review related changes
1 parent 223ad06 commit 2fa56b2

File tree

4 files changed

+99
-51
lines changed

4 files changed

+99
-51
lines changed

scarb/src/bin/scarb/args.rs

+7-9
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ pub enum Command {
187187
to a registry.
188188
")]
189189
Publish(PublishArgs),
190-
/// Run lint checker.
190+
/// Checks a package to catch common mistakes and improve your Cairo code.
191191
Lint(LintArgs),
192192
/// Run arbitrary package scripts.
193193
Run(ScriptsRunnerArgs),
@@ -538,20 +538,18 @@ pub struct LintArgs {
538538
/// Name of the package.
539539
#[command(flatten)]
540540
pub packages_filter: PackagesFilter,
541-
/// Path to the file or project to analyze
542-
pub path: Option<String>,
543-
/// Logging verbosity.
544-
#[command(flatten)]
545-
pub verbose: VerbositySpec,
546-
/// Comma separated list of target names to compile.
547-
#[arg(long, value_delimiter = ',', env = "SCARB_TARGET_NAMES")]
548-
pub target_names: Vec<String>,
541+
549542
/// Should lint the tests.
550543
#[arg(short, long, default_value_t = false)]
551544
pub test: bool,
545+
552546
/// Should fix the lint when it can.
553547
#[arg(short, long, default_value_t = false)]
554548
pub fix: bool,
549+
550+
/// Do not error on `cairo-version` mismatch.
551+
#[arg(long)]
552+
pub ignore_cairo_version: bool,
555553
}
556554

557555
/// Git reference specification arguments.

scarb/src/bin/scarb/commands/lint.rs

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ pub fn run(args: LintArgs, config: &Config) -> Result<()> {
1818
packages,
1919
test: args.test,
2020
fix: args.fix,
21+
ignore_cairo_version: args.ignore_cairo_version,
2122
},
2223
&ws,
2324
)

scarb/src/ops/lint.rs

+4-13
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@ use cairo_lang_filesystem::db::FilesGroup;
1414
use cairo_lang_filesystem::ids::{CrateLongId, FileId};
1515
use cairo_lang_semantic::db::SemanticGroup;
1616
use cairo_lang_semantic::diagnostic::SemanticDiagnosticKind;
17-
use cairo_lang_starknet::starknet_plugin_suite;
1817
use cairo_lang_syntax::node::SyntaxNode;
19-
use cairo_lang_test_plugin::test_plugin_suite;
2018
use cairo_lang_utils::Upcast;
2119
use cairo_lint_core::{
2220
diagnostics::format_diagnostic,
@@ -36,6 +34,7 @@ pub struct LintOptions {
3634
pub packages: Vec<Package>,
3735
pub test: bool,
3836
pub fix: bool,
37+
pub ignore_cairo_version: bool,
3938
}
4039

4140
#[tracing::instrument(skip_all, level = "debug")]
@@ -52,8 +51,8 @@ pub fn lint(opts: LintOptions, ws: &Workspace<'_>) -> Result<()> {
5251
&feature_opts,
5352
ws,
5453
CompilationUnitsOpts {
55-
ignore_cairo_version: false,
56-
load_prebuilt_macros: false,
54+
ignore_cairo_version: opts.ignore_cairo_version,
55+
load_prebuilt_macros: true,
5756
},
5857
)?;
5958

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

98-
let additional_plugins = if opts.test {
99-
vec![
100-
test_plugin_suite(),
101-
cairo_lint_plugin_suite(),
102-
starknet_plugin_suite(),
103-
]
104-
} else {
105-
vec![cairo_lint_plugin_suite(), starknet_plugin_suite()]
106-
};
97+
let additional_plugins = vec![cairo_lint_plugin_suite()];
10798
let ScarbDatabase { db, .. } =
10899
build_scarb_root_database(compilation_unit, ws, additional_plugins)?;
109100

scarb/tests/lint.rs

+87-29
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use assert_fs::fixture::FileWriteStr;
12
use assert_fs::{prelude::PathChild, TempDir};
23
use indoc::indoc;
34
use scarb_test_support::{
@@ -19,18 +20,20 @@ fn lint_main_package() {
1920
"#})
2021
.build(&t);
2122

22-
Scarb::quick_snapbox()
23+
Scarb::quick_snapbox().env("CLICOLOR", "0")
2324
.arg("lint")
2425
.current_dir(&t)
2526
.assert()
27+
// Current expected values include ANSI color codes because lint has custom renderer.
2628
.stdout_matches(indoc! {r#"
27-
Checking hello v1.0.0 ([..]Scarb.toml)
28-
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
29-
--> [..]
30-
|
31-
3 | if x == false {
32-
| ----------
33-
|
29+
Checking hello v1.0.0 ([..]/Scarb.toml)
30+
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
31+
--> [..]/lib.cairo:3:8
32+
|
33+
3 | if x == false {
34+
| ----------
35+
|
36+
3437
"#})
3538
.success();
3639
}
@@ -79,27 +82,82 @@ fn lint_workspace() {
7982
.arg("--workspace")
8083
.current_dir(&t)
8184
.assert()
85+
// Current expected values include ANSI color codes because lint has custom renderer.
8286
.stdout_matches(indoc! {r#"
83-
Checking first v1.0.0 ([..]first/Scarb.toml)
84-
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
85-
--> [..]/lib.cairo:3:8
86-
|
87-
3 | if first == false {
88-
| --------------
89-
|
90-
Checking main v1.0.0 ([..]/Scarb.toml)
91-
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
92-
--> [..]/lib.cairo:3:8
93-
|
94-
3 | if _main == false {
95-
| --------------
96-
|
97-
Checking second v1.0.0 ([..]second/Scarb.toml)
98-
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
99-
--> [..]/lib.cairo:3:8
100-
|
101-
3 | if second == false {
102-
| ---------------
103-
|
87+
Checking first v1.0.0 ([..]/first/Scarb.toml)
88+
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
89+
--> [..]/lib.cairo:3:8
90+
|
91+
3 | if first == false {
92+
| --------------
93+
|
94+
95+
Checking main v1.0.0 ([..]/Scarb.toml)
96+
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
97+
--> [..]/lib.cairo:3:8
98+
|
99+
3 | if _main == false {
100+
| --------------
101+
|
102+
103+
Checking second v1.0.0 ([..]/second/Scarb.toml)
104+
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
105+
--> [..]/lib.cairo:3:8
106+
|
107+
3 | if second == false {
108+
| ---------------
109+
|
110+
104111
"#}).success();
105112
}
113+
114+
#[test]
115+
fn lint_integration_tests() {
116+
let t = TempDir::new().unwrap();
117+
ProjectBuilder::start()
118+
.name("hello")
119+
.lib_cairo(indoc! {r#"
120+
pub fn f1() -> u32 {
121+
42
122+
}
123+
124+
fn main() {
125+
// This is a comment
126+
}
127+
"#})
128+
.dep_cairo_test()
129+
.build(&t);
130+
t.child("tests/test1.cairo")
131+
.write_str(indoc! {r#"
132+
use hello::f1;
133+
#[test]
134+
fn it_works() {
135+
let x = true;
136+
if false == x {
137+
println!("x is false");
138+
}
139+
assert_eq!(1, f1());
140+
}
141+
"#})
142+
.unwrap();
143+
144+
Scarb::quick_snapbox()
145+
.arg("lint")
146+
.arg("-t")
147+
.current_dir(&t)
148+
.assert()
149+
// Current expected values include ANSI color codes because lint has custom renderer.
150+
.stdout_matches(indoc! {r#"
151+
Checking hello v1.0.0 ([..]/Scarb.toml)
152+
Checking test(hello_unittest) hello v1.0.0 ([..]/Scarb.toml)
153+
Checking test(hello_integrationtest) hello_integrationtest v1.0.0 ([..]/Scarb.toml)
154+
warning: Plugin diagnostic: Unnecessary comparison with a boolean value. Use the variable directly.
155+
--> [..]/tests/test1.cairo:5:8
156+
|
157+
5 | if false == x {
158+
| ----------
159+
|
160+
161+
"#})
162+
.success();
163+
}

0 commit comments

Comments
 (0)