From 15ccd864c8869f87a564e5ab7e923dcc2bb54340 Mon Sep 17 00:00:00 2001 From: Akshay Date: Sun, 31 Oct 2021 21:31:04 +0530 Subject: add explainations to each lint --- bin/src/main.rs | 4 ++-- lib/src/lints/bool_comparison.rs | 27 ++++++++++++--------------- lib/src/lints/collapsible_let_in.rs | 28 ++++++++++++++++++++++++++++ lib/src/lints/empty_let_in.rs | 19 ++++++++++++++++++- lib/src/lints/empty_pattern.rs | 27 +++++++++++++++++++++++++++ lib/src/lints/eta_reduction.rs | 26 ++++++++++++++++++++++++++ lib/src/lints/legacy_let_syntax.rs | 28 ++++++++++++++++++++++++++++ lib/src/lints/manual_inherit.rs | 24 ++++++++++++++++++++++++ lib/src/lints/manual_inherit_from.rs | 24 ++++++++++++++++++++++++ lib/src/lints/redundant_pattern_bind.rs | 21 ++++++++++++++++++++- lib/src/lints/unquoted_splice.rs | 24 ++++++++++++++++++++++++ lib/src/lints/useless_parens.rs | 25 +++++++++++++++++++++++++ macros/src/explain.rs | 14 +++++++------- 13 files changed, 265 insertions(+), 26 deletions(-) diff --git a/bin/src/main.rs b/bin/src/main.rs index 138b243..31f6823 100644 --- a/bin/src/main.rs +++ b/bin/src/main.rs @@ -88,8 +88,8 @@ fn _main() -> Result<(), StatixErr> { } } SubCommand::Explain(explain_config) => { - let explaination = explain::explain(explain_config.target)?; - println!("{}", explaination); + let explanation = explain::explain(explain_config.target)?; + println!("{}", explanation) } } Ok(()) diff --git a/lib/src/lints/bool_comparison.rs b/lib/src/lints/bool_comparison.rs index 6636faf..5c9bee8 100644 --- a/lib/src/lints/bool_comparison.rs +++ b/lib/src/lints/bool_comparison.rs @@ -7,28 +7,25 @@ use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode, }; -/// What it does -/// ------------ -/// Checks for expressions of the form x == true, x != true and +/// ## What it does +/// Checks for expressions of the form `x == true`, `x != true` and /// suggests using the variable directly. /// -/// Why is this bad? -/// ---------------- +/// ## Why is this bad? /// Unnecessary code. /// -/// Example -/// -------- -/// Instead of checking the value of x: +/// ## Example +/// Instead of checking the value of `x`: /// -/// if x == true -/// then 0 -/// else 1 +/// ``` +/// if x == true then 0 else 1 +/// ``` /// -/// Use x directly: +/// Use `x` directly: /// -/// if x -/// then 0 -/// else 1 +/// ``` +/// if x then 0 else 1 +/// ``` #[lint( name = "bool_comparison", note = "Unnecessary comparison with boolean", diff --git a/lib/src/lints/collapsible_let_in.rs b/lib/src/lints/collapsible_let_in.rs index cb76c37..21199a8 100644 --- a/lib/src/lints/collapsible_let_in.rs +++ b/lib/src/lints/collapsible_let_in.rs @@ -8,6 +8,34 @@ use rnix::{ }; use rowan::Direction; +/// ## What it does +/// Checks for `let-in` expressions whose body is another `let-in` +/// expression. +/// +/// ## Why is this bad? +/// Unnecessary code, the `let-in` expressions can be merged. +/// +/// ## Example +/// +/// ``` +/// let +/// a = 2; +/// in +/// let +/// b = 3; +/// in +/// a + b +/// ``` +/// +/// Merge both `let-in` expressions: +/// +/// ``` +/// let +/// a = 2; +/// b = 3; +/// in +/// a + b +/// ``` #[lint( name = "collapsible let in", note = "These let-in expressions are collapsible", diff --git a/lib/src/lints/empty_let_in.rs b/lib/src/lints/empty_let_in.rs index 4e0887d..b255c23 100644 --- a/lib/src/lints/empty_let_in.rs +++ b/lib/src/lints/empty_let_in.rs @@ -7,7 +7,24 @@ use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, }; -/// empty let-in found +/// ## What it does +/// Checks for `let-in` expressions which create no new bindings. +/// +/// ## Why is this bad? +/// `let-in` expressions that create no new bindings are useless. +/// These are probably remnants from debugging or editing expressions. +/// +/// ## Example +/// +/// ``` +/// let in pkgs.statix +/// ``` +/// +/// Preserve only the body of the `let-in` expression: +/// +/// ``` +/// pkgs.statix +/// ``` #[lint( name = "empty let-in", note = "Useless let-in expression", diff --git a/lib/src/lints/empty_pattern.rs b/lib/src/lints/empty_pattern.rs index ef47c69..5312548 100644 --- a/lib/src/lints/empty_pattern.rs +++ b/lib/src/lints/empty_pattern.rs @@ -7,6 +7,33 @@ use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, }; +/// ## What it does +/// Checks for an empty variadic pattern: `{...}`, in a function +/// argument. +/// +/// ## Why is this bad? +/// The intention with empty patterns is not instantly obvious. Prefer +/// an underscore identifier instead, to indicate that the argument +/// is being ignored. +/// +/// ## Example +/// +/// ``` +/// client = { ... }: { +/// imports = [ self.nixosModules.irmaseal-pkg ]; +/// services.irmaseal-pkg.enable = true; +/// }; +/// ``` +/// +/// Replace the empty variadic pattern with `_` to indicate that you +/// intend to ignore the argument: +/// +/// ``` +/// client = _: { +/// imports = [ self.nixosModules.irmaseal-pkg ]; +/// services.irmaseal-pkg.enable = true; +/// }; +/// ``` #[lint( name = "empty pattern", note = "Found empty pattern in function argument", diff --git a/lib/src/lints/eta_reduction.rs b/lib/src/lints/eta_reduction.rs index b329ae8..3a483d0 100644 --- a/lib/src/lints/eta_reduction.rs +++ b/lib/src/lints/eta_reduction.rs @@ -7,6 +7,32 @@ use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode, }; +/// ## What it does +/// Checks for eta-reducible functions, i.e.: converts lambda +/// expressions into free standing functions where applicable. +/// +/// ## Why is this bad? +/// Oftentimes, eta-reduction results in code that is more natural +/// to read. +/// +/// ## Example +/// +/// ``` +/// let +/// double = i: 2 * i; +/// in +/// map (x: double x) [ 1 2 3 ] +/// ``` +/// +/// The lambda passed to the `map` function is eta-reducible, and the +/// result reads more naturally: +/// +/// ``` +/// let +/// double = i: 2 * i; +/// in +/// map double [ 1 2 3 ] +/// ``` #[lint( name = "eta reduction", note = "This function expression is eta reducible", diff --git a/lib/src/lints/legacy_let_syntax.rs b/lib/src/lints/legacy_let_syntax.rs index 8b37df8..139f633 100644 --- a/lib/src/lints/legacy_let_syntax.rs +++ b/lib/src/lints/legacy_let_syntax.rs @@ -7,6 +7,34 @@ use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, }; +/// ## What it does +/// Checks for legacy-let syntax that was never formalized. +/// +/// ## Why is this bad? +/// This syntax construct is undocumented, refrain from using it. +/// +/// ## Example +/// +/// Legacy let syntax makes use of an attribute set annotated with +/// `let` and expects a `body` attribute. +/// ``` +/// let { +/// body = x + y; +/// x = 2; +/// y = 3; +/// } +/// ``` +/// +/// This is trivially representible via `rec`, which is documented +/// and more widely known: +/// +/// ``` +/// rec { +/// body = x + y; +/// x = 2; +/// y = 3; +/// }.body +/// ``` #[lint( name = "legacy let syntax", note = "Using undocumented `let` syntax", diff --git a/lib/src/lints/manual_inherit.rs b/lib/src/lints/manual_inherit.rs index 1c10cbf..2d119c3 100644 --- a/lib/src/lints/manual_inherit.rs +++ b/lib/src/lints/manual_inherit.rs @@ -7,6 +7,30 @@ use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, }; +/// ## What it does +/// Checks for bindings of the form `a = a`. +/// +/// ## Why is this bad? +/// If the aim is to bring attributes from a larger scope into +/// the current scope, prefer an inherit statement. +/// +/// ## Example +/// +/// ``` +/// let +/// a = 2; +/// in +/// { a = a; b = 3; } +/// ``` +/// +/// Try `inherit` instead: +/// +/// ``` +/// let +/// a = 2; +/// in +/// { inherit a; b = 3; } +/// ``` #[lint( name = "manual inherit", note = "Assignment instead of inherit", diff --git a/lib/src/lints/manual_inherit_from.rs b/lib/src/lints/manual_inherit_from.rs index 146d55b..8d0f539 100644 --- a/lib/src/lints/manual_inherit_from.rs +++ b/lib/src/lints/manual_inherit_from.rs @@ -7,6 +7,30 @@ use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, }; +/// ## What it does +/// Checks for bindings of the form `a = someAttr.a`. +/// +/// ## Why is this bad? +/// If the aim is to extract or bring attributes of an attrset into +/// scope, prefer an inherit statement. +/// +/// ## Example +/// +/// ``` +/// let +/// mtl = pkgs.haskellPackages.mtl; +/// in +/// null +/// ``` +/// +/// Try `inherit` instead: +/// +/// ``` +/// let +/// inherit (pkgs.haskellPackages) mtl; +/// in +/// null +/// ``` #[lint( name = "manual inherit from", note = "Assignment instead of inherit from", diff --git a/lib/src/lints/redundant_pattern_bind.rs b/lib/src/lints/redundant_pattern_bind.rs index ad1aba8..5b0711f 100644 --- a/lib/src/lints/redundant_pattern_bind.rs +++ b/lib/src/lints/redundant_pattern_bind.rs @@ -7,10 +7,29 @@ use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, }; +/// ## What it does +/// Checks for binds of the form `inputs @ { ... }` in function +/// arguments. +/// +/// ## Why is this bad? +/// The variadic pattern here is redundant, as it does not capture +/// anything. +/// +/// ## Example +/// +/// ``` +/// inputs @ { ... }: inputs.nixpkgs +/// ``` +/// +/// Remove the pattern altogether: +/// +/// ``` +/// inputs: inputs.nixpkgs +/// ``` #[lint( name = "redundant pattern bind", note = "Found redundant pattern bind in function argument", - code = 10, + code = 11, match_with = SyntaxKind::NODE_PATTERN )] struct RedundantPatternBind; diff --git a/lib/src/lints/unquoted_splice.rs b/lib/src/lints/unquoted_splice.rs index 2831c1b..c2fd6e4 100644 --- a/lib/src/lints/unquoted_splice.rs +++ b/lib/src/lints/unquoted_splice.rs @@ -7,6 +7,30 @@ use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, }; +/// ## What it does +/// Checks for antiquote/splice expressions that are not quoted. +/// +/// ## Why is this bad? +/// An *anti*quoted expression should always occur within a *quoted* +/// expression. +/// +/// ## Example +/// +/// ``` +/// let +/// pkgs = nixpkgs.legacyPackages.${system}; +/// in +/// pkgs +/// ``` +/// +/// Quote the splice expression: +/// +/// ``` +/// let +/// pkgs = nixpkgs.legacyPackages."${system}"; +/// in +/// pkgs +/// ``` #[lint( name = "unquoted splice", note = "Found unquoted splice expression", diff --git a/lib/src/lints/useless_parens.rs b/lib/src/lints/useless_parens.rs index 9dfeabb..36ad1b7 100644 --- a/lib/src/lints/useless_parens.rs +++ b/lib/src/lints/useless_parens.rs @@ -7,6 +7,31 @@ use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, }; +/// ## What it does +/// Checks for unnecessary parentheses. +/// +/// ## Why is this bad? +/// Unnecessarily parenthesized code is hard to read. +/// +/// ## Example +/// +/// ``` +/// let +/// double = (x: 2 * x); +/// ls = map (double) [ 1 2 3 ]; +/// in +/// (2 + 3) +/// ``` +/// +/// Remove unnecessary parentheses: +/// +/// ``` +/// let +/// double = x: 2 * x; +/// ls = map double [ 1 2 3 ]; +/// in +/// 2 + 3 +/// ``` #[lint( name = "useless parens", note = "These parentheses can be omitted", diff --git a/macros/src/explain.rs b/macros/src/explain.rs index 8b00740..9bc3c29 100644 --- a/macros/src/explain.rs +++ b/macros/src/explain.rs @@ -7,15 +7,15 @@ pub fn generate_explain_impl(struct_item: &ItemStruct) -> TokenStream2 { let explain = struct_item .attrs .iter() - .filter_map(|a| a.parse_meta().ok()) - .filter_map(|meta| match meta { - Meta::NameValue(MetaNameValue { path, lit, .. }) if path.is_ident("doc") => Some(lit), - _ => None, - }) - .filter_map(|lit| match lit { - Lit::Str(str_lit) => Some(str_lit.value()), + .filter_map(|attr| match attr.parse_meta().ok() { + Some(Meta::NameValue(MetaNameValue { + path, + lit: Lit::Str(str_lit), + .. + })) if path.is_ident("doc") => Some(str_lit.value()), _ => None, }) + .map(|s| s.strip_prefix(' ').unwrap_or(&s).to_owned()) .collect::>() .join("\n"); quote! { -- cgit v1.2.3