diff --git a/Cargo.lock b/Cargo.lock index d7202e1..30d0956 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -58,15 +58,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "cbitset" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "29b6ad25ae296159fb0da12b970b2fe179b234584d7cd294c891e2bbb284466b" -dependencies = [ - "num-traits", -] - [[package]] name = "cfg-if" version = "1.0.0" @@ -119,9 +110,9 @@ dependencies = [ [[package]] name = "countme" -version = "2.0.4" +version = "3.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "328b822bdcba4d4e402be8d9adb6eebf269f969f8eadef977a553ff3c4fbcb58" +checksum = "7704b5fdd17b18ae31c4c1da5a2e0305a2bf17b5249300a9ee9ed7b72114c636" [[package]] name = "crossbeam-channel" @@ -153,7 +144,7 @@ dependencies = [ "cfg-if", "crossbeam-utils", "lazy_static", - "memoffset", + "memoffset 0.6.4", "scopeguard", ] @@ -206,15 +197,15 @@ dependencies = [ [[package]] name = "hashbrown" -version = "0.9.1" +version = "0.11.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7afe4a420e3fe79967a00898cc1f4db7c8a49a9333a29f8a4bd76a253d5cd04" +checksum = "ab5ef0d4909ef3724cc8cce6ccc8572c5c817592e9285f5464f8e86f8bd3726e" [[package]] name = "hashbrown" -version = "0.11.2" +version = "0.14.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ab5ef0d4909ef3724cc8cce6ccc8572c5c817592e9285f5464f8e86f8bd3726e" +checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" [[package]] name = "heck" @@ -347,10 +338,10 @@ dependencies = [ ] [[package]] -name = "num-traits" -version = "0.2.14" +name = "memoffset" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a64b1ec5cda2586e284722486d802acf1f7dbdc623e2bfc57e65ca1cd099290" +checksum = "488016bfae457b036d996092f6cb448677611ce4449e970ceaf42695203f218a" dependencies = [ "autocfg", ] @@ -472,24 +463,22 @@ checksum = "f497285884f3fcff424ffc933e56d7cbca511def0c9831a7f9b5f6153e3cc89b" [[package]] name = "rnix" -version = "0.10.2" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8024a523e8836f1a5d051203dc00d833357fee94e351b51348dfaeca5364daa9" +checksum = "bb35cedbeb70e0ccabef2a31bcff0aebd114f19566086300b8f42c725fc2cb5f" dependencies = [ - "cbitset", "rowan", - "smol_str", ] [[package]] name = "rowan" -version = "0.12.6" +version = "0.15.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1b36e449f3702f3b0c821411db1cbdf30fb451726a9456dce5dabcd44420043" +checksum = "d4f1e4a001f863f41ea8d0e6a0c34b356d5b733db50dadab3efef640bafb779b" dependencies = [ "countme", - "hashbrown 0.9.1", - "memoffset", + "hashbrown 0.14.5", + "memoffset 0.9.1", "rustc-hash", "text-size", ] @@ -576,15 +565,6 @@ version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2e24979f63a11545f5f2c60141afe249d4f19f84581ea2138065e400941d83d3" -[[package]] -name = "smol_str" -version = "0.1.20" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "559b173452ec4061933b0c0e22d7d429c90ecdc1b3ae1c6e64238e7c15c3ee15" -dependencies = [ - "serde", -] - [[package]] name = "statix" version = "0.5.8" @@ -597,6 +577,7 @@ dependencies = [ "paste", "rayon", "rnix", + "rowan", "serde", "serde_json", "similar 2.1.0", diff --git a/Cargo.toml b/Cargo.toml index 45662b6..ea11311 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,8 +20,8 @@ paste = "1.0.15" proc-macro2 = "1.0.27" quote = "1.0" rayon = "1.5.1" -rnix = "0.10.2" -rowan = "0.12.5" +rnix = "0.11.0" +rowan = "0.15.17" serde = { features = ["derive"], version = "1.0.68" } serde_json = { version = "1.0.68" } similar = "2.1.0" diff --git a/bin/Cargo.toml b/bin/Cargo.toml index fba12b8..06c3d09 100644 --- a/bin/Cargo.toml +++ b/bin/Cargo.toml @@ -21,6 +21,7 @@ ignore.workspace = true lib.workspace = true rayon.workspace = true rnix.workspace = true +rowan.workspace = true serde.workspace = true serde_json = { optional = true, workspace = true } similar.workspace = true diff --git a/bin/src/fix/all.rs b/bin/src/fix/all.rs index 14e3da1..730cd60 100644 --- a/bin/src/fix/all.rs +++ b/bin/src/fix/all.rs @@ -1,7 +1,8 @@ use std::borrow::Cow; use lib::{Report, session::SessionInfo}; -use rnix::{WalkEvent, parser::ParseError as RnixParseErr}; +use rnix::{Root, WalkEvent, parser::ParseError as RnixParseErr}; +use rowan::ast::AstNode as _; use crate::{ LintMap, @@ -13,10 +14,10 @@ fn collect_fixes( lints: &LintMap, sess: &SessionInfo, ) -> Result, RnixParseErr> { - let parsed = rnix::parse(source).as_result()?; + let parsed = Root::parse(source).ok()?; Ok(parsed - .node() + .syntax() .preorder_with_tokens() .filter_map(|event| match event { WalkEvent::Enter(child) => lints.get(&child.kind()).map(|rules| { @@ -93,7 +94,7 @@ pub fn all_with<'a>( sess: &'a SessionInfo, ) -> Option> { let src = Cow::from(src); - let _ = rnix::parse(&src).as_result().ok()?; + let _ = Root::parse(&src).ok().ok()?; let initial = FixResult::empty(src, lints, sess); initial.into_iter().last() } diff --git a/bin/src/fix/single.rs b/bin/src/fix/single.rs index ed4d10f..971e0c3 100644 --- a/bin/src/fix/single.rs +++ b/bin/src/fix/single.rs @@ -1,7 +1,7 @@ use std::{borrow::Cow, convert::TryFrom}; use lib::{Report, session::SessionInfo}; -use rnix::{TextSize, WalkEvent}; +use rnix::{Root, TextSize, WalkEvent}; use crate::{err::SingleFixErr, fix::Source, utils}; @@ -29,11 +29,11 @@ fn pos_to_byte(line: usize, col: usize, src: &str) -> Result Result { // we don't really need the source to form a completely parsed tree - let parsed = rnix::parse(src); + let parsed = Root::parse(src); let lints = utils::lint_map(); parsed - .node() + .syntax() .preorder_with_tokens() .filter_map(|event| match event { WalkEvent::Enter(child) => lints.get(&child.kind()).map(|rules| { diff --git a/bin/src/lint.rs b/bin/src/lint.rs index 34f4bae..0309a8b 100644 --- a/bin/src/lint.rs +++ b/bin/src/lint.rs @@ -1,7 +1,7 @@ use crate::{LintMap, utils}; use lib::{Report, session::SessionInfo}; -use rnix::WalkEvent; +use rnix::{Root, WalkEvent}; use vfs::{FileId, VfsEntry}; #[derive(Debug)] @@ -14,14 +14,14 @@ pub struct LintResult { pub fn lint_with(vfs_entry: &VfsEntry, lints: &LintMap, sess: &SessionInfo) -> LintResult { let file_id = vfs_entry.file_id; let source = vfs_entry.contents; - let parsed = rnix::parse(source); + let parsed = Root::parse(source); let error_reports = parsed .errors() - .into_iter() - .map(|err: rnix::parser::ParseError| Report::from_parse_err(&err)); + .iter() + .map(|err: &rnix::parser::ParseError| Report::from_parse_err(err)); let reports = parsed - .node() + .syntax() .preorder_with_tokens() .filter_map(|event| match event { WalkEvent::Enter(child) => lints.get(&child.kind()).map(|rules| { diff --git a/bin/tests/snapshots/main__empty_list_concat_lint.snap b/bin/tests/snapshots/main__empty_list_concat_lint.snap index d385caf..d6985b9 100644 --- a/bin/tests/snapshots/main__empty_list_concat_lint.snap +++ b/bin/tests/snapshots/main__empty_list_concat_lint.snap @@ -31,9 +31,9 @@ expression: "& out" · ╰──────── Concatenation with the empty list, [], is a no-op ────╯ [W23] Warning: Unnecessary concatenation with empty list - ╭─[data/empty_list_concat.nix:15:4] + ╭─[data/empty_list_concat.nix:15:10] │ 15 │ ([] ++ [] ++ []) - · ────┬─── - · ╰───── Concatenation with the empty list, [], is a no-op + · ────┬─── + · ╰───── Concatenation with the empty list, [], is a no-op ────╯ diff --git a/lib/src/lints/bool_comparison.rs b/lib/src/lints/bool_comparison.rs index 3168e2c..af97262 100644 --- a/lib/src/lints/bool_comparison.rs +++ b/lib/src/lints/bool_comparison.rs @@ -3,8 +3,9 @@ use crate::{Metadata, Report, Rule, Suggestion, make, session::SessionInfo}; use macros::lint; use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode, - types::{BinOp, BinOpKind, Ident, TokenWrapper, TypedNode}, + ast::{BinOp, BinOpKind, Ident}, }; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for expressions of the form `x == true`, `x != true` and @@ -40,13 +41,14 @@ impl Rule for BoolComparison { }; let bin_expr = BinOp::cast(node.clone())?; let (lhs, rhs) = (bin_expr.lhs()?, bin_expr.rhs()?); + let (lhs, rhs) = (lhs.syntax(), rhs.syntax()); let op = EqualityBinOpKind::try_from(bin_expr.operator()?)?; let (bool_side, non_bool_side): (NixBoolean, &SyntaxNode) = - match (boolean_ident(&lhs), boolean_ident(&rhs)) { + match (boolean_ident(lhs), boolean_ident(rhs)) { (None, None) => return None, - (None, Some(bool)) => (bool, &lhs), - (Some(bool), _) => (bool, &rhs), + (None, Some(bool)) => (bool, lhs), + (Some(bool), _) => (bool, rhs), }; let replacement = match (&bool_side, op) { @@ -59,23 +61,16 @@ impl Rule for BoolComparison { | (NixBoolean::False, EqualityBinOpKind::Equal) => { // `a != true`, `a == false` replace with `!a` match non_bool_side.kind() { - SyntaxKind::NODE_APPLY | SyntaxKind::NODE_PAREN | SyntaxKind::NODE_IDENT => { + SyntaxKind::NODE_APPLY + | SyntaxKind::NODE_PAREN + | SyntaxKind::NODE_IDENT + | SyntaxKind::NODE_HAS_ATTR => { // do not parenthsize the replacement - make::unary_not(non_bool_side).node().clone() - } - SyntaxKind::NODE_BIN_OP => { - let inner = BinOp::cast(non_bool_side.clone()).unwrap(); - // `!a ? b`, no paren required - if inner.operator()? == BinOpKind::IsSet { - make::unary_not(non_bool_side).node().clone() - } else { - let parens = make::parenthesize(non_bool_side); - make::unary_not(parens.node()).node().clone() - } + make::unary_not(non_bool_side).syntax().clone() } _ => { let parens = make::parenthesize(non_bool_side); - make::unary_not(parens.node()).node().clone() + make::unary_not(parens.syntax()).syntax().clone() } } } @@ -125,7 +120,7 @@ impl std::fmt::Display for NixBoolean { // not entirely accurate, underhanded nix programmers might write `true = false` fn boolean_ident(node: &SyntaxNode) -> Option { - Ident::cast(node.clone()).and_then(|ident_expr| match ident_expr.as_str() { + Ident::cast(node.clone()).and_then(|ident_expr| match ident_expr.to_string().as_str() { "true" => Some(NixBoolean::True), "false" => Some(NixBoolean::False), _ => None, diff --git a/lib/src/lints/bool_simplification.rs b/lib/src/lints/bool_simplification.rs index 9f8395d..8cca34e 100644 --- a/lib/src/lints/bool_simplification.rs +++ b/lib/src/lints/bool_simplification.rs @@ -3,8 +3,9 @@ use crate::{Metadata, Report, Rule, Suggestion, make, session::SessionInfo}; use macros::lint; use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, - types::{BinOp, BinOpKind, Paren, TypedNode, UnaryOp, UnaryOpKind, Wrapper}, + ast::{BinOpKind, Expr, UnaryOp, UnaryOpKind}, }; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for boolean expressions that can be simplified. @@ -38,14 +39,21 @@ impl Rule for BoolSimplification { let unary_expr = UnaryOp::cast(node.clone())?; - if unary_expr.operator() != UnaryOpKind::Invert { + if unary_expr.operator() != Some(UnaryOpKind::Invert) { return None; } - let value_expr = unary_expr.value()?; - let paren_expr = Paren::cast(value_expr)?; - let inner_expr = paren_expr.inner()?; - let bin_expr = BinOp::cast(inner_expr)?; + let value_expr = unary_expr.expr()?; + + let Expr::Paren(paren_expr) = value_expr else { + return None; + }; + + let inner_expr = paren_expr.expr()?; + + let Expr::BinOp(bin_expr) = inner_expr else { + return None; + }; let Some(BinOpKind::Equal) = bin_expr.operator() else { return None; @@ -56,7 +64,9 @@ impl Rule for BoolSimplification { let lhs = bin_expr.lhs()?; let rhs = bin_expr.rhs()?; - let replacement = make::binary(&lhs, "!=", &rhs).node().clone(); + let replacement = make::binary(lhs.syntax(), "!=", rhs.syntax()) + .syntax() + .clone(); Some( self.report() .suggest(at, message, Suggestion::with_replacement(at, replacement)), diff --git a/lib/src/lints/collapsible_let_in.rs b/lib/src/lints/collapsible_let_in.rs index f633593..cac075b 100644 --- a/lib/src/lints/collapsible_let_in.rs +++ b/lib/src/lints/collapsible_let_in.rs @@ -3,9 +3,9 @@ use crate::{Metadata, Report, Rule, Suggestion, session::SessionInfo}; use macros::lint; use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, TextRange, - types::{LetIn, TypedNode}, + ast::{Expr, LetIn}, }; -use rowan::Direction; +use rowan::{Direction, ast::AstNode as _}; /// ## What it does /// Checks for `let-in` expressions whose body is another `let-in` @@ -52,21 +52,25 @@ impl Rule for CollapsibleLetIn { let let_in_expr = LetIn::cast(node.clone())?; let body = let_in_expr.body()?; - LetIn::cast(body.clone())?; + let Expr::LetIn(_) = body else { + return None; + }; let first_annotation = node.text_range(); let first_message = "This `let in` expression contains a nested `let in` expression"; - let second_annotation = body.text_range(); + let second_annotation = body.syntax().text_range(); let second_message = "This `let in` expression is nested"; let replacement_at = { let start = body + .syntax() .siblings_with_tokens(Direction::Prev) .find(|elem| elem.kind() == SyntaxKind::TOKEN_IN)? .text_range() .start(); let end = body + .syntax() .descendants_with_tokens() .find(|elem| elem.kind() == SyntaxKind::TOKEN_LET)? .text_range() diff --git a/lib/src/lints/deprecated_to_path.rs b/lib/src/lints/deprecated_to_path.rs index bdf4ec7..192e645 100644 --- a/lib/src/lints/deprecated_to_path.rs +++ b/lib/src/lints/deprecated_to_path.rs @@ -1,10 +1,8 @@ use crate::{Metadata, Report, Rule, session::SessionInfo}; use macros::lint; -use rnix::{ - NodeOrToken, SyntaxElement, SyntaxKind, - types::{Apply, TypedNode}, -}; +use rnix::{NodeOrToken, SyntaxElement, SyntaxKind, ast::Apply}; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for usage of the `toPath` function. diff --git a/lib/src/lints/empty_inherit.rs b/lib/src/lints/empty_inherit.rs index 2632e25..6986c1e 100644 --- a/lib/src/lints/empty_inherit.rs +++ b/lib/src/lints/empty_inherit.rs @@ -1,10 +1,8 @@ use crate::{Metadata, Report, Rule, Suggestion, session::SessionInfo, utils}; use macros::lint; -use rnix::{ - NodeOrToken, SyntaxElement, SyntaxKind, - types::{Inherit, TypedNode}, -}; +use rnix::{NodeOrToken, SyntaxElement, SyntaxKind, ast::Inherit}; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for empty inherit statements. @@ -39,7 +37,7 @@ impl Rule for EmptyInherit { return None; } - if inherit_stmt.idents().count() != 0 { + if inherit_stmt.attrs().count() != 0 { return None; } diff --git a/lib/src/lints/empty_let_in.rs b/lib/src/lints/empty_let_in.rs index cce5cef..73d2de2 100644 --- a/lib/src/lints/empty_let_in.rs +++ b/lib/src/lints/empty_let_in.rs @@ -3,8 +3,9 @@ use crate::{Metadata, Report, Rule, Suggestion, session::SessionInfo}; use macros::lint; use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, - types::{EntryHolder, LetIn, TypedNode}, + ast::{HasEntry as _, LetIn}, }; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for `let-in` expressions which create no new bindings. @@ -48,13 +49,16 @@ impl Rule for EmptyLetIn { .any(|el| el.kind() == SyntaxKind::TOKEN_COMMENT); let at = node.text_range(); - let replacement = body; + let replacement = body.syntax(); let message = "This let-in expression has no entries"; Some(if has_comments { self.report().diagnostic(at, message) } else { - self.report() - .suggest(at, message, Suggestion::with_replacement(at, replacement)) + self.report().suggest( + at, + message, + Suggestion::with_replacement(at, replacement.clone()), + ) }) } else { None diff --git a/lib/src/lints/empty_list_concat.rs b/lib/src/lints/empty_list_concat.rs index fe6624a..4c9e9d7 100644 --- a/lib/src/lints/empty_list_concat.rs +++ b/lib/src/lints/empty_list_concat.rs @@ -2,9 +2,10 @@ use crate::{Metadata, Report, Rule, Suggestion, session::SessionInfo}; use macros::lint; use rnix::{ - NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode, - types::{BinOp, BinOpKind, List, TypedNode}, + NodeOrToken, SyntaxElement, SyntaxKind, + ast::{BinOp, BinOpKind, Expr}, }; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for concatenations to empty lists @@ -54,13 +55,17 @@ impl Rule for EmptyListConcat { return None; }; - Some( - self.report() - .suggest(at, message, Suggestion::with_replacement(at, empty_array)), - ) + Some(self.report().suggest( + at, + message, + Suggestion::with_replacement(at, empty_array.syntax().clone()), + )) } } -fn is_empty_array(node: &SyntaxNode) -> bool { - List::cast(node.clone()).is_some_and(|list| list.items().count() == 0) +fn is_empty_array(expr: &Expr) -> bool { + let Expr::List(list) = expr else { + return false; + }; + list.items().count() == 0 } diff --git a/lib/src/lints/empty_pattern.rs b/lib/src/lints/empty_pattern.rs index f8dc716..48feaa6 100644 --- a/lib/src/lints/empty_pattern.rs +++ b/lib/src/lints/empty_pattern.rs @@ -3,8 +3,9 @@ use crate::{Metadata, Report, Rule, Suggestion, make, session::SessionInfo}; use macros::lint; use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode, - types::{AttrSet, EntryHolder, Lambda, Pattern, TypedNode}, + ast::{AttrSet, Entry, HasEntry as _, Lambda, Param}, }; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for an empty variadic pattern: `{...}`, in a function @@ -46,28 +47,31 @@ impl Rule for EmptyPattern { }; let lambda_expr = Lambda::cast(node.clone())?; - let pattern = Pattern::cast(lambda_expr.arg()?)?; + + let Some(Param::Pattern(pattern)) = lambda_expr.param() else { + return None; + }; // no patterns within `{ }` - if pattern.entries().count() != 0 { + if pattern.pat_entries().count() != 0 { return None; } // pattern is not bound - if pattern.at().is_some() { + if pattern.pat_bind().is_some() { return None; } - if is_module(&lambda_expr.body()?) { + if is_module(lambda_expr.body()?.syntax()) { return None; } Some(self.report().suggest( - pattern.node().text_range(), + pattern.syntax().text_range(), "This pattern is empty, use `_` instead", Suggestion::with_replacement( - pattern.node().text_range(), - make::ident("_").node().clone(), + pattern.syntax().text_range(), + make::ident("_").syntax().clone(), ), )) } @@ -80,6 +84,12 @@ fn is_module(body: &SyntaxNode) -> bool { attr_set .entries() - .filter_map(|e| e.key()) - .any(|k| k.node().to_string() == "imports") + .filter_map(|e| { + let Entry::AttrpathValue(attrpath_value) = e else { + return None; + }; + + attrpath_value.attrpath() + }) + .any(|k| k.to_string() == "imports") } diff --git a/lib/src/lints/eta_reduction.rs b/lib/src/lints/eta_reduction.rs index 3511d98..c9135d2 100644 --- a/lib/src/lints/eta_reduction.rs +++ b/lib/src/lints/eta_reduction.rs @@ -3,8 +3,9 @@ use crate::{Metadata, Report, Rule, Suggestion, session::SessionInfo}; use macros::lint; use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode, - types::{Apply, Ident, Lambda, TokenWrapper, TypedNode}, + ast::{Expr, Ident, Lambda, Param}, }; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for eta-reducible functions, i.e.: converts lambda @@ -47,36 +48,51 @@ impl Rule for EtaReduction { }; let lambda_expr = Lambda::cast(node.clone())?; - let ident = Ident::cast(lambda_expr.arg()?)?; - let body = Apply::cast(lambda_expr.body()?)?; - if ident.as_str() != Ident::cast(body.value()?)?.as_str() { + let Some(Param::IdentParam(ident_param)) = lambda_expr.param() else { + return None; + }; + + let ident = ident_param.ident()?; + + let Some(Expr::Apply(body)) = lambda_expr.body() else { + return None; + }; + + let Some(Expr::Ident(body_ident)) = body.argument() else { + return None; + }; + + if ident.to_string() != body_ident.to_string() { return None; } let lambda_node = body.lambda()?; - if mentions_ident(&ident, &lambda_node) { + if mentions_ident(&ident, lambda_node.syntax()) { return None; } // lambda body should be no more than a single Ident to // retain code readability - Ident::cast(lambda_node)?; + let Expr::Ident(_) = lambda_node else { + return None; + }; let at = node.text_range(); let replacement = body.lambda()?; - let message = format!("Found eta-reduction: `{}`", replacement.text()); - Some( - self.report() - .suggest(at, message, Suggestion::with_replacement(at, replacement)), - ) + let message = format!("Found eta-reduction: `{}`", replacement.syntax().text()); + Some(self.report().suggest( + at, + message, + Suggestion::with_replacement(at, replacement.syntax().clone()), + )) } } fn mentions_ident(ident: &Ident, node: &SyntaxNode) -> bool { if let Some(node_ident) = Ident::cast(node.clone()) { - node_ident.as_str() == ident.as_str() + node_ident.to_string() == ident.to_string() } else { node.children().any(|child| mentions_ident(ident, &child)) } diff --git a/lib/src/lints/legacy_let_syntax.rs b/lib/src/lints/legacy_let_syntax.rs index 107da69..8b3fd7d 100644 --- a/lib/src/lints/legacy_let_syntax.rs +++ b/lib/src/lints/legacy_let_syntax.rs @@ -3,8 +3,9 @@ use crate::{Metadata, Report, Rule, Suggestion, make, session::SessionInfo}; use macros::lint; use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, - types::{EntryHolder, Ident, LegacyLet, TokenWrapper, TypedNode}, + ast::{Attr, Entry, HasEntry, LegacyLet}, }; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for legacy-let syntax that was never formalized. @@ -52,12 +53,20 @@ impl Rule for ManualInherit { if !legacy_let .entries() - .filter_map(|kv| { - let key = kv.key()?; - let first_component = key.path().next()?; - Ident::cast(first_component) + .filter_map(|entry| { + let Entry::AttrpathValue(attrpath_value) = entry else { + return None; + }; + + let first_component = attrpath_value.attrpath()?.attrs().next()?; + + let Attr::Ident(ident) = first_component else { + return None; + }; + + Some(ident) }) - .any(|ident| ident.as_str() == "body") + .any(|ident| ident.to_string() == "body") { return None; } @@ -65,12 +74,12 @@ impl Rule for ManualInherit { let inherits = legacy_let.inherits(); let entries = legacy_let.entries(); let attrset = make::attrset(inherits, entries, true); - let parenthesized = make::parenthesize(attrset.node()); - let selected = make::select(parenthesized.node(), make::ident("body").node()); + let parenthesized = make::parenthesize(attrset.syntax()); + let selected = make::select(parenthesized.syntax(), make::ident("body").syntax()); let at = node.text_range(); let message = "Prefer `rec` over undocumented `let` syntax"; - let replacement = selected.node().clone(); + let replacement = selected.syntax().clone(); Some( self.report() diff --git a/lib/src/lints/manual_inherit.rs b/lib/src/lints/manual_inherit.rs index 7da7518..be61720 100644 --- a/lib/src/lints/manual_inherit.rs +++ b/lib/src/lints/manual_inherit.rs @@ -3,8 +3,9 @@ use crate::{Metadata, Report, Rule, Suggestion, make, session::SessionInfo}; use macros::lint; use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, - types::{Ident, KeyValue, TokenWrapper, TypedNode}, + ast::{Attr, AttrpathValue, Expr}, }; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for bindings of the form `a = a`. @@ -34,7 +35,7 @@ use rnix::{ name = "manual_inherit", note = "Assignment instead of inherit", code = 3, - match_with = SyntaxKind::NODE_KEY_VALUE + match_with = SyntaxKind::NODE_ATTRPATH_VALUE )] struct ManualInherit; @@ -44,23 +45,28 @@ impl Rule for ManualInherit { return None; }; - let key = KeyValue::cast(node.clone())?.key()?; - let mut key_path = key.path(); - let key_node = key_path.next()?; + let attrpath_value = AttrpathValue::cast(node.clone())?; + let attrpath = attrpath_value.attrpath()?; + let mut attrs = attrpath.attrs(); + let first_attr = attrs.next()?; - if key_path.next().is_some() { + if attrs.next().is_some() { return None; } - let key = Ident::cast(key_node)?; - let value_node = KeyValue::cast(node.clone())?.value()?; - let value = Ident::cast(value_node)?; + let Attr::Ident(key) = first_attr else { + return None; + }; + + let Some(Expr::Ident(value)) = attrpath_value.value() else { + return None; + }; - if key.as_str() != value.as_str() { + if key.to_string() != value.to_string() { return None; } - let replacement = make::inherit_stmt(&[key]).node().clone(); + let replacement = make::inherit_stmt(&[key]).syntax().clone(); Some(self.report().suggest( node.text_range(), diff --git a/lib/src/lints/manual_inherit_from.rs b/lib/src/lints/manual_inherit_from.rs index 17fd0b4..239d81a 100644 --- a/lib/src/lints/manual_inherit_from.rs +++ b/lib/src/lints/manual_inherit_from.rs @@ -3,8 +3,9 @@ use crate::{Metadata, Report, Rule, Suggestion, make, session::SessionInfo}; use macros::lint; use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, - types::{Ident, KeyValue, Select, TokenWrapper, TypedNode}, + ast::{Attr, AttrpathValue, Expr}, }; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for bindings of the form `a = someAttr.a`. @@ -34,7 +35,7 @@ use rnix::{ name = "manual_inherit_from", note = "Assignment instead of inherit from", code = 4, - match_with = SyntaxKind::NODE_KEY_VALUE + match_with = SyntaxKind::NODE_ATTRPATH_VALUE, )] struct ManualInherit; @@ -44,27 +45,45 @@ impl Rule for ManualInherit { return None; }; - let key_value_stmt = KeyValue::cast(node.clone())?; - let key = key_value_stmt.key()?; - let mut key_path = key.path(); + let key_value_stmt = AttrpathValue::cast(node.clone())?; + let key = key_value_stmt.attrpath()?; + let mut key_path = key.attrs(); let key_node = key_path.next()?; if key_path.next().is_some() { return None; } - let key = Ident::cast(key_node)?; - let value = Select::cast(key_value_stmt.value()?)?; - let index = Ident::cast(value.index()?)?; + let Attr::Ident(key) = key_node else { + return None; + }; + + let Some(Expr::Select(value)) = key_value_stmt.value() else { + return None; + }; + let select_attrpath = value.attrpath()?; + let mut select_attrpath_attrs = select_attrpath.attrs(); + let first_attr = select_attrpath_attrs.next()?; + + if select_attrpath_attrs.next().is_some() { + return None; + } + + let Attr::Ident(index) = first_attr else { + return None; + }; - if key.as_str() != index.as_str() { + if key.to_string() != index.to_string() { return None; } let at = node.text_range(); + let replacement = { - let set = value.set()?; - make::inherit_from_stmt(&set, &[key]).node().clone() + let set = value.expr()?; + make::inherit_from_stmt(set.syntax(), &[key]) + .syntax() + .clone() }; Some(self.report().suggest( diff --git a/lib/src/lints/redundant_pattern_bind.rs b/lib/src/lints/redundant_pattern_bind.rs index 0f4c82f..1c59c62 100644 --- a/lib/src/lints/redundant_pattern_bind.rs +++ b/lib/src/lints/redundant_pattern_bind.rs @@ -1,10 +1,8 @@ use crate::{Metadata, Report, Rule, Suggestion, session::SessionInfo}; use macros::lint; -use rnix::{ - NodeOrToken, SyntaxElement, SyntaxKind, - types::{Pattern, TokenWrapper, TypedNode}, -}; +use rnix::{NodeOrToken, SyntaxElement, SyntaxKind, ast::Pattern}; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for binds of the form `inputs @ { ... }` in function @@ -42,23 +40,19 @@ impl Rule for RedundantPatternBind { let pattern = Pattern::cast(node.clone())?; // no patterns within `{ }` - if pattern.entries().count() != 0 { + if pattern.pat_entries().count() != 0 { return None; } // pattern is just ellipsis - if !pattern.ellipsis() { - return None; - } + pattern.ellipsis_token()?; // pattern is bound - let ident = pattern.at()?; + let pat_bind = pattern.pat_bind()?; + let ident = pat_bind.ident()?; let at = node.text_range(); - let message = format!( - "This pattern bind is redundant, use `{}` instead", - ident.as_str() - ); - let replacement = ident.node().clone(); + let message = format!("This pattern bind is redundant, use `{ident}` instead"); + let replacement = ident.syntax().clone(); Some( self.report() diff --git a/lib/src/lints/repeated_keys.rs b/lib/src/lints/repeated_keys.rs index 658e7d3..d7c71dc 100644 --- a/lib/src/lints/repeated_keys.rs +++ b/lib/src/lints/repeated_keys.rs @@ -5,8 +5,9 @@ use crate::{Metadata, Report, Rule, session::SessionInfo}; use macros::lint; use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, - types::{AttrSet, EntryHolder, Ident, KeyValue, TokenWrapper, TypedNode}, + ast::{Attr, AttrSet, AttrpathValue, Entry, HasEntry as _}, }; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for keys in attribute sets with repetitive keys, and suggests using @@ -39,7 +40,7 @@ use rnix::{ name = "repeated_keys", note = "Avoid repeated keys in attribute sets", code = 20, - match_with = SyntaxKind::NODE_KEY_VALUE + match_with = SyntaxKind::NODE_ATTRPATH_VALUE )] struct RepeatedKeys; @@ -49,11 +50,14 @@ impl Rule for RepeatedKeys { return None; }; - let key_value = KeyValue::cast(node.clone())?; - let key = key_value.key()?; - let mut components = key.path(); + let attrpath_value = AttrpathValue::cast(node.clone())?; + let attrpath = attrpath_value.attrpath()?; + let mut components = attrpath.attrs(); let first_component = components.next()?; - let first_component_ident = Ident::cast(first_component)?; + + let Attr::Ident(first_component_ident) = first_component else { + return None; + }; // ensure that there are >1 components components.next()?; @@ -61,24 +65,31 @@ impl Rule for RepeatedKeys { let parent_node = node.parent()?; let parent_attr_set = AttrSet::cast(parent_node)?; - if parent_attr_set.recursive() { + if parent_attr_set.rec_token().is_some() { return None; } let occurrences = parent_attr_set .entries() .filter_map(|kv_scrutinee| { - let scrutinee_key = kv_scrutinee.key()?; - let mut kv_scrutinee_components = scrutinee_key.path(); + let Entry::AttrpathValue(kv_scrutinee) = kv_scrutinee else { + return None; + }; + + let scrutinee_key = kv_scrutinee.attrpath()?; + let mut kv_scrutinee_components = scrutinee_key.attrs(); let kv_scrutinee_first_component = kv_scrutinee_components.next()?; - let kv_scrutinee_ident = Ident::cast(kv_scrutinee_first_component)?; - if kv_scrutinee_ident.as_str() != first_component_ident.as_str() { + let Attr::Ident(kv_scrutinee_ident) = kv_scrutinee_first_component else { + return None; + }; + + if kv_scrutinee_ident.to_string() != first_component_ident.to_string() { return None; } Some(( - kv_scrutinee.key()?.node().text_range(), + scrutinee_key.syntax().text_range(), kv_scrutinee_components .map(|n| n.to_string()) .collect::>() @@ -87,7 +98,7 @@ impl Rule for RepeatedKeys { }) .collect::>(); - if occurrences.first()?.0 != key.node().text_range() { + if occurrences.first()?.0 != attrpath.syntax().text_range() { return None; } @@ -98,10 +109,7 @@ impl Rule for RepeatedKeys { let mut iter = occurrences.into_iter(); let (first_annotation, first_subkey) = iter.next().unwrap(); - let first_message = format!( - "The key `{}` is first assigned here ...", - first_component_ident.as_str() - ); + let first_message = format!("The key `{first_component_ident}` is first assigned here ..."); let (second_annotation, second_subkey) = iter.next().unwrap(); let second_message = "... repeated here ..."; @@ -116,11 +124,7 @@ impl Rule for RepeatedKeys { }; write!( message, - " Try `{} = {{ {}=...; {}=...; {}=...; }}` instead.", - first_component_ident.as_str(), - first_subkey, - second_subkey, - third_subkey + " Try `{first_component_ident} = {{ {first_subkey}=...; {second_subkey}=...; {third_subkey}=...; }}` instead." ) .unwrap(); message diff --git a/lib/src/lints/unquoted_uri.rs b/lib/src/lints/unquoted_uri.rs index d2c4e4c..d46d03d 100644 --- a/lib/src/lints/unquoted_uri.rs +++ b/lib/src/lints/unquoted_uri.rs @@ -1,7 +1,8 @@ use crate::{Metadata, Report, Rule, Suggestion, make, session::SessionInfo}; +use rowan::ast::AstNode as _; use macros::lint; -use rnix::{NodeOrToken, SyntaxElement, SyntaxKind, types::TypedNode}; +use rnix::{NodeOrToken, SyntaxElement, SyntaxKind}; /// ## What it does /// Checks for URI expressions that are not quoted. @@ -50,13 +51,14 @@ impl Rule for UnquotedUri { return None; }; - let parent_node = token.parent(); + let parent_node = token.parent()?; let at = token.text_range(); - let replacement = make::quote(&parent_node).node().clone(); + let replacement = make::quote(&parent_node); let message = "Consider quoting this URI expression"; - Some( - self.report() - .suggest(at, message, Suggestion::with_replacement(at, replacement)), - ) + Some(self.report().suggest( + at, + message, + Suggestion::with_replacement(at, replacement.syntax().clone()), + )) } } diff --git a/lib/src/lints/useless_has_attr.rs b/lib/src/lints/useless_has_attr.rs index 4abd8e6..8f2a9ac 100644 --- a/lib/src/lints/useless_has_attr.rs +++ b/lib/src/lints/useless_has_attr.rs @@ -3,8 +3,9 @@ use crate::{Metadata, Report, Rule, Suggestion, make, session::SessionInfo}; use macros::lint; use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, - types::{BinOp, BinOpKind, IfElse, Select, TypedNode}, + ast::{Expr, IfElse}, }; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for expressions that use the "has attribute" operator: `?`, @@ -40,43 +41,48 @@ impl Rule for UselessHasAttr { let if_else_expr = IfElse::cast(node.clone())?; let condition_expr = if_else_expr.condition()?; let default_expr = if_else_expr.else_body()?; - let cond_bin_expr = BinOp::cast(condition_expr)?; - let Some(BinOpKind::IsSet) = cond_bin_expr.operator() else { + + let Expr::HasAttr(has_attr) = condition_expr else { return None; }; // set ? attr_path - // ^^^--------------- lhs - // ^^^^^^^^^^--- rhs - let set = cond_bin_expr.lhs()?; - let attr_path = cond_bin_expr.rhs()?; + let set = has_attr.expr()?; + let attr_path = has_attr.attrpath()?; // check if body of the `if` expression is of the form `set.attr_path` let body_expr = if_else_expr.body()?; - let body_select_expr = Select::cast(body_expr)?; - let expected_body = make::select(&set, &attr_path); + let Expr::Select(body_select_expr) = body_expr else { + return None; + }; + + let expected_body = make::select(set.syntax(), attr_path.syntax()); // text comparison will do for now - if body_select_expr.node().text() != expected_body.node().text() { + if body_select_expr.syntax().text() != expected_body.syntax().text() { return None; } let at = node.text_range(); // `or` is tightly binding, we need to parenthesize non-literal exprs - let default_with_parens = match default_expr.kind() { - SyntaxKind::NODE_LIST - | SyntaxKind::NODE_PAREN - | SyntaxKind::NODE_STRING - | SyntaxKind::NODE_ATTR_SET - | SyntaxKind::NODE_IDENT - | SyntaxKind::NODE_SELECT => default_expr, - _ => make::parenthesize(&default_expr).node().clone(), + let default_with_parens = match default_expr { + Expr::List(_) + | Expr::Paren(_) + | Expr::Str(_) + | Expr::AttrSet(_) + | Expr::Ident(_) + | Expr::Select(_) => default_expr, + _ => Expr::Paren(make::parenthesize(default_expr.syntax())), }; - let replacement = make::or_default(&set, &attr_path, &default_with_parens) - .node() - .clone(); + let replacement = make::or_default( + set.syntax(), + attr_path.syntax(), + default_with_parens.syntax(), + ) + .syntax() + .clone(); let message = format!("Consider using `{replacement}` instead of this `if` expression"); Some( self.report() diff --git a/lib/src/lints/useless_parens.rs b/lib/src/lints/useless_parens.rs index ca5ddbd..6234769 100644 --- a/lib/src/lints/useless_parens.rs +++ b/lib/src/lints/useless_parens.rs @@ -3,8 +3,9 @@ use crate::{Diagnostic, Metadata, Report, Rule, Suggestion, session::SessionInfo use macros::lint; use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, - types::{KeyValue, LetIn, Paren, ParsedType, TypedNode, Wrapper}, + ast::{AttrpathValue, Expr, LetIn, Paren}, }; +use rowan::ast::AstNode as _; /// ## What it does /// Checks for unnecessary parentheses. @@ -36,7 +37,7 @@ use rnix::{ note = "These parentheses can be omitted", code = 8, match_with = [ - SyntaxKind::NODE_KEY_VALUE, + SyntaxKind::NODE_ATTRPATH_VALUE, SyntaxKind::NODE_PAREN, SyntaxKind::NODE_LET_IN, ] @@ -49,35 +50,40 @@ impl Rule for UselessParens { return None; }; - let parsed_type_node = ParsedType::cast(node.clone())?; - - let diagnostic = match parsed_type_node { - ParsedType::KeyValue(kv) => { - let value_node = kv.value()?; - let value_range = value_node.text_range(); + let diagnostic = match (AttrpathValue::cast(node.clone()), Expr::cast(node.clone())) { + (Some(attrpath_value), _) => { + let value_node = attrpath_value.value()?; + let value_range = value_node.syntax().text_range(); + let paren = Paren::cast(value_node.syntax().clone())?; + let suggestion = + Suggestion::with_replacement(value_range, paren.expr()?.syntax().clone()); Diagnostic::suggest( value_range, "Useless parentheses around value in binding", - Suggestion::with_replacement(value_range, Paren::cast(value_node)?.inner()?), + suggestion, ) } - ParsedType::LetIn(let_in) => { + (_, Some(Expr::LetIn(let_in))) => { let body_node = let_in.body()?; - let body_range = body_node.text_range(); + let body_range = body_node.syntax().text_range(); + let paren = Paren::cast(body_node.syntax().clone())?; + let suggestion = + Suggestion::with_replacement(body_range, paren.expr()?.syntax().clone()); + Diagnostic::suggest( body_range, "Useless parentheses around body of `let` expression", - Suggestion::with_replacement(body_range, Paren::cast(body_node)?.inner()?), + suggestion, ) } - ParsedType::Paren(paren_expr) => { - let paren_expr_range = paren_expr.node().text_range(); - let father_node = paren_expr.node().parent()?; + (_, Some(Expr::Paren(paren_expr))) => { + let paren_expr_range = paren_expr.syntax().text_range(); + let father_node = paren_expr.syntax().parent()?; // ensure that we don't lint inside let-in statements // we already lint such cases in previous match stmt - if KeyValue::cast(father_node.clone()).is_some() { + if AttrpathValue::cast(father_node.clone()).is_some() { return None; } @@ -87,24 +93,22 @@ impl Rule for UselessParens { return None; } - let parsed_inner = ParsedType::cast(paren_expr.inner()?)?; + let parsed_inner = Expr::cast(paren_expr.expr()?.syntax().clone())?; - if !matches!( - parsed_inner, - ParsedType::List(_) - | ParsedType::Paren(_) - | ParsedType::Str(_) - | ParsedType::AttrSet(_) - | ParsedType::Select(_) - | ParsedType::Ident(_) - ) { - return None; + match &parsed_inner { + Expr::List(_) + | Expr::Paren(_) + | Expr::Str(_) + | Expr::AttrSet(_) + | Expr::Ident(_) => {} + Expr::Select(select) if select.or_token().is_none() => {} + _ => return None, } Diagnostic::suggest( paren_expr_range, "Useless parentheses around primitive expression", - Suggestion::with_replacement(paren_expr_range, parsed_inner.node().clone()), + Suggestion::with_replacement(paren_expr_range, parsed_inner.syntax().clone()), ) } _ => return None, diff --git a/lib/src/make.rs b/lib/src/make.rs index 22e14eb..b0a6244 100644 --- a/lib/src/make.rs +++ b/lib/src/make.rs @@ -1,14 +1,19 @@ use std::{fmt::Write, iter::IntoIterator}; use rnix::{ - SyntaxNode, - types::{self, TokenWrapper, TypedNode}, + Root, SyntaxNode, + ast::{self, AstNode}, }; +use rowan::ast::AstNode as _; -fn ast_from_text(text: &str) -> N { - let parse = rnix::parse(text); +fn ast_from_text(text: &str) -> N { + let parse = Root::parse(text).ok(); - let Some(node) = parse.node().descendants().find_map(N::cast) else { + let Ok(parse) = parse else { + panic!("Failed to parse `{text:?}`") + }; + + let Some(node) = parse.syntax().descendants().find_map(N::cast) else { panic!( "Failed to make ast node `{}` from text `{}`", std::any::type_name::(), @@ -19,22 +24,22 @@ fn ast_from_text(text: &str) -> N { node } -pub fn parenthesize(node: &SyntaxNode) -> types::Paren { +pub fn parenthesize(node: &SyntaxNode) -> ast::Paren { ast_from_text(&format!("({node})")) } -pub fn quote(node: &SyntaxNode) -> types::Str { +pub fn quote(node: &SyntaxNode) -> ast::Str { ast_from_text(&format!("\"{node}\"")) } -pub fn unary_not(node: &SyntaxNode) -> types::UnaryOp { +pub fn unary_not(node: &SyntaxNode) -> ast::UnaryOp { ast_from_text(&format!("!{node}")) } -pub fn inherit_stmt<'a>(nodes: impl IntoIterator) -> types::Inherit { +pub fn inherit_stmt<'a>(nodes: impl IntoIterator) -> ast::Inherit { let inherited_idents = nodes .into_iter() - .map(|i| i.as_str().to_owned()) + .map(std::string::ToString::to_string) .collect::>() .join(" "); ast_from_text(&format!("{{ inherit {inherited_idents}; }}")) @@ -42,48 +47,48 @@ pub fn inherit_stmt<'a>(nodes: impl IntoIterator) -> ty pub fn inherit_from_stmt<'a>( from: &SyntaxNode, - nodes: impl IntoIterator, -) -> types::Inherit { + nodes: impl IntoIterator, +) -> ast::Inherit { let inherited_idents = nodes .into_iter() - .map(|i| i.as_str().to_owned()) + .map(std::string::ToString::to_string) .collect::>() .join(" "); ast_from_text(&format!("{{ inherit ({from}) {inherited_idents}; }}")) } pub fn attrset( - inherits: impl IntoIterator, - entries: impl IntoIterator, + inherits: impl IntoIterator, + entries: impl IntoIterator, recursive: bool, -) -> types::AttrSet { +) -> ast::AttrSet { let mut buffer = String::new(); writeln!(buffer, "{}{{", if recursive { "rec " } else { "" }).unwrap(); for inherit in inherits { - writeln!(buffer, " {}", inherit.node().text()).unwrap(); + writeln!(buffer, " {inherit}").unwrap(); } for entry in entries { - writeln!(buffer, " {}", entry.node().text()).unwrap(); + writeln!(buffer, " {entry}").unwrap(); } write!(buffer, "}}").unwrap(); ast_from_text(&buffer) } -pub fn select(set: &SyntaxNode, index: &SyntaxNode) -> types::Select { +pub fn select(set: &SyntaxNode, index: &SyntaxNode) -> ast::Select { ast_from_text(&format!("{set}.{index}")) } -pub fn ident(text: &str) -> types::Ident { +pub fn ident(text: &str) -> ast::Ident { ast_from_text(text) } // LATER: make `op` strongly typed here -pub fn binary(lhs: &SyntaxNode, op: &str, rhs: &SyntaxNode) -> types::BinOp { +pub fn binary(lhs: &SyntaxNode, op: &str, rhs: &SyntaxNode) -> ast::BinOp { ast_from_text(&format!("{lhs} {op} {rhs}")) } -pub fn or_default(set: &SyntaxNode, index: &SyntaxNode, default: &SyntaxNode) -> types::OrDefault { +pub fn or_default(set: &SyntaxNode, index: &SyntaxNode, default: &SyntaxNode) -> ast::Select { ast_from_text(&format!("{set}.{index} or {default}")) }