From 71286af374206649fc9f4326c4aa83b018a9fb53 Mon Sep 17 00:00:00 2001 From: Sall Date: Sat, 18 Jul 2026 09:08:08 +0100 Subject: [PATCH 1/4] docs(adr): reconcile zsh-lint analyzer architecture --- ...zsh-lint-semantic-analyzer-architecture.md | 129 ++++++++++++++---- 1 file changed, 104 insertions(+), 25 deletions(-) diff --git a/decisions/0011-zsh-lint-semantic-analyzer-architecture.md b/decisions/0011-zsh-lint-semantic-analyzer-architecture.md index f9c9e7e53..8c6e74ca9 100644 --- a/decisions/0011-zsh-lint-semantic-analyzer-architecture.md +++ b/decisions/0011-zsh-lint-semantic-analyzer-architecture.md @@ -8,46 +8,125 @@ PROPOSED ## Context -`zsh-lint` is transitioning from a legacy interactive shell plugin to a standalone, Go-based semantic analyzer (Epic ZSH-3). The parser front end relies on `mvdan/sh/syntax`. We need a unified architecture for how the tool will traverse the Abstract Syntax Tree (AST), manage contextual state (like variable scoping), and evaluate linting rules. +`zsh-lint` is a standalone Go semantic analyzer for Zsh. Its parser front end +produces an `mvdan/sh/syntax` tree, and its semantic engine must support rules +that need only the current syntax node as well as rules that need declarations +collected from the complete file. -Shell scripts are highly dynamic, meaning a single-pass naive visitor pattern is often insufficient to detect complex issues (e.g., using a variable before it is declared, or aliasing). +Zsh permits dynamic behavior and declarations whose textual order does not +necessarily match the facts an analysis needs. A complete-file declaration +index can therefore be useful, but requiring one for every rule would couple +syntax-only checks to state they do not consume. The index is an intentionally +approximate symbol map, not flow-sensitive local/global resolution. + +Rule behavior and diagnostic compatibility also depend on contracts outside +the traversal strategy. Stable rule IDs and evidence requirements, parser-gap +handling, inline suppression, and machine-readable output are governed by the +linked `zsh-lint` contracts rather than duplicated here. ## Decision -We will implement a **Two-Pass Analysis Architecture** for the semantic engine: +Use a **conditional two-pass semantic core inside a three-phase diagnostic +pipeline**: + +1. **Optional scope indexing.** Before rule evaluation, build the declaration + index only when at least one registered rule implements `ScopeAwareRule` and + returns `NeedsScope() == true`. The index records the declarations and + approximate function-local/global associations exposed by `scope.Map`. +2. **Rule evaluation.** Walk the syntax tree and pass each node, with a shared + `*Context`, to every registered rule. `Context` carries the parsed file, + source path, diagnostics, and a declaration index populated only when + requested. Its `Report` method accepts source positions, a stable rule ID, + severity, and message. +3. **Suppression and finalization.** Collect and apply inline suppression + directives, preserve or add `meta/*` diagnostics required by the suppression + contract, and sort the resulting diagnostics once in deterministic order. -1. **Pass 1: Context & Scope Resolution (The Indexer)** - - Traverses the `syntax.File` AST to build a `ScopeMap`. - - Records variable declarations, function definitions, and alias definitions. - - Determines the boundaries of local vs. global scope. -2. **Pass 2: Rule Evaluation (The Linter)** - - Traverses the AST a second time. - - Feeds each `syntax.Node` to a registry of initialized `Rule` implementations. - - Passes a rich `*AnalyzerContext` object alongside the node, which provides the rules access to the `ScopeMap` generated in Pass 1, as well as a `Report(diagnostic)` method. +The analyzer's extension interfaces are: -### The Rule Interface -To ensure extensibility, every rule must satisfy a strict interface: ```go type Rule interface { + ID() diag.RuleID Name() string - Analyze(ctx *AnalyzerContext, node syntax.Node) + Analyze(ctx *Context, node syntax.Node) +} + +type ScopeAwareRule interface { + NeedsScope() bool } ``` +`ScopeAwareRule` is an opt-in capability, not a requirement for all rules. +Scope-dependent rules may query the declaration index, but they must account +for its approximate, non-flow-sensitive model when defining their semantics. + +The semantic pipeline produces the common diagnostic model. Suppression +semantics and the versioned JSON envelope remain product contracts in the +owning repository; changing either contract requires following its documented +compatibility rules rather than changing this ADR alone. + ## Consequences ### Positive -- **Decoupled Rules**: Rule authors do not need to worry about scope resolution; they can simply query `ctx.IsDeclared("varName")`. -- **Extensibility**: Adding a new lint rule requires only writing a struct that satisfies the `Rule` interface and registering it in the engine. -- **Precision**: Two passes allow the engine to detect "use before declaration" errors with high accuracy. -### Negative -- **Performance Overhead**: Walking the AST twice per file is slower than a single pass. However, `mvdan/sh` is highly optimized in Go, so the impact on typical shell scripts should be negligible compared to the architectural clarity gained. -- **Complexity**: Managing the `AnalyzerContext` state between passes introduces slight complexity to the core engine. +- Syntax-only rules do not pay the indexing cost or depend on scope state. +- Rules that need complete-file declaration facts can opt into a shared index + without embedding traversal-order mutation in each rule. +- A common reporting and finalization path keeps rule diagnostics, suppression, + metadata diagnostics, and deterministic output aligned. +- The scope implementation can evolve behind the existing `scope.Map` boundary + while preserving its rule-facing API and applicable diagnostic contracts. + +### Costs and limits + +- Enabling scope indexing adds a tree traversal; the cost depends on the + enabled rule capabilities and input, and this ADR makes no unmeasured + performance guarantee. +- The declaration index cannot by itself justify flow-sensitive or high-accuracy + claims. A rule that needs stronger resolution must first define and test that + capability in the owning repository. +- Shipping a rule requires more than satisfying the Go interface: it also needs + a stable ID, registry entry, tests, generated reference documentation, and the + evidence required by the rule policy. +- Suppression and finalization form a separate phase that the analyzer must keep + consistent across human- and machine-readable output. + +## Alternatives considered + +1. **Unconditional two-pass analysis.** Always build the declaration index + before evaluating rules. Rejected because it makes syntax-only rules depend + on and pay for state they do not consume. +2. **Single-pass state mutation as the only engine model.** Build context while + evaluating rules in one traversal. Rejected as the sole model because + analyses that require complete-file facts would become dependent on textual + traversal order. Rules that need only local syntax still operate entirely in + the evaluation phase. +3. **Flow-sensitive symbol analysis.** Build control-flow and data-flow models + for precise runtime ordering and scope. Not adopted by this decision, which + specifies an approximate declaration index. A rule that genuinely requires + stronger semantics should motivate a separate design change with Zsh-manual + grounding and corpus evidence. +4. **Regex-based linting over raw source.** Use regular expressions instead of a + syntax tree. Rejected as the semantic engine because raw text does not retain + shell grammar context and cannot reliably distinguish code from strings or + comments. + +## Decision review + +This reconciliation remains a proposal. Maintainers should explicitly accept +it with a named decider and date, amend it, supersede it with another ADR, or +reject it. Until that decision is recorded, this document describes the +implemented architecture but does not claim maintainer acceptance. -## Alternatives Considered +## References -1. **Single-Pass State Mutation**: A single traversal that builds scope and evaluates rules simultaneously. - - *Rejected* because shell functions can be declared at the bottom of a file but invoked at the top. A single pass would yield false positives for "undefined function" errors. -2. **Regex-Based Linting**: Using `regexp` against the raw file string. - - *Rejected* because it completely ignores the structural context of the shell grammar, leading to massive false-positive rates (e.g., matching a keyword inside a string literal). \ No newline at end of file +- [Issue #455 — Reconcile proposed zsh-lint architecture ADR 0011](https://github.com/z-shell/.github/issues/455) +- [`zsh-lint` analyzer orchestration](https://github.com/z-shell/zsh-lint/blob/main/internal/analyzer/analyzer.go) +- [`Rule` and `ScopeAwareRule` interfaces](https://github.com/z-shell/zsh-lint/blob/main/internal/analyzer/rule.go) +- [`Context` reporting API](https://github.com/z-shell/zsh-lint/blob/main/internal/analyzer/context.go) +- [`scope.Map` declaration model](https://github.com/z-shell/zsh-lint/blob/main/internal/scope/scope.go) +- [`zsh-lint` rule policy](https://github.com/z-shell/zsh-lint/blob/main/docs/project/rule-policy.md) +- [`zsh-lint` inline suppression contract](https://github.com/z-shell/zsh-lint/blob/main/docs/project/suppression.md) +- [`zsh-lint` machine-readable output contract](https://github.com/z-shell/zsh-lint/blob/main/docs/project/output-contract.md) +- [`zsh-lint` parser-gap workflow](https://github.com/z-shell/zsh-lint/blob/main/docs/project/parser-gap-workflow.md) +- [Zsh manual](https://zsh.sourceforge.io/Doc/Release/) From 0e910767bb74f162907f0ac44e468b1a78f5e8f0 Mon Sep 17 00:00:00 2001 From: Sall Date: Sat, 25 Jul 2026 00:00:42 +0100 Subject: [PATCH 2/4] docs(adr): clarify ScopeAwareRule composes with Rule Copilot review: the interface pairing implied a scope-aware rule must also implement Rule but never said so, leaving room to misread ScopeAwareRule as independently registrable. --- decisions/0011-zsh-lint-semantic-analyzer-architecture.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/decisions/0011-zsh-lint-semantic-analyzer-architecture.md b/decisions/0011-zsh-lint-semantic-analyzer-architecture.md index 8c6e74ca9..e19e6eed2 100644 --- a/decisions/0011-zsh-lint-semantic-analyzer-architecture.md +++ b/decisions/0011-zsh-lint-semantic-analyzer-architecture.md @@ -56,9 +56,11 @@ type ScopeAwareRule interface { } ``` -`ScopeAwareRule` is an opt-in capability, not a requirement for all rules. -Scope-dependent rules may query the declaration index, but they must account -for its approximate, non-flow-sensitive model when defining their semantics. +`ScopeAwareRule` is an opt-in capability that a registered `Rule` +implementation may additionally satisfy; it is not a replacement for `Rule` +and has no standalone registration path. Scope-dependent rules may query the +declaration index, but they must account for its approximate, +non-flow-sensitive model when defining their semantics. The semantic pipeline produces the common diagnostic model. Suppression semantics and the versioned JSON envelope remain product contracts in the From 265b27c704be2fd7b33e701823912aab3e2deba8 Mon Sep 17 00:00:00 2001 From: Sall Date: Sat, 25 Jul 2026 00:15:30 +0100 Subject: [PATCH 3/4] docs(adr): fix mvdan/sh import path Copilot review: the shipped module is mvdan.cc/sh/v3, so the syntax package's canonical import path is mvdan.cc/sh/v3/syntax, not mvdan/sh/syntax. Verified against zsh-lint's go.mod. --- decisions/0011-zsh-lint-semantic-analyzer-architecture.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/decisions/0011-zsh-lint-semantic-analyzer-architecture.md b/decisions/0011-zsh-lint-semantic-analyzer-architecture.md index e19e6eed2..63244743c 100644 --- a/decisions/0011-zsh-lint-semantic-analyzer-architecture.md +++ b/decisions/0011-zsh-lint-semantic-analyzer-architecture.md @@ -9,7 +9,7 @@ PROPOSED ## Context `zsh-lint` is a standalone Go semantic analyzer for Zsh. Its parser front end -produces an `mvdan/sh/syntax` tree, and its semantic engine must support rules +produces an `mvdan.cc/sh/v3/syntax` tree, and its semantic engine must support rules that need only the current syntax node as well as rules that need declarations collected from the complete file. From 12177316a7af1a0574c00c4552487b93d0d8a275 Mon Sep 17 00:00:00 2001 From: Sall Date: Sat, 25 Jul 2026 01:40:39 +0100 Subject: [PATCH 4/4] docs(adr): accept ADR-0011 as the Conditional Semantic Analysis Pipeline Per the decision packet in #455 (comment 5013562757), decider ss-o, 2026-07-25: 11A (retain the opt-in ScopeAwareRule extension point as shipped, no content change needed) / T1 (rename the heading to 'zsh-lint Conditional Semantic Analysis Pipeline', filename kept stable to avoid link churn) / D1 (revise in place and accept rather than supersede). Removed the now-resolved 'Decision review' section and added the Deciders field this ADR's template was missing. --- ...011-zsh-lint-semantic-analyzer-architecture.md | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/decisions/0011-zsh-lint-semantic-analyzer-architecture.md b/decisions/0011-zsh-lint-semantic-analyzer-architecture.md index 63244743c..5e529ed02 100644 --- a/decisions/0011-zsh-lint-semantic-analyzer-architecture.md +++ b/decisions/0011-zsh-lint-semantic-analyzer-architecture.md @@ -1,10 +1,12 @@ -# 11. zsh-lint Semantic Analyzer Architecture +# 11. zsh-lint Conditional Semantic Analysis Pipeline -Date: 2026-05-29 +Date: 2026-07-25 + +Deciders: ss-o ## Status -PROPOSED +ACCEPTED ## Context @@ -113,13 +115,6 @@ compatibility rules rather than changing this ADR alone. shell grammar context and cannot reliably distinguish code from strings or comments. -## Decision review - -This reconciliation remains a proposal. Maintainers should explicitly accept -it with a named decider and date, amend it, supersede it with another ADR, or -reject it. Until that decision is recorded, this document describes the -implemented architecture but does not claim maintainer acceptance. - ## References - [Issue #455 — Reconcile proposed zsh-lint architecture ADR 0011](https://github.com/z-shell/.github/issues/455)