diff --git a/.config/ast-grep/rule-tests/__snapshots__/is-err-then-unwrap-err-snapshot.yml b/.config/ast-grep/rule-tests/__snapshots__/is-err-then-unwrap-err-snapshot.yml new file mode 100644 index 0000000..dff7614 --- /dev/null +++ b/.config/ast-grep/rule-tests/__snapshots__/is-err-then-unwrap-err-snapshot.yml @@ -0,0 +1,22 @@ +id: is-err-then-unwrap-err +snapshots: + ? | + if result.is_err() { + report(result.unwrap_err()); + } + : labels: + - source: |- + if result.is_err() { + report(result.unwrap_err()); + } + style: primary + start: 0 + end: 55 + - source: result.is_err() + style: secondary + start: 3 + end: 18 + - source: result.unwrap_err() + style: secondary + start: 32 + end: 51 diff --git a/.config/ast-grep/rule-tests/__snapshots__/is-ok-then-unwrap-snapshot.yml b/.config/ast-grep/rule-tests/__snapshots__/is-ok-then-unwrap-snapshot.yml new file mode 100644 index 0000000..5c65957 --- /dev/null +++ b/.config/ast-grep/rule-tests/__snapshots__/is-ok-then-unwrap-snapshot.yml @@ -0,0 +1,22 @@ +id: is-ok-then-unwrap +snapshots: + ? | + if result.is_ok() { + consume(result.unwrap()); + } + : labels: + - source: |- + if result.is_ok() { + consume(result.unwrap()); + } + style: primary + start: 0 + end: 51 + - source: result.is_ok() + style: secondary + start: 3 + end: 17 + - source: result.unwrap() + style: secondary + start: 32 + end: 47 diff --git a/.config/ast-grep/rule-tests/__snapshots__/is-some-then-unwrap-snapshot.yml b/.config/ast-grep/rule-tests/__snapshots__/is-some-then-unwrap-snapshot.yml new file mode 100644 index 0000000..168d872 --- /dev/null +++ b/.config/ast-grep/rule-tests/__snapshots__/is-some-then-unwrap-snapshot.yml @@ -0,0 +1,22 @@ +id: is-some-then-unwrap +snapshots: + ? | + if value.is_some() { + consume(value.unwrap()); + } + : labels: + - source: |- + if value.is_some() { + consume(value.unwrap()); + } + style: primary + start: 0 + end: 51 + - source: value.is_some() + style: secondary + start: 3 + end: 18 + - source: value.unwrap() + style: secondary + start: 33 + end: 47 diff --git a/.config/ast-grep/rule-tests/__snapshots__/prefer-less-than-comparisons-snapshot.yml b/.config/ast-grep/rule-tests/__snapshots__/prefer-less-than-comparisons-snapshot.yml new file mode 100644 index 0000000..b2242ac --- /dev/null +++ b/.config/ast-grep/rule-tests/__snapshots__/prefer-less-than-comparisons-snapshot.yml @@ -0,0 +1,14 @@ +id: prefer-less-than-comparisons +snapshots: + larger > smaller: + labels: + - source: larger > smaller + style: primary + start: 0 + end: 16 + larger >= smaller: + labels: + - source: larger >= smaller + style: primary + start: 0 + end: 17 diff --git a/.config/ast-grep/rule-tests/__snapshots__/unwrap-in-production-snapshot.yml b/.config/ast-grep/rule-tests/__snapshots__/unwrap-in-production-snapshot.yml new file mode 100644 index 0000000..d8539f4 --- /dev/null +++ b/.config/ast-grep/rule-tests/__snapshots__/unwrap-in-production-snapshot.yml @@ -0,0 +1,21 @@ +id: unwrap-in-production +snapshots: + ? | + #[cfg(test)] + fn test_helper() {} + + #[allow(dead_code)] + fn production() { + value.unwrap(); + } + : labels: + - source: value.unwrap() + style: primary + start: 76 + end: 90 + value.unwrap(): + labels: + - source: value.unwrap() + style: primary + start: 0 + end: 14 diff --git a/.config/ast-grep/rule-tests/__snapshots__/unwrap-or-default-snapshot.yml b/.config/ast-grep/rule-tests/__snapshots__/unwrap-or-default-snapshot.yml new file mode 100644 index 0000000..82fb7b0 --- /dev/null +++ b/.config/ast-grep/rule-tests/__snapshots__/unwrap-or-default-snapshot.yml @@ -0,0 +1,14 @@ +id: unwrap-or-default +snapshots: + value.unwrap_or(Default::default()): + labels: + - source: value.unwrap_or(Default::default()) + style: primary + start: 0 + end: 35 + value.unwrap_or(String::default()): + labels: + - source: value.unwrap_or(String::default()) + style: primary + start: 0 + end: 34 diff --git a/.config/ast-grep/rule-tests/is-err-then-unwrap-err-test.yml b/.config/ast-grep/rule-tests/is-err-then-unwrap-err-test.yml new file mode 100644 index 0000000..f4b79f9 --- /dev/null +++ b/.config/ast-grep/rule-tests/is-err-then-unwrap-err-test.yml @@ -0,0 +1,15 @@ +id: is-err-then-unwrap-err +valid: + - | + if let Err(error) = result { + report(error); + } + - | + if result.is_err() { + report(other.unwrap_err()); + } +invalid: + - | + if result.is_err() { + report(result.unwrap_err()); + } diff --git a/.config/ast-grep/rule-tests/is-ok-then-unwrap-test.yml b/.config/ast-grep/rule-tests/is-ok-then-unwrap-test.yml new file mode 100644 index 0000000..760b92f --- /dev/null +++ b/.config/ast-grep/rule-tests/is-ok-then-unwrap-test.yml @@ -0,0 +1,15 @@ +id: is-ok-then-unwrap +valid: + - | + if let Ok(value) = result { + consume(value); + } + - | + if result.is_ok() { + consume(other.unwrap()); + } +invalid: + - | + if result.is_ok() { + consume(result.unwrap()); + } diff --git a/.config/ast-grep/rule-tests/is-some-then-unwrap-test.yml b/.config/ast-grep/rule-tests/is-some-then-unwrap-test.yml new file mode 100644 index 0000000..58abee4 --- /dev/null +++ b/.config/ast-grep/rule-tests/is-some-then-unwrap-test.yml @@ -0,0 +1,15 @@ +id: is-some-then-unwrap +valid: + - | + if let Some(value) = value { + consume(value); + } + - | + if value.is_some() { + consume(other.unwrap()); + } +invalid: + - | + if value.is_some() { + consume(value.unwrap()); + } diff --git a/.config/ast-grep/rule-tests/prefer-less-than-comparisons-test.yml b/.config/ast-grep/rule-tests/prefer-less-than-comparisons-test.yml new file mode 100644 index 0000000..8f4d7b7 --- /dev/null +++ b/.config/ast-grep/rule-tests/prefer-less-than-comparisons-test.yml @@ -0,0 +1,9 @@ +id: prefer-less-than-comparisons +valid: + - smaller < larger + - smaller <= larger + - next_left() > next_right() + - object.left > object.right +invalid: + - larger > smaller + - larger >= smaller diff --git a/.config/ast-grep/rule-tests/unwrap-in-production-test.yml b/.config/ast-grep/rule-tests/unwrap-in-production-test.yml new file mode 100644 index 0000000..bb96f4c --- /dev/null +++ b/.config/ast-grep/rule-tests/unwrap-in-production-test.yml @@ -0,0 +1,60 @@ +id: unwrap-in-production +valid: + - value? + - value.expect("value was validated above") + - | + #[cfg(test)] + mod tests { + #[test] + fn works() { + value.unwrap(); + } + } + - | + #[cfg(test)] + fn test_helper() { + value.unwrap(); + } + - | + #[test] + fn works() { + value.unwrap(); + } + - | + #[test] + #[should_panic] + fn panics() { + value.unwrap(); + } + - | + #[cfg(test)] + #[allow(dead_code)] + fn test_helper() { + value.unwrap(); + } + - | + #[cfg(test)] + #[allow(dead_code)] + mod tests { + fn helper() { + value.unwrap(); + } + } + - | + #[cfg(test)] + #[allow(dead_code)] + impl Fixture { + fn helper() { + value.unwrap(); + } + } +invalid: + - value.unwrap() + - | + #[cfg(test)] + fn test_helper() {} + + #[allow(dead_code)] + fn production() { + value.unwrap(); + } diff --git a/.config/ast-grep/rule-tests/unwrap-or-default-test.yml b/.config/ast-grep/rule-tests/unwrap-or-default-test.yml new file mode 100644 index 0000000..8d62342 --- /dev/null +++ b/.config/ast-grep/rule-tests/unwrap-or-default-test.yml @@ -0,0 +1,7 @@ +id: unwrap-or-default +valid: + - value.unwrap_or_default() + - value.unwrap_or(fallback) +invalid: + - value.unwrap_or(Default::default()) + - value.unwrap_or(String::default()) diff --git a/.config/ast-grep/rules/is-err-then-unwrap-err.yml b/.config/ast-grep/rules/is-err-then-unwrap-err.yml new file mode 100644 index 0000000..f7a0f4b --- /dev/null +++ b/.config/ast-grep/rules/is-err-then-unwrap-err.yml @@ -0,0 +1,14 @@ +id: is-err-then-unwrap-err +language: Rust +severity: warning +message: Replace an is_err() check followed by unwrap_err() with if let Err(...). +rule: + kind: if_expression + all: + - has: + field: condition + pattern: $VALUE.is_err() + - has: + field: consequence + stopBy: end + pattern: $VALUE.unwrap_err() diff --git a/.config/ast-grep/rules/is-ok-then-unwrap.yml b/.config/ast-grep/rules/is-ok-then-unwrap.yml new file mode 100644 index 0000000..7bc579f --- /dev/null +++ b/.config/ast-grep/rules/is-ok-then-unwrap.yml @@ -0,0 +1,14 @@ +id: is-ok-then-unwrap +language: Rust +severity: warning +message: Replace an is_ok() check followed by unwrap() with if let Ok(...). +rule: + kind: if_expression + all: + - has: + field: condition + pattern: $VALUE.is_ok() + - has: + field: consequence + stopBy: end + pattern: $VALUE.unwrap() diff --git a/.config/ast-grep/rules/is-some-then-unwrap.yml b/.config/ast-grep/rules/is-some-then-unwrap.yml new file mode 100644 index 0000000..70ff4fd --- /dev/null +++ b/.config/ast-grep/rules/is-some-then-unwrap.yml @@ -0,0 +1,14 @@ +id: is-some-then-unwrap +language: Rust +severity: warning +message: Replace an is_some() check followed by unwrap() with if let Some(...). +rule: + kind: if_expression + all: + - has: + field: condition + pattern: $VALUE.is_some() + - has: + field: consequence + stopBy: end + pattern: $VALUE.unwrap() diff --git a/.config/ast-grep/rules/prefer-less-than-comparisons.yml b/.config/ast-grep/rules/prefer-less-than-comparisons.yml new file mode 100644 index 0000000..5d98df2 --- /dev/null +++ b/.config/ast-grep/rules/prefer-less-than-comparisons.yml @@ -0,0 +1,17 @@ +id: prefer-less-than-comparisons +language: Rust +severity: hint +message: Put the lesser value on the left and use < or <= when comparing simple values. +rule: + any: + - pattern: $LEFT > $RIGHT + - pattern: $LEFT >= $RIGHT +constraints: + LEFT: + any: + - kind: identifier + - kind: integer_literal + RIGHT: + any: + - kind: identifier + - kind: integer_literal diff --git a/.config/ast-grep/rules/unwrap-in-production.yml b/.config/ast-grep/rules/unwrap-in-production.yml new file mode 100644 index 0000000..ed4c8ec --- /dev/null +++ b/.config/ast-grep/rules/unwrap-in-production.yml @@ -0,0 +1,40 @@ +id: unwrap-in-production +language: Rust +severity: warning +message: Avoid unwrap() outside test code; propagate the error or document the invariant with expect(). +ignores: + - "**/benches/**" + - "**/examples/**" + - "**/tests/**" + - "**/tests.rs" + - "**/*-tests.rs" + - "**/*_test.rs" + - "**/*_tests.rs" + - "**/test_*.rs" +rule: + all: + - pattern: $VALUE.unwrap() + - not: + inside: + stopBy: end + any: + - kind: mod_item + follows: + stopBy: + not: + kind: attribute_item + pattern: "#[cfg(test)]" + - kind: function_item + follows: + stopBy: + not: + kind: attribute_item + any: + - pattern: "#[cfg(test)]" + - pattern: "#[test]" + - kind: impl_item + follows: + stopBy: + not: + kind: attribute_item + pattern: "#[cfg(test)]" diff --git a/.config/ast-grep/rules/unwrap-or-default.yml b/.config/ast-grep/rules/unwrap-or-default.yml new file mode 100644 index 0000000..ab0f57e --- /dev/null +++ b/.config/ast-grep/rules/unwrap-or-default.yml @@ -0,0 +1,8 @@ +id: unwrap-or-default +language: Rust +severity: warning +message: Use unwrap_or_default() instead of constructing a default value eagerly. +rule: + any: + - pattern: $VALUE.unwrap_or(Default::default()) + - pattern: $VALUE.unwrap_or($TYPE::default()) diff --git a/checks/ast-grep-rules.nix b/checks/ast-grep-rules.nix new file mode 100644 index 0000000..f9f8f67 --- /dev/null +++ b/checks/ast-grep-rules.nix @@ -0,0 +1,45 @@ +{ pkgs }: +pkgs.runCommand "ast-grep-rules" + { + nativeBuildInputs = [ pkgs.ast-grep ]; + src = pkgs.lib.cleanSource ../.; + } + '' + cp -r "$src" source + chmod -R u+w source + cd source + + # Promote the behavioral rules to CI errors while retaining the comparison + # orientation rule as an interactive style hint. + ast-grep scan --error --hint=prefer-less-than-comparisons + ast-grep test + + mkdir -p \ + ast-grep-fixtures/benches \ + ast-grep-fixtures/examples \ + ast-grep-fixtures/tests \ + ast-grep-fixtures/src + for path in \ + ast-grep-fixtures/benches/case.rs \ + ast-grep-fixtures/examples/case.rs \ + ast-grep-fixtures/tests/case.rs \ + ast-grep-fixtures/src/check-tests.rs \ + ast-grep-fixtures/src/check_test.rs \ + ast-grep-fixtures/src/check_tests.rs \ + ast-grep-fixtures/src/test_check.rs \ + ast-grep-fixtures/src/tests.rs + do + printf 'fn fixture() { value.unwrap(); }\n' > "$path" + ast-grep scan --error "$path" + done + + production_fixture=ast-grep-fixtures/src/check.rs + printf 'fn fixture() { value.unwrap(); }\n' > "$production_fixture" + if ast-grep scan --error "$production_fixture" > violation.log 2>&1; then + echo "production unwrap fixture unexpectedly passed" >&2 + exit 1 + fi + grep -Fq 'unwrap-in-production' violation.log + + touch "$out" + '' diff --git a/checks/cell-aware-cross-inputs/src/main.rs b/checks/cell-aware-cross-inputs/src/main.rs index fa0f5a4..0525c6d 100644 --- a/checks/cell-aware-cross-inputs/src/main.rs +++ b/checks/cell-aware-cross-inputs/src/main.rs @@ -1,5 +1,7 @@ use openssl::ssl::{SslConnector, SslMethod}; fn main() { - let _connector = SslConnector::builder(SslMethod::tls()).unwrap().build(); + let _connector = SslConnector::builder(SslMethod::tls()) + .expect("OpenSSL supports TLS") + .build(); } diff --git a/checks/default.nix b/checks/default.nix index 2c26bf6..096c979 100644 --- a/checks/default.nix +++ b/checks/default.nix @@ -24,6 +24,7 @@ onlyDrvs ( craneMultiBuildTests = callPackage ./crane-multi-build-tests.nix { }; cargoCrapModule = callPackage ./cargo-crap-module.nix { inherit mkLib; }; astGrepModule = callPackage ./ast-grep-module.nix { inherit mkLib; }; + astGrepRules = callPackage ./ast-grep-rules.nix { }; gitHooks = callPackage ./git-hooks.nix { inherit mkLib; }; } // lib.optionalAttrs (pkgs.stdenv.buildPlatform.system == "x86_64-linux") { diff --git a/lib/pkgs/git-hook-check-timer.rs b/lib/pkgs/git-hook-check-timer.rs index fae7304..a9b12a8 100644 --- a/lib/pkgs/git-hook-check-timer.rs +++ b/lib/pkgs/git-hook-check-timer.rs @@ -72,7 +72,7 @@ impl ChildSignalState { if current_state == SHUTTING_DOWN { return None; } - if current_state > 0 { + if 0 < current_state { return Some(current_state); } diff --git a/sgconfig.yml b/sgconfig.yml new file mode 100644 index 0000000..732904d --- /dev/null +++ b/sgconfig.yml @@ -0,0 +1,5 @@ +ruleDirs: + - .config/ast-grep/rules + +testConfigs: + - testDir: .config/ast-grep/rule-tests