From e8c955da4cbb042e6f9b89307d143f5bfa6779fa Mon Sep 17 00:00:00 2001 From: Akshay Date: Sun, 31 Oct 2021 14:35:26 +0530 Subject: add `explain` subcommand and explanations to all lints --- bin/src/config.rs | 24 +++++ bin/src/err.rs | 10 ++ bin/src/explain.rs | 15 +++ bin/src/main.rs | 5 + lib/src/lib.rs | 30 +++--- lib/src/lints/bool_comparison.rs | 23 ++++- lib/src/lints/collapsible_let_in.rs | 37 ++++++- lib/src/lints/empty_let_in.rs | 25 ++++- lib/src/lints/empty_pattern.rs | 31 +++++- lib/src/lints/eta_reduction.rs | 30 +++++- lib/src/lints/legacy_let_syntax.rs | 32 +++++- lib/src/lints/manual_inherit.rs | 28 ++++- lib/src/lints/manual_inherit_from.rs | 28 ++++- lib/src/lints/redundant_pattern_bind.rs | 25 ++++- lib/src/lints/unquoted_splice.rs | 28 ++++- lib/src/lints/useless_parens.rs | 35 ++++++- macros/src/explain.rs | 28 +++++ macros/src/lib.rs | 173 ++----------------------------- macros/src/metadata.rs | 177 ++++++++++++++++++++++++++++++++ 19 files changed, 578 insertions(+), 206 deletions(-) create mode 100644 bin/src/explain.rs create mode 100644 macros/src/explain.rs create mode 100644 macros/src/metadata.rs diff --git a/bin/src/config.rs b/bin/src/config.rs index ef3a29d..1649017 100644 --- a/bin/src/config.rs +++ b/bin/src/config.rs @@ -26,6 +26,8 @@ pub enum SubCommand { Fix(Fix), /// Fix exactly one issue at provided position Single(Single), + /// Print detailed explanation for a lint warning + Explain(Explain), } #[derive(Clap, Debug)] @@ -88,6 +90,13 @@ pub struct Single { pub diff_only: bool, } +#[derive(Clap, Debug)] +pub struct Explain { + /// Warning code to explain + #[clap(parse(try_from_str = parse_warning_code))] + pub target: u32, +} + mod dirs { use std::{ fs, @@ -160,6 +169,21 @@ fn parse_line_col(src: &str) -> Result<(usize, usize), ConfigErr> { } } +fn parse_warning_code(src: &str) -> Result { + let mut char_stream = src.chars(); + let severity = char_stream + .next() + .ok_or(ConfigErr::InvalidWarningCode(src.to_owned()))? + .to_ascii_lowercase(); + match severity { + 'w' => char_stream + .collect::() + .parse::() + .map_err(|_| ConfigErr::InvalidWarningCode(src.to_owned())), + _ => Ok(0), + } +} + fn build_ignore_set(ignores: &[String]) -> Result { let mut set = GlobSetBuilder::new(); for pattern in ignores { diff --git a/bin/src/err.rs b/bin/src/err.rs index 4c16d69..1e52c2b 100644 --- a/bin/src/err.rs +++ b/bin/src/err.rs @@ -12,6 +12,8 @@ pub enum ConfigErr { InvalidPath(#[from] io::Error), #[error("unable to parse `{0}` as line and column")] InvalidPosition(String), + #[error("unable to parse `{0}` as warning code")] + InvalidWarningCode(String), } // #[derive(Error, Debug)] @@ -40,6 +42,12 @@ pub enum SingleFixErr { NoOp, } +#[derive(Error, Debug)] +pub enum ExplainErr { + #[error("lint with code `{0}` not found")] + LintNotFound(u32), +} + #[derive(Error, Debug)] pub enum StatixErr { // #[error("linter error: {0}")] @@ -50,4 +58,6 @@ pub enum StatixErr { Single(#[from] SingleFixErr), #[error("config error: {0}")] Config(#[from] ConfigErr), + #[error("explain error: {0}")] + Explain(#[from] ExplainErr), } diff --git a/bin/src/explain.rs b/bin/src/explain.rs new file mode 100644 index 0000000..6aefa7e --- /dev/null +++ b/bin/src/explain.rs @@ -0,0 +1,15 @@ +use crate::err::ExplainErr; + +use lib::LINTS; + +pub fn explain(code: u32) -> Result<&'static str, ExplainErr> { + match code { + 0 => Ok("syntax error"), + _ => LINTS + .values() + .flatten() + .find(|l| l.code() == code) + .map(|l| l.explanation()) + .ok_or(ExplainErr::LintNotFound(code)), + } +} diff --git a/bin/src/main.rs b/bin/src/main.rs index 90b79ce..31f6823 100644 --- a/bin/src/main.rs +++ b/bin/src/main.rs @@ -1,5 +1,6 @@ mod config; mod err; +mod explain; mod fix; mod lint; mod traits; @@ -86,6 +87,10 @@ fn _main() -> Result<(), StatixErr> { print!("{}", &*single_fix_result.src) } } + SubCommand::Explain(explain_config) => { + let explanation = explain::explain(explain_config.target)?; + println!("{}", explanation) + } } Ok(()) } diff --git a/lib/src/lib.rs b/lib/src/lib.rs index 196cbf8..5347666 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -226,25 +226,29 @@ pub trait Rule { /// Contains information about the lint itself. Do not implement manually, /// look at the `lint` attribute macro instead for implementing rules pub trait Metadata { - fn name() -> &'static str - where - Self: Sized; - fn note() -> &'static str - where - Self: Sized; - fn code() -> u32 - where - Self: Sized; - fn report() -> Report - where - Self: Sized; + fn name(&self) -> &'static str; + fn note(&self) -> &'static str; + fn code(&self) -> u32; + fn report(&self) -> Report; fn match_with(&self, with: &SyntaxKind) -> bool; fn match_kind(&self) -> Vec; } +/// Contains offline explanation for each lint +/// The `lint` macro scans nearby doc comments for +/// explanations and derives this trait. +/// +/// FIXME: the lint macro does way too much, maybe +/// split it into smaller macros. +pub trait Explain { + fn explanation(&self) -> &'static str { + "no explanation found" + } +} + /// Combines Rule and Metadata, do not implement manually, this is derived by /// the `lint` macro. -pub trait Lint: Metadata + Rule + Send + Sync {} +pub trait Lint: Metadata + Explain + Rule + Send + Sync {} /// Helper utility to take lints from modules and insert them into a map for efficient /// access. Mapping is from a SyntaxKind to a list of lints that apply on that Kind. diff --git a/lib/src/lints/bool_comparison.rs b/lib/src/lints/bool_comparison.rs index 0b5733b..5c9bee8 100644 --- a/lib/src/lints/bool_comparison.rs +++ b/lib/src/lints/bool_comparison.rs @@ -1,4 +1,4 @@ -use crate::{make, Lint, Metadata, Report, Rule, Suggestion}; +use crate::{make, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -7,6 +7,25 @@ use rnix::{ NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode, }; +/// ## What it does +/// Checks for expressions of the form `x == true`, `x != true` and +/// suggests using the variable directly. +/// +/// ## Why is this bad? +/// Unnecessary code. +/// +/// ## Example +/// Instead of checking the value of `x`: +/// +/// ``` +/// if x == true then 0 else 1 +/// ``` +/// +/// Use `x` directly: +/// +/// ``` +/// if x then 0 else 1 +/// ``` #[lint( name = "bool_comparison", note = "Unnecessary comparison with boolean", @@ -71,7 +90,7 @@ impl Rule for BoolComparison { non_bool_side, bool_side ); - Some(Self::report().suggest(at, message, Suggestion::new(at, replacement))) + Some(self.report().suggest(at, message, Suggestion::new(at, replacement))) } else { None } diff --git a/lib/src/lints/collapsible_let_in.rs b/lib/src/lints/collapsible_let_in.rs index 878d218..21199a8 100644 --- a/lib/src/lints/collapsible_let_in.rs +++ b/lib/src/lints/collapsible_let_in.rs @@ -1,13 +1,41 @@ -use crate::{make, Lint, Metadata, Report, Rule, Suggestion}; +use crate::{make, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; -use rowan::Direction; use rnix::{ types::{LetIn, TypedNode}, - NodeOrToken, SyntaxElement, SyntaxKind, TextRange + NodeOrToken, SyntaxElement, SyntaxKind, TextRange, }; +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", @@ -47,7 +75,7 @@ impl Rule for CollapsibleLetIn { let replacement = make::empty().node().clone(); Some( - Self::report() + self.report() .diagnostic(first_annotation, first_message) .suggest(second_annotation, second_message, Suggestion::new(replacement_at, replacement)) ) @@ -57,4 +85,3 @@ impl Rule for CollapsibleLetIn { } } } - diff --git a/lib/src/lints/empty_let_in.rs b/lib/src/lints/empty_let_in.rs index aae1377..b255c23 100644 --- a/lib/src/lints/empty_let_in.rs +++ b/lib/src/lints/empty_let_in.rs @@ -1,12 +1,30 @@ -use crate::{Lint, Metadata, Report, Rule, Suggestion}; +use crate::{Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; use rnix::{ - types::{LetIn, TypedNode, EntryHolder}, + types::{EntryHolder, LetIn, TypedNode}, NodeOrToken, SyntaxElement, SyntaxKind, }; +/// ## 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", @@ -31,11 +49,10 @@ impl Rule for EmptyLetIn { let at = node.text_range(); let replacement = body; let message = "This let-in expression has no entries"; - Some(Self::report().suggest(at, message, Suggestion::new(at, replacement))) + Some(self.report().suggest(at, message, Suggestion::new(at, replacement))) } else { None } } } } - diff --git a/lib/src/lints/empty_pattern.rs b/lib/src/lints/empty_pattern.rs index 6fb7558..5312548 100644 --- a/lib/src/lints/empty_pattern.rs +++ b/lib/src/lints/empty_pattern.rs @@ -1,4 +1,4 @@ -use crate::{make, Lint, Metadata, Report, Rule, Suggestion}; +use crate::{make, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -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", @@ -28,7 +55,7 @@ impl Rule for EmptyPattern { let at = node.text_range(); let message = "This pattern is empty, use `_` instead"; let replacement = make::ident("_").node().clone(); - Some(Self::report().suggest(at, message, Suggestion::new(at, replacement))) + Some(self.report().suggest(at, message, Suggestion::new(at, replacement))) } else { None } diff --git a/lib/src/lints/eta_reduction.rs b/lib/src/lints/eta_reduction.rs index 79a5101..3a483d0 100644 --- a/lib/src/lints/eta_reduction.rs +++ b/lib/src/lints/eta_reduction.rs @@ -1,4 +1,4 @@ -use crate::{Lint, Metadata, Report, Rule, Suggestion}; +use crate::{Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -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", @@ -43,7 +69,7 @@ impl Rule for EtaReduction { "Found eta-reduction: `{}`", replacement.text().to_string() ); - Some(Self::report().suggest(at, message, Suggestion::new(at, replacement))) + 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 2087e27..139f633 100644 --- a/lib/src/lints/legacy_let_syntax.rs +++ b/lib/src/lints/legacy_let_syntax.rs @@ -1,4 +1,4 @@ -use crate::{make, Lint, Metadata, Report, Rule, Suggestion}; +use crate::{make, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -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", @@ -36,7 +64,7 @@ impl Rule for ManualInherit { let message = "Prefer `rec` over undocumented `let` syntax"; let replacement = selected.node().clone(); - Some(Self::report().suggest(at, message, Suggestion::new(at, replacement))) + Some(self.report().suggest(at, message, Suggestion::new(at, replacement))) } else { None } diff --git a/lib/src/lints/manual_inherit.rs b/lib/src/lints/manual_inherit.rs index 0a6933c..2d119c3 100644 --- a/lib/src/lints/manual_inherit.rs +++ b/lib/src/lints/manual_inherit.rs @@ -1,4 +1,4 @@ -use crate::{make, Lint, Metadata, Report, Rule, Suggestion}; +use crate::{make, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -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", @@ -35,7 +59,7 @@ impl Rule for ManualInherit { let at = node.text_range(); let replacement = make::inherit_stmt(&[key]).node().clone(); let message = "This assignment is better written with `inherit`"; - Some(Self::report().suggest(at, message, Suggestion::new(at, replacement))) + Some(self.report().suggest(at, message, Suggestion::new(at, replacement))) } else { None } diff --git a/lib/src/lints/manual_inherit_from.rs b/lib/src/lints/manual_inherit_from.rs index 794aaf9..8d0f539 100644 --- a/lib/src/lints/manual_inherit_from.rs +++ b/lib/src/lints/manual_inherit_from.rs @@ -1,4 +1,4 @@ -use crate::{make, Lint, Metadata, Report, Rule, Suggestion}; +use crate::{make, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -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", @@ -40,7 +64,7 @@ impl Rule for ManualInherit { make::inherit_from_stmt(set, &[key]).node().clone() }; let message = "This assignment is better written with `inherit`"; - Some(Self::report().suggest(at, message, Suggestion::new(at, replacement))) + Some(self.report().suggest(at, message, Suggestion::new(at, replacement))) } else { None } diff --git a/lib/src/lints/redundant_pattern_bind.rs b/lib/src/lints/redundant_pattern_bind.rs index aebc549..5b0711f 100644 --- a/lib/src/lints/redundant_pattern_bind.rs +++ b/lib/src/lints/redundant_pattern_bind.rs @@ -1,4 +1,4 @@ -use crate::{Lint, Metadata, Report, Rule, Suggestion}; +use crate::{Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -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; @@ -32,7 +51,7 @@ impl Rule for RedundantPatternBind { let at = node.text_range(); let message = format!("This pattern bind is redundant, use `{}` instead", ident.as_str()); let replacement = ident.node().clone(); - Some(Self::report().suggest(at, message, Suggestion::new(at, replacement))) + Some(self.report().suggest(at, message, Suggestion::new(at, replacement))) } else { None } diff --git a/lib/src/lints/unquoted_splice.rs b/lib/src/lints/unquoted_splice.rs index 4d1ed69..c2fd6e4 100644 --- a/lib/src/lints/unquoted_splice.rs +++ b/lib/src/lints/unquoted_splice.rs @@ -1,4 +1,4 @@ -use crate::{make, Lint, Metadata, Report, Rule, Suggestion}; +use crate::{make, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; @@ -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", @@ -24,7 +48,7 @@ impl Rule for UnquotedSplice { let at = node.text_range(); let replacement = make::quote(&node).node().clone(); let message = "Consider quoting this splice expression"; - Some(Self::report().suggest(at, message, Suggestion::new(at, replacement))) + Some(self.report().suggest(at, message, Suggestion::new(at, replacement))) } else { None } diff --git a/lib/src/lints/useless_parens.rs b/lib/src/lints/useless_parens.rs index 2d6ba8f..36ad1b7 100644 --- a/lib/src/lints/useless_parens.rs +++ b/lib/src/lints/useless_parens.rs @@ -1,12 +1,37 @@ -use crate::{Lint, Metadata, Report, Rule, Suggestion, Diagnostic}; +use crate::{Diagnostic, Metadata, Report, Rule, Suggestion}; use if_chain::if_chain; use macros::lint; use rnix::{ - types::{ParsedType, KeyValue, Paren, TypedNode, Wrapper}, + types::{KeyValue, Paren, ParsedType, TypedNode, Wrapper}, 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", @@ -27,7 +52,7 @@ impl Rule for UselessParens { if let Some(diagnostic) = do_thing(parsed_type_node); then { - let mut report = Self::report(); + let mut report = self.report(); report.diagnostics.push(diagnostic); Some(report) } else { @@ -79,7 +104,7 @@ fn do_thing(parsed_type_node: ParsedType) -> Option { if let Some(parsed_inner) = ParsedType::cast(inner_node); if matches!( parsed_inner, - ParsedType::List(_) + ParsedType::List(_) | ParsedType::Paren(_) | ParsedType::Str(_) | ParsedType::AttrSet(_) @@ -95,6 +120,6 @@ fn do_thing(parsed_type_node: ParsedType) -> Option { None } }, - _ => None + _ => None, } } diff --git a/macros/src/explain.rs b/macros/src/explain.rs new file mode 100644 index 0000000..41dc5d4 --- /dev/null +++ b/macros/src/explain.rs @@ -0,0 +1,28 @@ +use proc_macro2::TokenStream as TokenStream2; +use quote::quote; +use syn::{ItemStruct, Lit, Meta, MetaNameValue}; + +pub fn generate_explain_impl(struct_item: &ItemStruct) -> TokenStream2 { + let struct_name = &struct_item.ident; + let explain = struct_item + .attrs + .iter() + .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! { + impl crate::Explain for #struct_name { + fn explanation(&self) -> &'static str { + #explain + } + } + } +} diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 127b4cb..86fa509 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -1,44 +1,12 @@ -use std::collections::HashMap; +mod explain; +mod metadata; +use explain::generate_explain_impl; +use metadata::{generate_meta_impl, RawLintMeta}; use proc_macro::TokenStream; use proc_macro2::TokenStream as TokenStream2; - -use quote::{format_ident, quote}; -use syn::{ - parse::{Parse, ParseStream, Result as ParseResult}, - parse_macro_input, - punctuated::Punctuated, - Expr, Ident, ItemStruct, Lit, Token, -}; - -struct KeyValue { - key: Ident, - _eq: Token![=], - value: Expr, -} - -impl Parse for KeyValue { - fn parse(input: ParseStream) -> ParseResult { - Ok(Self { - key: input.parse()?, - _eq: input.parse()?, - value: input.parse()?, - }) - } -} - -struct LintMeta(HashMap); - -impl Parse for LintMeta { - fn parse(input: ParseStream) -> ParseResult { - Ok(Self( - Punctuated::::parse_terminated(input)? - .into_iter() - .map(|item| (item.key, item.value)) - .collect(), - )) - } -} +use quote::quote; +use syn::{parse_macro_input, Ident, ItemStruct}; fn generate_self_impl(struct_name: &Ident) -> TokenStream2 { quote! { @@ -50,136 +18,16 @@ fn generate_self_impl(struct_name: &Ident) -> TokenStream2 { } } -fn generate_meta_impl(struct_name: &Ident, meta: &LintMeta) -> TokenStream2 { - let name_fn = generate_name_fn(meta); - let note_fn = generate_note_fn(meta); - let code_fn = generate_code_fn(meta); - let report_fn = generate_report_fn(); - let match_with_fn = generate_match_with_fn(meta); - let match_kind = generate_match_kind(meta); - quote! { - impl Metadata for #struct_name { - #name_fn - #note_fn - #code_fn - #report_fn - #match_with_fn - #match_kind - } - } -} - -fn generate_name_fn(meta: &LintMeta) -> TokenStream2 { - let name = meta - .0 - .get(&format_ident!("name")) - .unwrap_or_else(|| panic!("`name` not present")); - if let syn::Expr::Lit(name_lit) = name { - if let Lit::Str(name_str) = &name_lit.lit { - return quote! { - fn name() -> &'static str { - #name_str - } - }; - } - } - panic!("Invalid value for `name`"); -} - -fn generate_note_fn(meta: &LintMeta) -> TokenStream2 { - let note = meta - .0 - .get(&format_ident!("note")) - .unwrap_or_else(|| panic!("`note` not present")); - if let syn::Expr::Lit(note_lit) = note { - if let Lit::Str(note_str) = ¬e_lit.lit { - return quote! { - fn note() -> &'static str { - #note_str - } - }; - } - } - panic!("Invalid value for `note`"); -} - -fn generate_code_fn(meta: &LintMeta) -> TokenStream2 { - let code = meta - .0 - .get(&format_ident!("code")) - .unwrap_or_else(|| panic!("`code` not present")); - if let syn::Expr::Lit(code_lit) = code { - if let Lit::Int(code_int) = &code_lit.lit { - return quote! { - fn code() -> u32 { - #code_int - } - }; - } - } - panic!("Invalid value for `note`"); -} - -fn generate_report_fn() -> TokenStream2 { - quote! { - fn report() -> Report { - Report::new(Self::note(), Self::code()) - } - } -} - -fn generate_match_with_fn(meta: &LintMeta) -> TokenStream2 { - let match_with_lit = meta - .0 - .get(&format_ident!("match_with")) - .unwrap_or_else(|| panic!("`match_with` not present")); - if let syn::Expr::Path(match_path) = match_with_lit { - quote! { - fn match_with(&self, with: &SyntaxKind) -> bool { - #match_path == *with - } - } - } else if let syn::Expr::Array(array_expr) = match_with_lit { - quote! { - fn match_with(&self, with: &SyntaxKind) -> bool { - #array_expr.contains(with) - } - } - } else { - panic!("`match_with` has non-path value") - } -} - -fn generate_match_kind(meta: &LintMeta) -> TokenStream2 { - let match_with_lit = meta - .0 - .get(&format_ident!("match_with")) - .unwrap_or_else(|| panic!("`match_with` not present")); - if let syn::Expr::Path(match_path) = match_with_lit { - quote! { - fn match_kind(&self) -> Vec { - vec![#match_path] - } - } - } else if let syn::Expr::Array(array_expr) = match_with_lit { - quote! { - fn match_kind(&self) -> Vec { - #array_expr.to_vec() - } - } - } else { - panic!("`match_with` has non-path value") - } -} - #[proc_macro_attribute] pub fn lint(attr: TokenStream, item: TokenStream) -> TokenStream { let struct_item = parse_macro_input!(item as ItemStruct); - let meta = parse_macro_input!(attr as LintMeta); + let meta = parse_macro_input!(attr as RawLintMeta); let struct_name = &struct_item.ident; let self_impl = generate_self_impl(struct_name); let meta_impl = generate_meta_impl(struct_name, &meta); + let explain_impl = generate_explain_impl(&struct_item); + (quote! { #struct_item @@ -189,8 +37,9 @@ pub fn lint(attr: TokenStream, item: TokenStream) -> TokenStream { #self_impl #meta_impl + #explain_impl - impl Lint for #struct_name {} + impl crate::Lint for #struct_name {} }) .into() } diff --git a/macros/src/metadata.rs b/macros/src/metadata.rs new file mode 100644 index 0000000..b41eb9c --- /dev/null +++ b/macros/src/metadata.rs @@ -0,0 +1,177 @@ +use std::collections::HashMap; + +use proc_macro2::TokenStream as TokenStream2; +use quote::{format_ident, quote}; +use syn::{ + parse::{Parse, ParseStream, Result}, + punctuated::Punctuated, + Expr, ExprArray, Ident, Lit, Path, Token, +}; + +struct KeyValue { + key: Ident, + _eq: Token![=], + value: Expr, +} + +impl Parse for KeyValue { + fn parse(input: ParseStream) -> Result { + Ok(Self { + key: input.parse()?, + _eq: input.parse()?, + value: input.parse()?, + }) + } +} + +pub struct RawLintMeta(HashMap); + +impl Parse for RawLintMeta { + fn parse(input: ParseStream) -> Result { + Ok(Self( + Punctuated::::parse_terminated(input)? + .into_iter() + .map(|item| (item.key, item.value)) + .collect(), + )) + } +} + +pub struct LintMeta<'μ> { + name: &'μ Lit, + note: &'μ Lit, + code: &'μ Lit, + match_with: MatchWith<'μ>, +} + +enum MatchWith<'π> { + Path(&'π Path), + Array(&'π ExprArray), +} + +fn extract<'λ>(id: &str, raw: &'λ RawLintMeta) -> &'λ Expr { + raw.0 + .get(&format_ident!("{}", id)) + .unwrap_or_else(|| panic!("`{}` not present", id)) +} + +fn as_lit(e: &Expr) -> &Lit { + match e { + Expr::Lit(l) => &l.lit, + _ => panic!("expected a literal"), + } +} + +impl<'μ> LintMeta<'μ> { + fn from_raw(raw: &'μ RawLintMeta) -> Self { + let name = as_lit(extract("name", raw)); + let note = as_lit(extract("note", raw)); + let code = as_lit(extract("code", raw)); + let match_with_expr = extract("match_with", raw); + let match_with = match match_with_expr { + Expr::Path(p) => MatchWith::Path(&p.path), + Expr::Array(a) => MatchWith::Array(a), + _ => panic!("`match_with` is neither a path nor an array"), + }; + Self { + name, + note, + code, + match_with, + } + } + + fn generate_name_fn(&self) -> TokenStream2 { + let name_str = self.name; + return quote! { + fn name(&self) -> &'static str { + #name_str + } + }; + } + + fn generate_note_fn(&self) -> TokenStream2 { + let note_str = self.note; + return quote! { + fn note(&self) -> &'static str { + #note_str + } + }; + } + + fn generate_code_fn(&self) -> TokenStream2 { + let code_int = self.code; + return quote! { + fn code(&self) -> u32 { + #code_int + } + }; + } + + fn generate_match_with_fn(&self) -> TokenStream2 { + match self.match_with { + MatchWith::Path(p) => { + quote! { + fn match_with(&self, with: &SyntaxKind) -> bool { + #p == *with + } + } + } + MatchWith::Array(a) => { + quote! { + fn match_with(&self, with: &SyntaxKind) -> bool { + #a.contains(with) + } + } + } + } + } + + fn generate_match_kind_fn(&self) -> TokenStream2 { + match self.match_with { + MatchWith::Path(p) => { + quote! { + fn match_kind(&self) -> Vec { + vec![#p] + } + } + } + MatchWith::Array(a) => { + quote! { + fn match_kind(&self) -> Vec { + #a.to_vec() + } + } + } + } + } + + fn generate_report_fn(&self) -> TokenStream2 { + quote! { + fn report(&self) -> crate::Report { + crate::Report::new(self.note(), self.code()) + } + } + } +} + +pub fn generate_meta_impl(struct_name: &Ident, meta: &RawLintMeta) -> TokenStream2 { + let not_raw = LintMeta::from_raw(&meta); + let name_fn = not_raw.generate_name_fn(); + let note_fn = not_raw.generate_note_fn(); + let code_fn = not_raw.generate_code_fn(); + let match_with_fn = not_raw.generate_match_with_fn(); + let match_kind = not_raw.generate_match_kind_fn(); + let report_fn = not_raw.generate_report_fn(); + + quote! { + impl crate::Metadata for #struct_name { + #name_fn + #note_fn + #code_fn + #match_with_fn + #match_kind + #report_fn + } + } +} -- cgit v1.2.3