From d1f95f97af85b297eda9e30736978603d74f0af9 Mon Sep 17 00:00:00 2001 From: harehare Date: Fri, 5 Jun 2026 23:11:30 +0900 Subject: [PATCH 1/7] =?UTF-8?q?=E2=9C=A8=20feat(mq-lint):=20add=20mq-lint?= =?UTF-8?q?=20crate=20with=20LintRule=20trait=20and=20rule=20stubs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add new mq-lint crate providing the linting infrastructure and empty rule implementations for all 28 rules defined in mq-lint-rules.md. - LintRule trait with id(), severity(), check() - Diagnostic, LintContext, Linter, Severity types in lib.rs - LintConfig with per-rule enable/disable and complexity thresholds - Rule stubs organized into 5 categories (no mod.rs): correctness/ (10 rules), style/ (9 rules), complexity/ (5 rules), selector/ (5 rules), module/ (4 rules) --- Cargo.lock | 10 ++ Cargo.toml | 1 + crates/mq-lint/Cargo.toml | 10 ++ crates/mq-lint/src/config.rs | 65 +++++++++ crates/mq-lint/src/lib.rs | 133 ++++++++++++++++++ crates/mq-lint/src/rules.rs | 20 +++ crates/mq-lint/src/rules/complexity.rs | 17 +++ .../rules/complexity/complex_interpolation.rs | 17 +++ .../src/rules/complexity/deeply_nested.rs | 17 +++ .../src/rules/complexity/function_too_long.rs | 17 +++ .../rules/complexity/too_many_match_arms.rs | 17 +++ .../src/rules/complexity/too_many_params.rs | 17 +++ crates/mq-lint/src/rules/correctness.rs | 27 ++++ .../correctness/always_true_condition.rs | 17 +++ .../rules/correctness/duplicate_match_arm.rs | 17 +++ .../src/rules/correctness/infinite_loop.rs | 17 +++ .../rules/correctness/missing_else_in_expr.rs | 17 +++ .../src/rules/correctness/shadow_variable.rs | 17 +++ .../src/rules/correctness/unreachable_code.rs | 17 +++ .../rules/correctness/unused_catch_binding.rs | 17 +++ .../src/rules/correctness/unused_function.rs | 17 +++ .../src/rules/correctness/unused_import.rs | 17 +++ .../src/rules/correctness/unused_variable.rs | 17 +++ crates/mq-lint/src/rules/module.rs | 15 ++ .../module/ambiguous_qualified_access.rs | 17 +++ .../src/rules/module/circular_import.rs | 17 +++ .../src/rules/module/missing_module_doc.rs | 17 +++ .../src/rules/module/reexport_private.rs | 17 +++ crates/mq-lint/src/rules/selector.rs | 17 +++ .../src/rules/selector/deprecated_selector.rs | 17 +++ .../src/rules/selector/env_var_in_selector.rs | 17 +++ .../rules/selector/inefficient_selector.rs | 17 +++ .../src/rules/selector/missing_depth_guard.rs | 17 +++ .../rules/selector/selector_always_empty.rs | 17 +++ crates/mq-lint/src/rules/style.rs | 25 ++++ .../src/rules/style/boolean_comparison.rs | 17 +++ .../src/rules/style/naming_convention.rs | 17 +++ .../src/rules/style/prefer_coalesce.rs | 17 +++ .../src/rules/style/prefer_let_over_var.rs | 17 +++ .../src/rules/style/prefer_pipe_style.rs | 17 +++ .../rules/style/prefer_specific_heading.rs | 17 +++ .../rules/style/redundant_boolean_literal.rs | 17 +++ .../mq-lint/src/rules/style/redundant_try.rs | 17 +++ .../src/rules/style/unnecessary_parens.rs | 17 +++ 44 files changed, 901 insertions(+) create mode 100644 crates/mq-lint/Cargo.toml create mode 100644 crates/mq-lint/src/config.rs create mode 100644 crates/mq-lint/src/lib.rs create mode 100644 crates/mq-lint/src/rules.rs create mode 100644 crates/mq-lint/src/rules/complexity.rs create mode 100644 crates/mq-lint/src/rules/complexity/complex_interpolation.rs create mode 100644 crates/mq-lint/src/rules/complexity/deeply_nested.rs create mode 100644 crates/mq-lint/src/rules/complexity/function_too_long.rs create mode 100644 crates/mq-lint/src/rules/complexity/too_many_match_arms.rs create mode 100644 crates/mq-lint/src/rules/complexity/too_many_params.rs create mode 100644 crates/mq-lint/src/rules/correctness.rs create mode 100644 crates/mq-lint/src/rules/correctness/always_true_condition.rs create mode 100644 crates/mq-lint/src/rules/correctness/duplicate_match_arm.rs create mode 100644 crates/mq-lint/src/rules/correctness/infinite_loop.rs create mode 100644 crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs create mode 100644 crates/mq-lint/src/rules/correctness/shadow_variable.rs create mode 100644 crates/mq-lint/src/rules/correctness/unreachable_code.rs create mode 100644 crates/mq-lint/src/rules/correctness/unused_catch_binding.rs create mode 100644 crates/mq-lint/src/rules/correctness/unused_function.rs create mode 100644 crates/mq-lint/src/rules/correctness/unused_import.rs create mode 100644 crates/mq-lint/src/rules/correctness/unused_variable.rs create mode 100644 crates/mq-lint/src/rules/module.rs create mode 100644 crates/mq-lint/src/rules/module/ambiguous_qualified_access.rs create mode 100644 crates/mq-lint/src/rules/module/circular_import.rs create mode 100644 crates/mq-lint/src/rules/module/missing_module_doc.rs create mode 100644 crates/mq-lint/src/rules/module/reexport_private.rs create mode 100644 crates/mq-lint/src/rules/selector.rs create mode 100644 crates/mq-lint/src/rules/selector/deprecated_selector.rs create mode 100644 crates/mq-lint/src/rules/selector/env_var_in_selector.rs create mode 100644 crates/mq-lint/src/rules/selector/inefficient_selector.rs create mode 100644 crates/mq-lint/src/rules/selector/missing_depth_guard.rs create mode 100644 crates/mq-lint/src/rules/selector/selector_always_empty.rs create mode 100644 crates/mq-lint/src/rules/style.rs create mode 100644 crates/mq-lint/src/rules/style/boolean_comparison.rs create mode 100644 crates/mq-lint/src/rules/style/naming_convention.rs create mode 100644 crates/mq-lint/src/rules/style/prefer_coalesce.rs create mode 100644 crates/mq-lint/src/rules/style/prefer_let_over_var.rs create mode 100644 crates/mq-lint/src/rules/style/prefer_pipe_style.rs create mode 100644 crates/mq-lint/src/rules/style/prefer_specific_heading.rs create mode 100644 crates/mq-lint/src/rules/style/redundant_boolean_literal.rs create mode 100644 crates/mq-lint/src/rules/style/redundant_try.rs create mode 100644 crates/mq-lint/src/rules/style/unnecessary_parens.rs diff --git a/Cargo.lock b/Cargo.lock index 6fdca8571..a62f44ab3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2613,6 +2613,16 @@ dependencies = [ "yaml-rust2", ] +[[package]] +name = "mq-lint" +version = "0.1.0" +dependencies = [ + "miette", + "mq-hir", + "mq-lang", + "thiserror 2.0.18", +] + [[package]] name = "mq-lsp" version = "0.6.3" diff --git a/Cargo.toml b/Cargo.toml index 3e65d6b89..a4c4e9b78 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ members = [ "crates/mq-crawler", "crates/mq-check", "crates/mq-test", + "crates/mq-lint", "editors/zed", ] resolver = "3" diff --git a/crates/mq-lint/Cargo.toml b/crates/mq-lint/Cargo.toml new file mode 100644 index 000000000..493ffe8e5 --- /dev/null +++ b/crates/mq-lint/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "mq-lint" +version = "0.1.0" +edition = "2024" + +[dependencies] +mq-hir = {workspace = true} +mq-lang = {workspace = true} +miette = {workspace = true} +thiserror = {workspace = true} diff --git a/crates/mq-lint/src/config.rs b/crates/mq-lint/src/config.rs new file mode 100644 index 000000000..cdb053680 --- /dev/null +++ b/crates/mq-lint/src/config.rs @@ -0,0 +1,65 @@ +//! Configuration for the mq linter. + +use std::collections::HashMap; + +/// Per-rule enable/disable flag. +#[derive(Debug, Clone)] +pub struct RuleConfig { + pub enabled: bool, +} + +impl Default for RuleConfig { + fn default() -> Self { + Self { enabled: true } + } +} + +/// Thresholds for complexity rules. +#[derive(Debug, Clone)] +pub struct ComplexityThresholds { + /// Max lines in a function body before `function_too_long` fires. + pub function_max_lines: usize, + /// Max number of parameters before `too_many_params` fires. + pub max_params: usize, + /// Max nesting depth before `deeply_nested` fires. + pub max_nesting_depth: usize, + /// Max number of match arms before `too_many_match_arms` fires. + pub max_match_arms: usize, + /// Max interpolated expressions before `complex_interpolation` fires. + pub max_interpolation_exprs: usize, +} + +impl Default for ComplexityThresholds { + fn default() -> Self { + Self { + function_max_lines: 50, + max_params: 5, + max_nesting_depth: 4, + max_match_arms: 15, + max_interpolation_exprs: 3, + } + } +} + +/// Top-level linter configuration. +#[derive(Debug, Clone, Default)] +pub struct LintConfig { + /// Per-rule overrides. Rules not listed here use their default enabled state. + pub rules: HashMap, + pub complexity: ComplexityThresholds, +} + +impl LintConfig { + /// Returns `true` if the given rule should run. + pub fn is_rule_enabled(&self, rule_id: &str) -> bool { + self.rules + .get(rule_id) + .map(|r| r.enabled) + .unwrap_or(true) + } + + /// Disable a specific rule by ID. + pub fn disable_rule(&mut self, rule_id: impl Into) { + self.rules.insert(rule_id.into(), RuleConfig { enabled: false }); + } +} diff --git a/crates/mq-lint/src/lib.rs b/crates/mq-lint/src/lib.rs new file mode 100644 index 000000000..d3a1c65e6 --- /dev/null +++ b/crates/mq-lint/src/lib.rs @@ -0,0 +1,133 @@ +//! Linter for the mq language. +//! +//! This crate provides static analysis rules for mq programs, organized into +//! categories: correctness, style, complexity, selector, and module. +//! +//! ## Example +//! +//! ```rust +//! use mq_lint::{Linter, LintContext, LintConfig}; +//! use mq_hir::Hir; +//! +//! let mut hir = Hir::default(); +//! let (source_id, _) = hir.add_code(None, "let x = .h1;"); +//! +//! let config = LintConfig::default(); +//! let ctx = LintContext::new(&hir, source_id, &config); +//! let linter = Linter::default(); +//! let diagnostics = linter.run(&ctx); +//! ``` + +pub mod config; +pub mod rules; + +pub use config::LintConfig; + +use mq_hir::{Hir, SourceId}; + +/// Severity level for a lint diagnostic. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum Severity { + /// Code style suggestion — does not affect correctness. + Style, + /// Performance hint. + Perf, + /// Likely unintentional or suspicious code. + Warn, + /// Definite error that must be fixed. + Error, +} + +impl std::fmt::Display for Severity { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Severity::Style => write!(f, "style"), + Severity::Perf => write!(f, "perf"), + Severity::Warn => write!(f, "warn"), + Severity::Error => write!(f, "error"), + } + } +} + +/// A lint finding produced by a [`LintRule`]. +#[derive(Debug, Clone)] +pub struct Diagnostic { + /// The rule that produced this diagnostic. + pub rule_id: &'static str, + pub severity: Severity, + pub message: String, + /// Source location of the finding, if available. + pub range: Option, + /// Optional suggestion for how to fix the issue. + pub help: Option, +} + +impl Diagnostic { + pub fn new(rule_id: &'static str, severity: Severity, message: impl Into) -> Self { + Self { + rule_id, + severity, + message: message.into(), + range: None, + help: None, + } + } + + pub fn with_range(mut self, range: mq_lang::Range) -> Self { + self.range = Some(range); + self + } + + pub fn with_help(mut self, help: impl Into) -> Self { + self.help = Some(help.into()); + self + } +} + +/// Context passed to each [`LintRule`] during analysis. +pub struct LintContext<'a> { + pub hir: &'a Hir, + pub source_id: SourceId, + pub config: &'a LintConfig, +} + +impl<'a> LintContext<'a> { + pub fn new(hir: &'a Hir, source_id: SourceId, config: &'a LintConfig) -> Self { + Self { hir, source_id, config } + } +} + +/// A single lint rule. +pub trait LintRule: Send + Sync { + /// Unique identifier for this rule (e.g. `"unused_variable"`). + fn id(&self) -> &'static str; + + /// Default severity when the rule fires. + fn severity(&self) -> Severity; + + /// Analyze the HIR and return any diagnostics. + fn check(&self, ctx: &LintContext<'_>) -> Vec; +} + +/// Runs all registered [`LintRule`]s against a [`LintContext`]. +#[derive(Default)] +pub struct Linter { + rules: Vec>, +} + +impl Linter { + /// Create a linter with the full default rule set. + pub fn with_default_rules() -> Self { + Self { + rules: rules::all_rules(), + } + } + + pub fn run(&self, ctx: &LintContext<'_>) -> Vec { + self.rules + .iter() + .filter(|rule| ctx.config.is_rule_enabled(rule.id())) + .flat_map(|rule| rule.check(ctx)) + .collect() + } +} diff --git a/crates/mq-lint/src/rules.rs b/crates/mq-lint/src/rules.rs new file mode 100644 index 000000000..1d2f94f94 --- /dev/null +++ b/crates/mq-lint/src/rules.rs @@ -0,0 +1,20 @@ +//! All lint rules, organized by category. + +pub mod complexity; +pub mod correctness; +pub mod module; +pub mod selector; +pub mod style; + +use crate::LintRule; + +/// Returns all built-in lint rules. +pub fn all_rules() -> Vec> { + let mut rules: Vec> = Vec::new(); + rules.extend(correctness::all()); + rules.extend(style::all()); + rules.extend(complexity::all()); + rules.extend(selector::all()); + rules.extend(module::all()); + rules +} diff --git a/crates/mq-lint/src/rules/complexity.rs b/crates/mq-lint/src/rules/complexity.rs new file mode 100644 index 000000000..ce3039b4c --- /dev/null +++ b/crates/mq-lint/src/rules/complexity.rs @@ -0,0 +1,17 @@ +pub mod complex_interpolation; +pub mod deeply_nested; +pub mod function_too_long; +pub mod too_many_match_arms; +pub mod too_many_params; + +use crate::LintRule; + +pub fn all() -> Vec> { + vec![ + Box::new(function_too_long::FunctionTooLong), + Box::new(too_many_params::TooManyParams), + Box::new(deeply_nested::DeeplyNested), + Box::new(too_many_match_arms::TooManyMatchArms), + Box::new(complex_interpolation::ComplexInterpolation), + ] +} diff --git a/crates/mq-lint/src/rules/complexity/complex_interpolation.rs b/crates/mq-lint/src/rules/complexity/complex_interpolation.rs new file mode 100644 index 000000000..f1c6c250c --- /dev/null +++ b/crates/mq-lint/src/rules/complexity/complex_interpolation.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct ComplexInterpolation; + +impl LintRule for ComplexInterpolation { + fn id(&self) -> &'static str { + "complex_interpolation" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/complexity/deeply_nested.rs b/crates/mq-lint/src/rules/complexity/deeply_nested.rs new file mode 100644 index 000000000..b7be7a19d --- /dev/null +++ b/crates/mq-lint/src/rules/complexity/deeply_nested.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct DeeplyNested; + +impl LintRule for DeeplyNested { + fn id(&self) -> &'static str { + "deeply_nested" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/complexity/function_too_long.rs b/crates/mq-lint/src/rules/complexity/function_too_long.rs new file mode 100644 index 000000000..b988b7e85 --- /dev/null +++ b/crates/mq-lint/src/rules/complexity/function_too_long.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct FunctionTooLong; + +impl LintRule for FunctionTooLong { + fn id(&self) -> &'static str { + "function_too_long" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/complexity/too_many_match_arms.rs b/crates/mq-lint/src/rules/complexity/too_many_match_arms.rs new file mode 100644 index 000000000..b2761d93e --- /dev/null +++ b/crates/mq-lint/src/rules/complexity/too_many_match_arms.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct TooManyMatchArms; + +impl LintRule for TooManyMatchArms { + fn id(&self) -> &'static str { + "too_many_match_arms" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/complexity/too_many_params.rs b/crates/mq-lint/src/rules/complexity/too_many_params.rs new file mode 100644 index 000000000..714783fd8 --- /dev/null +++ b/crates/mq-lint/src/rules/complexity/too_many_params.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct TooManyParams; + +impl LintRule for TooManyParams { + fn id(&self) -> &'static str { + "too_many_params" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/correctness.rs b/crates/mq-lint/src/rules/correctness.rs new file mode 100644 index 000000000..f78770d08 --- /dev/null +++ b/crates/mq-lint/src/rules/correctness.rs @@ -0,0 +1,27 @@ +pub mod always_true_condition; +pub mod duplicate_match_arm; +pub mod infinite_loop; +pub mod missing_else_in_expr; +pub mod shadow_variable; +pub mod unreachable_code; +pub mod unused_catch_binding; +pub mod unused_function; +pub mod unused_import; +pub mod unused_variable; + +use crate::LintRule; + +pub fn all() -> Vec> { + vec![ + Box::new(unused_variable::UnusedVariable), + Box::new(unused_function::UnusedFunction), + Box::new(unused_import::UnusedImport), + Box::new(unreachable_code::UnreachableCode), + Box::new(infinite_loop::InfiniteLoop), + Box::new(duplicate_match_arm::DuplicateMatchArm), + Box::new(shadow_variable::ShadowVariable), + Box::new(missing_else_in_expr::MissingElseInExpr), + Box::new(always_true_condition::AlwaysTrueCondition), + Box::new(unused_catch_binding::UnusedCatchBinding), + ] +} diff --git a/crates/mq-lint/src/rules/correctness/always_true_condition.rs b/crates/mq-lint/src/rules/correctness/always_true_condition.rs new file mode 100644 index 000000000..d9233f775 --- /dev/null +++ b/crates/mq-lint/src/rules/correctness/always_true_condition.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct AlwaysTrueCondition; + +impl LintRule for AlwaysTrueCondition { + fn id(&self) -> &'static str { + "always_true_condition" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/correctness/duplicate_match_arm.rs b/crates/mq-lint/src/rules/correctness/duplicate_match_arm.rs new file mode 100644 index 000000000..6bbd5440d --- /dev/null +++ b/crates/mq-lint/src/rules/correctness/duplicate_match_arm.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct DuplicateMatchArm; + +impl LintRule for DuplicateMatchArm { + fn id(&self) -> &'static str { + "duplicate_match_arm" + } + + fn severity(&self) -> Severity { + Severity::Error + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/correctness/infinite_loop.rs b/crates/mq-lint/src/rules/correctness/infinite_loop.rs new file mode 100644 index 000000000..f6f6ae784 --- /dev/null +++ b/crates/mq-lint/src/rules/correctness/infinite_loop.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct InfiniteLoop; + +impl LintRule for InfiniteLoop { + fn id(&self) -> &'static str { + "infinite_loop" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs b/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs new file mode 100644 index 000000000..caac39325 --- /dev/null +++ b/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct MissingElseInExpr; + +impl LintRule for MissingElseInExpr { + fn id(&self) -> &'static str { + "missing_else_in_expr" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/correctness/shadow_variable.rs b/crates/mq-lint/src/rules/correctness/shadow_variable.rs new file mode 100644 index 000000000..270859b65 --- /dev/null +++ b/crates/mq-lint/src/rules/correctness/shadow_variable.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct ShadowVariable; + +impl LintRule for ShadowVariable { + fn id(&self) -> &'static str { + "shadow_variable" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/correctness/unreachable_code.rs b/crates/mq-lint/src/rules/correctness/unreachable_code.rs new file mode 100644 index 000000000..1cca20a80 --- /dev/null +++ b/crates/mq-lint/src/rules/correctness/unreachable_code.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct UnreachableCode; + +impl LintRule for UnreachableCode { + fn id(&self) -> &'static str { + "unreachable_code" + } + + fn severity(&self) -> Severity { + Severity::Error + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/correctness/unused_catch_binding.rs b/crates/mq-lint/src/rules/correctness/unused_catch_binding.rs new file mode 100644 index 000000000..b12b9bfb2 --- /dev/null +++ b/crates/mq-lint/src/rules/correctness/unused_catch_binding.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct UnusedCatchBinding; + +impl LintRule for UnusedCatchBinding { + fn id(&self) -> &'static str { + "unused_catch_binding" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/correctness/unused_function.rs b/crates/mq-lint/src/rules/correctness/unused_function.rs new file mode 100644 index 000000000..bc1067d71 --- /dev/null +++ b/crates/mq-lint/src/rules/correctness/unused_function.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct UnusedFunction; + +impl LintRule for UnusedFunction { + fn id(&self) -> &'static str { + "unused_function" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/correctness/unused_import.rs b/crates/mq-lint/src/rules/correctness/unused_import.rs new file mode 100644 index 000000000..c4ae13d31 --- /dev/null +++ b/crates/mq-lint/src/rules/correctness/unused_import.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct UnusedImport; + +impl LintRule for UnusedImport { + fn id(&self) -> &'static str { + "unused_import" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/correctness/unused_variable.rs b/crates/mq-lint/src/rules/correctness/unused_variable.rs new file mode 100644 index 000000000..efb7c9cbf --- /dev/null +++ b/crates/mq-lint/src/rules/correctness/unused_variable.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct UnusedVariable; + +impl LintRule for UnusedVariable { + fn id(&self) -> &'static str { + "unused_variable" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/module.rs b/crates/mq-lint/src/rules/module.rs new file mode 100644 index 000000000..18dc1d6aa --- /dev/null +++ b/crates/mq-lint/src/rules/module.rs @@ -0,0 +1,15 @@ +pub mod ambiguous_qualified_access; +pub mod circular_import; +pub mod missing_module_doc; +pub mod reexport_private; + +use crate::LintRule; + +pub fn all() -> Vec> { + vec![ + Box::new(circular_import::CircularImport), + Box::new(missing_module_doc::MissingModuleDoc), + Box::new(reexport_private::ReexportPrivate), + Box::new(ambiguous_qualified_access::AmbiguousQualifiedAccess), + ] +} diff --git a/crates/mq-lint/src/rules/module/ambiguous_qualified_access.rs b/crates/mq-lint/src/rules/module/ambiguous_qualified_access.rs new file mode 100644 index 000000000..d74c79241 --- /dev/null +++ b/crates/mq-lint/src/rules/module/ambiguous_qualified_access.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct AmbiguousQualifiedAccess; + +impl LintRule for AmbiguousQualifiedAccess { + fn id(&self) -> &'static str { + "ambiguous_qualified_access" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/module/circular_import.rs b/crates/mq-lint/src/rules/module/circular_import.rs new file mode 100644 index 000000000..3d6b4904a --- /dev/null +++ b/crates/mq-lint/src/rules/module/circular_import.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct CircularImport; + +impl LintRule for CircularImport { + fn id(&self) -> &'static str { + "circular_import" + } + + fn severity(&self) -> Severity { + Severity::Error + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/module/missing_module_doc.rs b/crates/mq-lint/src/rules/module/missing_module_doc.rs new file mode 100644 index 000000000..4f9a2aa6c --- /dev/null +++ b/crates/mq-lint/src/rules/module/missing_module_doc.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct MissingModuleDoc; + +impl LintRule for MissingModuleDoc { + fn id(&self) -> &'static str { + "missing_module_doc" + } + + fn severity(&self) -> Severity { + Severity::Style + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/module/reexport_private.rs b/crates/mq-lint/src/rules/module/reexport_private.rs new file mode 100644 index 000000000..23a7a65b6 --- /dev/null +++ b/crates/mq-lint/src/rules/module/reexport_private.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct ReexportPrivate; + +impl LintRule for ReexportPrivate { + fn id(&self) -> &'static str { + "reexport_private" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/selector.rs b/crates/mq-lint/src/rules/selector.rs new file mode 100644 index 000000000..a9bb95781 --- /dev/null +++ b/crates/mq-lint/src/rules/selector.rs @@ -0,0 +1,17 @@ +pub mod deprecated_selector; +pub mod env_var_in_selector; +pub mod inefficient_selector; +pub mod missing_depth_guard; +pub mod selector_always_empty; + +use crate::LintRule; + +pub fn all() -> Vec> { + vec![ + Box::new(inefficient_selector::InefficientSelector), + Box::new(selector_always_empty::SelectorAlwaysEmpty), + Box::new(env_var_in_selector::EnvVarInSelector), + Box::new(missing_depth_guard::MissingDepthGuard), + Box::new(deprecated_selector::DeprecatedSelector), + ] +} diff --git a/crates/mq-lint/src/rules/selector/deprecated_selector.rs b/crates/mq-lint/src/rules/selector/deprecated_selector.rs new file mode 100644 index 000000000..e90d708a7 --- /dev/null +++ b/crates/mq-lint/src/rules/selector/deprecated_selector.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct DeprecatedSelector; + +impl LintRule for DeprecatedSelector { + fn id(&self) -> &'static str { + "deprecated_selector" + } + + fn severity(&self) -> Severity { + Severity::Error + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/selector/env_var_in_selector.rs b/crates/mq-lint/src/rules/selector/env_var_in_selector.rs new file mode 100644 index 000000000..0e3e4b172 --- /dev/null +++ b/crates/mq-lint/src/rules/selector/env_var_in_selector.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct EnvVarInSelector; + +impl LintRule for EnvVarInSelector { + fn id(&self) -> &'static str { + "env_var_in_selector" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/selector/inefficient_selector.rs b/crates/mq-lint/src/rules/selector/inefficient_selector.rs new file mode 100644 index 000000000..ffd6f6e49 --- /dev/null +++ b/crates/mq-lint/src/rules/selector/inefficient_selector.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct InefficientSelector; + +impl LintRule for InefficientSelector { + fn id(&self) -> &'static str { + "inefficient_selector" + } + + fn severity(&self) -> Severity { + Severity::Perf + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/selector/missing_depth_guard.rs b/crates/mq-lint/src/rules/selector/missing_depth_guard.rs new file mode 100644 index 000000000..05d8aeae4 --- /dev/null +++ b/crates/mq-lint/src/rules/selector/missing_depth_guard.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct MissingDepthGuard; + +impl LintRule for MissingDepthGuard { + fn id(&self) -> &'static str { + "missing_depth_guard" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/selector/selector_always_empty.rs b/crates/mq-lint/src/rules/selector/selector_always_empty.rs new file mode 100644 index 000000000..a76be8c89 --- /dev/null +++ b/crates/mq-lint/src/rules/selector/selector_always_empty.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct SelectorAlwaysEmpty; + +impl LintRule for SelectorAlwaysEmpty { + fn id(&self) -> &'static str { + "selector_always_empty" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/style.rs b/crates/mq-lint/src/rules/style.rs new file mode 100644 index 000000000..eefc27ed3 --- /dev/null +++ b/crates/mq-lint/src/rules/style.rs @@ -0,0 +1,25 @@ +pub mod boolean_comparison; +pub mod naming_convention; +pub mod prefer_coalesce; +pub mod prefer_let_over_var; +pub mod prefer_pipe_style; +pub mod prefer_specific_heading; +pub mod redundant_boolean_literal; +pub mod redundant_try; +pub mod unnecessary_parens; + +use crate::LintRule; + +pub fn all() -> Vec> { + vec![ + Box::new(prefer_let_over_var::PreferLetOverVar), + Box::new(unnecessary_parens::UnnecessaryParens), + Box::new(prefer_pipe_style::PreferPipeStyle), + Box::new(prefer_coalesce::PreferCoalesce), + Box::new(prefer_specific_heading::PreferSpecificHeading), + Box::new(redundant_try::RedundantTry), + Box::new(naming_convention::NamingConvention), + Box::new(boolean_comparison::BooleanComparison), + Box::new(redundant_boolean_literal::RedundantBooleanLiteral), + ] +} diff --git a/crates/mq-lint/src/rules/style/boolean_comparison.rs b/crates/mq-lint/src/rules/style/boolean_comparison.rs new file mode 100644 index 000000000..497712e9d --- /dev/null +++ b/crates/mq-lint/src/rules/style/boolean_comparison.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct BooleanComparison; + +impl LintRule for BooleanComparison { + fn id(&self) -> &'static str { + "boolean_comparison" + } + + fn severity(&self) -> Severity { + Severity::Style + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/style/naming_convention.rs b/crates/mq-lint/src/rules/style/naming_convention.rs new file mode 100644 index 000000000..71005148b --- /dev/null +++ b/crates/mq-lint/src/rules/style/naming_convention.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct NamingConvention; + +impl LintRule for NamingConvention { + fn id(&self) -> &'static str { + "naming_convention" + } + + fn severity(&self) -> Severity { + Severity::Style + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/style/prefer_coalesce.rs b/crates/mq-lint/src/rules/style/prefer_coalesce.rs new file mode 100644 index 000000000..07afda0ca --- /dev/null +++ b/crates/mq-lint/src/rules/style/prefer_coalesce.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct PreferCoalesce; + +impl LintRule for PreferCoalesce { + fn id(&self) -> &'static str { + "prefer_coalesce" + } + + fn severity(&self) -> Severity { + Severity::Style + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/style/prefer_let_over_var.rs b/crates/mq-lint/src/rules/style/prefer_let_over_var.rs new file mode 100644 index 000000000..cea781f6d --- /dev/null +++ b/crates/mq-lint/src/rules/style/prefer_let_over_var.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct PreferLetOverVar; + +impl LintRule for PreferLetOverVar { + fn id(&self) -> &'static str { + "prefer_let_over_var" + } + + fn severity(&self) -> Severity { + Severity::Warn + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/style/prefer_pipe_style.rs b/crates/mq-lint/src/rules/style/prefer_pipe_style.rs new file mode 100644 index 000000000..60dbc722c --- /dev/null +++ b/crates/mq-lint/src/rules/style/prefer_pipe_style.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct PreferPipeStyle; + +impl LintRule for PreferPipeStyle { + fn id(&self) -> &'static str { + "prefer_pipe_style" + } + + fn severity(&self) -> Severity { + Severity::Style + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/style/prefer_specific_heading.rs b/crates/mq-lint/src/rules/style/prefer_specific_heading.rs new file mode 100644 index 000000000..051297a4a --- /dev/null +++ b/crates/mq-lint/src/rules/style/prefer_specific_heading.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct PreferSpecificHeading; + +impl LintRule for PreferSpecificHeading { + fn id(&self) -> &'static str { + "prefer_specific_heading" + } + + fn severity(&self) -> Severity { + Severity::Style + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/style/redundant_boolean_literal.rs b/crates/mq-lint/src/rules/style/redundant_boolean_literal.rs new file mode 100644 index 000000000..8670dc320 --- /dev/null +++ b/crates/mq-lint/src/rules/style/redundant_boolean_literal.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct RedundantBooleanLiteral; + +impl LintRule for RedundantBooleanLiteral { + fn id(&self) -> &'static str { + "redundant_boolean_literal" + } + + fn severity(&self) -> Severity { + Severity::Style + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/style/redundant_try.rs b/crates/mq-lint/src/rules/style/redundant_try.rs new file mode 100644 index 000000000..69de32045 --- /dev/null +++ b/crates/mq-lint/src/rules/style/redundant_try.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct RedundantTry; + +impl LintRule for RedundantTry { + fn id(&self) -> &'static str { + "redundant_try" + } + + fn severity(&self) -> Severity { + Severity::Style + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} diff --git a/crates/mq-lint/src/rules/style/unnecessary_parens.rs b/crates/mq-lint/src/rules/style/unnecessary_parens.rs new file mode 100644 index 000000000..83615aabe --- /dev/null +++ b/crates/mq-lint/src/rules/style/unnecessary_parens.rs @@ -0,0 +1,17 @@ +use crate::{Diagnostic, LintContext, LintRule, Severity}; + +pub struct UnnecessaryParens; + +impl LintRule for UnnecessaryParens { + fn id(&self) -> &'static str { + "unnecessary_parens" + } + + fn severity(&self) -> Severity { + Severity::Style + } + + fn check(&self, _ctx: &LintContext<'_>) -> Vec { + Vec::new() + } +} From 47483fb962be1ba72463c26dfd2d9922919c7341 Mon Sep 17 00:00:00 2001 From: harehare Date: Sat, 6 Jun 2026 22:39:00 +0900 Subject: [PATCH 2/7] =?UTF-8?q?=E2=9C=A8=20feat(mq-lint):=20implement=20li?= =?UTF-8?q?nt=20rules=20and=20add=20README?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement 15 lint rules across correctness, style, complexity, and selector categories: Correctness: - unused_variable, unused_function, unused_import - unreachable_code, infinite_loop, duplicate_match_arm - shadow_variable, missing_else_in_expr, always_true_condition Style: - prefer_let_over_var, naming_convention, boolean_comparison - redundant_boolean_literal, prefer_specific_heading Complexity: - function_too_long, too_many_params, deeply_nested - too_many_match_arms, complex_interpolation Selector: - inefficient_selector, missing_depth_guard Also fix mq-hir selector_from_cst_node to map DoubleDot tokens to Selector::Recursive so that `..` creates a proper HIR symbol. Add LintContext::all_symbols() helper to cover symbols registered via insert_symbol (Variable, Selector, Ref, Keyword) that are not visible through symbols_for_source(). Add README.md for the mq-lint crate with usage examples, rule tables, and architecture overview. ✨ feat(mq-lint): implement remaining rules and wire lint into mq-lsp Implements check() logic for the rules that were previously stubs (ambiguous_qualified_access, missing_module_doc, selector_always_empty, prefer_coalesce, prefer_pipe_style, redundant_try) and drops rules that were out of scope for this pass (unused_catch_binding, circular_import, reexport_private, deprecated_selector, env_var_in_selector, unnecessary_parens). Packages mq-lint as a standalone CLI binary and adds --enable-lint/--disable-lint-rule support to mq-lsp, surfaced in the VS Code, Neovim, and Zed clients. --- Cargo.lock | 7 +- Cargo.toml | 1 + crates/mq-hir/src/hir/lower.rs | 5 + crates/mq-lint/Cargo.toml | 33 +- crates/mq-lint/README.md | 238 +++++++++++++++ crates/mq-lint/src/config.rs | 5 +- crates/mq-lint/src/lib.rs | 15 +- crates/mq-lint/src/main.rs | 281 ++++++++++++++++++ .../rules/complexity/complex_interpolation.rs | 61 +++- .../src/rules/complexity/deeply_nested.rs | 84 +++++- .../src/rules/complexity/function_too_long.rs | 49 ++- .../rules/complexity/too_many_match_arms.rs | 66 +++- .../src/rules/complexity/too_many_params.rs | 59 +++- crates/mq-lint/src/rules/correctness.rs | 2 - .../correctness/always_true_condition.rs | 83 +++++- .../rules/correctness/duplicate_match_arm.rs | 85 +++++- .../src/rules/correctness/infinite_loop.rs | 74 ++++- .../rules/correctness/missing_else_in_expr.rs | 70 ++++- .../src/rules/correctness/shadow_variable.rs | 81 ++++- .../src/rules/correctness/unreachable_code.rs | 66 +++- .../rules/correctness/unused_catch_binding.rs | 17 -- .../src/rules/correctness/unused_function.rs | 64 +++- .../src/rules/correctness/unused_import.rs | 56 +++- .../src/rules/correctness/unused_variable.rs | 66 +++- crates/mq-lint/src/rules/module.rs | 4 - .../module/ambiguous_qualified_access.rs | 90 +++++- .../src/rules/module/circular_import.rs | 17 -- .../src/rules/module/missing_module_doc.rs | 49 ++- .../src/rules/module/reexport_private.rs | 17 -- crates/mq-lint/src/rules/selector.rs | 4 - .../src/rules/selector/deprecated_selector.rs | 17 -- .../src/rules/selector/env_var_in_selector.rs | 17 -- .../rules/selector/inefficient_selector.rs | 89 +++++- .../src/rules/selector/missing_depth_guard.rs | 72 ++++- .../rules/selector/selector_always_empty.rs | 99 +++++- crates/mq-lint/src/rules/style.rs | 2 - .../src/rules/style/boolean_comparison.rs | 68 ++++- .../src/rules/style/naming_convention.rs | 91 +++++- .../src/rules/style/prefer_coalesce.rs | 128 +++++++- .../src/rules/style/prefer_let_over_var.rs | 94 +++++- .../src/rules/style/prefer_pipe_style.rs | 74 ++++- .../rules/style/prefer_specific_heading.rs | 49 ++- .../rules/style/redundant_boolean_literal.rs | 136 ++++++++- .../mq-lint/src/rules/style/redundant_try.rs | 61 +++- .../src/rules/style/unnecessary_parens.rs | 17 -- crates/mq-lsp/Cargo.toml | 1 + crates/mq-lsp/README.md | 11 + crates/mq-lsp/src/error.rs | 115 +++++-- crates/mq-lsp/src/main.rs | 21 ++ crates/mq-lsp/src/server.rs | 150 +++++++++- docs/books/src/start/external_subcommands.md | 1 + editors/neovim/README.md | 17 ++ editors/neovim/lua/mq/config.lua | 4 + editors/neovim/lua/mq/lsp.lua | 11 + editors/vscode/package.json | 15 + editors/vscode/src/extension.ts | 9 + editors/zed/README.md | 22 ++ editors/zed/src/lib.rs | 30 +- 58 files changed, 2962 insertions(+), 208 deletions(-) create mode 100644 crates/mq-lint/README.md create mode 100644 crates/mq-lint/src/main.rs delete mode 100644 crates/mq-lint/src/rules/correctness/unused_catch_binding.rs delete mode 100644 crates/mq-lint/src/rules/module/circular_import.rs delete mode 100644 crates/mq-lint/src/rules/module/reexport_private.rs delete mode 100644 crates/mq-lint/src/rules/selector/deprecated_selector.rs delete mode 100644 crates/mq-lint/src/rules/selector/env_var_in_selector.rs delete mode 100644 crates/mq-lint/src/rules/style/unnecessary_parens.rs diff --git a/Cargo.lock b/Cargo.lock index a62f44ab3..9cb54477b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2615,11 +2615,15 @@ dependencies = [ [[package]] name = "mq-lint" -version = "0.1.0" +version = "0.6.3" dependencies = [ + "clap", + "colored 3.1.1", "miette", "mq-hir", "mq-lang", + "rstest", + "rustc-hash", "thiserror 2.0.18", ] @@ -2635,6 +2639,7 @@ dependencies = [ "mq-formatter", "mq-hir", "mq-lang", + "mq-lint", "mq-markdown", "rustc-hash", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index a4c4e9b78..d406a5085 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ mq-lang = {path = "crates/mq-lang", version = "0.6.3"} mq-macros = {path = "crates/mq-macros", version = "0.6.3"} mq-markdown = {path = "crates/mq-markdown", version = "0.6.3"} mq-repl = {path = "crates/mq-repl", version = "0.6.3"} +mq-lint = {path = "crates/mq-lint", version = "0.6.2"} # External dependencies arbitrary = {version = "1.4.2", features = ["derive"]} arboard = {version = "3.6.1", default-features = false} diff --git a/crates/mq-hir/src/hir/lower.rs b/crates/mq-hir/src/hir/lower.rs index 249bcd406..42a28d396 100644 --- a/crates/mq-hir/src/hir/lower.rs +++ b/crates/mq-hir/src/hir/lower.rs @@ -22,6 +22,11 @@ use crate::{ fn selector_from_cst_node(node: &mq_lang::CstNode) -> Option { let token = node.token.as_ref()?; + // `..` is tokenized as `DoubleDot`, not as `Selector(".."), so handle it explicitly. + if matches!(token.kind, TokenKind::DoubleDot) { + return Some(mq_lang::Selector::Recursive); + } + if !matches!(&token.kind, TokenKind::Selector(s) if s == ".") { return mq_lang::Selector::try_from(&**token).ok(); } diff --git a/crates/mq-lint/Cargo.toml b/crates/mq-lint/Cargo.toml index 493ffe8e5..4fd0e47ea 100644 --- a/crates/mq-lint/Cargo.toml +++ b/crates/mq-lint/Cargo.toml @@ -1,10 +1,37 @@ [package] -name = "mq-lint" -version = "0.1.0" +authors = ["Takahiro Sato "] +categories = ["command-line-utilities", "text-processing"] +description = "Static analysis linter for mq" edition = "2024" +homepage = "https://mqlang.org/" +keywords = ["markdown", "jq", "query", "lint"] +license = "MIT" +name = "mq-lint" +readme = "README.md" +repository = "https://github.com/harehare/mq" +version = "0.6.3" + +[features] +cli = ["dep:clap", "dep:colored"] + +[[bin]] +name = "mq-lint" +path = "src/main.rs" +required-features = ["cli"] + +[package.metadata.binstall] +pkg-fmt = "bin" +pkg-url = "{ repo }/releases/download/v{ version }/{ bin }-{ target }{ binary-ext }" +disabled-strategies = ["quick-install"] [dependencies] +clap = {workspace = true, features = ["derive"], optional = true} +colored = {workspace = true, optional = true} +miette = {workspace = true} mq-hir = {workspace = true} mq-lang = {workspace = true} -miette = {workspace = true} +rustc-hash = {workspace = true} thiserror = {workspace = true} + +[dev-dependencies] +rstest = {workspace = true} diff --git a/crates/mq-lint/README.md b/crates/mq-lint/README.md new file mode 100644 index 000000000..223dda482 --- /dev/null +++ b/crates/mq-lint/README.md @@ -0,0 +1,238 @@ +

