From d1ff222bcf94152cd657233cffd8c14a45788c26 Mon Sep 17 00:00:00 2001 From: Akshay Date: Wed, 29 Dec 2021 10:53:38 +0530 Subject: [PATCH] allow for version based lints --- bin/src/fix.rs | 30 ++++- bin/src/fix/all.rs | 21 +++- bin/src/fix/single.rs | 15 ++- bin/src/lib.rs | 1 + bin/src/lint.rs | 23 ++-- bin/src/session.rs | 0 bin/src/utils.rs | 11 ++ bin/tests/data/faster_groupby.nix | 15 +++ bin/tests/main.rs | 58 ++++++---- bin/tests/snapshots/main__faster_groupby.snap | 20 ++++ flake.lock | 18 +-- flake.nix | 4 +- lib/src/lib.rs | 4 +- lib/src/lints.rs | 1 + lib/src/lints/bool_comparison.rs | 4 +- lib/src/lints/collapsible_let_in.rs | 4 +- lib/src/lints/deprecated_is_null.rs | 4 +- lib/src/lints/empty_inherit.rs | 4 +- lib/src/lints/empty_let_in.rs | 4 +- lib/src/lints/empty_pattern.rs | 4 +- lib/src/lints/eta_reduction.rs | 4 +- lib/src/lints/faster_groupby.rs | 72 ++++++++++++ lib/src/lints/legacy_let_syntax.rs | 4 +- lib/src/lints/manual_inherit.rs | 4 +- lib/src/lints/manual_inherit_from.rs | 4 +- lib/src/lints/redundant_pattern_bind.rs | 4 +- lib/src/lints/unquoted_splice.rs | 4 +- lib/src/lints/unquoted_uri.rs | 4 +- lib/src/lints/useless_parens.rs | 4 +- lib/src/session.rs | 103 ++++++++++++++++++ 30 files changed, 371 insertions(+), 81 deletions(-) create mode 100644 bin/src/session.rs create mode 100644 bin/tests/data/faster_groupby.nix create mode 100644 bin/tests/snapshots/main__faster_groupby.snap create mode 100644 lib/src/lints/faster_groupby.rs create mode 100644 lib/src/session.rs diff --git a/bin/src/fix.rs b/bin/src/fix.rs index a035379..4b2ce0c 100644 --- a/bin/src/fix.rs +++ b/bin/src/fix.rs @@ -2,6 +2,7 @@ use std::borrow::Cow; use crate::LintMap; +use lib::session::SessionInfo; use rnix::TextRange; mod all; @@ -16,6 +17,7 @@ pub struct FixResult<'a> { pub src: Source<'a>, pub fixed: Vec, pub lints: &'a LintMap, + pub sess: &'a SessionInfo, } #[derive(Debug, Clone)] @@ -25,11 +27,12 @@ pub struct Fixed { } impl<'a> FixResult<'a> { - fn empty(src: Source<'a>, lints: &'a LintMap) -> Self { + fn empty(src: Source<'a>, lints: &'a LintMap, sess: &'a SessionInfo) -> Self { Self { src, fixed: Vec::new(), lints, + sess, } } } @@ -40,15 +43,27 @@ pub mod main { use crate::{ config::{Fix as FixConfig, FixOut, Single as SingleConfig}, err::{FixErr, StatixErr}, + utils, }; + use lib::session::{SessionInfo, Version}; use similar::TextDiff; pub fn all(fix_config: FixConfig) -> Result<(), StatixErr> { let vfs = fix_config.vfs()?; let lints = fix_config.lints()?; + + let version = utils::get_version_info() + .unwrap() + .parse::() + .unwrap(); + let session = SessionInfo::from_version(version); + for entry in vfs.iter() { - match (fix_config.out(), super::all_with(entry.contents, &lints)) { + match ( + fix_config.out(), + super::all_with(entry.contents, &lints, &session), + ) { (FixOut::Diff, fix_result) => { let src = fix_result .map(|r| r.src) @@ -87,7 +102,16 @@ pub mod main { let original_src = entry.contents; let (line, col) = single_config.position; - match (single_config.out(), super::single(line, col, original_src)) { + let version = utils::get_version_info() + .unwrap() + .parse::() + .unwrap(); + let session = SessionInfo::from_version(version); + + match ( + single_config.out(), + super::single(line, col, original_src, &session), + ) { (FixOut::Diff, single_result) => { let fixed_src = single_result .map(|r| r.src) diff --git a/bin/src/fix/all.rs b/bin/src/fix/all.rs index 7e51d16..d7f7fff 100644 --- a/bin/src/fix/all.rs +++ b/bin/src/fix/all.rs @@ -1,6 +1,6 @@ use std::borrow::Cow; -use lib::Report; +use lib::{session::SessionInfo, Report}; use rnix::{parser::ParseError as RnixParseErr, WalkEvent}; use crate::{ @@ -8,7 +8,11 @@ use crate::{ LintMap, }; -fn collect_fixes(source: &str, lints: &LintMap) -> Result, RnixParseErr> { +fn collect_fixes( + source: &str, + lints: &LintMap, + sess: &SessionInfo, +) -> Result, RnixParseErr> { let parsed = rnix::parse(source).as_result()?; Ok(parsed @@ -18,7 +22,7 @@ fn collect_fixes(source: &str, lints: &LintMap) -> Result, RnixParse WalkEvent::Enter(child) => lints.get(&child.kind()).map(|rules| { rules .iter() - .filter_map(|rule| rule.validate(&child)) + .filter_map(|rule| rule.validate(&child, sess)) .filter(|report| report.total_suggestion_range().is_some()) .collect::>() }), @@ -57,7 +61,7 @@ fn reorder(mut reports: Vec) -> Vec { impl<'a> Iterator for FixResult<'a> { type Item = FixResult<'a>; fn next(&mut self) -> Option { - let all_reports = collect_fixes(&self.src, self.lints).ok()?; + let all_reports = collect_fixes(&self.src, self.lints, &self.sess).ok()?; if all_reports.is_empty() { return None; } @@ -78,13 +82,18 @@ impl<'a> Iterator for FixResult<'a> { src: self.src.clone(), fixed, lints: self.lints, + sess: self.sess, }) } } -pub fn all_with<'a>(src: &'a str, lints: &'a LintMap) -> Option> { +pub fn all_with<'a>( + src: &'a str, + lints: &'a LintMap, + sess: &'a SessionInfo, +) -> Option> { let src = Cow::from(src); let _ = rnix::parse(&src).as_result().ok()?; - let initial = FixResult::empty(src, lints); + 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 d95cfda..67b6b8f 100644 --- a/bin/src/fix/single.rs +++ b/bin/src/fix/single.rs @@ -1,6 +1,6 @@ use std::{borrow::Cow, convert::TryFrom}; -use lib::Report; +use lib::{session::SessionInfo, Report}; use rnix::{TextSize, WalkEvent}; use crate::{err::SingleFixErr, fix::Source, utils}; @@ -27,7 +27,7 @@ fn pos_to_byte(line: usize, col: usize, src: &str) -> Result Result { +fn find(offset: TextSize, src: &str, sess: &SessionInfo) -> Result { // we don't really need the source to form a completely parsed tree let parsed = rnix::parse(src); let lints = utils::lint_map(); @@ -39,7 +39,7 @@ fn find(offset: TextSize, src: &str) -> Result { WalkEvent::Enter(child) => lints.get(&child.kind()).map(|rules| { rules .iter() - .filter_map(|rule| rule.validate(&child)) + .filter_map(|rule| rule.validate(&child, sess)) .find(|report| report.total_suggestion_range().is_some()) }), _ => None, @@ -49,10 +49,15 @@ fn find(offset: TextSize, src: &str) -> Result { .ok_or(SingleFixErr::NoOp) } -pub fn single(line: usize, col: usize, src: &str) -> Result { +pub fn single<'a, 'b>( + line: usize, + col: usize, + src: &'a str, + sess: &'b SessionInfo, +) -> Result, SingleFixErr> { let mut src = Cow::from(src); let offset = pos_to_byte(line, col, &*src)?; - let report = find(offset, &*src)?; + let report = find(offset, &*src, &sess)?; report.apply(src.to_mut()); diff --git a/bin/src/lib.rs b/bin/src/lib.rs index 0105334..01d3ea7 100644 --- a/bin/src/lib.rs +++ b/bin/src/lib.rs @@ -4,6 +4,7 @@ pub mod err; pub mod explain; pub mod fix; pub mod lint; +pub mod session; pub mod traits; mod utils; diff --git a/bin/src/lint.rs b/bin/src/lint.rs index 3482d46..c385007 100644 --- a/bin/src/lint.rs +++ b/bin/src/lint.rs @@ -1,6 +1,6 @@ use crate::{utils, LintMap}; -use lib::Report; +use lib::{session::SessionInfo, Report}; use rnix::WalkEvent; use vfs::{FileId, VfsEntry}; @@ -10,7 +10,7 @@ pub struct LintResult { pub reports: Vec, } -pub fn lint_with(vfs_entry: VfsEntry, lints: &LintMap) -> 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); @@ -23,7 +23,7 @@ pub fn lint_with(vfs_entry: VfsEntry, lints: &LintMap) -> LintResult { WalkEvent::Enter(child) => lints.get(&child.kind()).map(|rules| { rules .iter() - .filter_map(|rule| rule.validate(&child)) + .filter_map(|rule| rule.validate(&child, sess)) .collect::>() }), _ => None, @@ -35,21 +35,30 @@ pub fn lint_with(vfs_entry: VfsEntry, lints: &LintMap) -> LintResult { LintResult { file_id, reports } } -pub fn lint(vfs_entry: VfsEntry) -> LintResult { - lint_with(vfs_entry, &utils::lint_map()) +pub fn lint(vfs_entry: VfsEntry, sess: &SessionInfo) -> LintResult { + lint_with(vfs_entry, &utils::lint_map(), &sess) } pub mod main { use std::io; use super::lint_with; - use crate::{config::Check as CheckConfig, err::StatixErr, traits::WriteDiagnostic}; + use crate::{config::Check as CheckConfig, err::StatixErr, traits::WriteDiagnostic, utils}; + + use lib::session::{SessionInfo, Version}; pub fn main(check_config: CheckConfig) -> Result<(), StatixErr> { let vfs = check_config.vfs()?; let mut stdout = io::stdout(); let lints = check_config.lints()?; - let lint = |vfs_entry| lint_with(vfs_entry, &lints); + + let version = utils::get_version_info() + .unwrap() + .parse::() + .unwrap(); + let session = SessionInfo::from_version(version); + + let lint = |vfs_entry| lint_with(vfs_entry, &lints, &session); vfs.iter().map(lint).for_each(|r| { stdout.write(&r, &vfs, check_config.format).unwrap(); }); diff --git a/bin/src/session.rs b/bin/src/session.rs new file mode 100644 index 0000000..e69de29 diff --git a/bin/src/utils.rs b/bin/src/utils.rs index 747a761..d374b4b 100644 --- a/bin/src/utils.rs +++ b/bin/src/utils.rs @@ -22,3 +22,14 @@ pub fn lint_map_of( pub fn lint_map() -> HashMap>> { lint_map_of(&*LINTS) } + +pub fn get_version_info() -> Option { + use std::process::Command; + let program = Command::new("nix") + .arg("--version") + .output() + .expect("failed to execute"); + std::str::from_utf8(&program.stdout) + .ok() + .map(ToOwned::to_owned) +} diff --git a/bin/tests/data/faster_groupby.nix b/bin/tests/data/faster_groupby.nix new file mode 100644 index 0000000..30d1031 --- /dev/null +++ b/bin/tests/data/faster_groupby.nix @@ -0,0 +1,15 @@ +{ + # trivial case + _ = lib.groupBy (x: if x > 2 then "big" else "small") [ 1 2 3 4 5 ]; + + # offer lint heuristically on this too + _ = nixpkgs.lib.groupBy (x: if x > 2 then "big" else "small") [ 1 2 3 4 5 ]; + + # do not lint on `builtins` + _ = builtins.groupBy (x: x.name) [ + { name = "foo"; idx = 1; } + { name = "foo"; idx = 2; } + { name = "bar"; idx = 1; } + { name = "bar"; idx = 2; } + ]; +} diff --git a/bin/tests/main.rs b/bin/tests/main.rs index de5266f..991c876 100644 --- a/bin/tests/main.rs +++ b/bin/tests/main.rs @@ -1,31 +1,48 @@ +use lib::session::{SessionInfo, Version}; + +macro_rules! session_info { + ($version:expr) => {{ + let v: Version = $version.parse().unwrap(); + SessionInfo::from_version(v) + }}; +} + mod util { #[macro_export] macro_rules! test_lint { - ($($tname:ident),*,) => { - test_lint!($($tname),*); + ($tname:ident => $sess:expr, $($tail:tt)*) => { + test_lint!($tname => $sess); + test_lint!($($tail)*); }; - ($($tname:ident),*) => { - $( - #[test] - fn $tname() { - use statix::{config::OutFormat, traits::WriteDiagnostic, lint}; - use vfs::ReadOnlyVfs; + ($tname:ident, $($tail:tt)*) => { + test_lint!($tname); + test_lint!($($tail)*); + }; + ($tname:ident) => { + test_lint!($tname => session_info!("nix (Nix) 2.5")); + }; + ($tname:ident => $sess:expr) => { + #[test] + fn $tname() { + use statix::{config::OutFormat, traits::WriteDiagnostic, lint}; + use vfs::ReadOnlyVfs; - let file_path = concat!("data/", stringify!($tname), ".nix"); - let contents = include_str!(concat!("data/", stringify!($tname), ".nix")); + let file_path = concat!("data/", stringify!($tname), ".nix"); + let contents = include_str!(concat!("data/", stringify!($tname), ".nix")); - let vfs = ReadOnlyVfs::singleton(file_path, contents.as_bytes()); + let vfs = ReadOnlyVfs::singleton(file_path, contents.as_bytes()); - let mut buffer = Vec::new(); - vfs.iter().map(lint::lint).for_each(|r| { - buffer.write(&r, &vfs, OutFormat::StdErr).unwrap(); - }); + let session = $sess; - let stripped = strip_ansi_escapes::strip(&buffer).unwrap(); - let out = std::str::from_utf8(&stripped).unwrap(); - insta::assert_snapshot!(&out); - } - )* + let mut buffer = Vec::new(); + vfs.iter().map(|entry| lint::lint(entry, &session)).for_each(|r| { + buffer.write(&r, &vfs, OutFormat::StdErr).unwrap(); + }); + + let stripped = strip_ansi_escapes::strip(&buffer).unwrap(); + let out = std::str::from_utf8(&stripped).unwrap(); + insta::assert_snapshot!(&out); + } }; } } @@ -44,4 +61,5 @@ test_lint! { unquoted_uri, deprecated_is_null, empty_inherit, + faster_groupby => session_info!("nix (Nix) 2.5") } diff --git a/bin/tests/snapshots/main__faster_groupby.snap b/bin/tests/snapshots/main__faster_groupby.snap new file mode 100644 index 0000000..6ff3380 --- /dev/null +++ b/bin/tests/snapshots/main__faster_groupby.snap @@ -0,0 +1,20 @@ +--- +source: bin/tests/main.rs +expression: "&out" + +--- +[W15] Warning: Found lib.groupBy + ╭─[data/faster_groupby.nix:3:7] + │ + 3 │ _ = lib.groupBy (x: if x > 2 then "big" else "small") [ 1 2 3 4 5 ]; + · ─────┬───── + · ╰─────── Prefer builtins.groupBy over lib.groupBy +───╯ +[W15] Warning: Found lib.groupBy + ╭─[data/faster_groupby.nix:6:7] + │ + 6 │ _ = nixpkgs.lib.groupBy (x: if x > 2 then "big" else "small") [ 1 2 3 4 5 ]; + · ─────────┬───────── + · ╰─────────── Prefer builtins.groupBy over nixpkgs.lib.groupBy +───╯ + diff --git a/flake.lock b/flake.lock index ace5efe..ee2d078 100644 --- a/flake.lock +++ b/flake.lock @@ -8,11 +8,11 @@ "rust-analyzer-src": "rust-analyzer-src" }, "locked": { - "lastModified": 1638080655, - "narHash": "sha256-ZPx8e8CukEBx31IcgivAWnN9Jg0r+LTBPHV7fREf+QI=", + "lastModified": 1640413491, + "narHash": "sha256-3VdQNPd9k4Fcjv/JBlwtJZWXg4fhrqTrEoSqhEOEWaU=", "owner": "nix-community", "repo": "fenix", - "rev": "78a0c55b6f9d8bb6f3b89eb995fa5bbdd73e9475", + "rev": "730771574878f1fd5c31b4d7b1595df389f01ea3", "type": "github" }, "original": { @@ -43,11 +43,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1638036523, - "narHash": "sha256-ZL6gogsuBmhBvIro+YwRKrypYhwVPCOOO7FmhOV/xyE=", + "lastModified": 1640418986, + "narHash": "sha256-a8GGtxn2iL3WAkY5H+4E0s3Q7XJt6bTOvos9qqxT5OQ=", "owner": "nixos", "repo": "nixpkgs", - "rev": "9c191ebcdfe917043195c54ab6ae8e934434fe7b", + "rev": "5c37ad87222cfc1ec36d6cd1364514a9efc2f7f2", "type": "github" }, "original": { @@ -67,11 +67,11 @@ "rust-analyzer-src": { "flake": false, "locked": { - "lastModified": 1638036899, - "narHash": "sha256-vh7z8jupVxXPOko3sWUsOB7eji/7lKfwJ/CE3iw97Sw=", + "lastModified": 1640270361, + "narHash": "sha256-g0canOfW6Iu/wSy0XUctgmlHJahfh5NqwDYddcRHXJQ=", "owner": "rust-analyzer", "repo": "rust-analyzer", - "rev": "d9b2291f546abc77d24499339a72a89127464b95", + "rev": "7b7a1ed062edd438caa824b6a71aaaa56b48e7d4", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index c598bdf..9bb9332 100644 --- a/flake.nix +++ b/flake.nix @@ -33,9 +33,9 @@ }); chanspec = { - date = "2021-11-01"; + date = "2021-12-01"; channel = "nightly"; - sha256 = "2BmxGawDNjXHJvnQToxmErMGgEPOfVzUvxhkvuixHYU="; # set zeros after modifying channel or date + sha256 = "DhIP1w63/hMbWlgElJGBumEK/ExFWCdLaeBV5F8uWHc="; # set zeros after modifying channel or date }; rustChannel = p: (fenix.overlay p p).fenix.toolchainOf chanspec; diff --git a/lib/src/lib.rs b/lib/src/lib.rs index a25b814..cc4818a 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -1,9 +1,11 @@ #![recursion_limit = "1024"] mod lints; mod make; +pub mod session; mod utils; pub use lints::LINTS; +use session::SessionInfo; use rnix::{parser::ParseError, SyntaxElement, SyntaxKind, TextRange}; use std::{convert::Into, default::Default}; @@ -221,7 +223,7 @@ impl Serialize for Suggestion { /// Lint logic is defined via this trait. Do not implement manually, /// look at the `lint` attribute macro instead for implementing rules pub trait Rule { - fn validate(&self, node: &SyntaxElement) -> Option; + fn validate(&self, node: &SyntaxElement, sess: &SessionInfo) -> Option; } /// Contains information about the lint itself. Do not implement manually, diff --git a/lib/src/lints.rs b/lib/src/lints.rs index 5de6b65..0add458 100644 --- a/lib/src/lints.rs +++ b/lib/src/lints.rs @@ -15,4 +15,5 @@ lints! { unquoted_uri, deprecated_is_null, empty_inherit, + faster_groupby, } diff --git a/lib/src/lints/bool_comparison.rs b/lib/src/lints/bool_comparison.rs index ed1e8bb..ef7f5d2 100644 --- a/lib/src/lints/bool_comparison.rs +++ b/lib/src/lints/bool_comparison.rs @@ -1,4 +1,4 @@ -use crate::{make, Metadata, Report, Rule, Suggestion}; +use crate::{make, session::SessionInfo, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -35,7 +35,7 @@ use rnix::{ struct BoolComparison; impl Rule for BoolComparison { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if let Some(bin_expr) = BinOp::cast(node.clone()); diff --git a/lib/src/lints/collapsible_let_in.rs b/lib/src/lints/collapsible_let_in.rs index aa7e5a7..1c04d9c 100644 --- a/lib/src/lints/collapsible_let_in.rs +++ b/lib/src/lints/collapsible_let_in.rs @@ -1,4 +1,4 @@ -use crate::{make, Metadata, Report, Rule, Suggestion}; +use crate::{make, session::SessionInfo, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -45,7 +45,7 @@ use rowan::Direction; struct CollapsibleLetIn; impl Rule for CollapsibleLetIn { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if let Some(let_in_expr) = LetIn::cast(node.clone()); diff --git a/lib/src/lints/deprecated_is_null.rs b/lib/src/lints/deprecated_is_null.rs index c814e87..9e7c293 100644 --- a/lib/src/lints/deprecated_is_null.rs +++ b/lib/src/lints/deprecated_is_null.rs @@ -1,4 +1,4 @@ -use crate::{make, Metadata, Report, Rule, Suggestion}; +use crate::{make, session::SessionInfo, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -35,7 +35,7 @@ use rnix::{ struct DeprecatedIsNull; impl Rule for DeprecatedIsNull { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if let Some(apply) = Apply::cast(node.clone()); diff --git a/lib/src/lints/empty_inherit.rs b/lib/src/lints/empty_inherit.rs index 9ae4cf2..871c218 100644 --- a/lib/src/lints/empty_inherit.rs +++ b/lib/src/lints/empty_inherit.rs @@ -1,4 +1,4 @@ -use crate::{make, utils, Metadata, Report, Rule, Suggestion}; +use crate::{make, session::SessionInfo, utils, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -29,7 +29,7 @@ use rnix::{ struct EmptyInherit; impl Rule for EmptyInherit { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if let Some(inherit_stmt) = Inherit::cast(node.clone()); diff --git a/lib/src/lints/empty_let_in.rs b/lib/src/lints/empty_let_in.rs index d33d0ae..e42f658 100644 --- a/lib/src/lints/empty_let_in.rs +++ b/lib/src/lints/empty_let_in.rs @@ -1,4 +1,4 @@ -use crate::{Metadata, Report, Rule, Suggestion}; +use crate::{session::SessionInfo, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -34,7 +34,7 @@ use rnix::{ struct EmptyLetIn; impl Rule for EmptyLetIn { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if let Some(let_in_expr) = LetIn::cast(node.clone()); diff --git a/lib/src/lints/empty_pattern.rs b/lib/src/lints/empty_pattern.rs index f66a3b1..e03708b 100644 --- a/lib/src/lints/empty_pattern.rs +++ b/lib/src/lints/empty_pattern.rs @@ -1,4 +1,4 @@ -use crate::{make, Metadata, Report, Rule, Suggestion}; +use crate::{make, session::SessionInfo, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -43,7 +43,7 @@ use rnix::{ struct EmptyPattern; impl Rule for EmptyPattern { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if let Some(pattern) = Pattern::cast(node.clone()); diff --git a/lib/src/lints/eta_reduction.rs b/lib/src/lints/eta_reduction.rs index 580f4a0..8e9d2a3 100644 --- a/lib/src/lints/eta_reduction.rs +++ b/lib/src/lints/eta_reduction.rs @@ -1,4 +1,4 @@ -use crate::{Metadata, Report, Rule, Suggestion}; +use crate::{session::SessionInfo, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -42,7 +42,7 @@ use rnix::{ struct EtaReduction; impl Rule for EtaReduction { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if let Some(lambda_expr) = Lambda::cast(node.clone()); diff --git a/lib/src/lints/faster_groupby.rs b/lib/src/lints/faster_groupby.rs new file mode 100644 index 0000000..c496125 --- /dev/null +++ b/lib/src/lints/faster_groupby.rs @@ -0,0 +1,72 @@ +use crate::{ + make, + session::{SessionInfo, Version}, + Metadata, Report, Rule, Suggestion, +}; + +use if_chain::if_chain; +use macros::lint; +use rnix::{ + types::{Select, TypedNode}, + NodeOrToken, SyntaxElement, SyntaxKind, +}; + +/// ## What it does +/// Checks for `lib.groupBy`. +/// +/// ## Why is this bad? +/// Nix 2.5 introduces `builtins.groupBy` which is faster and does +/// not require a lib import. +/// +/// ## Example +/// +/// ```nix +/// lib.groupBy (x: if x > 2 then "big" else "small") [ 1 2 3 4 5 6 ]; +/// # { big = [ 3 4 5 6 ]; small = [ 1 2 ]; } +/// ``` +/// +/// Replace `lib.groupBy` with `builtins.groupBy`: +/// +/// ``` +/// builtins.groupBy (x: if x > 2 then "big" else "small") [ 1 2 3 4 5 6 ]; +/// ``` +#[lint( + name = "faster_groupby", + note = "Found lib.groupBy", + code = 15, + match_with = SyntaxKind::NODE_SELECT +)] +struct FasterGroupBy; + +impl Rule for FasterGroupBy { + fn validate(&self, node: &SyntaxElement, sess: &SessionInfo) -> Option { + let lint_version = "nix (Nix) 2.5".parse::().unwrap(); + if_chain! { + if sess.version() >= &lint_version; + if let NodeOrToken::Node(node) = node; + if let Some(select_expr) = Select::cast(node.clone()); + if let Some(select_from) = select_expr.set(); + if let Some(group_by_attr) = select_expr.index(); + + // a heuristic to lint on nixpkgs.lib.groupBy + // and lib.groupBy and its variants + if select_from.text().to_string() != "builtins"; + if group_by_attr.text().to_string() == "groupBy"; + + then { + let at = node.text_range(); + let replacement = { + let builtins = make::ident("builtins"); + make::select(builtins.node(), &group_by_attr).node().clone() + }; + let message = format!("Prefer `builtins.groupBy` over `{}.groupBy`", select_from); + Some( + self.report() + .suggest(at, message, Suggestion::new(at, replacement)), + ) + } else { + None + } + } + } +} diff --git a/lib/src/lints/legacy_let_syntax.rs b/lib/src/lints/legacy_let_syntax.rs index 5d0028b..e0b8980 100644 --- a/lib/src/lints/legacy_let_syntax.rs +++ b/lib/src/lints/legacy_let_syntax.rs @@ -1,4 +1,4 @@ -use crate::{make, Metadata, Report, Rule, Suggestion}; +use crate::{make, session::SessionInfo, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -44,7 +44,7 @@ use rnix::{ struct ManualInherit; impl Rule for ManualInherit { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if let Some(legacy_let) = LegacyLet::cast(node.clone()); diff --git a/lib/src/lints/manual_inherit.rs b/lib/src/lints/manual_inherit.rs index 7717dc9..4fddce5 100644 --- a/lib/src/lints/manual_inherit.rs +++ b/lib/src/lints/manual_inherit.rs @@ -1,4 +1,4 @@ -use crate::{make, Metadata, Report, Rule, Suggestion}; +use crate::{make, session::SessionInfo, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -40,7 +40,7 @@ use rnix::{ struct ManualInherit; impl Rule for ManualInherit { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if let Some(key_value_stmt) = KeyValue::cast(node.clone()); diff --git a/lib/src/lints/manual_inherit_from.rs b/lib/src/lints/manual_inherit_from.rs index 05d6bc8..a62a6c7 100644 --- a/lib/src/lints/manual_inherit_from.rs +++ b/lib/src/lints/manual_inherit_from.rs @@ -1,4 +1,4 @@ -use crate::{make, Metadata, Report, Rule, Suggestion}; +use crate::{make, session::SessionInfo, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -40,7 +40,7 @@ use rnix::{ struct ManualInherit; impl Rule for ManualInherit { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if let Some(key_value_stmt) = KeyValue::cast(node.clone()); diff --git a/lib/src/lints/redundant_pattern_bind.rs b/lib/src/lints/redundant_pattern_bind.rs index 88ce4b0..56957ce 100644 --- a/lib/src/lints/redundant_pattern_bind.rs +++ b/lib/src/lints/redundant_pattern_bind.rs @@ -1,4 +1,4 @@ -use crate::{Metadata, Report, Rule, Suggestion}; +use crate::{session::SessionInfo, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -35,7 +35,7 @@ use rnix::{ struct RedundantPatternBind; impl Rule for RedundantPatternBind { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if let Some(pattern) = Pattern::cast(node.clone()); diff --git a/lib/src/lints/unquoted_splice.rs b/lib/src/lints/unquoted_splice.rs index 7649fbc..ba1641a 100644 --- a/lib/src/lints/unquoted_splice.rs +++ b/lib/src/lints/unquoted_splice.rs @@ -1,4 +1,4 @@ -use crate::{make, Metadata, Report, Rule, Suggestion}; +use crate::{make, session::SessionInfo, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -40,7 +40,7 @@ use rnix::{ struct UnquotedSplice; impl Rule for UnquotedSplice { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if Dynamic::cast(node.clone()).is_some(); diff --git a/lib/src/lints/unquoted_uri.rs b/lib/src/lints/unquoted_uri.rs index 8835338..440b278 100644 --- a/lib/src/lints/unquoted_uri.rs +++ b/lib/src/lints/unquoted_uri.rs @@ -1,4 +1,4 @@ -use crate::{make, Metadata, Report, Rule, Suggestion}; +use crate::{make, session::SessionInfo, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -46,7 +46,7 @@ use rnix::{types::TypedNode, NodeOrToken, SyntaxElement, SyntaxKind}; struct UnquotedUri; impl Rule for UnquotedUri { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Token(token) = node; then { diff --git a/lib/src/lints/useless_parens.rs b/lib/src/lints/useless_parens.rs index 09d6f04..9cba4b3 100644 --- a/lib/src/lints/useless_parens.rs +++ b/lib/src/lints/useless_parens.rs @@ -1,4 +1,4 @@ -use crate::{Diagnostic, Metadata, Report, Rule, Suggestion}; +use crate::{session::SessionInfo, Diagnostic, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -45,7 +45,7 @@ use rnix::{ struct UselessParens; impl Rule for UselessParens { - fn validate(&self, node: &SyntaxElement) -> Option { + fn validate(&self, node: &SyntaxElement, _sess: &SessionInfo) -> Option { if_chain! { if let NodeOrToken::Node(node) = node; if let Some(parsed_type_node) = ParsedType::cast(node.clone()); diff --git a/lib/src/session.rs b/lib/src/session.rs new file mode 100644 index 0000000..8d142ec --- /dev/null +++ b/lib/src/session.rs @@ -0,0 +1,103 @@ +use std::{cmp::Ordering, str::FromStr}; + +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub struct Version { + major: u16, + minor: u16, + patch: Option, +} + +impl Ord for Version { + fn cmp(&self, other: &Self) -> Ordering { + let score = |v: &Version| v.major * 100 + v.minor * 10 + v.patch.unwrap_or(0); + score(self).cmp(&score(other)) + } +} + +impl PartialOrd for Version { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +fn parse_number(s: &str) -> Option { + s.chars() + .take_while(|c| c.is_digit(10)) + .collect::() + .parse::() + .ok() +} + +fn parse_version(s: &str) -> Option { + match s.split(' ').collect::>().as_slice() { + [_, _, version] => { + let mut parts = version.split('.'); + let major = parse_number(parts.next()?)?; + let minor = parse_number(parts.next()?)?; + let patch = parts.next().map(|p| parse_number(p)).flatten(); + Some(Version { + major, + minor, + patch, + }) + } + _ => None, + } +} + +impl FromStr for Version { + type Err = (); + fn from_str(s: &str) -> Result { + parse_version(s).ok_or(()) + } +} + +#[non_exhaustive] +pub struct SessionInfo { + nix_version: Version, +} + +impl SessionInfo { + pub fn from_version(nix_version: Version) -> Self { + Self { nix_version } + } + + pub fn version(&self) -> &Version { + &self.nix_version + } +} + +pub fn get_nix_version() -> Option { + "nix (Nix) 2.4pre20211006_53e4794".parse::().ok() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_trivial() { + let v = "nix (Nix) 1.6.1".parse::().ok(); + assert!(v.is_some()) + } + + #[test] + fn parse() { + let v = "nix (Nix) 2.4pre20211006_53e4794".parse::().ok(); + assert!(v.is_some()) + } + + #[test] + fn compare_trivial() { + let v1 = "nix (Nix) 1.6.1".parse::().ok(); + let v2 = "nix (Nix) 1.7.2".parse::().ok(); + assert!(v2 > v1); + } + + #[test] + fn compare() { + let v1 = "nix (Nix) 1.7".parse::().ok(); + let v2 = "nix (Nix) 2.4pre20211006_53e4794".parse::().ok(); + assert!(v2 >= v1); + } +}