-
Notifications
You must be signed in to change notification settings - Fork 5
docs(adr): reconcile zsh-lint analyzer architecture #456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+104
−28
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
71286af
docs(adr): reconcile zsh-lint analyzer architecture
ss-o eb7ea16
Merge branch 'main' into feature-455
ss-o 0e91076
docs(adr): clarify ScopeAwareRule composes with Rule
ss-o 265b27c
docs(adr): fix mvdan/sh import path
ss-o 1217731
docs(adr): accept ADR-0011 as the Conditional Semantic Analysis Pipeline
ss-o File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
132 changes: 104 additions & 28 deletions
132
decisions/0011-zsh-lint-semantic-analyzer-architecture.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,53 +1,129 @@ | ||
| # 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 | ||
|
|
||
| `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.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 | ||
|
Copilot marked this conversation as resolved.
|
||
| collected from the complete file. | ||
|
|
||
| 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. | ||
|
|
||
| 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). | ||
| 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 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 | ||
| 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. | ||
|
|
||
| ## 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). | ||
| - [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/) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.