mq-lint

+ +Static analysis linter for the mq language. + +`mq-lint` analyses mq programs by walking the HIR (High-level Intermediate Representation) and reporting diagnostics across five categories: correctness, style, complexity, selector, and module. + +## Usage + +### As a Library + +```rust +use mq_lint::{Linter, LintContext, LintConfig}; +use mq_hir::Hir; + +// Build the HIR from mq source code +let mut hir = Hir::default(); +let (source_id, _) = hir.add_code(None, "let x = .h1;"); + +// Configure and run the linter +let config = LintConfig::default(); +let ctx = LintContext::new(&hir, source_id, &config); +let linter = Linter::with_default_rules(); +let diagnostics = linter.run(&ctx); + +for d in diagnostics { + eprintln!("[{}] {} — {}", d.severity, d.rule_id, d.message); + if let Some(help) = &d.help { + eprintln!(" help: {}", help); + } +} +``` + +### As a CLI + +Build with the `cli` feature to get the `mq-lint` binary (also invocable as `mq lint` if placed on your `PATH`, see [External Subcommands](https://mqlang.org/book/start/external_subcommands.html)): + +```bash +cargo install --path crates/mq-lint --features cli +``` + +```bash +mq-lint script.mq +mq-lint script.mq another.mq # lint multiple files +echo "let x = .h1" | mq-lint # read from stdin +mq-lint --disable naming_convention script.mq +mq-lint --min-severity warn script.mq # only show warn/error diagnostics +mq-lint --list-rules # print all rule IDs and their severity +``` + +Exits with a non-zero status if any diagnostic (at or above `--min-severity`) was reported. + +### Disabling Rules + +```rust +let mut config = LintConfig::default(); +config.disable_rule("naming_convention"); +config.disable_rule("shadow_variable"); +``` + +### Adjusting Complexity Thresholds + +```rust +let mut config = LintConfig::default(); +config.complexity.function_max_lines = 80; +config.complexity.max_params = 6; +config.complexity.max_nesting_depth = 5; +config.complexity.max_match_arms = 20; +config.complexity.max_interpolation_exprs = 4; +``` + +## Rules + +### Correctness + +| Rule ID | Severity | Description | +| ----------------------- | -------- | ------------------------------------------------------------------------------ | +| `unused_variable` | warn | `let`/`var` variable declared but never referenced | +| `unused_function` | warn | `def` function defined but never called | +| `unused_import` | warn | `import` module declared but never accessed | +| `unreachable_code` | error | Code following `break`/`continue` that can never execute | +| `infinite_loop` | warn | `loop` body without a `break` | +| `duplicate_match_arm` | error | Same pattern appears more than once in a `match` | +| `shadow_variable` | warn | Variable re-declared in an inner scope with the same name as an outer variable | +| `missing_else_in_expr` | warn | `if` expression with no `else` branch (evaluates to `none` on false) | +| `always_true_condition` | warn | `if` condition is a literal `true` or `false` | + +**Example — `unused_variable`** + +```mq +let x = .h1; # warn: x is never used +.text +``` + +**Example — `missing_else_in_expr`** + +```mq +if (.h1): "heading" # warn: no else branch — produces `none` when condition is false +``` + +**Example — `always_true_condition`** + +```mq +if (true): 1 else: 2; # warn: condition is always `true` +``` + +### Style / Best Practices + +| Rule ID | Severity | Description | +| --------------------------- | -------- | -------------------------------------------------------------------- | +| `prefer_let_over_var` | warn | `var` variable never reassigned — prefer `let` | +| `naming_convention` | style | Function or variable name is not `snake_case` | +| `boolean_comparison` | style | `x == true` → `x`, `x == false` → `not(x)` | +| `redundant_boolean_literal` | style | `if (cond): true else: false` simplifies to `cond` | +| `prefer_specific_heading` | style | `.h` without a level — prefer `.h1`–`.h6` | +| `prefer_coalesce` | style | `if (x == none): fallback else: x` simplifies to `x ?? fallback` | +| `prefer_pipe_style` | style | Nested unary call `f(g(x))` reads better as a pipe `x \| g() \| f()` | +| `redundant_try` | style | `try: ... catch: none` is exactly what the `?` operator does | + +**Example — `prefer_let_over_var`** + +```mq +# Before (warn): x is never reassigned +var x = .h1 +| x | to_text() + +# After +let x = .h1 +| x | to_text() +``` + +**Example — `boolean_comparison`** + +```mq +if (.checked == true): "yes" # style: simplify to `if (.checked): "yes"` +``` + +**Example — `redundant_boolean_literal`** + +```mq +if (.h1): true else: false; # style: simplify to `.h1` +``` + +**Example — `prefer_coalesce`** + +```mq +if (.value == none): "default" else: .value # style: simplify to `.value ?? "default"` +``` + +**Example — `prefer_pipe_style`** + +```mq +to_text(to_upper(x)) # style: rewrite as `x | to_upper() | to_text()` +``` + +**Example — `redundant_try`** + +```mq +try: get("x") catch: none # style: rewrite as `get("x")?` +``` + +### Complexity + +| Rule ID | Default Threshold | Description | +| ----------------------- | ----------------- | ------------------------------------------------------- | +| `function_too_long` | 50 lines | `def` body exceeds the line limit | +| `too_many_params` | 5 params | Function has too many parameters | +| `deeply_nested` | depth 4 | `if`/`match`/`loop`/`foreach` nesting exceeds the limit | +| `too_many_match_arms` | 15 arms | `match` expression has too many arms | +| `complex_interpolation` | 3 expressions | Interpolated string has too many `${...}` parts | + +**Example — `complex_interpolation`** + +```mq +# warn: 4 interpolated expressions exceeds the default limit of 3 +s"${.h1} ${.h2} ${.h3} ${.h4}" +``` + +### Selector + +| Rule ID | Severity | Description | +| ------------------------- | -------- | ---------------------------------------------------------------------------------- | +| `inefficient_selector` | perf | `..` followed by a specific selector — use the specific selector directly | +| `missing_depth_guard` | warn | `..` (recursive selector) used without any `.depth`/`.level` guard | +| `prefer_specific_heading` | style | `.h` selector without a level — prefer `.h1`–`.h6` | +| `selector_always_empty` | warn | Adjacent selectors that can never both match (e.g. `.h1 \| .h2`, `.todo \| .done`) | + +**Example — `inefficient_selector`** + +```mq +# Before (perf): .. traverses the whole document then filters to h1 +.. | .h1 + +# After +.h1 +``` + +**Example — `missing_depth_guard`** + +```mq +.. # warn: no depth guard — may traverse the entire document +``` + +**Example — `selector_always_empty`** + +```mq +.h1 | .h2 # warn: a heading already filtered to level 1 can never also be level 2 +``` + +### Module + +| Rule ID | Severity | Description | +| ---------------------------- | -------- | ------------------------------------------------------------- | +| `missing_module_doc` | style | `module` declaration has no documentation comment | +| `ambiguous_qualified_access` | warn | Same function name is defined in more than one `module` block | + +**Example — `missing_module_doc`** + +```mq +module a: def foo(): 1; end # style: module `a` has no documentation comment +``` + +**Example — `ambiguous_qualified_access`** + +```mq +# warn: `foo` is defined in both `a` and `b` — qualify the call to avoid ambiguity +module a: def foo(): 1; end +| module b: def foo(): 2; end +``` + +## Support + +- 🐛 [Report bugs](https://github.com/harehare/mq/issues) +- 💡 [Request features](https://github.com/harehare/mq/issues) +- 📖 [Read the documentation](https://mqlang.org/book/) + +## License + +Licensed under the MIT License. diff --git a/crates/mq-lint/src/config.rs b/crates/mq-lint/src/config.rs index cdb053680..61caa6d01 100644 --- a/crates/mq-lint/src/config.rs +++ b/crates/mq-lint/src/config.rs @@ -52,10 +52,7 @@ pub struct LintConfig { impl LintConfig { /// Returns `true` if the given rule should run. pub fn is_rule_enabled(&self, rule_id: &str) -> bool { - self.rules - .get(rule_id) - .map(|r| r.enabled) - .unwrap_or(true) + self.rules.get(rule_id).map(|r| r.enabled).unwrap_or(true) } /// Disable a specific rule by ID. diff --git a/crates/mq-lint/src/lib.rs b/crates/mq-lint/src/lib.rs index d3a1c65e6..8c285ae44 100644 --- a/crates/mq-lint/src/lib.rs +++ b/crates/mq-lint/src/lib.rs @@ -14,7 +14,7 @@ //! //! let config = LintConfig::default(); //! let ctx = LintContext::new(&hir, source_id, &config); -//! let linter = Linter::default(); +//! let linter = Linter::with_default_rules(); //! let diagnostics = linter.run(&ctx); //! ``` @@ -95,6 +95,19 @@ impl<'a> LintContext<'a> { pub fn new(hir: &'a Hir, source_id: SourceId, config: &'a LintConfig) -> Self { Self { hir, source_id, config } } + + /// Returns all symbols that belong to this source, including those added + /// via `insert_symbol` (e.g. Variable, Selector, Ref, Keyword). + /// + /// This is broader than `hir.symbols_for_source()`, which only returns + /// symbols registered with `add_symbol` (structured constructs like + /// Function, Match, Block, etc.). + pub fn all_symbols(&self) -> impl Iterator + '_ { + let source_id = self.source_id; + self.hir + .symbols() + .filter(move |(_, s)| s.source.source_id == Some(source_id)) + } } /// A single lint rule. diff --git a/crates/mq-lint/src/main.rs b/crates/mq-lint/src/main.rs new file mode 100644 index 000000000..80cec34f5 --- /dev/null +++ b/crates/mq-lint/src/main.rs @@ -0,0 +1,281 @@ +use std::io::{self, BufWriter, Read, Write}; +use std::path::PathBuf; +use std::process::ExitCode; +use std::str::FromStr; + +use clap::Parser; +use colored::Colorize; +use mq_hir::Hir; +use mq_lint::{Diagnostic, LintConfig, LintContext, Linter, Severity}; + +/// Static analysis linter for mq programs +#[derive(Parser)] +#[command(name = "mq-lint", about = "Lint mq programs")] +struct Cli { + /// Paths to .mq files to lint (reads from stdin if omitted) + files: Vec, + + /// Disable a rule by ID (repeatable) + #[arg(long = "disable", value_name = "RULE_ID")] + disable: Vec, + + /// Only report diagnostics at or above this severity (style, perf, warn, error) + #[arg(long, default_value = "style")] + min_severity: SeverityArg, + + /// Print all available rule IDs and their default severity, then exit + #[arg(long)] + list_rules: bool, +} + +#[derive(Clone, Copy)] +struct SeverityArg(Severity); + +impl FromStr for SeverityArg { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "style" => Ok(SeverityArg(Severity::Style)), + "perf" => Ok(SeverityArg(Severity::Perf)), + "warn" => Ok(SeverityArg(Severity::Warn)), + "error" => Ok(SeverityArg(Severity::Error)), + other => Err(format!( + "invalid severity `{other}` (expected style, perf, warn, or error)" + )), + } + } +} + +fn main() -> ExitCode { + match run() { + Ok(had_diagnostics) => { + if had_diagnostics { + ExitCode::FAILURE + } else { + ExitCode::SUCCESS + } + } + Err(e) => { + eprintln!("{} {}", "error:".bright_red().bold(), e); + ExitCode::FAILURE + } + } +} + +fn run() -> io::Result { + let cli = Cli::parse(); + let mut w = BufWriter::new(io::stdout()); + + if cli.list_rules { + list_rules(&mut w)?; + return Ok(false); + } + + let mut config = LintConfig::default(); + for rule_id in &cli.disable { + config.disable_rule(rule_id.clone()); + } + let min_severity = cli.min_severity.0; + let linter = Linter::with_default_rules(); + + if cli.files.is_empty() { + let mut code = String::new(); + io::stdin().read_to_string(&mut code)?; + let had_diagnostics = lint_source(&mut w, &code, None, &linter, &config, min_severity)?; + return Ok(had_diagnostics); + } + + let multi = cli.files.len() > 1; + let mut had_diagnostics = false; + + for path in &cli.files { + let code = std::fs::read_to_string(path) + .map_err(|e| io::Error::other(format!("reading file {}: {}", path.display(), e)))?; + let label = if multi { Some(path.display().to_string()) } else { None }; + if lint_source(&mut w, &code, label.as_deref(), &linter, &config, min_severity)? { + had_diagnostics = true; + } + if multi { + writeln!(w)?; + } + } + + Ok(had_diagnostics) +} + +fn list_rules(w: &mut impl Write) -> io::Result<()> { + let mut rules: Vec<_> = mq_lint::rules::all_rules(); + rules.sort_by_key(|r| r.id()); + for rule in &rules { + writeln!(w, "{:<28} {}", rule.id().bright_cyan(), rule.severity())?; + } + Ok(()) +} + +/// Lints a single source, writes diagnostics, and returns `true` if any were reported. +fn lint_source( + w: &mut impl Write, + code: &str, + label: Option<&str>, + linter: &Linter, + config: &LintConfig, + min_severity: Severity, +) -> io::Result { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let ctx = LintContext::new(&hir, source_id, config); + + let mut diagnostics: Vec<_> = linter + .run(&ctx) + .into_iter() + .filter(|d| d.severity >= min_severity) + .collect(); + diagnostics.sort_by_key(|d| d.range.map(|r| (r.start.line, r.start.column))); + + if let Some(lbl) = label { + writeln!(w, "{} {}", "──".dimmed(), lbl.bold())?; + } + + for diagnostic in &diagnostics { + write_diagnostic(w, diagnostic, code)?; + } + + if diagnostics.is_empty() { + writeln!( + w, + "{} {}", + "✓".bright_green().bold(), + "No lint issues found.".bright_green() + )?; + } else { + writeln!( + w, + "{} {} issue{} found.", + "✗".bright_red().bold(), + diagnostics.len().to_string().bright_red().bold(), + if diagnostics.len() == 1 { "" } else { "s" }, + )?; + } + + Ok(!diagnostics.is_empty()) +} + +fn severity_label(severity: Severity) -> colored::ColoredString { + match severity { + Severity::Style => "style".cyan().bold(), + Severity::Perf => "perf".blue().bold(), + Severity::Warn => "warn".bright_yellow().bold(), + Severity::Error => "error".bright_red().bold(), + } +} + +fn write_diagnostic(w: &mut impl Write, diagnostic: &Diagnostic, code: &str) -> io::Result<()> { + let loc_plain = diagnostic + .range + .map(|range| format!("{}:{}", range.start.line, range.start.column)) + .unwrap_or_default(); + let prefix_width = loc_plain.len(); + let sep = "│".dimmed(); + + writeln!( + w, + " {} {} {} {} {}", + loc_plain.dimmed(), + sep, + severity_label(diagnostic.severity), + diagnostic.rule_id.bright_magenta(), + diagnostic.message, + )?; + + if let Some(range) = &diagnostic.range { + write_snippet(w, code, range, prefix_width)?; + } + + if let Some(help) = &diagnostic.help { + writeln!( + w, + " {} {} {}", + " ".repeat(prefix_width), + sep, + format!("help: {help}").bright_blue(), + )?; + } + + Ok(()) +} + +/// Writes a source snippet for the diagnostic location with a caret underline. +fn write_snippet(w: &mut impl Write, code: &str, range: &mq_lang::Range, prefix_width: usize) -> io::Result<()> { + let sep = "│".dimmed(); + let lines: Vec<&str> = code.lines().collect(); + let line_idx = range.start.line.saturating_sub(1) as usize; + let Some(source_line) = lines.get(line_idx) else { + return Ok(()); + }; + + let col_start = range.start.column.saturating_sub(1); + let col_end = if range.end.line == range.start.line { + range.end.column.saturating_sub(1) + } else { + source_line.len() + }; + let underline_len = col_end.saturating_sub(col_start).max(1); + + let line_num = range.start.line.to_string(); + let gutter = " ".repeat(prefix_width.max(line_num.len())); + + writeln!(w, " {} {} {}", gutter, sep, source_line.dimmed())?; + writeln!( + w, + " {} {} {}{}", + format!("{:>width$}", line_num, width = prefix_width).dimmed(), + sep, + " ".repeat(col_start), + "^".repeat(underline_len).bright_red().bold(), + )?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use clap::Parser; + use rstest::rstest; + + #[rstest] + #[case(vec!["mq-lint", "test.mq"], vec!["test.mq"])] + #[case(vec!["mq-lint"], vec![])] + fn test_cli_parsing(#[case] args: Vec<&str>, #[case] expected_files: Vec<&str>) { + let cli = Cli::try_parse_from(args).unwrap(); + assert_eq!( + cli.files, + expected_files.into_iter().map(PathBuf::from).collect::>() + ); + } + + #[test] + fn test_cli_disable_rule() { + let cli = Cli::try_parse_from([ + "mq-lint", + "--disable", + "naming_convention", + "--disable", + "shadow_variable", + ]) + .unwrap(); + assert_eq!(cli.disable, vec!["naming_convention", "shadow_variable"]); + } + + #[test] + fn test_cli_min_severity() { + let cli = Cli::try_parse_from(["mq-lint", "--min-severity", "warn"]).unwrap(); + assert_eq!(cli.min_severity.0, Severity::Warn); + } + + #[test] + fn test_cli_min_severity_invalid() { + let result = Cli::try_parse_from(["mq-lint", "--min-severity", "bogus"]); + assert!(result.is_err()); + } +} diff --git a/crates/mq-lint/src/rules/complexity/complex_interpolation.rs b/crates/mq-lint/src/rules/complexity/complex_interpolation.rs index f1c6c250c..5b1701a1b 100644 --- a/crates/mq-lint/src/rules/complexity/complex_interpolation.rs +++ b/crates/mq-lint/src/rules/complexity/complex_interpolation.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct ComplexInterpolation; @@ -11,7 +12,63 @@ impl LintRule for ComplexInterpolation { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let max_exprs = ctx.config.complexity.max_interpolation_exprs; + + ctx.hir + .symbols_for_source(ctx.source_id) + .filter(|(_, s)| matches!(s.kind, SymbolKind::InterpolatedString)) + .filter_map(|(interp_id, sym)| { + // Variable children represent `${...}` interpolated expressions. + let expr_count = ctx + .all_symbols() + .filter(|(_, s)| s.parent == Some(interp_id) && matches!(s.kind, SymbolKind::Variable)) + .count(); + + if expr_count <= max_exprs { + return None; + } + + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("interpolated string has {expr_count} interpolated expressions (limit: {max_exprs})"), + ); + if let Some(range) = sym.source.text_range { + d = d.with_range(range); + } + Some(d.with_help("consider extracting parts into intermediate `let` bindings for readability")) + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check_with_max(code: &str, max: usize) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let mut config = LintConfig::default(); + config.complexity.max_interpolation_exprs = max; + let ctx = LintContext::new(&hir, source_id, &config); + ComplexInterpolation.check(&ctx) + } + + #[test] + fn detects_too_many_interpolations() { + // 4 interpolated expressions, limit 3 + let diags = check_with_max(r#"s"${.h1} ${.h2} ${.h3} ${.h4}""#, 3); + assert_eq!(diags.len(), 1); + } + + #[test] + fn no_diagnostic_within_limit() { + let diags = check_with_max(r#"s"${.h1} ${.h2}""#, 3); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/complexity/deeply_nested.rs b/crates/mq-lint/src/rules/complexity/deeply_nested.rs index b7be7a19d..8595e9cd5 100644 --- a/crates/mq-lint/src/rules/complexity/deeply_nested.rs +++ b/crates/mq-lint/src/rules/complexity/deeply_nested.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::{ScopeId, ScopeKind}; pub struct DeeplyNested; @@ -11,7 +12,86 @@ impl LintRule for DeeplyNested { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let max_depth = ctx.config.complexity.max_nesting_depth; + let mut diagnostics = Vec::new(); + + for (scope_id, scope) in ctx.hir.scopes() { + // Only consider scopes belonging to this source + if scope.source.source_id != Some(ctx.source_id) { + continue; + } + // Module scopes are depth 0; skip them + if scope.is_module() { + continue; + } + + let depth = scope_depth(ctx, scope_id); + if depth <= max_depth { + continue; + } + + // Report at the symbol that owns this scope + let range = match &scope.kind { + ScopeKind::Function(sym_id) + | ScopeKind::Block(sym_id) + | ScopeKind::Loop(sym_id) + | ScopeKind::MatchArm(sym_id) + | ScopeKind::Let(sym_id) => ctx.hir.symbol(*sym_id).and_then(|s| s.source.text_range), + ScopeKind::Module(_) => None, + }; + + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("nesting depth {depth} exceeds the limit of {max_depth}"), + ); + if let Some(r) = range { + d = d.with_range(r); + } + diagnostics.push(d.with_help("reduce nesting by extracting code into helper functions")); + } + + diagnostics + } +} + +/// Returns the nesting depth of `scope_id`, counting non-module parent scopes. +fn scope_depth(ctx: &LintContext<'_>, scope_id: ScopeId) -> usize { + let mut depth = 0; + let mut current = ctx.hir.scope(scope_id).and_then(|s| s.parent_id); + while let Some(sid) = current { + let scope = match ctx.hir.scope(sid) { + Some(s) => s, + None => break, + }; + if !scope.is_module() { + depth += 1; + } + current = scope.parent_id; + } + depth +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check_with_max(code: &str, max: usize) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let mut config = LintConfig::default(); + config.complexity.max_nesting_depth = max; + let ctx = LintContext::new(&hir, source_id, &config); + DeeplyNested.check(&ctx) + } + + #[test] + fn no_diagnostic_for_shallow_code() { + let diags = check_with_max("def f(): .h1;", 4); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/complexity/function_too_long.rs b/crates/mq-lint/src/rules/complexity/function_too_long.rs index b988b7e85..32b0886c4 100644 --- a/crates/mq-lint/src/rules/complexity/function_too_long.rs +++ b/crates/mq-lint/src/rules/complexity/function_too_long.rs @@ -11,7 +11,52 @@ impl LintRule for FunctionTooLong { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let max_lines = ctx.config.complexity.function_max_lines; + + ctx.hir + .symbols_for_source(ctx.source_id) + .filter_map(|(_, sym)| { + if !sym.is_function() { + return None; + } + let range = sym.source.text_range?; + let line_count = (range.end.line - range.start.line + 1) as usize; + if line_count <= max_lines { + return None; + } + let name = sym.value.as_deref().unwrap_or(""); + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("function `{name}` is {line_count} lines long (limit: {max_lines})"), + ); + d = d.with_range(range); + Some(d.with_help("consider splitting into smaller, focused helper functions")) + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check_with_max(code: &str, max: usize) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let mut config = LintConfig::default(); + config.complexity.function_max_lines = max; + let ctx = LintContext::new(&hir, source_id, &config); + FunctionTooLong.check(&ctx) + } + + #[test] + fn no_diagnostic_for_short_function() { + let diags = check_with_max("def f(): .h1;", 50); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/complexity/too_many_match_arms.rs b/crates/mq-lint/src/rules/complexity/too_many_match_arms.rs index b2761d93e..054bcbb3f 100644 --- a/crates/mq-lint/src/rules/complexity/too_many_match_arms.rs +++ b/crates/mq-lint/src/rules/complexity/too_many_match_arms.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct TooManyMatchArms; @@ -11,7 +12,68 @@ impl LintRule for TooManyMatchArms { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let max_arms = ctx.config.complexity.max_match_arms; + + // Match and MatchArm both use add_symbol + let match_ids: Vec<_> = ctx + .hir + .symbols_for_source(ctx.source_id) + .filter(|(_, s)| matches!(s.kind, SymbolKind::Match)) + .collect(); + + match_ids + .into_iter() + .filter_map(|(match_id, match_sym)| { + let arm_count = ctx + .hir + .symbols_for_source(ctx.source_id) + .filter(|(_, s)| s.parent == Some(match_id) && matches!(s.kind, SymbolKind::MatchArm { .. })) + .count(); + + if arm_count <= max_arms { + return None; + } + + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("match expression has {arm_count} arms (limit: {max_arms})"), + ); + if let Some(range) = match_sym.source.text_range { + d = d.with_range(range); + } + Some(d.with_help("consider refactoring into helper functions or a lookup table")) + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check_with_max(code: &str, max: usize) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let mut config = LintConfig::default(); + config.complexity.max_match_arms = max; + let ctx = LintContext::new(&hir, source_id, &config); + TooManyMatchArms.check(&ctx) + } + + #[test] + fn detects_too_many_arms() { + let diags = check_with_max(r#"match (1): | "a": 1 | "b": 2 | "c": 3 | _: 0 end"#, 3); + assert_eq!(diags.len(), 1); + } + + #[test] + fn no_diagnostic_within_limit() { + let diags = check_with_max(r#"match (1): | "a": 1 | "b": 2 | _: 0 end"#, 3); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/complexity/too_many_params.rs b/crates/mq-lint/src/rules/complexity/too_many_params.rs index 714783fd8..b4ab2a1bd 100644 --- a/crates/mq-lint/src/rules/complexity/too_many_params.rs +++ b/crates/mq-lint/src/rules/complexity/too_many_params.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct TooManyParams; @@ -11,7 +12,61 @@ impl LintRule for TooManyParams { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let max = ctx.config.complexity.max_params; + + // Function symbols use add_symbol; also cover all_symbols for completeness + ctx.all_symbols() + .filter_map(|(_, sym)| { + let params = match &sym.kind { + SymbolKind::Function(p) => p, + _ => return None, + }; + if params.len() <= max { + return None; + } + let name = sym.value.as_deref().unwrap_or(""); + let count = params.len(); + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("function `{name}` has {count} parameters (limit: {max})"), + ); + if let Some(range) = sym.source.text_range { + d = d.with_range(range); + } + Some(d.with_help("consider grouping related parameters or using default arguments")) + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check_with_max(code: &str, max: usize) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let mut config = LintConfig::default(); + config.complexity.max_params = max; + let ctx = LintContext::new(&hir, source_id, &config); + TooManyParams.check(&ctx) + } + + #[test] + fn detects_too_many_params() { + let diags = check_with_max("def f(a, b, c): a", 2); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("3 parameters")); + } + + #[test] + fn no_diagnostic_within_limit() { + let diags = check_with_max("def f(a, b): a", 2); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/correctness.rs b/crates/mq-lint/src/rules/correctness.rs index f78770d08..06297dad4 100644 --- a/crates/mq-lint/src/rules/correctness.rs +++ b/crates/mq-lint/src/rules/correctness.rs @@ -4,7 +4,6 @@ pub mod infinite_loop; pub mod missing_else_in_expr; pub mod shadow_variable; pub mod unreachable_code; -pub mod unused_catch_binding; pub mod unused_function; pub mod unused_import; pub mod unused_variable; @@ -22,6 +21,5 @@ pub fn all() -> Vec> { Box::new(shadow_variable::ShadowVariable), Box::new(missing_else_in_expr::MissingElseInExpr), Box::new(always_true_condition::AlwaysTrueCondition), - Box::new(unused_catch_binding::UnusedCatchBinding), ] } diff --git a/crates/mq-lint/src/rules/correctness/always_true_condition.rs b/crates/mq-lint/src/rules/correctness/always_true_condition.rs index d9233f775..c8ad39e3d 100644 --- a/crates/mq-lint/src/rules/correctness/always_true_condition.rs +++ b/crates/mq-lint/src/rules/correctness/always_true_condition.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct AlwaysTrueCondition; @@ -11,7 +12,85 @@ impl LintRule for AlwaysTrueCondition { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + // Find If symbols whose condition child is a literal `true` or `false`. + // The condition is the first child of the If node by source position. + let mut diagnostics = Vec::new(); + + let if_ids: Vec<_> = ctx + .hir + .symbols_for_source(ctx.source_id) + .filter(|(_, s)| matches!(s.kind, SymbolKind::If)) + .collect(); + + for (if_id, if_sym) in if_ids { + // Sort children by source position and take the first one (the condition). + let mut children: Vec<_> = ctx + .all_symbols() + .filter(|(_, s)| s.parent == Some(if_id)) + .filter_map(|(id, s)| s.source.text_range.map(|r| (id, r, s))) + .collect(); + children.sort_by_key(|(_, r, _)| (r.start.line, r.start.column)); + + let Some((_, _, cond_sym)) = children.first() else { + continue; + }; + + let Some(value) = cond_sym.value.as_deref() else { + continue; + }; + + if !matches!(cond_sym.kind, SymbolKind::Boolean) { + continue; + } + + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("condition is always `{value}` — this branch is never/always taken"), + ); + if let Some(range) = if_sym.source.text_range { + d = d.with_range(range); + } + diagnostics.push(d.with_help("replace the `if` with the branch that will always execute")); + } + + diagnostics + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + AlwaysTrueCondition.check(&ctx) + } + + #[test] + fn detects_literal_true_condition() { + let diags = check("if (true): 1 else: 2;"); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("always `true`")); + } + + #[test] + fn detects_literal_false_condition() { + let diags = check("if (false): 1 else: 2;"); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("always `false`")); + } + + #[test] + fn no_diagnostic_for_dynamic_condition() { + let diags = check("if (.h1): 1 else: 2;"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/correctness/duplicate_match_arm.rs b/crates/mq-lint/src/rules/correctness/duplicate_match_arm.rs index 6bbd5440d..e2b2225e6 100644 --- a/crates/mq-lint/src/rules/correctness/duplicate_match_arm.rs +++ b/crates/mq-lint/src/rules/correctness/duplicate_match_arm.rs @@ -1,4 +1,7 @@ +use rustc_hash::FxHashSet; + use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct DuplicateMatchArm; @@ -11,7 +14,85 @@ impl LintRule for DuplicateMatchArm { Severity::Error } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let mut diagnostics = Vec::new(); + + // Match symbols are tracked via add_symbol + let match_ids: Vec<_> = ctx + .hir + .symbols_for_source(ctx.source_id) + .filter(|(_, s)| matches!(s.kind, SymbolKind::Match)) + .map(|(id, _)| id) + .collect(); + + for match_id in match_ids { + // MatchArm children are also tracked via add_symbol + let arms: Vec<_> = ctx + .hir + .symbols_for_source(ctx.source_id) + .filter(|(_, s)| s.parent == Some(match_id) && matches!(s.kind, SymbolKind::MatchArm { .. })) + .collect(); + + let mut seen_patterns: FxHashSet = FxHashSet::default(); + + for (arm_id, arm_sym) in arms { + // Pattern children of MatchArm use insert_symbol → use all_symbols + let pattern = ctx + .all_symbols() + .find(|(_, s)| s.parent == Some(arm_id) && matches!(s.kind, SymbolKind::Pattern { .. })); + + let pattern_repr = pattern.and_then(|(_, p)| p.value.clone()).map(|v| v.to_string()); + + let Some(repr) = pattern_repr else { + continue; + }; + + if repr == "_" { + continue; + } + + if !seen_patterns.insert(repr.clone()) { + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("duplicate match arm pattern `{repr}`"), + ); + if let Some(range) = arm_sym.source.text_range { + d = d.with_range(range); + } + diagnostics.push(d.with_help("remove or merge this arm with the earlier identical pattern")); + } + } + } + + diagnostics + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + DuplicateMatchArm.check(&ctx) + } + + #[test] + fn detects_duplicate_arm() { + let diags = check(r#"match (1): | "a": "h" | "a": "hh" | _: "other" end"#); + assert_eq!(diags.len(), 1); + } + + #[test] + fn no_duplicate_arms() { + let diags = check(r#"match (1): | "a": "h" | "b": "t" | _: "other" end"#); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/correctness/infinite_loop.rs b/crates/mq-lint/src/rules/correctness/infinite_loop.rs index f6f6ae784..aa4bde097 100644 --- a/crates/mq-lint/src/rules/correctness/infinite_loop.rs +++ b/crates/mq-lint/src/rules/correctness/infinite_loop.rs @@ -1,4 +1,6 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolId; +use mq_hir::SymbolKind; pub struct InfiniteLoop; @@ -11,7 +13,75 @@ impl LintRule for InfiniteLoop { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let mut diagnostics = Vec::new(); + + // Loop symbols are tracked via add_symbol + let loops: Vec<_> = ctx + .hir + .symbols_for_source(ctx.source_id) + .filter(|(_, s)| matches!(s.kind, SymbolKind::Loop)) + .collect(); + + for (loop_id, loop_sym) in loops { + if !has_break_descendant(ctx, loop_id) { + let mut d = Diagnostic::new(self.id(), self.severity(), "loop without `break` may run forever"); + if let Some(range) = loop_sym.source.text_range { + d = d.with_range(range); + } + diagnostics.push(d.with_help("add a `break` expression to exit the loop")); + } + } + + diagnostics + } +} + +/// Returns true if any descendant of `ancestor_id` in the source is `Keyword("break")`. +/// Keyword symbols use `insert_symbol`, so we use `all_symbols`. +fn has_break_descendant(ctx: &LintContext<'_>, ancestor_id: SymbolId) -> bool { + ctx.all_symbols().any(|(_, s)| { + matches!(s.kind, SymbolKind::Keyword) + && s.value.as_deref() == Some("break") + && is_descendant_of(ctx, s.parent, ancestor_id) + }) +} + +fn is_descendant_of(ctx: &LintContext<'_>, maybe_parent: Option, target: SymbolId) -> bool { + let mut current = maybe_parent; + while let Some(id) = current { + if id == target { + return true; + } + current = ctx.hir.symbol(id).and_then(|s| s.parent); + } + false +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + InfiniteLoop.check(&ctx) + } + + #[test] + fn detects_loop_without_break() { + let diags = check("loop .h1 end"); + assert_eq!(diags.len(), 1); + } + + #[test] + fn no_diagnostic_when_break_present() { + let diags = check("loop break end"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs b/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs index caac39325..8e222e43f 100644 --- a/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs +++ b/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct MissingElseInExpr; @@ -11,7 +12,72 @@ impl LintRule for MissingElseInExpr { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + // Collect all If symbol IDs that have an Else or Elif child. + // Both If and Else use add_symbol, so symbols_for_source covers them. + let if_ids_with_else: std::collections::HashSet = ctx + .hir + .symbols_for_source(ctx.source_id) + .filter(|(_, s)| matches!(s.kind, SymbolKind::Else)) + .filter_map(|(_, s)| s.parent) + .collect(); + + ctx.hir + .symbols_for_source(ctx.source_id) + .filter(|(id, s)| matches!(s.kind, SymbolKind::If) && !if_ids_with_else.contains(id)) + .map(|(_, sym)| { + let mut d = Diagnostic::new( + self.id(), + self.severity(), + "`if` expression is missing an `else` branch (evaluates to `none` on false)", + ); + if let Some(range) = sym.source.text_range { + d = d.with_range(range); + } + d.with_help("add `else: ` to provide a value for the false branch") + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + MissingElseInExpr.check(&ctx) + } + + #[test] + fn detects_if_without_else() { + let diags = check("if (true): 1;"); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("missing an `else` branch")); + } + + #[test] + fn no_diagnostic_with_else() { + let diags = check("if (true): 1 else: 2;"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn no_diagnostic_with_elif_else() { + let diags = check("if (true): 1 elif (false): 2 else: 3;"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn detects_if_with_only_elif() { + // elif but no else is still missing an else + let diags = check("if (true): 1 elif (false): 2;"); + assert_eq!(diags.len(), 1); } } diff --git a/crates/mq-lint/src/rules/correctness/shadow_variable.rs b/crates/mq-lint/src/rules/correctness/shadow_variable.rs index 270859b65..e0d2f2e49 100644 --- a/crates/mq-lint/src/rules/correctness/shadow_variable.rs +++ b/crates/mq-lint/src/rules/correctness/shadow_variable.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::{ScopeId, SymbolKind}; pub struct ShadowVariable; @@ -11,7 +12,83 @@ impl LintRule for ShadowVariable { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let mut diagnostics = Vec::new(); + + // Variable symbols use insert_symbol, so use all_symbols + for (_, sym) in ctx.all_symbols() { + if !matches!(sym.kind, SymbolKind::Variable) { + continue; + } + let name = match sym.value.as_deref() { + Some(n) => n, + None => continue, + }; + + if shadows_outer_variable(ctx, sym.scope, name) { + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("variable `{name}` shadows a variable in an outer scope"), + ); + if let Some(range) = sym.source.text_range { + d = d.with_range(range); + } + diagnostics.push(d.with_help("consider renaming to avoid confusion")); + } + } + + diagnostics + } +} + +/// Returns true if any ancestor scope (excluding `current_scope`) contains +/// a Variable or Parameter with the given name defined in this same source. +fn shadows_outer_variable(ctx: &LintContext<'_>, current_scope: ScopeId, name: &str) -> bool { + let mut scope_id = ctx.hir.scope(current_scope).and_then(|s| s.parent_id); + + while let Some(sid) = scope_id { + let has_same_name = ctx.all_symbols().any(|(_, s)| { + s.scope == sid + && matches!(s.kind, SymbolKind::Variable | SymbolKind::Parameter) + && s.value.as_deref() == Some(name) + }); + + if has_same_name { + return true; + } + + scope_id = ctx.hir.scope(sid).and_then(|s| s.parent_id); + } + + false +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + ShadowVariable.check(&ctx) + } + + #[test] + fn detects_shadow() { + // Outer `x`, then inner `x` in function body + let diags = check("let x = 1 | def f(): let x = 2; x; end"); + assert!(!diags.is_empty()); + } + + #[test] + fn no_shadow_different_names() { + let diags = check("let x = 1 | def f(): let y = 2; y; end"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/correctness/unreachable_code.rs b/crates/mq-lint/src/rules/correctness/unreachable_code.rs index 1cca20a80..85f11ef90 100644 --- a/crates/mq-lint/src/rules/correctness/unreachable_code.rs +++ b/crates/mq-lint/src/rules/correctness/unreachable_code.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct UnreachableCode; @@ -11,7 +12,68 @@ impl LintRule for UnreachableCode { Severity::Error } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let mut diagnostics = Vec::new(); + + // Keyword symbols use insert_symbol → use all_symbols + for (_, sym) in ctx.all_symbols() { + if !matches!(sym.kind, SymbolKind::Keyword) { + continue; + } + let kw = sym.value.as_deref().unwrap_or(""); + if kw != "break" && kw != "continue" { + continue; + } + + let break_end = match sym.source.text_range { + Some(r) => r.end, + None => continue, + }; + + let parent_id = sym.parent; + + // Any sibling (same parent) that starts after this break's end position + // and is not a structural keyword (like "end") is unreachable. + let unreachable: Vec<_> = ctx + .all_symbols() + .filter(|(_, s)| { + s.parent == parent_id + && s.source.text_range.is_some_and(|r| r.start > break_end) + && !matches!(s.kind, SymbolKind::Keyword) + }) + .collect(); + + for (_, dead_sym) in unreachable { + let mut d = Diagnostic::new(self.id(), self.severity(), format!("unreachable code after `{kw}`")); + if let Some(range) = dead_sym.source.text_range { + d = d.with_range(range); + } + diagnostics.push(d.with_help("remove or move this code before the `break`/`continue`")); + } + } + + diagnostics + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + UnreachableCode.check(&ctx) + } + + #[test] + fn no_diagnostic_without_break() { + let diags = check(".h1 | .value"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/correctness/unused_catch_binding.rs b/crates/mq-lint/src/rules/correctness/unused_catch_binding.rs deleted file mode 100644 index b12b9bfb2..000000000 --- a/crates/mq-lint/src/rules/correctness/unused_catch_binding.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; - -pub struct UnusedCatchBinding; - -impl LintRule for UnusedCatchBinding { - fn id(&self) -> &'static str { - "unused_catch_binding" - } - - fn severity(&self) -> Severity { - Severity::Warn - } - - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() - } -} diff --git a/crates/mq-lint/src/rules/correctness/unused_function.rs b/crates/mq-lint/src/rules/correctness/unused_function.rs index bc1067d71..b8ef1a2cb 100644 --- a/crates/mq-lint/src/rules/correctness/unused_function.rs +++ b/crates/mq-lint/src/rules/correctness/unused_function.rs @@ -1,4 +1,7 @@ +use rustc_hash::FxHashSet; + use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct UnusedFunction; @@ -11,7 +14,64 @@ impl LintRule for UnusedFunction { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + // Collect all names referenced via Call/Ref/Ident in the entire HIR + // (cross-file usage counts too, so we search all symbols not just this source) + let used_names: FxHashSet<&str> = ctx + .hir + .symbols() + .filter(|(_, s)| matches!(s.kind, SymbolKind::Call | SymbolKind::Ref | SymbolKind::Ident)) + .filter_map(|(_, s)| s.value.as_deref()) + .collect(); + + ctx.all_symbols() + .filter(|(_, s)| s.is_function() && !ctx.hir.is_builtin_symbol(s) && !s.is_internal_function()) + .filter_map(|(_, sym)| { + let name = sym.value.as_deref()?; + if name.starts_with('_') || used_names.contains(name) { + return None; + } + let mut d = Diagnostic::new(self.id(), self.severity(), format!("unused function `{name}`")); + if let Some(range) = sym.source.text_range { + d = d.with_range(range); + } + Some(d.with_help(format!("if this is intentional, prefix with `_`: `_{name}`"))) + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + UnusedFunction.check(&ctx) + } + + #[test] + fn detects_unused_function() { + let diags = check("def foo(): .h1;"); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("unused function `foo`")); + } + + #[test] + fn no_diagnostic_when_function_called() { + let diags = check("def foo(): .h1; | foo"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn no_diagnostic_when_function_called_with_parens() { + let diags = check("def foo(): .h1; | foo()"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/correctness/unused_import.rs b/crates/mq-lint/src/rules/correctness/unused_import.rs index c4ae13d31..43faad00b 100644 --- a/crates/mq-lint/src/rules/correctness/unused_import.rs +++ b/crates/mq-lint/src/rules/correctness/unused_import.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct UnusedImport; @@ -11,7 +12,58 @@ impl LintRule for UnusedImport { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + // Collect all module names referenced via QualifiedAccess (module::function). + let used_modules: std::collections::HashSet<&str> = ctx + .hir + .symbols() + .filter(|(_, s)| matches!(s.kind, SymbolKind::QualifiedAccess)) + .filter_map(|(_, s)| s.value.as_deref()) + .collect(); + + // Find Import symbols in this source whose module name is never accessed. + ctx.hir + .symbols_for_source(ctx.source_id) + .filter(|(_, s)| matches!(s.kind, SymbolKind::Import(_))) + .filter_map(|(_, sym)| { + let name = sym.value.as_deref()?; + if used_modules.contains(name) { + return None; + } + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("imported module `{name}` is never used"), + ); + if let Some(range) = sym.source.text_range { + d = d.with_range(range); + } + Some(d.with_help(format!("remove `import \"{name}\"` or use it with `{name}::function`"))) + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + // Note: import resolution requires a real module resolver; we test the symbol + // detection logic with a manually constructed HIR scenario. + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + UnusedImport.check(&ctx) + } + + #[test] + fn no_imports_no_diagnostic() { + let diags = check(".h1"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/correctness/unused_variable.rs b/crates/mq-lint/src/rules/correctness/unused_variable.rs index efb7c9cbf..100c9a1f4 100644 --- a/crates/mq-lint/src/rules/correctness/unused_variable.rs +++ b/crates/mq-lint/src/rules/correctness/unused_variable.rs @@ -1,4 +1,7 @@ +use rustc_hash::FxHashSet; + use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct UnusedVariable; @@ -11,7 +14,66 @@ impl LintRule for UnusedVariable { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + // Build a set of all names referenced by Ref/Ident/Call symbols + let used_names: FxHashSet<&str> = ctx + .all_symbols() + .filter(|(_, s)| matches!(s.kind, SymbolKind::Ref | SymbolKind::Ident | SymbolKind::Call)) + .filter_map(|(_, s)| s.value.as_deref()) + .collect(); + + ctx.all_symbols() + .filter(|(_, s)| matches!(s.kind, SymbolKind::Variable)) + .filter_map(|(_, sym)| { + let name = sym.value.as_deref()?; + // Variables prefixed with `_` are intentionally unused + if name.starts_with('_') { + return None; + } + if used_names.contains(name) { + return None; + } + let mut d = Diagnostic::new(self.id(), self.severity(), format!("unused variable `{name}`")); + if let Some(range) = sym.source.text_range { + d = d.with_range(range); + } + Some(d.with_help(format!("if this is intentional, prefix with `_`: `_{name}`"))) + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + UnusedVariable.check(&ctx) + } + + #[test] + fn detects_unused_variable() { + let diags = check("let x = .h1"); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("unused variable `x`")); + } + + #[test] + fn no_diagnostic_when_variable_used() { + let diags = check("let x = .h1 | x"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn underscore_prefix_suppresses_warning() { + let diags = check("let _x = .h1"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/module.rs b/crates/mq-lint/src/rules/module.rs index 18dc1d6aa..71d8b1381 100644 --- a/crates/mq-lint/src/rules/module.rs +++ b/crates/mq-lint/src/rules/module.rs @@ -1,15 +1,11 @@ pub mod ambiguous_qualified_access; -pub mod circular_import; pub mod missing_module_doc; -pub mod reexport_private; use crate::LintRule; pub fn all() -> Vec> { vec![ - Box::new(circular_import::CircularImport), Box::new(missing_module_doc::MissingModuleDoc), - Box::new(reexport_private::ReexportPrivate), Box::new(ambiguous_qualified_access::AmbiguousQualifiedAccess), ] } diff --git a/crates/mq-lint/src/rules/module/ambiguous_qualified_access.rs b/crates/mq-lint/src/rules/module/ambiguous_qualified_access.rs index d74c79241..4ab13a04f 100644 --- a/crates/mq-lint/src/rules/module/ambiguous_qualified_access.rs +++ b/crates/mq-lint/src/rules/module/ambiguous_qualified_access.rs @@ -1,4 +1,7 @@ +use rustc_hash::FxHashMap; + use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct AmbiguousQualifiedAccess; @@ -11,7 +14,90 @@ impl LintRule for AmbiguousQualifiedAccess { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + // Use the whole HIR, not just this source, since modules may live elsewhere. + let mut defining_modules: FxHashMap<&str, Vec<&str>> = FxHashMap::default(); + + for (_, sym) in ctx.hir.symbols() { + if !sym.is_function() { + continue; + } + let Some(fn_name) = sym.value.as_deref() else { continue }; + let Some(module_sym) = sym.parent.and_then(|p| ctx.hir.symbol(p)) else { + continue; + }; + let SymbolKind::Module(_) = module_sym.kind else { + continue; + }; + let Some(module_name) = module_sym.value.as_deref() else { + continue; + }; + + let modules = defining_modules.entry(fn_name).or_default(); + if !modules.contains(&module_name) { + modules.push(module_name); + } + } + + ctx.all_symbols() + .filter(|(_, sym)| sym.is_function()) + .filter_map(|(_, sym)| { + let fn_name = sym.value.as_deref()?; + let module_sym = sym.parent.and_then(|p| ctx.hir.symbol(p))?; + let SymbolKind::Module(_) = module_sym.kind else { + return None; + }; + let this_module = module_sym.value.as_deref()?; + + let other_module = defining_modules.get(fn_name)?.iter().find(|&&m| m != this_module)?; + + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("function `{fn_name}` is also defined in module `{other_module}`"), + ); + if let Some(range) = sym.source.text_range { + d = d.with_range(range); + } + Some(d.with_help(format!( + "use a fully qualified call (e.g. `{this_module}::{fn_name}()`) to avoid ambiguity" + ))) + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + AmbiguousQualifiedAccess.check(&ctx) + } + + #[test] + fn detects_same_function_name_in_two_modules() { + let diags = check("module a: def foo(): 1; end | module b: def foo(): 2; end"); + assert_eq!(diags.len(), 2); + assert!(diags.iter().all(|d| d.message.contains("foo"))); + } + + #[test] + fn no_diagnostic_for_distinct_function_names() { + let diags = check("module a: def foo(): 1; end | module b: def bar(): 2; end"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn no_diagnostic_for_function_outside_module() { + let diags = check("def foo(): 1; | foo()"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/module/circular_import.rs b/crates/mq-lint/src/rules/module/circular_import.rs deleted file mode 100644 index 3d6b4904a..000000000 --- a/crates/mq-lint/src/rules/module/circular_import.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; - -pub struct CircularImport; - -impl LintRule for CircularImport { - fn id(&self) -> &'static str { - "circular_import" - } - - fn severity(&self) -> Severity { - Severity::Error - } - - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() - } -} diff --git a/crates/mq-lint/src/rules/module/missing_module_doc.rs b/crates/mq-lint/src/rules/module/missing_module_doc.rs index 4f9a2aa6c..d24ad483f 100644 --- a/crates/mq-lint/src/rules/module/missing_module_doc.rs +++ b/crates/mq-lint/src/rules/module/missing_module_doc.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct MissingModuleDoc; @@ -11,7 +12,51 @@ impl LintRule for MissingModuleDoc { Severity::Style } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + ctx.all_symbols() + .filter(|(_, sym)| matches!(sym.kind, SymbolKind::Module(_))) + .filter(|(_, sym)| sym.doc.is_empty()) + .map(|(_, sym)| { + let name = sym.value.as_deref().unwrap_or(""); + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("module `{name}` has no documentation comment"), + ); + if let Some(range) = sym.source.text_range { + d = d.with_range(range); + } + d.with_help(format!("add a `#` doc comment above `module {name}:`")) + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + MissingModuleDoc.check(&ctx) + } + + #[test] + fn detects_module_without_doc() { + let diags = check("module a: def b(): 1; end"); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("module `a`")); + } + + #[test] + fn no_diagnostic_when_module_has_doc() { + let diags = check("# A module that does something.\nmodule a: def b(): 1; end"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/module/reexport_private.rs b/crates/mq-lint/src/rules/module/reexport_private.rs deleted file mode 100644 index 23a7a65b6..000000000 --- a/crates/mq-lint/src/rules/module/reexport_private.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; - -pub struct ReexportPrivate; - -impl LintRule for ReexportPrivate { - fn id(&self) -> &'static str { - "reexport_private" - } - - fn severity(&self) -> Severity { - Severity::Warn - } - - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() - } -} diff --git a/crates/mq-lint/src/rules/selector.rs b/crates/mq-lint/src/rules/selector.rs index a9bb95781..c28b0b1ed 100644 --- a/crates/mq-lint/src/rules/selector.rs +++ b/crates/mq-lint/src/rules/selector.rs @@ -1,5 +1,3 @@ -pub mod deprecated_selector; -pub mod env_var_in_selector; pub mod inefficient_selector; pub mod missing_depth_guard; pub mod selector_always_empty; @@ -10,8 +8,6 @@ pub fn all() -> Vec> { vec![ Box::new(inefficient_selector::InefficientSelector), Box::new(selector_always_empty::SelectorAlwaysEmpty), - Box::new(env_var_in_selector::EnvVarInSelector), Box::new(missing_depth_guard::MissingDepthGuard), - Box::new(deprecated_selector::DeprecatedSelector), ] } diff --git a/crates/mq-lint/src/rules/selector/deprecated_selector.rs b/crates/mq-lint/src/rules/selector/deprecated_selector.rs deleted file mode 100644 index e90d708a7..000000000 --- a/crates/mq-lint/src/rules/selector/deprecated_selector.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; - -pub struct DeprecatedSelector; - -impl LintRule for DeprecatedSelector { - fn id(&self) -> &'static str { - "deprecated_selector" - } - - fn severity(&self) -> Severity { - Severity::Error - } - - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() - } -} diff --git a/crates/mq-lint/src/rules/selector/env_var_in_selector.rs b/crates/mq-lint/src/rules/selector/env_var_in_selector.rs deleted file mode 100644 index 0e3e4b172..000000000 --- a/crates/mq-lint/src/rules/selector/env_var_in_selector.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; - -pub struct EnvVarInSelector; - -impl LintRule for EnvVarInSelector { - fn id(&self) -> &'static str { - "env_var_in_selector" - } - - fn severity(&self) -> Severity { - Severity::Warn - } - - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() - } -} diff --git a/crates/mq-lint/src/rules/selector/inefficient_selector.rs b/crates/mq-lint/src/rules/selector/inefficient_selector.rs index ffd6f6e49..86d7b6a13 100644 --- a/crates/mq-lint/src/rules/selector/inefficient_selector.rs +++ b/crates/mq-lint/src/rules/selector/inefficient_selector.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct InefficientSelector; @@ -11,7 +12,91 @@ impl LintRule for InefficientSelector { Severity::Perf } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let mut diagnostics = Vec::new(); + + // Selector symbols use insert_symbol → use all_symbols + let recursive_selectors: Vec<_> = ctx + .all_symbols() + .filter(|(_, s)| matches!(s.kind, SymbolKind::Selector(mq_lang::Selector::Recursive))) + .collect(); + + for (rec_id, rec_sym) in recursive_selectors { + let rec_range = match rec_sym.source.text_range { + Some(r) => r, + None => continue, + }; + + // Check if any sibling selector (same parent, starts after `..`) is a + // concrete non-recursive non-attr selector. + let has_redundant_follow = ctx.all_symbols().any(|(sid, s)| { + if sid == rec_id { + return false; + } + if s.parent != rec_sym.parent { + return false; + } + let s_start = match s.source.text_range { + Some(r) => r.start, + None => return false, + }; + if s_start <= rec_range.start { + return false; + } + matches!( + &s.kind, + SymbolKind::Selector(sel) if !matches!(sel, mq_lang::Selector::Recursive | mq_lang::Selector::Attr(_)) + ) + }); + + if has_redundant_follow { + let mut d = Diagnostic::new( + self.id(), + self.severity(), + "inefficient selector: `..` followed by a specific selector", + ); + d = d.with_range(rec_range); + diagnostics + .push(d.with_help("use the specific selector directly (e.g. replace `.. | .h1` with `.h1`)")); + } + } + + diagnostics + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + InefficientSelector.check(&ctx) + } + + #[test] + fn no_diagnostic_for_direct_selector() { + let diags = check(".h1"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn no_diagnostic_for_recursive_alone() { + let diags = check(".."); + assert_eq!(diags.len(), 0); + } + + #[test] + fn detects_recursive_followed_by_specific_selector() { + // `.. | .h1` — the `..` is redundant because `.h1` already selects h1 nodes directly. + let diags = check(".. | .h1"); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("inefficient selector")); } } diff --git a/crates/mq-lint/src/rules/selector/missing_depth_guard.rs b/crates/mq-lint/src/rules/selector/missing_depth_guard.rs index 05d8aeae4..996a5d7c3 100644 --- a/crates/mq-lint/src/rules/selector/missing_depth_guard.rs +++ b/crates/mq-lint/src/rules/selector/missing_depth_guard.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct MissingDepthGuard; @@ -11,7 +12,74 @@ impl LintRule for MissingDepthGuard { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + // Check for depth-guard attributes (`.depth` / `.level`) anywhere in this source. + // If any depth attribute is present, we assume the author is aware of depth guarding. + let has_depth_attr = ctx.all_symbols().any(|(_, s)| { + matches!( + s.kind, + SymbolKind::Selector(mq_lang::Selector::Attr(mq_lang::AttrKind::Depth)) + | SymbolKind::Selector(mq_lang::Selector::Attr(mq_lang::AttrKind::Level)) + ) + }); + + if has_depth_attr { + return Vec::new(); + } + + // Flag every `..` selector that appears without any depth guard in the source. + ctx.all_symbols() + .filter(|(_, s)| matches!(s.kind, SymbolKind::Selector(mq_lang::Selector::Recursive))) + .map(|(_, sym)| { + let mut d = Diagnostic::new( + self.id(), + self.severity(), + "`..` (recursive selector) used without a depth guard", + ); + if let Some(range) = sym.source.text_range { + d = d.with_range(range); + } + d.with_help( + "consider adding a depth limit, e.g. `.. | select(.depth <= 3)`, \ + to avoid traversing the entire document", + ) + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + MissingDepthGuard.check(&ctx) + } + + #[test] + fn detects_bare_recursive_selector() { + let diags = check(".."); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("depth guard")); + } + + #[test] + fn no_diagnostic_when_depth_attr_present() { + // When .depth is used somewhere, we consider depth guarding in place. + let diags = check(".. | .depth"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn no_diagnostic_for_non_recursive_selector() { + let diags = check(".h1"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/selector/selector_always_empty.rs b/crates/mq-lint/src/rules/selector/selector_always_empty.rs index a76be8c89..ab2c3dbf8 100644 --- a/crates/mq-lint/src/rules/selector/selector_always_empty.rs +++ b/crates/mq-lint/src/rules/selector/selector_always_empty.rs @@ -1,7 +1,22 @@ +use mq_hir::SymbolKind; +use mq_lang::Selector; + use crate::{Diagnostic, LintContext, LintRule, Severity}; pub struct SelectorAlwaysEmpty; +/// Conservative: only flags same-variant value mismatches (plus `.todo`/`.done`) +/// to avoid false positives across mq's many selector kinds. +fn mutually_exclusive(first: &Selector, second: &Selector) -> bool { + match (first, second) { + (Selector::Heading(Some(a)), Selector::Heading(Some(b))) => a != b, + (Selector::List(Some(a), _), Selector::List(Some(b), _)) => a != b, + (Selector::Table(Some(r1), Some(c1)), Selector::Table(Some(r2), Some(c2))) => r1 != r2 || c1 != c2, + (Selector::Todo, Selector::Done) | (Selector::Done, Selector::Todo) => true, + _ => false, + } +} + impl LintRule for SelectorAlwaysEmpty { fn id(&self) -> &'static str { "selector_always_empty" @@ -11,7 +26,87 @@ impl LintRule for SelectorAlwaysEmpty { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let mut by_parent: std::collections::HashMap, Vec<_>> = + std::collections::HashMap::new(); + for (id, sym) in ctx.all_symbols() { + by_parent.entry(sym.parent).or_default().push((id, sym)); + } + + let mut diagnostics = Vec::new(); + for siblings in by_parent.values_mut() { + siblings.sort_by_key(|(_, s)| s.source.text_range.map(|r| (r.start.line, r.start.column))); + + for pair in siblings.windows(2) { + let [(_, first), (_, second)] = pair else { continue }; + let (SymbolKind::Selector(sel1), SymbolKind::Selector(sel2)) = (&first.kind, &second.kind) else { + continue; + }; + if !mutually_exclusive(sel1, sel2) { + continue; + } + + let first_text = first.value.as_deref().unwrap_or(""); + let second_text = second.value.as_deref().unwrap_or(""); + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("`{first_text} | {second_text}` can never match: a node can't be both"), + ); + if let Some(range) = second.source.text_range { + d = d.with_range(range); + } + diagnostics + .push(d.with_help("remove one of the selectors, or replace the pipe with a different query")); + } + } + + diagnostics + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + SelectorAlwaysEmpty.check(&ctx) + } + + #[test] + fn detects_conflicting_heading_levels() { + let diags = check(".h1 | .h2"); + assert_eq!(diags.len(), 1); + } + + #[test] + fn detects_todo_then_done() { + let diags = check(".todo | .done"); + assert_eq!(diags.len(), 1); + } + + #[test] + fn no_diagnostic_for_same_selector_repeated() { + let diags = check(".h1 | .h1"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn no_diagnostic_for_generic_then_specific_heading() { + let diags = check(".heading | .h1"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn no_diagnostic_when_unrelated_step_is_between() { + let diags = check(".h1 | to_text() | .h2"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/style.rs b/crates/mq-lint/src/rules/style.rs index eefc27ed3..d67eda80f 100644 --- a/crates/mq-lint/src/rules/style.rs +++ b/crates/mq-lint/src/rules/style.rs @@ -6,14 +6,12 @@ pub mod prefer_pipe_style; pub mod prefer_specific_heading; pub mod redundant_boolean_literal; pub mod redundant_try; -pub mod unnecessary_parens; use crate::LintRule; pub fn all() -> Vec> { vec![ Box::new(prefer_let_over_var::PreferLetOverVar), - Box::new(unnecessary_parens::UnnecessaryParens), Box::new(prefer_pipe_style::PreferPipeStyle), Box::new(prefer_coalesce::PreferCoalesce), Box::new(prefer_specific_heading::PreferSpecificHeading), diff --git a/crates/mq-lint/src/rules/style/boolean_comparison.rs b/crates/mq-lint/src/rules/style/boolean_comparison.rs index 497712e9d..cc083c614 100644 --- a/crates/mq-lint/src/rules/style/boolean_comparison.rs +++ b/crates/mq-lint/src/rules/style/boolean_comparison.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct BooleanComparison; @@ -11,7 +12,70 @@ impl LintRule for BooleanComparison { Severity::Style } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let mut diagnostics = Vec::new(); + + // BinaryOp uses add_symbol; Boolean child may use insert_symbol. + // Use all_symbols for both. + for (bop_id, bop_sym) in ctx.all_symbols() { + if !matches!(bop_sym.kind, SymbolKind::BinaryOp) { + continue; + } + let op = bop_sym.value.as_deref().unwrap_or(""); + if op != "==" && op != "!=" { + continue; + } + + let bool_child = ctx + .all_symbols() + .find(|(_, s)| s.parent == Some(bop_id) && matches!(s.kind, SymbolKind::Boolean)); + + let Some((_, bool_sym)) = bool_child else { + continue; + }; + + let bool_val = bool_sym.value.as_deref().unwrap_or("true"); + let help = match (op, bool_val) { + ("==", "true") => "use the value directly instead of `== true`", + ("==", "false") => "use `!` prefix instead of `== false`", + ("!=", "true") => "use `!` prefix instead of `!= true`", + ("!=", "false") => "use the value directly instead of `!= false`", + _ => "simplify this boolean comparison", + }; + + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("unnecessary comparison with boolean literal `{bool_val}`"), + ); + if let Some(range) = bop_sym.source.text_range { + d = d.with_range(range); + } + diagnostics.push(d.with_help(help)); + } + + diagnostics + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + BooleanComparison.check(&ctx) + } + + #[test] + fn no_diagnostic_for_non_boolean_comparison() { + let diags = check(r#".type == "heading""#); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/style/naming_convention.rs b/crates/mq-lint/src/rules/style/naming_convention.rs index 71005148b..424692d20 100644 --- a/crates/mq-lint/src/rules/style/naming_convention.rs +++ b/crates/mq-lint/src/rules/style/naming_convention.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct NamingConvention; @@ -11,7 +12,93 @@ impl LintRule for NamingConvention { Severity::Style } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + // Function symbols use add_symbol; Variable symbols use insert_symbol. + // Use all_symbols to cover both. + ctx.all_symbols() + .filter(|(_, s)| matches!(s.kind, SymbolKind::Function(_) | SymbolKind::Variable)) + .filter_map(|(_, sym)| { + let name = sym.value.as_deref()?; + if name.starts_with('_') { + return None; + } + if is_snake_case(name) { + return None; + } + let suggested = to_snake_case(name); + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("`{name}` should be written in snake_case"), + ); + if let Some(range) = sym.source.text_range { + d = d.with_range(range); + } + Some(d.with_help(format!("rename to `{suggested}`"))) + }) + .collect() + } +} + +/// Returns true if the name is already valid snake_case (lowercase + underscores + digits). +fn is_snake_case(name: &str) -> bool { + name.chars() + .all(|c| c.is_ascii_lowercase() || c == '_' || c.is_ascii_digit()) +} + +/// Naive camelCase / PascalCase → snake_case conversion for the help message. +fn to_snake_case(name: &str) -> String { + let mut out = String::with_capacity(name.len() + 4); + for (i, c) in name.char_indices() { + if c.is_ascii_uppercase() { + if i > 0 { + out.push('_'); + } + out.push(c.to_ascii_lowercase()); + } else { + out.push(c); + } + } + out +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + NamingConvention.check(&ctx) + } + + #[test] + fn detects_camel_case_function() { + let diags = check("def myFunc(): .h1;"); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("myFunc")); + } + + #[test] + fn no_diagnostic_for_snake_case() { + let diags = check("def my_func(): .h1;"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn detects_camel_case_variable() { + let diags = check("let myVar = .h1"); + assert_eq!(diags.len(), 1); + } + + #[test] + fn underscore_prefix_is_ok() { + let diags = check("def _myHelper(): .h1;"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/style/prefer_coalesce.rs b/crates/mq-lint/src/rules/style/prefer_coalesce.rs index 07afda0ca..69cf6bc96 100644 --- a/crates/mq-lint/src/rules/style/prefer_coalesce.rs +++ b/crates/mq-lint/src/rules/style/prefer_coalesce.rs @@ -1,7 +1,77 @@ +use mq_hir::{Symbol, SymbolId, SymbolKind}; + use crate::{Diagnostic, LintContext, LintRule, Severity}; pub struct PreferCoalesce; +/// True if `id` has no children (e.g. a bare `Ref`/`Selector`, not a call). +fn is_leaf(ctx: &LintContext<'_>, id: SymbolId) -> bool { + !ctx.all_symbols().any(|(_, s)| s.parent == Some(id)) +} + +fn is_none_literal(sym: &Symbol) -> bool { + matches!(sym.kind, SymbolKind::None) || sym.value.as_deref() == Some("none") +} + +fn children_sorted_by_position<'a>(ctx: &'a LintContext<'_>, parent: SymbolId) -> Vec<(SymbolId, &'a Symbol)> { + let mut children: Vec<_> = ctx.all_symbols().filter(|(_, s)| s.parent == Some(parent)).collect(); + children.sort_by_key(|(_, s)| s.source.text_range.map(|r| (r.start.line, r.start.column))); + children +} + +/// Detects `if (x == none): fallback else: x` and its `!=` form, both equivalent to `x ?? fallback`. +fn matches_null_check_pattern(ctx: &LintContext<'_>, if_id: SymbolId) -> bool { + let children = children_sorted_by_position(ctx, if_id); + let Some(&(cond_id, cond)) = children.first() else { + return false; + }; + let Some(&(then_id, _)) = children.get(1) else { + return false; + }; + let Some(&(else_kw_id, _)) = children.iter().find(|(_, s)| matches!(s.kind, SymbolKind::Else)) else { + return false; + }; + let Some(&(else_expr_id, _)) = children_sorted_by_position(ctx, else_kw_id).first() else { + return false; + }; + + if !matches!(cond.kind, SymbolKind::BinaryOp) { + return false; + } + let Some(op) = cond.value.as_deref() else { return false }; + if op != "==" && op != "!=" { + return false; + } + + let cond_children = children_sorted_by_position(ctx, cond_id); + let (Some(&(lhs_id, lhs)), Some(&(rhs_id, rhs))) = (cond_children.first(), cond_children.get(1)) else { + return false; + }; + + // Whichever side isn't the `none` literal is the value being null-checked. + let value_id = if is_none_literal(rhs) && is_leaf(ctx, lhs_id) { + lhs_id + } else if is_none_literal(lhs) && is_leaf(ctx, rhs_id) { + rhs_id + } else { + return false; + }; + let Some(value_sym) = ctx.hir.symbol(value_id) else { + return false; + }; + + // `==`: the value must be repeated in the else-branch; `!=`: in the then-branch. + let value_branch_id = if op == "==" { else_expr_id } else { then_id }; + + if !is_leaf(ctx, value_branch_id) { + return false; + } + let Some(value_branch) = ctx.hir.symbol(value_branch_id) else { + return false; + }; + value_branch.kind == value_sym.kind && value_branch.value == value_sym.value +} + impl LintRule for PreferCoalesce { fn id(&self) -> &'static str { "prefer_coalesce" @@ -11,7 +81,61 @@ impl LintRule for PreferCoalesce { Severity::Style } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + ctx.all_symbols() + .filter(|(_, sym)| matches!(sym.kind, SymbolKind::If)) + .filter(|(if_id, _)| matches_null_check_pattern(ctx, *if_id)) + .map(|(_, if_sym)| { + let mut d = Diagnostic::new( + self.id(), + self.severity(), + "`if`/`else` null-check can be simplified using the `??` coalesce operator", + ); + if let Some(range) = if_sym.source.text_range { + d = d.with_range(range); + } + d.with_help("rewrite as ` ?? `") + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + PreferCoalesce.check(&ctx) + } + + #[test] + fn detects_eq_none_pattern() { + let diags = check(r#"if (.value == none): "default" else: .value"#); + assert_eq!(diags.len(), 1); + } + + #[test] + fn detects_ne_none_pattern() { + let diags = check(r#"if (x != none): x else: "default""#); + assert_eq!(diags.len(), 1); + } + + #[test] + fn no_diagnostic_when_branches_differ() { + let diags = check(r#"if (.value == none): "default" else: .other"#); + assert_eq!(diags.len(), 0); + } + + #[test] + fn no_diagnostic_for_unrelated_condition() { + let diags = check(r#"if (.checked == true): "yes" else: "no""#); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/style/prefer_let_over_var.rs b/crates/mq-lint/src/rules/style/prefer_let_over_var.rs index cea781f6d..208ee2434 100644 --- a/crates/mq-lint/src/rules/style/prefer_let_over_var.rs +++ b/crates/mq-lint/src/rules/style/prefer_let_over_var.rs @@ -1,4 +1,8 @@ +use rustc_hash::FxHashSet; + use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::{SymbolId, SymbolKind}; +use mq_lang::Range; pub struct PreferLetOverVar; @@ -11,7 +15,93 @@ impl LintRule for PreferLetOverVar { Severity::Warn } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + // Collect names that appear as the LHS of an Assign node (re-assigned variables). + let reassigned_names: FxHashSet<&str> = ctx + .all_symbols() + .filter(|(_, s)| matches!(s.kind, SymbolKind::Assign)) + .flat_map(|(assign_id, _)| { + ctx.all_symbols() + .filter(move |(_, s)| { + s.parent == Some(assign_id) && matches!(s.kind, SymbolKind::Ident | SymbolKind::Ref) + }) + .filter_map(|(_, s)| s.value.as_deref()) + .collect::>() + }) + .collect(); + + let mut diagnostics = Vec::new(); + + // Sort all source symbols by position for sequential pairing. + let mut syms: Vec<(SymbolId, Range, &mq_hir::Symbol)> = ctx + .all_symbols() + .filter_map(|(id, s)| s.source.text_range.map(|r| (id, r, s))) + .collect(); + syms.sort_by_key(|(_, r, _)| (r.start.line, r.start.column)); + + // Find Keyword("var") symbols and pair each with the next Variable in the same scope. + for i in 0..syms.len() { + let (_, kw_range, kw_sym) = syms[i]; + if !matches!(kw_sym.kind, SymbolKind::Keyword) || kw_sym.value.as_deref() != Some("var") { + continue; + } + + // The Variable for this `var` is the next Variable after the keyword + // in the same scope (same parent) at a later position. + let var_sym = syms[i + 1..].iter().find(|(_, _, s)| { + s.parent == kw_sym.parent && matches!(s.kind, SymbolKind::Variable) && s.scope == kw_sym.scope + }); + + let Some((_, _, var_sym)) = var_sym else { + continue; + }; + let name = match var_sym.value.as_deref() { + Some(n) => n, + None => continue, + }; + + if reassigned_names.contains(name) { + continue; + } + + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("`{name}` is never reassigned; prefer `let` over `var`"), + ); + d = d.with_range(kw_range); + diagnostics.push(d.with_help(format!("change `var {name}` to `let {name}`"))); + } + + diagnostics + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + PreferLetOverVar.check(&ctx) + } + + #[test] + fn detects_var_never_reassigned() { + let diags = check("var x = .h1 | x"); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("prefer `let` over `var`")); + } + + #[test] + fn no_diagnostic_for_let() { + let diags = check("let x = .h1 | x"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/style/prefer_pipe_style.rs b/crates/mq-lint/src/rules/style/prefer_pipe_style.rs index 60dbc722c..e080bc962 100644 --- a/crates/mq-lint/src/rules/style/prefer_pipe_style.rs +++ b/crates/mq-lint/src/rules/style/prefer_pipe_style.rs @@ -1,3 +1,5 @@ +use mq_hir::SymbolKind; + use crate::{Diagnostic, LintContext, LintRule, Severity}; pub struct PreferPipeStyle; @@ -11,7 +13,75 @@ impl LintRule for PreferPipeStyle { Severity::Style } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + // `f(g(x))` can always be rewritten as the pipe `x | g() | f()`. + ctx.all_symbols() + .filter(|(_, sym)| matches!(sym.kind, SymbolKind::Call | SymbolKind::CallDynamic)) + .filter_map(|(outer_id, outer_sym)| { + let mut args = ctx.all_symbols().filter(|(_, s)| s.parent == Some(outer_id)); + let (_, arg) = args.next()?; + if args.next().is_some() { + return None; + } + if !matches!(arg.kind, SymbolKind::Call | SymbolKind::CallDynamic) { + return None; + } + + let outer_name = outer_sym.value.as_deref().unwrap_or(""); + let inner_name = arg.value.as_deref().unwrap_or(""); + + let mut d = Diagnostic::new( + self.id(), + self.severity(), + format!("nested call `{outer_name}({inner_name}(...))` reads better as a pipe"), + ); + if let Some(range) = outer_sym.source.text_range { + d = d.with_range(range); + } + Some(d.with_help(format!("rewrite as `... | {inner_name}() | {outer_name}()`"))) + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + PreferPipeStyle.check(&ctx) + } + + #[test] + fn detects_nested_unary_calls() { + let diags = check("to_text(to_upper(x))"); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("to_text")); + assert!(diags[0].message.contains("to_upper")); + } + + #[test] + fn no_diagnostic_for_multi_arg_outer_call() { + let diags = check("add(foo(x), y)"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn no_diagnostic_for_non_call_argument() { + let diags = check("to_text(x)"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn no_diagnostic_for_already_piped_style() { + let diags = check("x | to_upper() | to_text()"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/style/prefer_specific_heading.rs b/crates/mq-lint/src/rules/style/prefer_specific_heading.rs index 051297a4a..590eb1990 100644 --- a/crates/mq-lint/src/rules/style/prefer_specific_heading.rs +++ b/crates/mq-lint/src/rules/style/prefer_specific_heading.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::SymbolKind; pub struct PreferSpecificHeading; @@ -11,7 +12,51 @@ impl LintRule for PreferSpecificHeading { Severity::Style } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + // Selector symbols use insert_symbol → use all_symbols + ctx.all_symbols() + .filter(|(_, s)| matches!(s.kind, SymbolKind::Selector(mq_lang::Selector::Heading(None)))) + .map(|(_, sym)| { + let mut d = Diagnostic::new( + self.id(), + self.severity(), + "prefer a specific heading level selector (`.h1`–`.h6`) over `.h`", + ); + if let Some(range) = sym.source.text_range { + d = d.with_range(range); + } + d.with_help("using `.h1`–`.h6` makes the intended heading level explicit") + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + PreferSpecificHeading.check(&ctx) + } + + #[test] + fn detects_generic_heading_selector() { + let diags = check(".h"); + assert_eq!(diags.len(), 1); + } + + #[test] + fn no_diagnostic_for_specific_heading() { + let diags = check(".h1"); + assert_eq!(diags.len(), 0); + let diags2 = check(".h6"); + assert_eq!(diags2.len(), 0); } } diff --git a/crates/mq-lint/src/rules/style/redundant_boolean_literal.rs b/crates/mq-lint/src/rules/style/redundant_boolean_literal.rs index 8670dc320..a245a4e66 100644 --- a/crates/mq-lint/src/rules/style/redundant_boolean_literal.rs +++ b/crates/mq-lint/src/rules/style/redundant_boolean_literal.rs @@ -1,4 +1,5 @@ use crate::{Diagnostic, LintContext, LintRule, Severity}; +use mq_hir::{SymbolId, SymbolKind}; pub struct RedundantBooleanLiteral; @@ -11,7 +12,138 @@ impl LintRule for RedundantBooleanLiteral { Severity::Style } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + let mut diagnostics = Vec::new(); + + // Find If symbols that have an Else child. + let if_ids: Vec<_> = ctx + .hir + .symbols_for_source(ctx.source_id) + .filter(|(_, s)| matches!(s.kind, SymbolKind::If)) + .collect(); + + for (if_id, if_sym) in if_ids { + let Some(else_id) = find_else_child(ctx, if_id) else { + continue; + }; + + // The then-body is the second child of If by position (first is the condition). + let then_bool = second_child_boolean(ctx, if_id); + // The else-body is the first (only) child of Else. + let else_bool = first_child_boolean(ctx, else_id); + + let (then_val, else_val) = match (then_bool, else_bool) { + (Some(t), Some(e)) => (t, e), + _ => continue, + }; + + // Pattern: if (cond): true else: false → cond + // if (cond): false else: true → !cond + if (then_val == "true" && else_val == "false") || (then_val == "false" && else_val == "true") { + let suggestion = if then_val == "true" { + "replace `if (cond): true else: false` with just `cond`" + } else { + "replace `if (cond): false else: true` with `not(cond)` or `!(cond)`" + }; + + let mut d = Diagnostic::new( + self.id(), + self.severity(), + "redundant boolean literal in `if`/`else` — condition already is the result", + ); + if let Some(range) = if_sym.source.text_range { + d = d.with_range(range); + } + diagnostics.push(d.with_help(suggestion)); + } + } + + diagnostics + } +} + +fn find_else_child(ctx: &LintContext<'_>, if_id: SymbolId) -> Option { + ctx.hir + .symbols_for_source(ctx.source_id) + .find(|(_, s)| s.parent == Some(if_id) && matches!(s.kind, SymbolKind::Else)) + .map(|(id, _)| id) +} + +/// Returns the value of the second child (by source position) of `parent_id` if it is Boolean. +fn second_child_boolean<'a>(ctx: &'a LintContext<'_>, parent_id: SymbolId) -> Option<&'a str> { + let mut children: Vec<_> = ctx + .all_symbols() + .filter(|(_, s)| { + s.parent == Some(parent_id) + && !matches!(s.kind, SymbolKind::Else | SymbolKind::Elif) + && s.source.text_range.is_some() + }) + .filter_map(|(id, s)| s.source.text_range.map(|r| (id, r, s))) + .collect(); + children.sort_by_key(|(_, r, _)| (r.start.line, r.start.column)); + + let (_, _, sym) = children.get(1)?; + if matches!(sym.kind, SymbolKind::Boolean) { + sym.value.as_deref() + } else { + None + } +} + +/// Returns the value of the first child (by source position) of `parent_id` if it is Boolean. +fn first_child_boolean<'a>(ctx: &'a LintContext<'_>, parent_id: SymbolId) -> Option<&'a str> { + let mut children: Vec<_> = ctx + .all_symbols() + .filter(|(_, s)| s.parent == Some(parent_id) && s.source.text_range.is_some()) + .filter_map(|(id, s)| s.source.text_range.map(|r| (id, r, s))) + .collect(); + children.sort_by_key(|(_, r, _)| (r.start.line, r.start.column)); + + let (_, _, sym) = children.first()?; + if matches!(sym.kind, SymbolKind::Boolean) { + sym.value.as_deref() + } else { + None + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + RedundantBooleanLiteral.check(&ctx) + } + + #[test] + fn detects_true_false_pattern() { + let diags = check("if (.h1): true else: false;"); + assert_eq!(diags.len(), 1); + assert!(diags[0].message.contains("redundant boolean literal")); + } + + #[test] + fn detects_false_true_pattern() { + let diags = check("if (.h1): false else: true;"); + assert_eq!(diags.len(), 1); + } + + #[test] + fn no_diagnostic_for_non_boolean_branches() { + let diags = check("if (.h1): 1 else: 2;"); + assert_eq!(diags.len(), 0); + } + + #[test] + fn no_diagnostic_for_mixed_branches() { + let diags = check("if (.h1): true else: 2;"); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/style/redundant_try.rs b/crates/mq-lint/src/rules/style/redundant_try.rs index 69de32045..0568fa870 100644 --- a/crates/mq-lint/src/rules/style/redundant_try.rs +++ b/crates/mq-lint/src/rules/style/redundant_try.rs @@ -1,3 +1,5 @@ +use mq_hir::SymbolKind; + use crate::{Diagnostic, LintContext, LintRule, Severity}; pub struct RedundantTry; @@ -11,7 +13,62 @@ impl LintRule for RedundantTry { Severity::Style } - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() + fn check(&self, ctx: &LintContext<'_>) -> Vec { + // `try: catch: none` is exactly what `?` does. + ctx.all_symbols() + .filter(|(_, sym)| matches!(sym.kind, SymbolKind::Try)) + .filter_map(|(try_id, try_sym)| { + let (catch_id, _) = ctx + .all_symbols() + .find(|(_, s)| s.parent == Some(try_id) && matches!(s.kind, SymbolKind::Catch))?; + let (catch_expr_id, catch_expr) = ctx.all_symbols().find(|(_, s)| s.parent == Some(catch_id))?; + + let is_none_literal = + matches!(catch_expr.kind, SymbolKind::None) || catch_expr.value.as_deref() == Some("none"); + let has_no_children = !ctx.all_symbols().any(|(_, s)| s.parent == Some(catch_expr_id)); + + if !is_none_literal || !has_no_children { + return None; + } + + let mut d = Diagnostic::new( + self.id(), + self.severity(), + "`try: ... catch: none` is equivalent to the `?` error-suppression operator", + ); + if let Some(range) = try_sym.source.text_range { + d = d.with_range(range); + } + Some(d.with_help("rewrite as `?`")) + }) + .collect() + } +} + +#[cfg(test)] +mod tests { + use mq_hir::Hir; + + use super::*; + use crate::{LintConfig, LintContext}; + + fn check(code: &str) -> Vec { + let mut hir = Hir::default(); + let (source_id, _) = hir.add_code(None, code); + let config = LintConfig::default(); + let ctx = LintContext::new(&hir, source_id, &config); + RedundantTry.check(&ctx) + } + + #[test] + fn detects_catch_none() { + let diags = check(r#"try: get("x") catch: none"#); + assert_eq!(diags.len(), 1); + } + + #[test] + fn no_diagnostic_for_catch_with_fallback() { + let diags = check(r#"try: get("x") catch: "default""#); + assert_eq!(diags.len(), 0); } } diff --git a/crates/mq-lint/src/rules/style/unnecessary_parens.rs b/crates/mq-lint/src/rules/style/unnecessary_parens.rs deleted file mode 100644 index 83615aabe..000000000 --- a/crates/mq-lint/src/rules/style/unnecessary_parens.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; - -pub struct UnnecessaryParens; - -impl LintRule for UnnecessaryParens { - fn id(&self) -> &'static str { - "unnecessary_parens" - } - - fn severity(&self) -> Severity { - Severity::Style - } - - fn check(&self, _ctx: &LintContext<'_>) -> Vec { - Vec::new() - } -} diff --git a/crates/mq-lsp/Cargo.toml b/crates/mq-lsp/Cargo.toml index b2a34c9f6..77b6a7eff 100644 --- a/crates/mq-lsp/Cargo.toml +++ b/crates/mq-lsp/Cargo.toml @@ -20,6 +20,7 @@ mq-check = {workspace = true} mq-formatter = {workspace = true} mq-hir = {workspace = true} mq-lang = {workspace = true, features = ["ast-json", "sync", "file-io"]} +mq-lint = {workspace = true} mq-markdown = {workspace = true} serde_json = {workspace = true} smol_str = {workspace = true} diff --git a/crates/mq-lsp/README.md b/crates/mq-lsp/README.md index c121f30d6..5b75f77b1 100644 --- a/crates/mq-lsp/README.md +++ b/crates/mq-lsp/README.md @@ -5,6 +5,7 @@ Language Server Protocol (LSP) implementation for the [mq](https://mqlang.org/) ## Features - 🔍 **Diagnostics**: Real-time syntax and semantic error reporting +- 🧹 **Linting**: Optional `mq-lint` diagnostics (correctness, style, complexity, selector, and module rules), toggled with `--enable-lint` - 💡 **Code Completion**: Intelligent suggestions for selectors, functions, and variables - 📖 **Hover Information**: Inline documentation and type information - 🎯 **Go To Definition**: Navigate to symbol definitions with a single click @@ -52,6 +53,16 @@ The LSP server communicates via stdin/stdout following the LSP protocol: mq-lsp ``` +### CLI Options + +| Option | Description | +| -------------------------------- | ------------------------------------------------------------ | +| `-M, --module-path ` | Search modules from the directory (repeatable) | +| `-T, --enable-type-checking` | Enable type checking for mq queries | +| `--strict-array` | Reject heterogeneous arrays (requires `--enable-type-checking`) | +| `-L, --enable-lint` | Enable `mq-lint` diagnostics | +| `--disable-lint-rule ` | Disable a specific lint rule by ID (repeatable, requires `--enable-lint`) | + ## Development ### Building from Source diff --git a/crates/mq-lsp/src/error.rs b/crates/mq-lsp/src/error.rs index 58256fa69..8e461d278 100644 --- a/crates/mq-lsp/src/error.rs +++ b/crates/mq-lsp/src/error.rs @@ -1,4 +1,5 @@ use tower_lsp_server::ls_types; +use tower_lsp_server::ls_types::NumberOrString; pub type SyntaxError = (std::string::String, mq_lang::Range); @@ -6,38 +7,39 @@ pub type SyntaxError = (std::string::String, mq_lang::Range); pub enum LspError { SyntaxError(SyntaxError), TypeError(mq_check::TypeError), + LintWarning(mq_lint::Diagnostic), +} + +fn lsp_range(range: mq_lang::Range) -> ls_types::Range { + ls_types::Range::new( + ls_types::Position { + line: range.start.line.saturating_sub(1), + character: range.start.column.saturating_sub(1) as u32, + }, + ls_types::Position { + line: range.end.line.saturating_sub(1), + character: range.end.column.saturating_sub(1) as u32, + }, + ) +} + +fn lint_severity(severity: mq_lint::Severity) -> ls_types::DiagnosticSeverity { + match severity { + mq_lint::Severity::Error => ls_types::DiagnosticSeverity::ERROR, + mq_lint::Severity::Warn => ls_types::DiagnosticSeverity::WARNING, + mq_lint::Severity::Perf => ls_types::DiagnosticSeverity::INFORMATION, + mq_lint::Severity::Style => ls_types::DiagnosticSeverity::HINT, + } } impl From<&LspError> for ls_types::Diagnostic { fn from(error: &LspError) -> Self { match error { - LspError::SyntaxError((message, range)) => ls_types::Diagnostic::new_simple( - ls_types::Range::new( - ls_types::Position { - line: range.start.line.saturating_sub(1), - character: range.start.column.saturating_sub(1) as u32, - }, - ls_types::Position { - line: range.end.line.saturating_sub(1), - character: range.end.column.saturating_sub(1) as u32, - }, - ), - message.to_string(), - ), + LspError::SyntaxError((message, range)) => { + ls_types::Diagnostic::new_simple(lsp_range(*range), message.to_string()) + } LspError::TypeError(type_error) => match type_error.location() { - Some(range) => ls_types::Diagnostic::new_simple( - ls_types::Range::new( - ls_types::Position { - line: range.start.line.saturating_sub(1), - character: range.start.column.saturating_sub(1) as u32, - }, - ls_types::Position { - line: range.end.line.saturating_sub(1), - character: range.end.column.saturating_sub(1) as u32, - }, - ), - type_error.to_string(), - ), + Some(range) => ls_types::Diagnostic::new_simple(lsp_range(range), type_error.to_string()), None => ls_types::Diagnostic::new_simple( ls_types::Range::new( ls_types::Position { line: 0, character: 0 }, @@ -46,6 +48,67 @@ impl From<&LspError> for ls_types::Diagnostic { type_error.to_string(), ), }, + LspError::LintWarning(diagnostic) => { + let range = diagnostic.range.map(lsp_range).unwrap_or_else(|| { + ls_types::Range::new( + ls_types::Position { line: 0, character: 0 }, + ls_types::Position { line: 0, character: 1 }, + ) + }); + let message = match &diagnostic.help { + Some(help) => format!("{} (help: {help})", diagnostic.message), + None => diagnostic.message.clone(), + }; + ls_types::Diagnostic::new( + range, + Some(lint_severity(diagnostic.severity)), + Some(NumberOrString::String(diagnostic.rule_id.to_string())), + Some("mq-lint".to_string()), + message, + None, + None, + ) + } } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn lint_warning_carries_rule_id_source_and_severity() { + let diagnostic = mq_lint::Diagnostic::new("unused_variable", mq_lint::Severity::Warn, "unused variable `x`") + .with_range(mq_lang::Range { + start: mq_lang::Position { line: 1, column: 5 }, + end: mq_lang::Position { line: 1, column: 6 }, + }) + .with_help("prefix with `_` if intentional"); + + let lsp_diagnostic: ls_types::Diagnostic = (&LspError::LintWarning(diagnostic)).into(); + + assert_eq!(lsp_diagnostic.severity, Some(ls_types::DiagnosticSeverity::WARNING)); + assert_eq!( + lsp_diagnostic.code, + Some(NumberOrString::String("unused_variable".to_string())) + ); + assert_eq!(lsp_diagnostic.source, Some("mq-lint".to_string())); + assert!(lsp_diagnostic.message.contains("unused variable `x`")); + assert!(lsp_diagnostic.message.contains("help: prefix with `_` if intentional")); + assert_eq!(lsp_diagnostic.range.start.line, 0); + assert_eq!(lsp_diagnostic.range.start.character, 4); + } + + #[test] + fn lint_warning_without_range_falls_back_to_origin() { + let diagnostic = + mq_lint::Diagnostic::new("duplicate_match_arm", mq_lint::Severity::Error, "duplicate match arm"); + + let lsp_diagnostic: ls_types::Diagnostic = (&LspError::LintWarning(diagnostic)).into(); + + assert_eq!(lsp_diagnostic.severity, Some(ls_types::DiagnosticSeverity::ERROR)); + assert_eq!(lsp_diagnostic.range.start.line, 0); + assert_eq!(lsp_diagnostic.range.start.character, 0); + } +} diff --git a/crates/mq-lsp/src/main.rs b/crates/mq-lsp/src/main.rs index 97cb70421..09459426e 100644 --- a/crates/mq-lsp/src/main.rs +++ b/crates/mq-lsp/src/main.rs @@ -30,6 +30,9 @@ struct Cli { #[clap(flatten)] type_check: TypeCheckArgs, + + #[clap(flatten)] + lint: LintArgs, } #[derive(Clone, Debug, clap::Args, Default)] @@ -47,6 +50,17 @@ struct TypeCheckArgs { tuple: bool, } +#[derive(Clone, Debug, clap::Args, Default)] +struct LintArgs { + /// Enable mq-lint diagnostics (style, correctness, complexity, selector, module rules) + #[arg(short = 'L', long, default_value_t = false)] + enable_lint: bool, + + /// Disable a specific lint rule by ID (repeatable) + #[arg(long = "disable-lint-rule", value_name = "RULE_ID")] + disable_lint_rule: Vec, +} + #[tokio::main] async fn main() { let cli = Cli::parse(); @@ -55,10 +69,17 @@ async fn main() { ..Default::default() }; + let mut lint_config = mq_lint::LintConfig::default(); + for rule_id in &cli.lint.disable_lint_rule { + lint_config.disable_rule(rule_id.clone()); + } + let config = LspConfig::new( cli.module_paths.unwrap_or_default(), cli.type_check.enable_type_checking, type_check_config, + cli.lint.enable_lint, + lint_config, ); server::start(config).await; } diff --git a/crates/mq-lsp/src/server.rs b/crates/mq-lsp/src/server.rs index e6ca62e7f..d4a324b56 100644 --- a/crates/mq-lsp/src/server.rs +++ b/crates/mq-lsp/src/server.rs @@ -509,6 +509,19 @@ impl Backend { None } })); + + // `unused_function` is skipped: already covered by the warning above. + if self.config.enable_lint { + let lint_ctx = mq_lint::LintContext::new(&hir_guard, *source_id, &self.config.lint_config); + let linter = mq_lint::Linter::with_default_rules(); + diagnostics.extend( + linter + .run(&lint_ctx) + .into_iter() + .filter(|d| d.rule_id != "unused_function") + .map(|d| (&LspError::LintWarning(d)).into()), + ); + } } } // Guards are dropped here, before the await @@ -523,6 +536,8 @@ pub struct LspConfig { module_paths: Vec, enable_type_checking: bool, type_checker_options: mq_check::TypeCheckerOptions, + enable_lint: bool, + lint_config: mq_lint::LintConfig, } impl LspConfig { @@ -534,15 +549,21 @@ impl LspConfig { /// to the LSP server. These paths are used to initialize the language server's environment, /// allowing it to provide features such as code completion, diagnostics, and navigation /// based on the specified modules. + /// * `enable_lint` - Whether to also publish `mq-lint` diagnostics. + /// * `lint_config` - Per-rule lint configuration, used when `enable_lint` is `true`. pub fn new( module_paths: Vec, enable_type_checking: bool, type_checker_options: mq_check::TypeCheckerOptions, + enable_lint: bool, + lint_config: mq_lint::LintConfig, ) -> Self { Self { module_paths, enable_type_checking, type_checker_options, + enable_lint, + lint_config, } } } @@ -1475,6 +1496,92 @@ mod tests { backend.diagnostics(uri, None).await; } + #[tokio::test] + async fn test_lint_diagnostics_enabled() { + let (service, _) = LspService::new(|client| Backend { + client, + hir: Arc::new(RwLock::new(mq_hir::Hir::default())), + source_map: RwLock::new(BiMap::new()), + type_env_map: DashMap::new(), + error_map: DashMap::new(), + text_map: DashMap::new(), + config: LspConfig::new( + vec![], + false, + mq_check::TypeCheckerOptions::default(), + true, + mq_lint::LintConfig::default(), + ), + }); + + let backend = service.inner(); + let uri = Url::parse("file:///test.mq").unwrap(); + + // `x` is declared but never used. + let code = "let x = .h1 | .text"; + let (nodes, errors) = mq_lang::parse_recovery(code); + let (source_id, _) = backend.hir.write().unwrap().add_nodes(uri.clone(), &nodes); + + backend.source_map.write().unwrap().insert(uri.to_string(), source_id); + backend.text_map.insert(uri.to_string(), code.to_string().into()); + backend.error_map.insert( + uri.to_string(), + errors + .error_ranges(code) + .into_iter() + .map(|(message, range)| LspError::SyntaxError((message, range))) + .collect(), + ); + + { + let hir_guard = backend.hir.read().unwrap(); + let lint_ctx = mq_lint::LintContext::new(&hir_guard, source_id, &backend.config.lint_config); + let diagnostics = mq_lint::Linter::with_default_rules().run(&lint_ctx); + assert!( + diagnostics.iter().any(|d| d.rule_id == "unused_variable"), + "expected an unused_variable lint diagnostic" + ); + } + + // Should complete without panic. + backend.diagnostics(uri, None).await; + } + + #[tokio::test] + async fn test_lint_diagnostics_disabled_by_default() { + let (service, _) = LspService::new(|client| Backend { + client, + hir: Arc::new(RwLock::new(mq_hir::Hir::default())), + source_map: RwLock::new(BiMap::new()), + type_env_map: DashMap::new(), + error_map: DashMap::new(), + text_map: DashMap::new(), + config: LspConfig::default(), + }); + + let backend = service.inner(); + assert!(!backend.config.enable_lint); + + let uri = Url::parse("file:///test.mq").unwrap(); + let code = "let x = .h1 | .text"; + let (nodes, errors) = mq_lang::parse_recovery(code); + let (source_id, _) = backend.hir.write().unwrap().add_nodes(uri.clone(), &nodes); + + backend.source_map.write().unwrap().insert(uri.to_string(), source_id); + backend.text_map.insert(uri.to_string(), code.to_string().into()); + backend.error_map.insert( + uri.to_string(), + errors + .error_ranges(code) + .into_iter() + .map(|(message, range)| LspError::SyntaxError((message, range))) + .collect(), + ); + + // Should complete without panic. + backend.diagnostics(uri, None).await; + } + #[tokio::test] async fn test_formatting_with_errors() { let (service, _) = LspService::new(|client| Backend { @@ -1697,6 +1804,8 @@ mod tests { #[tokio::test] async fn test_lsp_config_new() { + let mut lint_config = mq_lint::LintConfig::default(); + lint_config.disable_rule("naming_convention"); let config = LspConfig::new( vec![], true, @@ -1704,9 +1813,13 @@ mod tests { strict_array: true, ..Default::default() }, + true, + lint_config, ); assert!(config.enable_type_checking); assert!(config.type_checker_options.strict_array); + assert!(config.enable_lint); + assert!(!config.lint_config.is_rule_enabled("naming_convention")); } #[tokio::test] @@ -1714,6 +1827,7 @@ mod tests { let config = LspConfig::default(); assert!(!config.enable_type_checking); assert!(!config.type_checker_options.strict_array); + assert!(!config.enable_lint); } #[tokio::test] @@ -1725,7 +1839,13 @@ mod tests { type_env_map: DashMap::new(), error_map: DashMap::new(), text_map: DashMap::new(), - config: LspConfig::new(vec![], false, mq_check::TypeCheckerOptions::default()), + config: LspConfig::new( + vec![], + false, + mq_check::TypeCheckerOptions::default(), + false, + mq_lint::LintConfig::default(), + ), }); let backend = service.inner(); @@ -1761,7 +1881,13 @@ mod tests { type_env_map: DashMap::new(), error_map: DashMap::new(), text_map: DashMap::new(), - config: LspConfig::new(vec![], true, mq_check::TypeCheckerOptions::default()), + config: LspConfig::new( + vec![], + true, + mq_check::TypeCheckerOptions::default(), + false, + mq_lint::LintConfig::default(), + ), }); let backend = service.inner(); @@ -1798,7 +1924,13 @@ mod tests { type_env_map: DashMap::new(), error_map: DashMap::new(), text_map: DashMap::new(), - config: LspConfig::new(vec![], true, mq_check::TypeCheckerOptions::default()), + config: LspConfig::new( + vec![], + true, + mq_check::TypeCheckerOptions::default(), + false, + mq_lint::LintConfig::default(), + ), }); let backend = service.inner(); @@ -1835,7 +1967,13 @@ mod tests { type_env_map: DashMap::new(), error_map: DashMap::new(), text_map: DashMap::new(), - config: LspConfig::new(vec![], true, mq_check::TypeCheckerOptions::default()), + config: LspConfig::new( + vec![], + true, + mq_check::TypeCheckerOptions::default(), + false, + mq_lint::LintConfig::default(), + ), }); let backend = service.inner(); @@ -1878,6 +2016,8 @@ mod tests { strict_array: true, ..Default::default() }, + false, + mq_lint::LintConfig::default(), ), }); @@ -1921,6 +2061,8 @@ mod tests { strict_array: false, ..Default::default() }, + false, + mq_lint::LintConfig::default(), ), }); diff --git a/docs/books/src/start/external_subcommands.md b/docs/books/src/start/external_subcommands.md index 36135929d..94a827976 100644 --- a/docs/books/src/start/external_subcommands.md +++ b/docs/books/src/start/external_subcommands.md @@ -31,6 +31,7 @@ The following external tools are available to extend mq's functionality: - [mq-docs](https://github.com/harehare/mq-docs) - A documentation generator for mq functions, macros, and selectors. - [mq-fmt](https://github.com/harehare/mq/blob/main/crates/mq-formatter/README.md)- Formatter for mq query language (.mq) files. - [mq-http](https://github.com/harehare/mq-http) - A lightweight HTTP server that executes mq scripts for each request. +- [mq-lint](https://github.com/harehare/mq/blob/main/crates/mq-lint/README.md) - Static analysis linter for mq files (correctness, style, complexity, selector, and module rules). - [mq-lsp](https://github.com/harehare/mq/tree/main/crates/mq-lsp/README.md) - Language Server Protocol (LSP) implementation for mq query files, providing IDE features like completion, hover, and diagnostics. - [mq-mcp](https://github.com/harehare/mq-mcp) - Model Context Protocol (MCP) server implementation for AI assistants. - [mq-serve](https://github.com/harehare/mq-serve) - A browser-based Markdown viewer with mq query support. diff --git a/editors/neovim/README.md b/editors/neovim/README.md index 54477ad8e..d2354f26f 100644 --- a/editors/neovim/README.md +++ b/editors/neovim/README.md @@ -100,6 +100,12 @@ require("mq").setup({ -- Enable strict array mode (passes --strict-array to mq-lsp, requires enable_type_check) strict_array = false, + -- Enable mq-lint diagnostics (passes --enable-lint to mq-lsp) + enable_lint = false, + + -- Lint rule IDs to disable (passes --disable-lint-rule to mq-lsp for each, requires enable_lint) + lint_disabled_rules = {}, + -- Enable LSP inlay hints (requires Neovim 0.10+) enable_inlay_hints = true, @@ -130,6 +136,17 @@ require("mq").setup({ }) ``` +### Linting + +Enable `mq-lint` diagnostics (correctness, style, complexity, selector, and module rules): + +```lua +require("mq").setup({ + enable_lint = true, + lint_disabled_rules = { "naming_convention", "shadow_variable" }, +}) +``` + ### Inlay Hints Inlay hints show inferred types inline in the editor. They are enabled by default on Neovim 0.10+. diff --git a/editors/neovim/lua/mq/config.lua b/editors/neovim/lua/mq/config.lua index 7c0cd9365..4ff8eea74 100644 --- a/editors/neovim/lua/mq/config.lua +++ b/editors/neovim/lua/mq/config.lua @@ -37,6 +37,10 @@ M.defaults = { enable_type_check = false, -- Enable strict array mode (passes --strict-array to mq-lsp, requires enable_type_check) strict_array = false, + -- Enable mq-lint diagnostics (passes --enable-lint to mq-lsp) + enable_lint = false, + -- Lint rule IDs to disable (passes --disable-lint-rule to mq-lsp for each, requires enable_lint) + lint_disabled_rules = {}, -- Enable LSP inlay hints (requires Neovim 0.10+) enable_inlay_hints = true, -- LSP server configuration diff --git a/editors/neovim/lua/mq/lsp.lua b/editors/neovim/lua/mq/lsp.lua index 89751b101..e342bebe6 100644 --- a/editors/neovim/lua/mq/lsp.lua +++ b/editors/neovim/lua/mq/lsp.lua @@ -39,8 +39,19 @@ function M.start() end end + -- Build lint args from config + local lint_args = {} + if config.get("enable_lint") then + table.insert(lint_args, "--enable-lint") + for _, rule_id in ipairs(config.get("lint_disabled_rules")) do + table.insert(lint_args, "--disable-lint-rule") + table.insert(lint_args, rule_id) + end + end + local args = vim.list_extend(vim.deepcopy(config.get("lsp_args")), multi_workspace_args) vim.list_extend(args, type_check_args) + vim.list_extend(args, lint_args) local lsp_config = config.get("lsp") diff --git a/editors/vscode/package.json b/editors/vscode/package.json index 8453e31c7..773b526d2 100644 --- a/editors/vscode/package.json +++ b/editors/vscode/package.json @@ -144,6 +144,21 @@ "type": "boolean", "default": false, "description": "Enable or disable strict array mode. When enabled, arrays must contain elements of a single type." + }, + "mq.lint.enableLint": { + "title": "Enable lint", + "type": "boolean", + "default": false, + "description": "Enable or disable mq-lint diagnostics (correctness, style, complexity, selector, and module rules) for mq queries." + }, + "mq.lint.disabledRules": { + "title": "Disabled lint rules", + "type": "array", + "items": { + "type": "string" + }, + "default": [], + "description": "Lint rule IDs to disable (e.g. \"naming_convention\", \"shadow_variable\"). Only effective when mq.lint.enableLint is true." } } }, diff --git a/editors/vscode/src/extension.ts b/editors/vscode/src/extension.ts index 40764dcac..c6715f1e6 100644 --- a/editors/vscode/src/extension.ts +++ b/editors/vscode/src/extension.ts @@ -605,6 +605,8 @@ const startLspServer = async (providedLspPath?: string) => { const enableTypeCheck = config.get("typeCheck.enableTypeCheck"); const strictArray = config.get("typeCheck.strictArray"); + const enableLint = config.get("lint.enableLint"); + const disabledLintRules = config.get("lint.disabledRules") ?? []; const workspaceFolders = vscode.workspace.workspaceFolders; const workspacePaths = workspaceFolders && workspaceFolders.length > 0 @@ -612,6 +614,12 @@ const startLspServer = async (providedLspPath?: string) => { : [process.cwd()]; const multiWorkspaceArgs = workspacePaths.flatMap((path) => ["-M", path]); + const lintArgs = enableLint + ? [ + "--enable-lint", + ...disabledLintRules.flatMap((ruleId) => ["--disable-lint-rule", ruleId]), + ] + : []; const run: lc.Executable = { command: lspPath, @@ -621,6 +629,7 @@ const startLspServer = async (providedLspPath?: string) => { enableTypeCheck ? "--enable-type-checking" : "", enableTypeCheck && strictArray ? "--strict-array" : "", ].filter((v) => v !== ""), + ...lintArgs, ], options: {}, }; diff --git a/editors/zed/README.md b/editors/zed/README.md index aa3f31195..05b954d91 100644 --- a/editors/zed/README.md +++ b/editors/zed/README.md @@ -90,6 +90,28 @@ Enable type checking by adding the following to your Zed `settings.json` (`cmd-, | `enableTypeCheck` | `false` | Enable type checking; passes `--enable-type-checking` to `mq-lsp` | | `strictArray` | `false` | Arrays must contain elements of a single type (requires `enableTypeCheck`) | +### Linting + +Enable `mq-lint` diagnostics (correctness, style, complexity, selector, and module rules): + +```json +{ + "lsp": { + "mq-lsp": { + "initialization_options": { + "enableLint": true, + "lintDisabledRules": ["naming_convention", "shadow_variable"] + } + } + } +} +``` + +| Option | Default | Description | +|---|---|---| +| `enableLint` | `false` | Enable `mq-lint` diagnostics; passes `--enable-lint` to `mq-lsp` | +| `lintDisabledRules` | `[]` | Lint rule IDs to disable (requires `enableLint`) | + ### Inlay Hints Inlay hints show inferred types inline in the editor. Enable them in your Zed `settings.json`: diff --git a/editors/zed/src/lib.rs b/editors/zed/src/lib.rs index 0c842db4c..304095eb0 100644 --- a/editors/zed/src/lib.rs +++ b/editors/zed/src/lib.rs @@ -127,6 +127,32 @@ impl MqExtension { args } + + fn lint_args_from_settings(worktree: &Worktree) -> Vec { + let Ok(lsp_settings) = LspSettings::for_worktree("mq-lsp", worktree) else { + return vec![]; + }; + let Some(init_opts) = lsp_settings.initialization_options else { + return vec![]; + }; + + let enable_lint = init_opts.get("enableLint").and_then(|v| v.as_bool()).unwrap_or(false); + + if !enable_lint { + return vec![]; + } + + let mut args = vec!["--enable-lint".to_string()]; + + if let Some(rules) = init_opts.get("lintDisabledRules").and_then(|v| v.as_array()) { + for rule_id in rules.iter().filter_map(|v| v.as_str()) { + args.push("--disable-lint-rule".to_string()); + args.push(rule_id.to_string()); + } + } + + args + } } fn find_mq_binary(worktree: Option<&Worktree>) -> Result { @@ -297,7 +323,9 @@ impl zed::Extension for MqExtension { let args = if let Some(args) = user_args { args } else { - Self::type_check_args_from_settings(worktree) + let mut args = Self::type_check_args_from_settings(worktree); + args.extend(Self::lint_args_from_settings(worktree)); + args }; Ok(zed::Command { From 4e1a5a7e344975b6000e33628cff1d4931947085 Mon Sep 17 00:00:00 2001 From: Takahiro Sato Date: Mon, 22 Jun 2026 23:51:39 +0900 Subject: [PATCH 3/7] Update Cargo.toml --- crates/mq-lint/Cargo.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/mq-lint/Cargo.toml b/crates/mq-lint/Cargo.toml index 4fd0e47ea..c5ab0a8bb 100644 --- a/crates/mq-lint/Cargo.toml +++ b/crates/mq-lint/Cargo.toml @@ -27,11 +27,9 @@ disabled-strategies = ["quick-install"] [dependencies] clap = {workspace = true, features = ["derive"], optional = true} colored = {workspace = true, optional = true} -miette = {workspace = true} mq-hir = {workspace = true} mq-lang = {workspace = true} rustc-hash = {workspace = true} -thiserror = {workspace = true} [dev-dependencies] rstest = {workspace = true} From 4adf0f3cc46c9e7bfaf7eaf153edd073413233e4 Mon Sep 17 00:00:00 2001 From: harehare Date: Tue, 23 Jun 2026 21:52:15 +0900 Subject: [PATCH 4/7] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor(mq-lint):=20r?= =?UTF-8?q?eplace=20string-based=20rule=20ids=20and=20messages=20with=20en?= =?UTF-8?q?ums?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce RuleId (unit enum) and LintMessage (data-carrying enum) in mq-lint/src/message.rs so every rule has a compile-time-checked id and message/help text, instead of scattered string literals and format! calls across 27 rule files. Diagnostic now derives rule_id()/message()/ help() from a single LintMessage value. Update mq-lsp accordingly: Diagnostic::new(...) calls, rule_id comparisons, and disable_rule/is_rule_enabled now use RuleId instead of &str. --- Cargo.lock | 2 - crates/mq-lint/README.md | 12 +- crates/mq-lint/src/config.rs | 12 +- crates/mq-lint/src/lib.rs | 35 +- crates/mq-lint/src/main.rs | 16 +- crates/mq-lint/src/message.rs | 450 ++++++++++++++++++ .../rules/complexity/complex_interpolation.rs | 11 +- .../src/rules/complexity/deeply_nested.rs | 14 +- .../src/rules/complexity/function_too_long.rs | 17 +- .../rules/complexity/too_many_match_arms.rs | 14 +- .../src/rules/complexity/too_many_params.rs | 18 +- .../correctness/always_true_condition.rs | 17 +- .../rules/correctness/duplicate_match_arm.rs | 14 +- .../src/rules/correctness/infinite_loop.rs | 10 +- .../rules/correctness/missing_else_in_expr.rs | 16 +- .../src/rules/correctness/shadow_variable.rs | 14 +- .../src/rules/correctness/unreachable_code.rs | 15 +- .../src/rules/correctness/unused_function.rs | 12 +- .../src/rules/correctness/unused_import.rs | 14 +- .../src/rules/correctness/unused_variable.rs | 12 +- .../module/ambiguous_qualified_access.rs | 19 +- .../src/rules/module/missing_module_doc.rs | 18 +- .../rules/selector/inefficient_selector.rs | 17 +- .../src/rules/selector/missing_depth_guard.rs | 19 +- .../rules/selector/selector_always_empty.rs | 19 +- .../src/rules/style/boolean_comparison.rs | 21 +- .../src/rules/style/naming_convention.rs | 16 +- .../src/rules/style/prefer_coalesce.rs | 14 +- .../src/rules/style/prefer_let_over_var.rs | 13 +- .../src/rules/style/prefer_pipe_style.rs | 22 +- .../rules/style/prefer_specific_heading.rs | 14 +- .../rules/style/redundant_boolean_literal.rs | 21 +- .../mq-lint/src/rules/style/redundant_try.rs | 14 +- crates/mq-lsp/src/error.rs | 32 +- crates/mq-lsp/src/main.rs | 4 +- crates/mq-lsp/src/server.rs | 10 +- 36 files changed, 704 insertions(+), 294 deletions(-) create mode 100644 crates/mq-lint/src/message.rs diff --git a/Cargo.lock b/Cargo.lock index 9cb54477b..6e43ec0bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2619,12 +2619,10 @@ version = "0.6.3" dependencies = [ "clap", "colored 3.1.1", - "miette", "mq-hir", "mq-lang", "rstest", "rustc-hash", - "thiserror 2.0.18", ] [[package]] diff --git a/crates/mq-lint/README.md b/crates/mq-lint/README.md index 223dda482..0ea806ce6 100644 --- a/crates/mq-lint/README.md +++ b/crates/mq-lint/README.md @@ -23,13 +23,15 @@ let linter = Linter::with_default_rules(); let diagnostics = linter.run(&ctx); for d in diagnostics { - eprintln!("[{}] {} — {}", d.severity, d.rule_id, d.message); - if let Some(help) = &d.help { + eprintln!("[{}] {} — {}", d.severity, d.rule_id(), d.message()); + if let Some(help) = d.help() { eprintln!(" help: {}", help); } } ``` +Each diagnostic's `rule_id()` and `message()` are derived from `d.kind`, a [`LintMessage`](src/message.rs) enum with one variant per rule. Rule identity (`RuleId`) and message text are both enums rather than free-form strings, so adding or renaming a rule is a compile-time-checked change in one place (`src/message.rs`). + ### As a CLI Build with the `cli` feature to get the `mq-lint` binary (also invocable as `mq lint` if placed on your `PATH`, see [External Subcommands](https://mqlang.org/book/start/external_subcommands.html)): @@ -52,9 +54,11 @@ Exits with a non-zero status if any diagnostic (at or above `--min-severity`) wa ### Disabling Rules ```rust +use mq_lint::RuleId; + let mut config = LintConfig::default(); -config.disable_rule("naming_convention"); -config.disable_rule("shadow_variable"); +config.disable_rule(RuleId::NamingConvention); +config.disable_rule(RuleId::ShadowVariable); ``` ### Adjusting Complexity Thresholds diff --git a/crates/mq-lint/src/config.rs b/crates/mq-lint/src/config.rs index 61caa6d01..e39e07302 100644 --- a/crates/mq-lint/src/config.rs +++ b/crates/mq-lint/src/config.rs @@ -2,6 +2,8 @@ use std::collections::HashMap; +use crate::RuleId; + /// Per-rule enable/disable flag. #[derive(Debug, Clone)] pub struct RuleConfig { @@ -45,18 +47,18 @@ impl Default for ComplexityThresholds { #[derive(Debug, Clone, Default)] pub struct LintConfig { /// Per-rule overrides. Rules not listed here use their default enabled state. - pub rules: HashMap, + pub rules: HashMap, pub complexity: ComplexityThresholds, } impl LintConfig { /// Returns `true` if the given rule should run. - pub fn is_rule_enabled(&self, rule_id: &str) -> bool { - self.rules.get(rule_id).map(|r| r.enabled).unwrap_or(true) + pub fn is_rule_enabled(&self, rule_id: RuleId) -> bool { + self.rules.get(&rule_id).map(|r| r.enabled).unwrap_or(true) } /// Disable a specific rule by ID. - pub fn disable_rule(&mut self, rule_id: impl Into) { - self.rules.insert(rule_id.into(), RuleConfig { enabled: false }); + pub fn disable_rule(&mut self, rule_id: RuleId) { + self.rules.insert(rule_id, RuleConfig { enabled: false }); } } diff --git a/crates/mq-lint/src/lib.rs b/crates/mq-lint/src/lib.rs index 8c285ae44..41d254095 100644 --- a/crates/mq-lint/src/lib.rs +++ b/crates/mq-lint/src/lib.rs @@ -19,9 +19,11 @@ //! ``` pub mod config; +pub mod message; pub mod rules; pub use config::LintConfig; +pub use message::{LintMessage, RuleId}; use mq_hir::{Hir, SourceId}; @@ -52,24 +54,19 @@ impl std::fmt::Display for Severity { /// A lint finding produced by a [`LintRule`]. #[derive(Debug, Clone)] pub struct Diagnostic { - /// The rule that produced this diagnostic. - pub rule_id: &'static str, + /// The message data that identifies the rule and renders its text. + pub kind: LintMessage, pub severity: Severity, - pub message: String, /// Source location of the finding, if available. pub range: Option, - /// Optional suggestion for how to fix the issue. - pub help: Option, } impl Diagnostic { - pub fn new(rule_id: &'static str, severity: Severity, message: impl Into) -> Self { + pub fn new(kind: LintMessage, severity: Severity) -> Self { Self { - rule_id, + kind, severity, - message: message.into(), range: None, - help: None, } } @@ -78,9 +75,19 @@ impl Diagnostic { self } - pub fn with_help(mut self, help: impl Into) -> Self { - self.help = Some(help.into()); - self + /// The rule that produced this diagnostic. + pub fn rule_id(&self) -> RuleId { + self.kind.rule_id() + } + + /// Human-readable diagnostic text. + pub fn message(&self) -> String { + self.kind.to_string() + } + + /// Suggested fix text, if the rule has one. + pub fn help(&self) -> Option { + self.kind.help() } } @@ -112,8 +119,8 @@ impl<'a> LintContext<'a> { /// A single lint rule. pub trait LintRule: Send + Sync { - /// Unique identifier for this rule (e.g. `"unused_variable"`). - fn id(&self) -> &'static str; + /// Unique identifier for this rule. + fn id(&self) -> RuleId; /// Default severity when the rule fires. fn severity(&self) -> Severity; diff --git a/crates/mq-lint/src/main.rs b/crates/mq-lint/src/main.rs index 80cec34f5..4050a22ea 100644 --- a/crates/mq-lint/src/main.rs +++ b/crates/mq-lint/src/main.rs @@ -6,7 +6,7 @@ use std::str::FromStr; use clap::Parser; use colored::Colorize; use mq_hir::Hir; -use mq_lint::{Diagnostic, LintConfig, LintContext, Linter, Severity}; +use mq_lint::{Diagnostic, LintConfig, LintContext, Linter, RuleId, Severity}; /// Static analysis linter for mq programs #[derive(Parser)] @@ -17,7 +17,7 @@ struct Cli { /// Disable a rule by ID (repeatable) #[arg(long = "disable", value_name = "RULE_ID")] - disable: Vec, + disable: Vec, /// Only report diagnostics at or above this severity (style, perf, warn, error) #[arg(long, default_value = "style")] @@ -74,7 +74,7 @@ fn run() -> io::Result { let mut config = LintConfig::default(); for rule_id in &cli.disable { - config.disable_rule(rule_id.clone()); + config.disable_rule(*rule_id); } let min_severity = cli.min_severity.0; let linter = Linter::with_default_rules(); @@ -108,7 +108,7 @@ fn list_rules(w: &mut impl Write) -> io::Result<()> { let mut rules: Vec<_> = mq_lint::rules::all_rules(); rules.sort_by_key(|r| r.id()); for rule in &rules { - writeln!(w, "{:<28} {}", rule.id().bright_cyan(), rule.severity())?; + writeln!(w, "{:<28} {}", rule.id().as_str().bright_cyan(), rule.severity())?; } Ok(()) } @@ -184,15 +184,15 @@ fn write_diagnostic(w: &mut impl Write, diagnostic: &Diagnostic, code: &str) -> loc_plain.dimmed(), sep, severity_label(diagnostic.severity), - diagnostic.rule_id.bright_magenta(), - diagnostic.message, + diagnostic.rule_id().as_str().bright_magenta(), + diagnostic.message(), )?; if let Some(range) = &diagnostic.range { write_snippet(w, code, range, prefix_width)?; } - if let Some(help) = &diagnostic.help { + if let Some(help) = diagnostic.help() { writeln!( w, " {} {} {}", @@ -264,7 +264,7 @@ mod tests { "shadow_variable", ]) .unwrap(); - assert_eq!(cli.disable, vec!["naming_convention", "shadow_variable"]); + assert_eq!(cli.disable, vec![RuleId::NamingConvention, RuleId::ShadowVariable]); } #[test] diff --git a/crates/mq-lint/src/message.rs b/crates/mq-lint/src/message.rs new file mode 100644 index 000000000..8b34da584 --- /dev/null +++ b/crates/mq-lint/src/message.rs @@ -0,0 +1,450 @@ +//! Rule identity and diagnostic message types. +//! +//! Every lint rule is identified by a [`RuleId`] variant and, when it fires, +//! produces a [`LintMessage`] carrying whatever data is needed to render the +//! diagnostic text. Keeping both as enums (rather than free-form strings) +//! means the compiler enforces that every rule has exactly one ID and that +//! every message variant maps to a real rule. + +use std::fmt; +use std::str::FromStr; + +/// Unique identifier for a lint rule. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum RuleId { + UnusedVariable, + UnusedFunction, + UnusedImport, + UnreachableCode, + InfiniteLoop, + DuplicateMatchArm, + ShadowVariable, + MissingElseInExpr, + AlwaysTrueCondition, + FunctionTooLong, + TooManyParams, + DeeplyNested, + TooManyMatchArms, + ComplexInterpolation, + InefficientSelector, + MissingDepthGuard, + SelectorAlwaysEmpty, + MissingModuleDoc, + AmbiguousQualifiedAccess, + PreferLetOverVar, + PreferPipeStyle, + PreferCoalesce, + PreferSpecificHeading, + RedundantTry, + NamingConvention, + BooleanComparison, + RedundantBooleanLiteral, +} + +impl RuleId { + /// All known rule IDs. + pub const ALL: &'static [RuleId] = &[ + RuleId::UnusedVariable, + RuleId::UnusedFunction, + RuleId::UnusedImport, + RuleId::UnreachableCode, + RuleId::InfiniteLoop, + RuleId::DuplicateMatchArm, + RuleId::ShadowVariable, + RuleId::MissingElseInExpr, + RuleId::AlwaysTrueCondition, + RuleId::FunctionTooLong, + RuleId::TooManyParams, + RuleId::DeeplyNested, + RuleId::TooManyMatchArms, + RuleId::ComplexInterpolation, + RuleId::InefficientSelector, + RuleId::MissingDepthGuard, + RuleId::SelectorAlwaysEmpty, + RuleId::MissingModuleDoc, + RuleId::AmbiguousQualifiedAccess, + RuleId::PreferLetOverVar, + RuleId::PreferPipeStyle, + RuleId::PreferCoalesce, + RuleId::PreferSpecificHeading, + RuleId::RedundantTry, + RuleId::NamingConvention, + RuleId::BooleanComparison, + RuleId::RedundantBooleanLiteral, + ]; + + /// The rule's `snake_case` identifier, as used in config and CLI flags. + pub fn as_str(&self) -> &'static str { + match self { + RuleId::UnusedVariable => "unused_variable", + RuleId::UnusedFunction => "unused_function", + RuleId::UnusedImport => "unused_import", + RuleId::UnreachableCode => "unreachable_code", + RuleId::InfiniteLoop => "infinite_loop", + RuleId::DuplicateMatchArm => "duplicate_match_arm", + RuleId::ShadowVariable => "shadow_variable", + RuleId::MissingElseInExpr => "missing_else_in_expr", + RuleId::AlwaysTrueCondition => "always_true_condition", + RuleId::FunctionTooLong => "function_too_long", + RuleId::TooManyParams => "too_many_params", + RuleId::DeeplyNested => "deeply_nested", + RuleId::TooManyMatchArms => "too_many_match_arms", + RuleId::ComplexInterpolation => "complex_interpolation", + RuleId::InefficientSelector => "inefficient_selector", + RuleId::MissingDepthGuard => "missing_depth_guard", + RuleId::SelectorAlwaysEmpty => "selector_always_empty", + RuleId::MissingModuleDoc => "missing_module_doc", + RuleId::AmbiguousQualifiedAccess => "ambiguous_qualified_access", + RuleId::PreferLetOverVar => "prefer_let_over_var", + RuleId::PreferPipeStyle => "prefer_pipe_style", + RuleId::PreferCoalesce => "prefer_coalesce", + RuleId::PreferSpecificHeading => "prefer_specific_heading", + RuleId::RedundantTry => "redundant_try", + RuleId::NamingConvention => "naming_convention", + RuleId::BooleanComparison => "boolean_comparison", + RuleId::RedundantBooleanLiteral => "redundant_boolean_literal", + } + } +} + +impl fmt::Display for RuleId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.as_str()) + } +} + +impl FromStr for RuleId { + type Err = String; + + fn from_str(s: &str) -> Result { + RuleId::ALL + .iter() + .copied() + .find(|id| id.as_str() == s) + .ok_or_else(|| format!("unknown rule id `{s}`")) + } +} + +/// A diagnostic finding, carrying whatever data its rule needs to render a +/// message and help text. Each variant corresponds to exactly one [`RuleId`]. +#[derive(Debug, Clone, PartialEq)] +pub enum LintMessage { + UnusedVariable { + name: String, + }, + UnusedFunction { + name: String, + }, + UnusedImport { + name: String, + }, + UnreachableCode { + keyword: String, + }, + InfiniteLoop, + DuplicateMatchArm { + pattern: String, + }, + ShadowVariable { + name: String, + }, + MissingElseInExpr, + AlwaysTrueCondition { + value: String, + }, + FunctionTooLong { + name: String, + line_count: usize, + max_lines: usize, + }, + TooManyParams { + name: String, + count: usize, + max: usize, + }, + DeeplyNested { + depth: usize, + max_depth: usize, + }, + TooManyMatchArms { + arm_count: usize, + max_arms: usize, + }, + ComplexInterpolation { + expr_count: usize, + max_exprs: usize, + }, + InefficientSelector, + MissingDepthGuard, + SelectorAlwaysEmpty { + first: String, + second: String, + }, + MissingModuleDoc { + name: String, + }, + AmbiguousQualifiedAccess { + fn_name: String, + this_module: String, + other_module: String, + }, + PreferLetOverVar { + name: String, + }, + PreferPipeStyle { + outer_name: String, + inner_name: String, + }, + PreferCoalesce, + PreferSpecificHeading, + RedundantTry, + NamingConvention { + name: String, + suggested: String, + }, + BooleanComparison { + op: String, + bool_val: String, + }, + RedundantBooleanLiteral { + then_val: String, + }, +} + +impl LintMessage { + /// The rule that produces this message. + pub fn rule_id(&self) -> RuleId { + match self { + LintMessage::UnusedVariable { .. } => RuleId::UnusedVariable, + LintMessage::UnusedFunction { .. } => RuleId::UnusedFunction, + LintMessage::UnusedImport { .. } => RuleId::UnusedImport, + LintMessage::UnreachableCode { .. } => RuleId::UnreachableCode, + LintMessage::InfiniteLoop => RuleId::InfiniteLoop, + LintMessage::DuplicateMatchArm { .. } => RuleId::DuplicateMatchArm, + LintMessage::ShadowVariable { .. } => RuleId::ShadowVariable, + LintMessage::MissingElseInExpr => RuleId::MissingElseInExpr, + LintMessage::AlwaysTrueCondition { .. } => RuleId::AlwaysTrueCondition, + LintMessage::FunctionTooLong { .. } => RuleId::FunctionTooLong, + LintMessage::TooManyParams { .. } => RuleId::TooManyParams, + LintMessage::DeeplyNested { .. } => RuleId::DeeplyNested, + LintMessage::TooManyMatchArms { .. } => RuleId::TooManyMatchArms, + LintMessage::ComplexInterpolation { .. } => RuleId::ComplexInterpolation, + LintMessage::InefficientSelector => RuleId::InefficientSelector, + LintMessage::MissingDepthGuard => RuleId::MissingDepthGuard, + LintMessage::SelectorAlwaysEmpty { .. } => RuleId::SelectorAlwaysEmpty, + LintMessage::MissingModuleDoc { .. } => RuleId::MissingModuleDoc, + LintMessage::AmbiguousQualifiedAccess { .. } => RuleId::AmbiguousQualifiedAccess, + LintMessage::PreferLetOverVar { .. } => RuleId::PreferLetOverVar, + LintMessage::PreferPipeStyle { .. } => RuleId::PreferPipeStyle, + LintMessage::PreferCoalesce => RuleId::PreferCoalesce, + LintMessage::PreferSpecificHeading => RuleId::PreferSpecificHeading, + LintMessage::RedundantTry => RuleId::RedundantTry, + LintMessage::NamingConvention { .. } => RuleId::NamingConvention, + LintMessage::BooleanComparison { .. } => RuleId::BooleanComparison, + LintMessage::RedundantBooleanLiteral { .. } => RuleId::RedundantBooleanLiteral, + } + } + + /// Suggested fix text, if the rule has one. + pub fn help(&self) -> Option { + match self { + LintMessage::UnusedVariable { name } | LintMessage::UnusedFunction { name } => { + Some(format!("if this is intentional, prefix with `_`: `_{name}`")) + } + LintMessage::UnusedImport { name } => { + Some(format!("remove `import \"{name}\"` or use it with `{name}::function`")) + } + LintMessage::UnreachableCode { .. } => { + Some("remove or move this code before the `break`/`continue`".to_string()) + } + LintMessage::InfiniteLoop => Some("add a `break` expression to exit the loop".to_string()), + LintMessage::DuplicateMatchArm { .. } => { + Some("remove or merge this arm with the earlier identical pattern".to_string()) + } + LintMessage::ShadowVariable { .. } => Some("consider renaming to avoid confusion".to_string()), + LintMessage::MissingElseInExpr => { + Some("add `else: ` to provide a value for the false branch".to_string()) + } + LintMessage::AlwaysTrueCondition { .. } => { + Some("replace the `if` with the branch that will always execute".to_string()) + } + LintMessage::FunctionTooLong { .. } => { + Some("consider splitting into smaller, focused helper functions".to_string()) + } + LintMessage::TooManyParams { .. } => { + Some("consider grouping related parameters or using default arguments".to_string()) + } + LintMessage::DeeplyNested { .. } => { + Some("reduce nesting by extracting code into helper functions".to_string()) + } + LintMessage::TooManyMatchArms { .. } => { + Some("consider refactoring into helper functions or a lookup table".to_string()) + } + LintMessage::ComplexInterpolation { .. } => { + Some("consider extracting parts into intermediate `let` bindings for readability".to_string()) + } + LintMessage::InefficientSelector => { + Some("use the specific selector directly (e.g. replace `.. | .h1` with `.h1`)".to_string()) + } + LintMessage::MissingDepthGuard => Some( + "consider adding a depth limit, e.g. `.. | select(.depth <= 3)`, \ + to avoid traversing the entire document" + .to_string(), + ), + LintMessage::SelectorAlwaysEmpty { .. } => { + Some("remove one of the selectors, or replace the pipe with a different query".to_string()) + } + LintMessage::MissingModuleDoc { name } => Some(format!("add a `#` doc comment above `module {name}:`")), + LintMessage::AmbiguousQualifiedAccess { + fn_name, this_module, .. + } => Some(format!( + "use a fully qualified call (e.g. `{this_module}::{fn_name}()`) to avoid ambiguity" + )), + LintMessage::PreferLetOverVar { name } => Some(format!("change `var {name}` to `let {name}`")), + LintMessage::PreferPipeStyle { outer_name, inner_name } => { + Some(format!("rewrite as `... | {inner_name}() | {outer_name}()`")) + } + LintMessage::PreferCoalesce => Some("rewrite as ` ?? `".to_string()), + LintMessage::PreferSpecificHeading => { + Some("using `.h1`–`.h6` makes the intended heading level explicit".to_string()) + } + LintMessage::RedundantTry => Some("rewrite as `?`".to_string()), + LintMessage::NamingConvention { suggested, .. } => Some(format!("rename to `{suggested}`")), + LintMessage::BooleanComparison { op, bool_val } => Some( + match (op.as_str(), bool_val.as_str()) { + ("==", "true") => "use the value directly instead of `== true`", + ("==", "false") => "use `!` prefix instead of `== false`", + ("!=", "true") => "use `!` prefix instead of `!= true`", + ("!=", "false") => "use the value directly instead of `!= false`", + _ => "simplify this boolean comparison", + } + .to_string(), + ), + LintMessage::RedundantBooleanLiteral { then_val } => Some( + if then_val == "true" { + "replace `if (cond): true else: false` with just `cond`" + } else { + "replace `if (cond): false else: true` with `not(cond)` or `!(cond)`" + } + .to_string(), + ), + } + } +} + +impl fmt::Display for LintMessage { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + LintMessage::UnusedVariable { name } => write!(f, "unused variable `{name}`"), + LintMessage::UnusedFunction { name } => write!(f, "unused function `{name}`"), + LintMessage::UnusedImport { name } => write!(f, "imported module `{name}` is never used"), + LintMessage::UnreachableCode { keyword } => write!(f, "unreachable code after `{keyword}`"), + LintMessage::InfiniteLoop => write!(f, "loop without `break` may run forever"), + LintMessage::DuplicateMatchArm { pattern } => write!(f, "duplicate match arm pattern `{pattern}`"), + LintMessage::ShadowVariable { name } => { + write!(f, "variable `{name}` shadows a variable in an outer scope") + } + LintMessage::MissingElseInExpr => { + write!( + f, + "`if` expression is missing an `else` branch (evaluates to `none` on false)" + ) + } + LintMessage::AlwaysTrueCondition { value } => { + write!(f, "condition is always `{value}` — this branch is never/always taken") + } + LintMessage::FunctionTooLong { + name, + line_count, + max_lines, + } => { + write!(f, "function `{name}` is {line_count} lines long (limit: {max_lines})") + } + LintMessage::TooManyParams { name, count, max } => { + write!(f, "function `{name}` has {count} parameters (limit: {max})") + } + LintMessage::DeeplyNested { depth, max_depth } => { + write!(f, "nesting depth {depth} exceeds the limit of {max_depth}") + } + LintMessage::TooManyMatchArms { arm_count, max_arms } => { + write!(f, "match expression has {arm_count} arms (limit: {max_arms})") + } + LintMessage::ComplexInterpolation { expr_count, max_exprs } => { + write!( + f, + "interpolated string has {expr_count} interpolated expressions (limit: {max_exprs})" + ) + } + LintMessage::InefficientSelector => write!(f, "inefficient selector: `..` followed by a specific selector"), + LintMessage::MissingDepthGuard => write!(f, "`..` (recursive selector) used without a depth guard"), + LintMessage::SelectorAlwaysEmpty { first, second } => { + write!(f, "`{first} | {second}` can never match: a node can't be both") + } + LintMessage::MissingModuleDoc { name } => write!(f, "module `{name}` has no documentation comment"), + LintMessage::AmbiguousQualifiedAccess { + fn_name, other_module, .. + } => { + write!(f, "function `{fn_name}` is also defined in module `{other_module}`") + } + LintMessage::PreferLetOverVar { name } => { + write!(f, "`{name}` is never reassigned; prefer `let` over `var`") + } + LintMessage::PreferPipeStyle { outer_name, inner_name } => { + write!( + f, + "nested call `{outer_name}({inner_name}(...))` reads better as a pipe" + ) + } + LintMessage::PreferCoalesce => { + write!( + f, + "`if`/`else` null-check can be simplified using the `??` coalesce operator" + ) + } + LintMessage::PreferSpecificHeading => { + write!(f, "prefer a specific heading level selector (`.h1`–`.h6`) over `.h`") + } + LintMessage::RedundantTry => { + write!( + f, + "`try: ... catch: none` is equivalent to the `?` error-suppression operator" + ) + } + LintMessage::NamingConvention { name, .. } => write!(f, "`{name}` should be written in snake_case"), + LintMessage::BooleanComparison { bool_val, .. } => { + write!(f, "unnecessary comparison with boolean literal `{bool_val}`") + } + LintMessage::RedundantBooleanLiteral { .. } => { + write!( + f, + "redundant boolean literal in `if`/`else` — condition already is the result" + ) + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn rule_id_round_trips_through_str() { + for id in RuleId::ALL { + assert_eq!(id.as_str().parse::().unwrap(), *id); + } + } + + #[test] + fn rule_id_from_str_rejects_unknown() { + assert!("not_a_real_rule".parse::().is_err()); + } + + #[test] + fn message_rule_id_matches_intent() { + let msg = LintMessage::UnusedVariable { name: "x".to_string() }; + assert_eq!(msg.rule_id(), RuleId::UnusedVariable); + assert_eq!(msg.to_string(), "unused variable `x`"); + assert!(msg.help().unwrap().contains("_x")); + } +} diff --git a/crates/mq-lint/src/rules/complexity/complex_interpolation.rs b/crates/mq-lint/src/rules/complexity/complex_interpolation.rs index 5b1701a1b..bc9ac4e4d 100644 --- a/crates/mq-lint/src/rules/complexity/complex_interpolation.rs +++ b/crates/mq-lint/src/rules/complexity/complex_interpolation.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct ComplexInterpolation; impl LintRule for ComplexInterpolation { - fn id(&self) -> &'static str { - "complex_interpolation" + fn id(&self) -> RuleId { + RuleId::ComplexInterpolation } fn severity(&self) -> Severity { @@ -30,14 +30,13 @@ impl LintRule for ComplexInterpolation { } let mut d = Diagnostic::new( - self.id(), + LintMessage::ComplexInterpolation { expr_count, max_exprs }, self.severity(), - format!("interpolated string has {expr_count} interpolated expressions (limit: {max_exprs})"), ); if let Some(range) = sym.source.text_range { d = d.with_range(range); } - Some(d.with_help("consider extracting parts into intermediate `let` bindings for readability")) + Some(d) }) .collect() } diff --git a/crates/mq-lint/src/rules/complexity/deeply_nested.rs b/crates/mq-lint/src/rules/complexity/deeply_nested.rs index 8595e9cd5..fe052e18a 100644 --- a/crates/mq-lint/src/rules/complexity/deeply_nested.rs +++ b/crates/mq-lint/src/rules/complexity/deeply_nested.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::{ScopeId, ScopeKind}; pub struct DeeplyNested; impl LintRule for DeeplyNested { - fn id(&self) -> &'static str { - "deeply_nested" + fn id(&self) -> RuleId { + RuleId::DeeplyNested } fn severity(&self) -> Severity { @@ -41,15 +41,11 @@ impl LintRule for DeeplyNested { ScopeKind::Module(_) => None, }; - let mut d = Diagnostic::new( - self.id(), - self.severity(), - format!("nesting depth {depth} exceeds the limit of {max_depth}"), - ); + let mut d = Diagnostic::new(LintMessage::DeeplyNested { depth, max_depth }, self.severity()); if let Some(r) = range { d = d.with_range(r); } - diagnostics.push(d.with_help("reduce nesting by extracting code into helper functions")); + diagnostics.push(d); } diagnostics diff --git a/crates/mq-lint/src/rules/complexity/function_too_long.rs b/crates/mq-lint/src/rules/complexity/function_too_long.rs index 32b0886c4..5485e8bc5 100644 --- a/crates/mq-lint/src/rules/complexity/function_too_long.rs +++ b/crates/mq-lint/src/rules/complexity/function_too_long.rs @@ -1,10 +1,10 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; pub struct FunctionTooLong; impl LintRule for FunctionTooLong { - fn id(&self) -> &'static str { - "function_too_long" + fn id(&self) -> RuleId { + RuleId::FunctionTooLong } fn severity(&self) -> Severity { @@ -25,14 +25,17 @@ impl LintRule for FunctionTooLong { if line_count <= max_lines { return None; } - let name = sym.value.as_deref().unwrap_or(""); + let name = sym.value.as_deref().unwrap_or("").to_string(); let mut d = Diagnostic::new( - self.id(), + LintMessage::FunctionTooLong { + name, + line_count, + max_lines, + }, self.severity(), - format!("function `{name}` is {line_count} lines long (limit: {max_lines})"), ); d = d.with_range(range); - Some(d.with_help("consider splitting into smaller, focused helper functions")) + Some(d) }) .collect() } diff --git a/crates/mq-lint/src/rules/complexity/too_many_match_arms.rs b/crates/mq-lint/src/rules/complexity/too_many_match_arms.rs index 054bcbb3f..4689dd9bb 100644 --- a/crates/mq-lint/src/rules/complexity/too_many_match_arms.rs +++ b/crates/mq-lint/src/rules/complexity/too_many_match_arms.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct TooManyMatchArms; impl LintRule for TooManyMatchArms { - fn id(&self) -> &'static str { - "too_many_match_arms" + fn id(&self) -> RuleId { + RuleId::TooManyMatchArms } fn severity(&self) -> Severity { @@ -35,15 +35,11 @@ impl LintRule for TooManyMatchArms { return None; } - let mut d = Diagnostic::new( - self.id(), - self.severity(), - format!("match expression has {arm_count} arms (limit: {max_arms})"), - ); + let mut d = Diagnostic::new(LintMessage::TooManyMatchArms { arm_count, max_arms }, self.severity()); if let Some(range) = match_sym.source.text_range { d = d.with_range(range); } - Some(d.with_help("consider refactoring into helper functions or a lookup table")) + Some(d) }) .collect() } diff --git a/crates/mq-lint/src/rules/complexity/too_many_params.rs b/crates/mq-lint/src/rules/complexity/too_many_params.rs index b4ab2a1bd..65d577417 100644 --- a/crates/mq-lint/src/rules/complexity/too_many_params.rs +++ b/crates/mq-lint/src/rules/complexity/too_many_params.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct TooManyParams; impl LintRule for TooManyParams { - fn id(&self) -> &'static str { - "too_many_params" + fn id(&self) -> RuleId { + RuleId::TooManyParams } fn severity(&self) -> Severity { @@ -25,17 +25,13 @@ impl LintRule for TooManyParams { if params.len() <= max { return None; } - let name = sym.value.as_deref().unwrap_or(""); + let name = sym.value.as_deref().unwrap_or("").to_string(); let count = params.len(); - let mut d = Diagnostic::new( - self.id(), - self.severity(), - format!("function `{name}` has {count} parameters (limit: {max})"), - ); + let mut d = Diagnostic::new(LintMessage::TooManyParams { name, count, max }, self.severity()); if let Some(range) = sym.source.text_range { d = d.with_range(range); } - Some(d.with_help("consider grouping related parameters or using default arguments")) + Some(d) }) .collect() } @@ -61,7 +57,7 @@ mod tests { fn detects_too_many_params() { let diags = check_with_max("def f(a, b, c): a", 2); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("3 parameters")); + assert!(diags[0].message().contains("3 parameters")); } #[test] diff --git a/crates/mq-lint/src/rules/correctness/always_true_condition.rs b/crates/mq-lint/src/rules/correctness/always_true_condition.rs index c8ad39e3d..f534e0be6 100644 --- a/crates/mq-lint/src/rules/correctness/always_true_condition.rs +++ b/crates/mq-lint/src/rules/correctness/always_true_condition.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct AlwaysTrueCondition; impl LintRule for AlwaysTrueCondition { - fn id(&self) -> &'static str { - "always_true_condition" + fn id(&self) -> RuleId { + RuleId::AlwaysTrueCondition } fn severity(&self) -> Severity { @@ -45,14 +45,15 @@ impl LintRule for AlwaysTrueCondition { } let mut d = Diagnostic::new( - self.id(), + LintMessage::AlwaysTrueCondition { + value: value.to_string(), + }, self.severity(), - format!("condition is always `{value}` — this branch is never/always taken"), ); if let Some(range) = if_sym.source.text_range { d = d.with_range(range); } - diagnostics.push(d.with_help("replace the `if` with the branch that will always execute")); + diagnostics.push(d); } diagnostics @@ -78,14 +79,14 @@ mod tests { fn detects_literal_true_condition() { let diags = check("if (true): 1 else: 2;"); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("always `true`")); + assert!(diags[0].message().contains("always `true`")); } #[test] fn detects_literal_false_condition() { let diags = check("if (false): 1 else: 2;"); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("always `false`")); + assert!(diags[0].message().contains("always `false`")); } #[test] diff --git a/crates/mq-lint/src/rules/correctness/duplicate_match_arm.rs b/crates/mq-lint/src/rules/correctness/duplicate_match_arm.rs index e2b2225e6..f35c7603a 100644 --- a/crates/mq-lint/src/rules/correctness/duplicate_match_arm.rs +++ b/crates/mq-lint/src/rules/correctness/duplicate_match_arm.rs @@ -1,13 +1,13 @@ use rustc_hash::FxHashSet; -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct DuplicateMatchArm; impl LintRule for DuplicateMatchArm { - fn id(&self) -> &'static str { - "duplicate_match_arm" + fn id(&self) -> RuleId { + RuleId::DuplicateMatchArm } fn severity(&self) -> Severity { @@ -52,15 +52,11 @@ impl LintRule for DuplicateMatchArm { } if !seen_patterns.insert(repr.clone()) { - let mut d = Diagnostic::new( - self.id(), - self.severity(), - format!("duplicate match arm pattern `{repr}`"), - ); + let mut d = Diagnostic::new(LintMessage::DuplicateMatchArm { pattern: repr }, self.severity()); if let Some(range) = arm_sym.source.text_range { d = d.with_range(range); } - diagnostics.push(d.with_help("remove or merge this arm with the earlier identical pattern")); + diagnostics.push(d); } } } diff --git a/crates/mq-lint/src/rules/correctness/infinite_loop.rs b/crates/mq-lint/src/rules/correctness/infinite_loop.rs index aa4bde097..95c3799a8 100644 --- a/crates/mq-lint/src/rules/correctness/infinite_loop.rs +++ b/crates/mq-lint/src/rules/correctness/infinite_loop.rs @@ -1,12 +1,12 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolId; use mq_hir::SymbolKind; pub struct InfiniteLoop; impl LintRule for InfiniteLoop { - fn id(&self) -> &'static str { - "infinite_loop" + fn id(&self) -> RuleId { + RuleId::InfiniteLoop } fn severity(&self) -> Severity { @@ -25,11 +25,11 @@ impl LintRule for InfiniteLoop { for (loop_id, loop_sym) in loops { if !has_break_descendant(ctx, loop_id) { - let mut d = Diagnostic::new(self.id(), self.severity(), "loop without `break` may run forever"); + let mut d = Diagnostic::new(LintMessage::InfiniteLoop, self.severity()); if let Some(range) = loop_sym.source.text_range { d = d.with_range(range); } - diagnostics.push(d.with_help("add a `break` expression to exit the loop")); + diagnostics.push(d); } } diff --git a/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs b/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs index 8e222e43f..1ed832afa 100644 --- a/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs +++ b/crates/mq-lint/src/rules/correctness/missing_else_in_expr.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct MissingElseInExpr; impl LintRule for MissingElseInExpr { - fn id(&self) -> &'static str { - "missing_else_in_expr" + fn id(&self) -> RuleId { + RuleId::MissingElseInExpr } fn severity(&self) -> Severity { @@ -26,15 +26,11 @@ impl LintRule for MissingElseInExpr { .symbols_for_source(ctx.source_id) .filter(|(id, s)| matches!(s.kind, SymbolKind::If) && !if_ids_with_else.contains(id)) .map(|(_, sym)| { - let mut d = Diagnostic::new( - self.id(), - self.severity(), - "`if` expression is missing an `else` branch (evaluates to `none` on false)", - ); + let mut d = Diagnostic::new(LintMessage::MissingElseInExpr, self.severity()); if let Some(range) = sym.source.text_range { d = d.with_range(range); } - d.with_help("add `else: ` to provide a value for the false branch") + d }) .collect() } @@ -59,7 +55,7 @@ mod tests { fn detects_if_without_else() { let diags = check("if (true): 1;"); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("missing an `else` branch")); + assert!(diags[0].message().contains("missing an `else` branch")); } #[test] diff --git a/crates/mq-lint/src/rules/correctness/shadow_variable.rs b/crates/mq-lint/src/rules/correctness/shadow_variable.rs index e0d2f2e49..148348dda 100644 --- a/crates/mq-lint/src/rules/correctness/shadow_variable.rs +++ b/crates/mq-lint/src/rules/correctness/shadow_variable.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::{ScopeId, SymbolKind}; pub struct ShadowVariable; impl LintRule for ShadowVariable { - fn id(&self) -> &'static str { - "shadow_variable" + fn id(&self) -> RuleId { + RuleId::ShadowVariable } fn severity(&self) -> Severity { @@ -26,15 +26,11 @@ impl LintRule for ShadowVariable { }; if shadows_outer_variable(ctx, sym.scope, name) { - let mut d = Diagnostic::new( - self.id(), - self.severity(), - format!("variable `{name}` shadows a variable in an outer scope"), - ); + let mut d = Diagnostic::new(LintMessage::ShadowVariable { name: name.to_string() }, self.severity()); if let Some(range) = sym.source.text_range { d = d.with_range(range); } - diagnostics.push(d.with_help("consider renaming to avoid confusion")); + diagnostics.push(d); } } diff --git a/crates/mq-lint/src/rules/correctness/unreachable_code.rs b/crates/mq-lint/src/rules/correctness/unreachable_code.rs index 85f11ef90..fe3d3820f 100644 --- a/crates/mq-lint/src/rules/correctness/unreachable_code.rs +++ b/crates/mq-lint/src/rules/correctness/unreachable_code.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct UnreachableCode; impl LintRule for UnreachableCode { - fn id(&self) -> &'static str { - "unreachable_code" + fn id(&self) -> RuleId { + RuleId::UnreachableCode } fn severity(&self) -> Severity { @@ -44,11 +44,16 @@ impl LintRule for UnreachableCode { .collect(); for (_, dead_sym) in unreachable { - let mut d = Diagnostic::new(self.id(), self.severity(), format!("unreachable code after `{kw}`")); + let mut d = Diagnostic::new( + LintMessage::UnreachableCode { + keyword: kw.to_string(), + }, + self.severity(), + ); if let Some(range) = dead_sym.source.text_range { d = d.with_range(range); } - diagnostics.push(d.with_help("remove or move this code before the `break`/`continue`")); + diagnostics.push(d); } } diff --git a/crates/mq-lint/src/rules/correctness/unused_function.rs b/crates/mq-lint/src/rules/correctness/unused_function.rs index b8ef1a2cb..5a38ce028 100644 --- a/crates/mq-lint/src/rules/correctness/unused_function.rs +++ b/crates/mq-lint/src/rules/correctness/unused_function.rs @@ -1,13 +1,13 @@ use rustc_hash::FxHashSet; -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct UnusedFunction; impl LintRule for UnusedFunction { - fn id(&self) -> &'static str { - "unused_function" + fn id(&self) -> RuleId { + RuleId::UnusedFunction } fn severity(&self) -> Severity { @@ -31,11 +31,11 @@ impl LintRule for UnusedFunction { if name.starts_with('_') || used_names.contains(name) { return None; } - let mut d = Diagnostic::new(self.id(), self.severity(), format!("unused function `{name}`")); + let mut d = Diagnostic::new(LintMessage::UnusedFunction { name: name.to_string() }, self.severity()); if let Some(range) = sym.source.text_range { d = d.with_range(range); } - Some(d.with_help(format!("if this is intentional, prefix with `_`: `_{name}`"))) + Some(d) }) .collect() } @@ -60,7 +60,7 @@ mod tests { fn detects_unused_function() { let diags = check("def foo(): .h1;"); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("unused function `foo`")); + assert!(diags[0].message().contains("unused function `foo`")); } #[test] diff --git a/crates/mq-lint/src/rules/correctness/unused_import.rs b/crates/mq-lint/src/rules/correctness/unused_import.rs index 43faad00b..81e3506c9 100644 --- a/crates/mq-lint/src/rules/correctness/unused_import.rs +++ b/crates/mq-lint/src/rules/correctness/unused_import.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct UnusedImport; impl LintRule for UnusedImport { - fn id(&self) -> &'static str { - "unused_import" + fn id(&self) -> RuleId { + RuleId::UnusedImport } fn severity(&self) -> Severity { @@ -30,15 +30,11 @@ impl LintRule for UnusedImport { if used_modules.contains(name) { return None; } - let mut d = Diagnostic::new( - self.id(), - self.severity(), - format!("imported module `{name}` is never used"), - ); + let mut d = Diagnostic::new(LintMessage::UnusedImport { name: name.to_string() }, self.severity()); if let Some(range) = sym.source.text_range { d = d.with_range(range); } - Some(d.with_help(format!("remove `import \"{name}\"` or use it with `{name}::function`"))) + Some(d) }) .collect() } diff --git a/crates/mq-lint/src/rules/correctness/unused_variable.rs b/crates/mq-lint/src/rules/correctness/unused_variable.rs index 100c9a1f4..bc4d34322 100644 --- a/crates/mq-lint/src/rules/correctness/unused_variable.rs +++ b/crates/mq-lint/src/rules/correctness/unused_variable.rs @@ -1,13 +1,13 @@ use rustc_hash::FxHashSet; -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct UnusedVariable; impl LintRule for UnusedVariable { - fn id(&self) -> &'static str { - "unused_variable" + fn id(&self) -> RuleId { + RuleId::UnusedVariable } fn severity(&self) -> Severity { @@ -33,11 +33,11 @@ impl LintRule for UnusedVariable { if used_names.contains(name) { return None; } - let mut d = Diagnostic::new(self.id(), self.severity(), format!("unused variable `{name}`")); + let mut d = Diagnostic::new(LintMessage::UnusedVariable { name: name.to_string() }, self.severity()); if let Some(range) = sym.source.text_range { d = d.with_range(range); } - Some(d.with_help(format!("if this is intentional, prefix with `_`: `_{name}`"))) + Some(d) }) .collect() } @@ -62,7 +62,7 @@ mod tests { fn detects_unused_variable() { let diags = check("let x = .h1"); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("unused variable `x`")); + assert!(diags[0].message().contains("unused variable `x`")); } #[test] diff --git a/crates/mq-lint/src/rules/module/ambiguous_qualified_access.rs b/crates/mq-lint/src/rules/module/ambiguous_qualified_access.rs index 4ab13a04f..476d7da97 100644 --- a/crates/mq-lint/src/rules/module/ambiguous_qualified_access.rs +++ b/crates/mq-lint/src/rules/module/ambiguous_qualified_access.rs @@ -1,13 +1,13 @@ use rustc_hash::FxHashMap; -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct AmbiguousQualifiedAccess; impl LintRule for AmbiguousQualifiedAccess { - fn id(&self) -> &'static str { - "ambiguous_qualified_access" + fn id(&self) -> RuleId { + RuleId::AmbiguousQualifiedAccess } fn severity(&self) -> Severity { @@ -52,16 +52,17 @@ impl LintRule for AmbiguousQualifiedAccess { let other_module = defining_modules.get(fn_name)?.iter().find(|&&m| m != this_module)?; let mut d = Diagnostic::new( - self.id(), + LintMessage::AmbiguousQualifiedAccess { + fn_name: fn_name.to_string(), + this_module: this_module.to_string(), + other_module: other_module.to_string(), + }, self.severity(), - format!("function `{fn_name}` is also defined in module `{other_module}`"), ); if let Some(range) = sym.source.text_range { d = d.with_range(range); } - Some(d.with_help(format!( - "use a fully qualified call (e.g. `{this_module}::{fn_name}()`) to avoid ambiguity" - ))) + Some(d) }) .collect() } @@ -86,7 +87,7 @@ mod tests { fn detects_same_function_name_in_two_modules() { let diags = check("module a: def foo(): 1; end | module b: def foo(): 2; end"); assert_eq!(diags.len(), 2); - assert!(diags.iter().all(|d| d.message.contains("foo"))); + assert!(diags.iter().all(|d| d.message().contains("foo"))); } #[test] diff --git a/crates/mq-lint/src/rules/module/missing_module_doc.rs b/crates/mq-lint/src/rules/module/missing_module_doc.rs index d24ad483f..35457462c 100644 --- a/crates/mq-lint/src/rules/module/missing_module_doc.rs +++ b/crates/mq-lint/src/rules/module/missing_module_doc.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct MissingModuleDoc; impl LintRule for MissingModuleDoc { - fn id(&self) -> &'static str { - "missing_module_doc" + fn id(&self) -> RuleId { + RuleId::MissingModuleDoc } fn severity(&self) -> Severity { @@ -17,16 +17,12 @@ impl LintRule for MissingModuleDoc { .filter(|(_, sym)| matches!(sym.kind, SymbolKind::Module(_))) .filter(|(_, sym)| sym.doc.is_empty()) .map(|(_, sym)| { - let name = sym.value.as_deref().unwrap_or(""); - let mut d = Diagnostic::new( - self.id(), - self.severity(), - format!("module `{name}` has no documentation comment"), - ); + let name = sym.value.as_deref().unwrap_or("").to_string(); + let mut d = Diagnostic::new(LintMessage::MissingModuleDoc { name }, self.severity()); if let Some(range) = sym.source.text_range { d = d.with_range(range); } - d.with_help(format!("add a `#` doc comment above `module {name}:`")) + d }) .collect() } @@ -51,7 +47,7 @@ mod tests { fn detects_module_without_doc() { let diags = check("module a: def b(): 1; end"); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("module `a`")); + assert!(diags[0].message().contains("module `a`")); } #[test] diff --git a/crates/mq-lint/src/rules/selector/inefficient_selector.rs b/crates/mq-lint/src/rules/selector/inefficient_selector.rs index 86d7b6a13..8f13c7316 100644 --- a/crates/mq-lint/src/rules/selector/inefficient_selector.rs +++ b/crates/mq-lint/src/rules/selector/inefficient_selector.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct InefficientSelector; impl LintRule for InefficientSelector { - fn id(&self) -> &'static str { - "inefficient_selector" + fn id(&self) -> RuleId { + RuleId::InefficientSelector } fn severity(&self) -> Severity { @@ -50,14 +50,9 @@ impl LintRule for InefficientSelector { }); if has_redundant_follow { - let mut d = Diagnostic::new( - self.id(), - self.severity(), - "inefficient selector: `..` followed by a specific selector", - ); + let mut d = Diagnostic::new(LintMessage::InefficientSelector, self.severity()); d = d.with_range(rec_range); - diagnostics - .push(d.with_help("use the specific selector directly (e.g. replace `.. | .h1` with `.h1`)")); + diagnostics.push(d); } } @@ -97,6 +92,6 @@ mod tests { // `.. | .h1` — the `..` is redundant because `.h1` already selects h1 nodes directly. let diags = check(".. | .h1"); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("inefficient selector")); + assert!(diags[0].message().contains("inefficient selector")); } } diff --git a/crates/mq-lint/src/rules/selector/missing_depth_guard.rs b/crates/mq-lint/src/rules/selector/missing_depth_guard.rs index 996a5d7c3..3d118a376 100644 --- a/crates/mq-lint/src/rules/selector/missing_depth_guard.rs +++ b/crates/mq-lint/src/rules/selector/missing_depth_guard.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct MissingDepthGuard; impl LintRule for MissingDepthGuard { - fn id(&self) -> &'static str { - "missing_depth_guard" + fn id(&self) -> RuleId { + RuleId::MissingDepthGuard } fn severity(&self) -> Severity { @@ -31,18 +31,11 @@ impl LintRule for MissingDepthGuard { ctx.all_symbols() .filter(|(_, s)| matches!(s.kind, SymbolKind::Selector(mq_lang::Selector::Recursive))) .map(|(_, sym)| { - let mut d = Diagnostic::new( - self.id(), - self.severity(), - "`..` (recursive selector) used without a depth guard", - ); + let mut d = Diagnostic::new(LintMessage::MissingDepthGuard, self.severity()); if let Some(range) = sym.source.text_range { d = d.with_range(range); } - d.with_help( - "consider adding a depth limit, e.g. `.. | select(.depth <= 3)`, \ - to avoid traversing the entire document", - ) + d }) .collect() } @@ -67,7 +60,7 @@ mod tests { fn detects_bare_recursive_selector() { let diags = check(".."); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("depth guard")); + assert!(diags[0].message().contains("depth guard")); } #[test] diff --git a/crates/mq-lint/src/rules/selector/selector_always_empty.rs b/crates/mq-lint/src/rules/selector/selector_always_empty.rs index ab2c3dbf8..9a1b3d85c 100644 --- a/crates/mq-lint/src/rules/selector/selector_always_empty.rs +++ b/crates/mq-lint/src/rules/selector/selector_always_empty.rs @@ -1,7 +1,7 @@ use mq_hir::SymbolKind; use mq_lang::Selector; -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; pub struct SelectorAlwaysEmpty; @@ -18,8 +18,8 @@ fn mutually_exclusive(first: &Selector, second: &Selector) -> bool { } impl LintRule for SelectorAlwaysEmpty { - fn id(&self) -> &'static str { - "selector_always_empty" + fn id(&self) -> RuleId { + RuleId::SelectorAlwaysEmpty } fn severity(&self) -> Severity { @@ -46,18 +46,19 @@ impl LintRule for SelectorAlwaysEmpty { continue; } - let first_text = first.value.as_deref().unwrap_or(""); - let second_text = second.value.as_deref().unwrap_or(""); + let first_text = first.value.as_deref().unwrap_or("").to_string(); + let second_text = second.value.as_deref().unwrap_or("").to_string(); let mut d = Diagnostic::new( - self.id(), + LintMessage::SelectorAlwaysEmpty { + first: first_text, + second: second_text, + }, self.severity(), - format!("`{first_text} | {second_text}` can never match: a node can't be both"), ); if let Some(range) = second.source.text_range { d = d.with_range(range); } - diagnostics - .push(d.with_help("remove one of the selectors, or replace the pipe with a different query")); + diagnostics.push(d); } } diff --git a/crates/mq-lint/src/rules/style/boolean_comparison.rs b/crates/mq-lint/src/rules/style/boolean_comparison.rs index cc083c614..54c58a185 100644 --- a/crates/mq-lint/src/rules/style/boolean_comparison.rs +++ b/crates/mq-lint/src/rules/style/boolean_comparison.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct BooleanComparison; impl LintRule for BooleanComparison { - fn id(&self) -> &'static str { - "boolean_comparison" + fn id(&self) -> RuleId { + RuleId::BooleanComparison } fn severity(&self) -> Severity { @@ -35,23 +35,18 @@ impl LintRule for BooleanComparison { }; let bool_val = bool_sym.value.as_deref().unwrap_or("true"); - let help = match (op, bool_val) { - ("==", "true") => "use the value directly instead of `== true`", - ("==", "false") => "use `!` prefix instead of `== false`", - ("!=", "true") => "use `!` prefix instead of `!= true`", - ("!=", "false") => "use the value directly instead of `!= false`", - _ => "simplify this boolean comparison", - }; let mut d = Diagnostic::new( - self.id(), + LintMessage::BooleanComparison { + op: op.to_string(), + bool_val: bool_val.to_string(), + }, self.severity(), - format!("unnecessary comparison with boolean literal `{bool_val}`"), ); if let Some(range) = bop_sym.source.text_range { d = d.with_range(range); } - diagnostics.push(d.with_help(help)); + diagnostics.push(d); } diagnostics diff --git a/crates/mq-lint/src/rules/style/naming_convention.rs b/crates/mq-lint/src/rules/style/naming_convention.rs index 424692d20..93b1119f2 100644 --- a/crates/mq-lint/src/rules/style/naming_convention.rs +++ b/crates/mq-lint/src/rules/style/naming_convention.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct NamingConvention; impl LintRule for NamingConvention { - fn id(&self) -> &'static str { - "naming_convention" + fn id(&self) -> RuleId { + RuleId::NamingConvention } fn severity(&self) -> Severity { @@ -27,14 +27,16 @@ impl LintRule for NamingConvention { } let suggested = to_snake_case(name); let mut d = Diagnostic::new( - self.id(), + LintMessage::NamingConvention { + name: name.to_string(), + suggested, + }, self.severity(), - format!("`{name}` should be written in snake_case"), ); if let Some(range) = sym.source.text_range { d = d.with_range(range); } - Some(d.with_help(format!("rename to `{suggested}`"))) + Some(d) }) .collect() } @@ -81,7 +83,7 @@ mod tests { fn detects_camel_case_function() { let diags = check("def myFunc(): .h1;"); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("myFunc")); + assert!(diags[0].message().contains("myFunc")); } #[test] diff --git a/crates/mq-lint/src/rules/style/prefer_coalesce.rs b/crates/mq-lint/src/rules/style/prefer_coalesce.rs index 69cf6bc96..e3b2d8ecd 100644 --- a/crates/mq-lint/src/rules/style/prefer_coalesce.rs +++ b/crates/mq-lint/src/rules/style/prefer_coalesce.rs @@ -1,6 +1,6 @@ use mq_hir::{Symbol, SymbolId, SymbolKind}; -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; pub struct PreferCoalesce; @@ -73,8 +73,8 @@ fn matches_null_check_pattern(ctx: &LintContext<'_>, if_id: SymbolId) -> bool { } impl LintRule for PreferCoalesce { - fn id(&self) -> &'static str { - "prefer_coalesce" + fn id(&self) -> RuleId { + RuleId::PreferCoalesce } fn severity(&self) -> Severity { @@ -86,15 +86,11 @@ impl LintRule for PreferCoalesce { .filter(|(_, sym)| matches!(sym.kind, SymbolKind::If)) .filter(|(if_id, _)| matches_null_check_pattern(ctx, *if_id)) .map(|(_, if_sym)| { - let mut d = Diagnostic::new( - self.id(), - self.severity(), - "`if`/`else` null-check can be simplified using the `??` coalesce operator", - ); + let mut d = Diagnostic::new(LintMessage::PreferCoalesce, self.severity()); if let Some(range) = if_sym.source.text_range { d = d.with_range(range); } - d.with_help("rewrite as ` ?? `") + d }) .collect() } diff --git a/crates/mq-lint/src/rules/style/prefer_let_over_var.rs b/crates/mq-lint/src/rules/style/prefer_let_over_var.rs index 208ee2434..20f42b324 100644 --- a/crates/mq-lint/src/rules/style/prefer_let_over_var.rs +++ b/crates/mq-lint/src/rules/style/prefer_let_over_var.rs @@ -1,14 +1,14 @@ use rustc_hash::FxHashSet; -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::{SymbolId, SymbolKind}; use mq_lang::Range; pub struct PreferLetOverVar; impl LintRule for PreferLetOverVar { - fn id(&self) -> &'static str { - "prefer_let_over_var" + fn id(&self) -> RuleId { + RuleId::PreferLetOverVar } fn severity(&self) -> Severity { @@ -65,12 +65,11 @@ impl LintRule for PreferLetOverVar { } let mut d = Diagnostic::new( - self.id(), + LintMessage::PreferLetOverVar { name: name.to_string() }, self.severity(), - format!("`{name}` is never reassigned; prefer `let` over `var`"), ); d = d.with_range(kw_range); - diagnostics.push(d.with_help(format!("change `var {name}` to `let {name}`"))); + diagnostics.push(d); } diagnostics @@ -96,7 +95,7 @@ mod tests { fn detects_var_never_reassigned() { let diags = check("var x = .h1 | x"); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("prefer `let` over `var`")); + assert!(diags[0].message().contains("prefer `let` over `var`")); } #[test] diff --git a/crates/mq-lint/src/rules/style/prefer_pipe_style.rs b/crates/mq-lint/src/rules/style/prefer_pipe_style.rs index e080bc962..7b9b94063 100644 --- a/crates/mq-lint/src/rules/style/prefer_pipe_style.rs +++ b/crates/mq-lint/src/rules/style/prefer_pipe_style.rs @@ -1,12 +1,12 @@ use mq_hir::SymbolKind; -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; pub struct PreferPipeStyle; impl LintRule for PreferPipeStyle { - fn id(&self) -> &'static str { - "prefer_pipe_style" + fn id(&self) -> RuleId { + RuleId::PreferPipeStyle } fn severity(&self) -> Severity { @@ -27,18 +27,14 @@ impl LintRule for PreferPipeStyle { return None; } - let outer_name = outer_sym.value.as_deref().unwrap_or(""); - let inner_name = arg.value.as_deref().unwrap_or(""); + let outer_name = outer_sym.value.as_deref().unwrap_or("").to_string(); + let inner_name = arg.value.as_deref().unwrap_or("").to_string(); - let mut d = Diagnostic::new( - self.id(), - self.severity(), - format!("nested call `{outer_name}({inner_name}(...))` reads better as a pipe"), - ); + let mut d = Diagnostic::new(LintMessage::PreferPipeStyle { outer_name, inner_name }, self.severity()); if let Some(range) = outer_sym.source.text_range { d = d.with_range(range); } - Some(d.with_help(format!("rewrite as `... | {inner_name}() | {outer_name}()`"))) + Some(d) }) .collect() } @@ -63,8 +59,8 @@ mod tests { fn detects_nested_unary_calls() { let diags = check("to_text(to_upper(x))"); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("to_text")); - assert!(diags[0].message.contains("to_upper")); + assert!(diags[0].message().contains("to_text")); + assert!(diags[0].message().contains("to_upper")); } #[test] diff --git a/crates/mq-lint/src/rules/style/prefer_specific_heading.rs b/crates/mq-lint/src/rules/style/prefer_specific_heading.rs index 590eb1990..5a342a6ed 100644 --- a/crates/mq-lint/src/rules/style/prefer_specific_heading.rs +++ b/crates/mq-lint/src/rules/style/prefer_specific_heading.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::SymbolKind; pub struct PreferSpecificHeading; impl LintRule for PreferSpecificHeading { - fn id(&self) -> &'static str { - "prefer_specific_heading" + fn id(&self) -> RuleId { + RuleId::PreferSpecificHeading } fn severity(&self) -> Severity { @@ -17,15 +17,11 @@ impl LintRule for PreferSpecificHeading { ctx.all_symbols() .filter(|(_, s)| matches!(s.kind, SymbolKind::Selector(mq_lang::Selector::Heading(None)))) .map(|(_, sym)| { - let mut d = Diagnostic::new( - self.id(), - self.severity(), - "prefer a specific heading level selector (`.h1`–`.h6`) over `.h`", - ); + let mut d = Diagnostic::new(LintMessage::PreferSpecificHeading, self.severity()); if let Some(range) = sym.source.text_range { d = d.with_range(range); } - d.with_help("using `.h1`–`.h6` makes the intended heading level explicit") + d }) .collect() } diff --git a/crates/mq-lint/src/rules/style/redundant_boolean_literal.rs b/crates/mq-lint/src/rules/style/redundant_boolean_literal.rs index a245a4e66..0f5ba34c8 100644 --- a/crates/mq-lint/src/rules/style/redundant_boolean_literal.rs +++ b/crates/mq-lint/src/rules/style/redundant_boolean_literal.rs @@ -1,11 +1,11 @@ -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; use mq_hir::{SymbolId, SymbolKind}; pub struct RedundantBooleanLiteral; impl LintRule for RedundantBooleanLiteral { - fn id(&self) -> &'static str { - "redundant_boolean_literal" + fn id(&self) -> RuleId { + RuleId::RedundantBooleanLiteral } fn severity(&self) -> Severity { @@ -40,21 +40,16 @@ impl LintRule for RedundantBooleanLiteral { // Pattern: if (cond): true else: false → cond // if (cond): false else: true → !cond if (then_val == "true" && else_val == "false") || (then_val == "false" && else_val == "true") { - let suggestion = if then_val == "true" { - "replace `if (cond): true else: false` with just `cond`" - } else { - "replace `if (cond): false else: true` with `not(cond)` or `!(cond)`" - }; - let mut d = Diagnostic::new( - self.id(), + LintMessage::RedundantBooleanLiteral { + then_val: then_val.to_string(), + }, self.severity(), - "redundant boolean literal in `if`/`else` — condition already is the result", ); if let Some(range) = if_sym.source.text_range { d = d.with_range(range); } - diagnostics.push(d.with_help(suggestion)); + diagnostics.push(d); } } @@ -126,7 +121,7 @@ mod tests { fn detects_true_false_pattern() { let diags = check("if (.h1): true else: false;"); assert_eq!(diags.len(), 1); - assert!(diags[0].message.contains("redundant boolean literal")); + assert!(diags[0].message().contains("redundant boolean literal")); } #[test] diff --git a/crates/mq-lint/src/rules/style/redundant_try.rs b/crates/mq-lint/src/rules/style/redundant_try.rs index 0568fa870..459199b13 100644 --- a/crates/mq-lint/src/rules/style/redundant_try.rs +++ b/crates/mq-lint/src/rules/style/redundant_try.rs @@ -1,12 +1,12 @@ use mq_hir::SymbolKind; -use crate::{Diagnostic, LintContext, LintRule, Severity}; +use crate::{Diagnostic, LintContext, LintMessage, LintRule, RuleId, Severity}; pub struct RedundantTry; impl LintRule for RedundantTry { - fn id(&self) -> &'static str { - "redundant_try" + fn id(&self) -> RuleId { + RuleId::RedundantTry } fn severity(&self) -> Severity { @@ -31,15 +31,11 @@ impl LintRule for RedundantTry { return None; } - let mut d = Diagnostic::new( - self.id(), - self.severity(), - "`try: ... catch: none` is equivalent to the `?` error-suppression operator", - ); + let mut d = Diagnostic::new(LintMessage::RedundantTry, self.severity()); if let Some(range) = try_sym.source.text_range { d = d.with_range(range); } - Some(d.with_help("rewrite as `?`")) + Some(d) }) .collect() } diff --git a/crates/mq-lsp/src/error.rs b/crates/mq-lsp/src/error.rs index 8e461d278..6aec99464 100644 --- a/crates/mq-lsp/src/error.rs +++ b/crates/mq-lsp/src/error.rs @@ -55,14 +55,14 @@ impl From<&LspError> for ls_types::Diagnostic { ls_types::Position { line: 0, character: 1 }, ) }); - let message = match &diagnostic.help { - Some(help) => format!("{} (help: {help})", diagnostic.message), - None => diagnostic.message.clone(), + let message = match diagnostic.help() { + Some(help) => format!("{} (help: {help})", diagnostic.message()), + None => diagnostic.message(), }; ls_types::Diagnostic::new( range, Some(lint_severity(diagnostic.severity)), - Some(NumberOrString::String(diagnostic.rule_id.to_string())), + Some(NumberOrString::String(diagnostic.rule_id().to_string())), Some("mq-lint".to_string()), message, None, @@ -79,12 +79,14 @@ mod tests { #[test] fn lint_warning_carries_rule_id_source_and_severity() { - let diagnostic = mq_lint::Diagnostic::new("unused_variable", mq_lint::Severity::Warn, "unused variable `x`") - .with_range(mq_lang::Range { - start: mq_lang::Position { line: 1, column: 5 }, - end: mq_lang::Position { line: 1, column: 6 }, - }) - .with_help("prefix with `_` if intentional"); + let diagnostic = mq_lint::Diagnostic::new( + mq_lint::LintMessage::UnusedVariable { name: "x".to_string() }, + mq_lint::Severity::Warn, + ) + .with_range(mq_lang::Range { + start: mq_lang::Position { line: 1, column: 5 }, + end: mq_lang::Position { line: 1, column: 6 }, + }); let lsp_diagnostic: ls_types::Diagnostic = (&LspError::LintWarning(diagnostic)).into(); @@ -95,15 +97,19 @@ mod tests { ); assert_eq!(lsp_diagnostic.source, Some("mq-lint".to_string())); assert!(lsp_diagnostic.message.contains("unused variable `x`")); - assert!(lsp_diagnostic.message.contains("help: prefix with `_` if intentional")); + assert!(lsp_diagnostic.message.contains("help: if this is intentional")); assert_eq!(lsp_diagnostic.range.start.line, 0); assert_eq!(lsp_diagnostic.range.start.character, 4); } #[test] fn lint_warning_without_range_falls_back_to_origin() { - let diagnostic = - mq_lint::Diagnostic::new("duplicate_match_arm", mq_lint::Severity::Error, "duplicate match arm"); + let diagnostic = mq_lint::Diagnostic::new( + mq_lint::LintMessage::DuplicateMatchArm { + pattern: "a".to_string(), + }, + mq_lint::Severity::Error, + ); let lsp_diagnostic: ls_types::Diagnostic = (&LspError::LintWarning(diagnostic)).into(); diff --git a/crates/mq-lsp/src/main.rs b/crates/mq-lsp/src/main.rs index 09459426e..2b1d28857 100644 --- a/crates/mq-lsp/src/main.rs +++ b/crates/mq-lsp/src/main.rs @@ -58,7 +58,7 @@ struct LintArgs { /// Disable a specific lint rule by ID (repeatable) #[arg(long = "disable-lint-rule", value_name = "RULE_ID")] - disable_lint_rule: Vec, + disable_lint_rule: Vec, } #[tokio::main] @@ -71,7 +71,7 @@ async fn main() { let mut lint_config = mq_lint::LintConfig::default(); for rule_id in &cli.lint.disable_lint_rule { - lint_config.disable_rule(rule_id.clone()); + lint_config.disable_rule(*rule_id); } let config = LspConfig::new( diff --git a/crates/mq-lsp/src/server.rs b/crates/mq-lsp/src/server.rs index d4a324b56..614342032 100644 --- a/crates/mq-lsp/src/server.rs +++ b/crates/mq-lsp/src/server.rs @@ -518,7 +518,7 @@ impl Backend { linter .run(&lint_ctx) .into_iter() - .filter(|d| d.rule_id != "unused_function") + .filter(|d| d.rule_id() != mq_lint::RuleId::UnusedFunction) .map(|d| (&LspError::LintWarning(d)).into()), ); } @@ -1538,7 +1538,9 @@ mod tests { let lint_ctx = mq_lint::LintContext::new(&hir_guard, source_id, &backend.config.lint_config); let diagnostics = mq_lint::Linter::with_default_rules().run(&lint_ctx); assert!( - diagnostics.iter().any(|d| d.rule_id == "unused_variable"), + diagnostics + .iter() + .any(|d| d.rule_id() == mq_lint::RuleId::UnusedVariable), "expected an unused_variable lint diagnostic" ); } @@ -1805,7 +1807,7 @@ mod tests { #[tokio::test] async fn test_lsp_config_new() { let mut lint_config = mq_lint::LintConfig::default(); - lint_config.disable_rule("naming_convention"); + lint_config.disable_rule(mq_lint::RuleId::NamingConvention); let config = LspConfig::new( vec![], true, @@ -1819,7 +1821,7 @@ mod tests { assert!(config.enable_type_checking); assert!(config.type_checker_options.strict_array); assert!(config.enable_lint); - assert!(!config.lint_config.is_rule_enabled("naming_convention")); + assert!(!config.lint_config.is_rule_enabled(mq_lint::RuleId::NamingConvention)); } #[tokio::test] From 130c1585d5f21506e42ded16f44fe933c9a04985 Mon Sep 17 00:00:00 2001 From: harehare Date: Tue, 23 Jun 2026 22:33:00 +0900 Subject: [PATCH 5/7] docs(vscode): document type-check and lint settings in README Configuration list referenced the stale mq-lsp.* prefix and omitted typeCheck/lint/enableCodeLens settings already present in package.json. --- editors/vscode/README.md | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/editors/vscode/README.md b/editors/vscode/README.md index 7b32d65bd..f6b387256 100644 --- a/editors/vscode/README.md +++ b/editors/vscode/README.md @@ -36,12 +36,39 @@ This extension provides essential coding assistance for `.mq` files, helping you The extension can be configured through Visual Studio Code settings: -- `mq-lsp.lspPath`: Path to the mq language server binary -- `mq-lsp.showExamplesInNewFile`: To Show/Hide examples in new file +- `mq.lspPath`: Path to the mq language server binary +- `mq.showExamplesInNewFile`: To Show/Hide examples in new file +- `mq.enableCodeLens`: Enable/disable the CodeLens for running mq queries +- `mq.typeCheck.enableTypeCheck`: Enable type checking diagnostics (passes `--enable-type-checking` to `mq-lsp`) +- `mq.typeCheck.strictArray`: Require arrays to contain elements of a single type (passes `--strict-array` to `mq-lsp`, requires `mq.typeCheck.enableTypeCheck`) +- `mq.lint.enableLint`: Enable `mq-lint` diagnostics (correctness, style, complexity, selector, and module rules) +- `mq.lint.disabledRules`: Lint rule IDs to disable (e.g. `"naming_convention"`, `"shadow_variable"`). Only effective when `mq.lint.enableLint` is `true` - `editor.semanticHighlighting.enabled`: Set to `true` to enable semantic token highlighting for improved code visualization You can customize these settings in your VS Code settings.json file or through the Settings UI. +### Type Checking + +Enable type checking to get real-time type errors and richer hover type information: + +```json +{ + "mq.typeCheck.enableTypeCheck": true, + "mq.typeCheck.strictArray": true +} +``` + +### Linting + +Enable `mq-lint` diagnostics (correctness, style, complexity, selector, and module rules): + +```json +{ + "mq.lint.enableLint": true, + "mq.lint.disabledRules": ["naming_convention", "shadow_variable"] +} +``` + ## Example ### Basic Example From 761b846f046e273cfdb7a2c468c63f7c256c56d1 Mon Sep 17 00:00:00 2001 From: harehare Date: Tue, 23 Jun 2026 23:58:16 +0900 Subject: [PATCH 6/7] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor(mq-lsp):=20re?= =?UTF-8?q?move=20unused=20function=20warnings=20from=20diagnostics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/mq-lint/src/main.rs | 177 ++++++++++++++++++++---------------- crates/mq-lsp/src/server.rs | 27 ------ 2 files changed, 98 insertions(+), 106 deletions(-) diff --git a/crates/mq-lint/src/main.rs b/crates/mq-lint/src/main.rs index 4050a22ea..685e312f8 100644 --- a/crates/mq-lint/src/main.rs +++ b/crates/mq-lint/src/main.rs @@ -82,23 +82,19 @@ fn run() -> io::Result { if cli.files.is_empty() { let mut code = String::new(); io::stdin().read_to_string(&mut code)?; - let had_diagnostics = lint_source(&mut w, &code, None, &linter, &config, min_severity)?; + let had_diagnostics = lint_source(&mut w, &code, "", &linter, &config, min_severity)?; return Ok(had_diagnostics); } - let multi = cli.files.len() > 1; let mut had_diagnostics = false; for path in &cli.files { let code = std::fs::read_to_string(path) .map_err(|e| io::Error::other(format!("reading file {}: {}", path.display(), e)))?; - let label = if multi { Some(path.display().to_string()) } else { None }; - if lint_source(&mut w, &code, label.as_deref(), &linter, &config, min_severity)? { + let label = path.display().to_string(); + if lint_source(&mut w, &code, &label, &linter, &config, min_severity)? { had_diagnostics = true; } - if multi { - writeln!(w)?; - } } Ok(had_diagnostics) @@ -113,11 +109,15 @@ fn list_rules(w: &mut impl Write) -> io::Result<()> { Ok(()) } -/// Lints a single source, writes diagnostics, and returns `true` if any were reported. +/// Severities in the order categories are displayed, most severe first. +const SEVERITY_ORDER: [Severity; 4] = [Severity::Error, Severity::Warn, Severity::Perf, Severity::Style]; + +/// Lints a single source, writes diagnostics grouped by severity in a Credo-style report, +/// and returns `true` if any were reported. fn lint_source( w: &mut impl Write, code: &str, - label: Option<&str>, + file_label: &str, linter: &Linter, config: &LintConfig, min_severity: Severity, @@ -133,12 +133,17 @@ fn lint_source( .collect(); diagnostics.sort_by_key(|d| d.range.map(|r| (r.start.line, r.start.column))); - if let Some(lbl) = label { - writeln!(w, "{} {}", "──".dimmed(), lbl.bold())?; - } - - for diagnostic in &diagnostics { - write_diagnostic(w, diagnostic, code)?; + let mut printed_category = false; + for severity in SEVERITY_ORDER { + let group: Vec<&Diagnostic> = diagnostics.iter().filter(|d| d.severity == severity).collect(); + if group.is_empty() { + continue; + } + if printed_category { + writeln!(w)?; + } + printed_category = true; + write_category(w, severity, &group, file_label)?; } if diagnostics.is_empty() { @@ -149,92 +154,106 @@ fn lint_source( "No lint issues found.".bright_green() )?; } else { - writeln!( - w, - "{} {} issue{} found.", - "✗".bright_red().bold(), - diagnostics.len().to_string().bright_red().bold(), - if diagnostics.len() == 1 { "" } else { "s" }, - )?; + writeln!(w)?; + write_summary(w, &diagnostics)?; } Ok(!diagnostics.is_empty()) } -fn severity_label(severity: Severity) -> colored::ColoredString { +/// Maps a severity to its Credo-style category title and one-letter marker. +fn severity_category(severity: Severity) -> (colored::ColoredString, colored::ColoredString) { match severity { - Severity::Style => "style".cyan().bold(), - Severity::Perf => "perf".blue().bold(), - Severity::Warn => "warn".bright_yellow().bold(), - Severity::Error => "error".bright_red().bold(), + Severity::Error => ("## Errors".bright_red().bold(), "[E]".bright_red().bold()), + Severity::Warn => ("## Warnings".bright_yellow().bold(), "[W]".bright_yellow().bold()), + Severity::Perf => ("## Performance".blue().bold(), "[P]".blue().bold()), + Severity::Style => ("## Style".cyan().bold(), "[S]".cyan().bold()), } } -fn write_diagnostic(w: &mut impl Write, diagnostic: &Diagnostic, code: &str) -> io::Result<()> { - let loc_plain = diagnostic - .range - .map(|range| format!("{}:{}", range.start.line, range.start.column)) - .unwrap_or_default(); - let prefix_width = loc_plain.len(); - let sep = "│".dimmed(); - - writeln!( - w, - " {} {} {} {} {}", - loc_plain.dimmed(), - sep, - severity_label(diagnostic.severity), - diagnostic.rule_id().as_str().bright_magenta(), - diagnostic.message(), - )?; - - if let Some(range) = &diagnostic.range { - write_snippet(w, code, range, prefix_width)?; +/// Uses mq's own pipe operator `|` as the gutter bar. +fn severity_bar(severity: Severity) -> colored::ColoredString { + match severity { + Severity::Error => "|".bright_red(), + Severity::Warn => "|".bright_yellow(), + Severity::Perf => "|".blue(), + Severity::Style => "|".cyan(), } +} + +/// Writes one severity category: a heading followed by its diagnostics, each as a +/// `[X] message` line with the `file:line:col .rule_id` location on the line below +/// (the rule id rendered as an mq selector, e.g. `.unused_variable`). +fn write_category( + w: &mut impl Write, + severity: Severity, + diagnostics: &[&Diagnostic], + file_label: &str, +) -> io::Result<()> { + let (title, letter) = severity_category(severity); + let bar = severity_bar(severity); + + writeln!(w, "{}\n", title)?; + writeln!(w, "{bar}")?; + + for (i, diagnostic) in diagnostics.iter().enumerate() { + match diagnostic.severity { + Severity::Error => writeln!(w, "{bar} {} {}", letter, diagnostic.message().bright_red().bold())?, + Severity::Warn => writeln!(w, "{bar} {} {}", letter, diagnostic.message().bright_yellow().bold())?, + Severity::Perf => writeln!(w, "{bar} {} {}", letter, diagnostic.message().blue().bold())?, + Severity::Style => writeln!(w, "{bar} {} {}", letter, diagnostic.message().cyan().bold())?, + } - if let Some(help) = diagnostic.help() { + let loc = match &diagnostic.range { + Some(range) => format!("{}:{}:{}", file_label, range.start.line, range.start.column), + None => file_label.to_string(), + }; writeln!( w, - " {} {} {}", - " ".repeat(prefix_width), - sep, - format!("help: {help}").bright_blue(), + "{bar} {} {}", + loc.dimmed(), + format!(".{}", diagnostic.rule_id().as_str()).dimmed(), )?; + + if let Some(help) = diagnostic.help() { + writeln!(w, "{bar} {}", format!("help: {help}").bright_blue())?; + } + + if i + 1 < diagnostics.len() { + writeln!(w, "{bar}")?; + } } Ok(()) } -/// Writes a source snippet for the diagnostic location with a caret underline. -fn write_snippet(w: &mut impl Write, code: &str, range: &mq_lang::Range, prefix_width: usize) -> io::Result<()> { - let sep = "│".dimmed(); - let lines: Vec<&str> = code.lines().collect(); - let line_idx = range.start.line.saturating_sub(1) as usize; - let Some(source_line) = lines.get(line_idx) else { - return Ok(()); - }; - - let col_start = range.start.column.saturating_sub(1); - let col_end = if range.end.line == range.start.line { - range.end.column.saturating_sub(1) - } else { - source_line.len() - }; - let underline_len = col_end.saturating_sub(col_start).max(1); - - let line_num = range.start.line.to_string(); - let gutter = " ".repeat(prefix_width.max(line_num.len())); +/// Writes the trailing summary line, e.g. `found 3 issues (2 warnings, 1 style).` +fn write_summary(w: &mut impl Write, diagnostics: &[Diagnostic]) -> io::Result<()> { + let breakdown: Vec = SEVERITY_ORDER + .into_iter() + .filter_map(|severity| { + let count = diagnostics.iter().filter(|d| d.severity == severity).count(); + if count == 0 { + return None; + } + let (singular, plural) = match severity { + Severity::Error => ("error".bright_red(), "errors".bright_red()), + Severity::Warn => ("warning".bright_yellow(), "warnings".bright_yellow()), + Severity::Perf => ("performance".blue(), "performance".blue()), + Severity::Style => ("style".cyan(), "style".cyan()), + }; + Some(format!("{count} {}", if count == 1 { singular } else { plural })) + }) + .collect(); - writeln!(w, " {} {} {}", gutter, sep, source_line.dimmed())?; writeln!( w, - " {} {} {}{}", - format!("{:>width$}", line_num, width = prefix_width).dimmed(), - sep, - " ".repeat(col_start), - "^".repeat(underline_len).bright_red().bold(), - )?; - Ok(()) + "{} {} issue{} ({}).", + "found".bold(), + diagnostics.len().to_string().bold(), + if diagnostics.len() == 1 { "" } else { "s" }, + breakdown.join(", "), + ) } #[cfg(test)] diff --git a/crates/mq-lsp/src/server.rs b/crates/mq-lsp/src/server.rs index 614342032..6db8612c4 100644 --- a/crates/mq-lsp/src/server.rs +++ b/crates/mq-lsp/src/server.rs @@ -462,31 +462,6 @@ impl Backend { } })); - // Add unused function warnings - let unused_functions = hir_guard.unused_functions(*source_id); - for (_, symbol) in unused_functions { - if let Some(text_range) = &symbol.source.text_range { - let mut diagnostic = ls_types::Diagnostic::new_simple( - ls_types::Range::new( - ls_types::Position { - line: text_range.start.line - 1, - character: (text_range.start.column - 1) as u32, - }, - ls_types::Position { - line: text_range.end.line - 1, - character: (text_range.end.column - 1) as u32, - }, - ), - format!( - "Function '{}' is defined but never used", - symbol.value.as_ref().unwrap_or(&"".into()) - ), - ); - diagnostic.severity = Some(ls_types::DiagnosticSeverity::WARNING); - diagnostics.push(diagnostic); - } - } - // Add HIR warnings (including unreachable code warnings) diagnostics.extend(hir_guard.warning_ranges().into_iter().filter_map(|(message, item)| { if range_set.contains(&item) { @@ -510,7 +485,6 @@ impl Backend { } })); - // `unused_function` is skipped: already covered by the warning above. if self.config.enable_lint { let lint_ctx = mq_lint::LintContext::new(&hir_guard, *source_id, &self.config.lint_config); let linter = mq_lint::Linter::with_default_rules(); @@ -518,7 +492,6 @@ impl Backend { linter .run(&lint_ctx) .into_iter() - .filter(|d| d.rule_id() != mq_lint::RuleId::UnusedFunction) .map(|d| (&LspError::LintWarning(d)).into()), ); } From c77269d32cfca1e13f53514d61dfc7427e6664d1 Mon Sep 17 00:00:00 2001 From: harehare Date: Fri, 26 Jun 2026 23:09:26 +0900 Subject: [PATCH 7/7] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor(mq-lint):=20r?= =?UTF-8?q?emove=20unnecessary=20severity=20bar=20output=20in=20write=5Fca?= =?UTF-8?q?tegory=20function?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/mq-lint/src/main.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/mq-lint/src/main.rs b/crates/mq-lint/src/main.rs index 685e312f8..edcbd6984 100644 --- a/crates/mq-lint/src/main.rs +++ b/crates/mq-lint/src/main.rs @@ -194,7 +194,6 @@ fn write_category( let bar = severity_bar(severity); writeln!(w, "{}\n", title)?; - writeln!(w, "{bar}")?; for (i, diagnostic) in diagnostics.iter().enumerate() { match diagnostic.severity {