diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index cca91d3c15974..149a3b9cfebb2 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -73,6 +73,28 @@ jobs: ${{ runner.os }}- - run: cargo clippy -- -D warnings + cargo_test: + name: "cargo test" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1 + with: + toolchain: stable + - uses: actions/cache@v3 + env: + cache-name: cache-cargo + with: + path: | + ~/.cargo/registry + ~/.cargo/git + key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/Cargo.lock') }} + restore-keys: | + ${{ runner.os }}-build-${{ env.cache-name }}- + ${{ runner.os }}-build- + ${{ runner.os }}- + - run: cargo test + maturin_build: name: "maturin build" runs-on: ubuntu-latest diff --git a/resources/test/src/duplicate_argument_name.py b/resources/test/src/duplicate_argument_name.py new file mode 100644 index 0000000000000..b866b4938f713 --- /dev/null +++ b/resources/test/src/duplicate_argument_name.py @@ -0,0 +1,10 @@ +def foo(x: int, y: int, x: int) -> None: + pass + + +def bar(x: int, y: int, *, x: int) -> None: + pass + + +def baz(x: int, y: int, **x: int) -> None: + pass diff --git a/resources/test/src/foo.py b/resources/test/src/foo.py deleted file mode 100644 index 2c739920afdc4..0000000000000 --- a/resources/test/src/foo.py +++ /dev/null @@ -1,5 +0,0 @@ -from bar import * - - -def baz(x: int, x: int) -> None: - pass diff --git a/resources/test/src/bar.py b/resources/test/src/if_tuple.py similarity index 82% rename from resources/test/src/bar.py rename to resources/test/src/if_tuple.py index d0c5c49e10df0..501a7f8ad23ec 100644 --- a/resources/test/src/bar.py +++ b/resources/test/src/if_tuple.py @@ -4,5 +4,5 @@ for _ in range(5): if True: pass - elif (1, 2): + elif (3, 4): pass diff --git a/resources/test/src/import_star_usage.py b/resources/test/src/import_star_usage.py new file mode 100644 index 0000000000000..ebebd784844ca --- /dev/null +++ b/resources/test/src/import_star_usage.py @@ -0,0 +1 @@ +from if_tuple import * diff --git a/src/checker.rs b/src/checker.rs index beb2599ca7af5..c5de2f9ef1d2f 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -2,19 +2,14 @@ use std::collections::HashSet; use rustpython_parser::ast::{Arg, Arguments, ExprKind, Stmt, StmtKind, Suite}; -use crate::check::{Check, CheckKind}; +use crate::checks::{Check, CheckKind}; use crate::visitor::{walk_arguments, walk_stmt, Visitor}; +#[derive(Default)] struct Checker { checks: Vec, } -impl Checker { - fn new() -> Self { - Checker { checks: vec![] } - } -} - impl Visitor for Checker { fn visit_stmt(&mut self, stmt: &Stmt) { match &stmt.node { @@ -79,9 +74,46 @@ pub fn check_ast(python_ast: &Suite) -> Vec { python_ast .iter() .flat_map(|stmt| { - let mut checker = Checker::new(); + let mut checker: Checker = Default::default(); checker.visit_stmt(stmt); checker.checks }) .collect() } + +#[cfg(test)] +mod tests { + use rustpython_parser::ast::{Alias, Location, Stmt, StmtKind}; + + use crate::checker::Checker; + use crate::checks::Check; + use crate::checks::CheckKind::ImportStarUsage; + use crate::visitor::Visitor; + + #[test] + fn import_star_usage() { + let mut checker: Checker = Default::default(); + checker.visit_stmt(&Stmt { + location: Location::new(1, 1), + custom: (), + node: StmtKind::ImportFrom { + module: Some("bar".to_string()), + names: vec![Alias { + name: "*".to_string(), + asname: None, + }], + level: 0, + }, + }); + + let actual = checker.checks; + let expected = vec![Check { + kind: ImportStarUsage, + location: Location::new(1, 1), + }]; + assert_eq!(actual.len(), expected.len()); + for i in 1..actual.len() { + assert_eq!(actual[i], expected[i]); + } + } +} diff --git a/src/check.rs b/src/checks.rs similarity index 91% rename from src/check.rs rename to src/checks.rs index 43fd027b9ca95..adebda18af0d2 100644 --- a/src/check.rs +++ b/src/checks.rs @@ -1,7 +1,7 @@ use rustpython_parser::ast::Location; use serde::{Deserialize, Serialize}; -#[derive(Serialize, Deserialize)] +#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] pub enum CheckKind { DuplicateArgumentName, ImportStarUsage, @@ -28,6 +28,7 @@ impl CheckKind { } } +#[derive(Debug, PartialEq, Eq)] pub struct Check { pub kind: CheckKind, pub location: Location, diff --git a/src/lib.rs b/src/lib.rs index 109e063155c7b..61547eeabb5b8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,6 @@ mod cache; -mod check; pub mod checker; +mod checks; pub mod fs; pub mod linter; pub mod logging; diff --git a/src/linter.rs b/src/linter.rs index a07c0221c559d..209fc881fbf42 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -28,3 +28,92 @@ pub fn check_path(path: &Path, mode: &cache::Mode) -> Result> { Ok(messages) } + +#[cfg(test)] +mod tests { + use std::path::Path; + + use anyhow::Result; + use rustpython_parser::ast::Location; + + use crate::cache; + use crate::checks::CheckKind::{DuplicateArgumentName, IfTuple, ImportStarUsage}; + use crate::linter::check_path; + use crate::message::Message; + + #[test] + fn duplicate_argument_name() -> Result<()> { + let actual = check_path( + &Path::new("./resources/test/src/duplicate_argument_name.py"), + &cache::Mode::None, + )?; + let expected = vec![ + Message { + kind: DuplicateArgumentName, + location: Location::new(1, 25), + filename: "./resources/test/src/duplicate_argument_name.py".to_string(), + }, + Message { + kind: DuplicateArgumentName, + location: Location::new(5, 9), + filename: "./resources/test/src/duplicate_argument_name.py".to_string(), + }, + Message { + kind: DuplicateArgumentName, + location: Location::new(9, 27), + filename: "./resources/test/src/duplicate_argument_name.py".to_string(), + }, + ]; + assert_eq!(actual.len(), expected.len()); + for i in 1..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } + + #[test] + fn if_tuple() -> Result<()> { + let actual = check_path( + &Path::new("./resources/test/src/if_tuple.py"), + &cache::Mode::None, + )?; + let expected = vec![ + Message { + kind: IfTuple, + location: Location::new(1, 1), + filename: "./resources/test/src/if_tuple.py".to_string(), + }, + Message { + kind: IfTuple, + location: Location::new(7, 5), + filename: "./resources/test/src/if_tuple.py".to_string(), + }, + ]; + assert_eq!(actual.len(), expected.len()); + for i in 1..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } + + #[test] + fn import_star_usage() -> Result<()> { + let actual = check_path( + &Path::new("./resources/test/src/import_star_usage.py"), + &cache::Mode::None, + )?; + let expected = vec![Message { + kind: ImportStarUsage, + location: Location::new(1, 1), + filename: "./resources/test/src/import_star_usage.py".to_string(), + }]; + assert_eq!(actual.len(), expected.len()); + for i in 1..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } +} diff --git a/src/message.rs b/src/message.rs index b72400a6eb425..b6d3c38b92723 100644 --- a/src/message.rs +++ b/src/message.rs @@ -4,7 +4,7 @@ use colored::Colorize; use rustpython_parser::ast::Location; use serde::{Deserialize, Serialize}; -use crate::check::CheckKind; +use crate::checks::CheckKind; #[derive(Serialize, Deserialize)] #[serde(remote = "Location")] @@ -21,7 +21,7 @@ impl From for Location { } } -#[derive(Serialize, Deserialize)] +#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct Message { pub kind: CheckKind, #[serde(with = "LocationDef")]