|
| 1 | +## Context |
| 2 | + |
| 3 | +See proposal.md — Why. Two aspects of the current state shape this design. |
| 4 | + |
| 5 | +**Scala is already half-wired.** `patterns.py` lists `*.scala` among code-file globs and |
| 6 | +maps `".scala": "scala"` in `CODE_EXTENSIONS`. Scala files are therefore already |
| 7 | +discovered and tagged with a language, then dropped at two later points: |
| 8 | + |
| 9 | +``` |
| 10 | + [CODE_EXTENSIONS] .scala -> "scala" already wired |
| 11 | + | |
| 12 | + [call_graph_analyzer._find_code_files] passes through |
| 13 | + | |
| 14 | + [analysis_service._filter_supported_languages] DROPPED (whitelist) |
| 15 | + | |
| 16 | + [call_graph_analyzer._analyze_file if/elif chain] SILENT NO-OP (no branch, |
| 17 | + | else-warning commented out) |
| 18 | + [analyzers/scala.py] does not exist |
| 19 | +``` |
| 20 | + |
| 21 | +The whitelist is the hard gate. Until `"scala"` joins it, nothing else in this change |
| 22 | +executes, and because the dispatch fallthrough logs nothing, the failure mode during |
| 23 | +development is silence rather than an error. |
| 24 | + |
| 25 | +**Leaf-node selection is typed, and its type set is narrow.** `leaf_selection.py` |
| 26 | +defines `OOP_TYPES = {"class", "interface", "struct"}`. This drives two things: which |
| 27 | +components survive `filter_leaf_nodes`, and the `n_oop` count feeding the issue-#75 |
| 28 | +heuristic that decides whether free functions carry a codebase's architecture. Component |
| 29 | +types outside that set — `trait` (emitted by PHP), `object` (Kotlin), `enum` and |
| 30 | +`record` (Java), `delegate` (C#), `type_alias` (C++) — are already excluded from leaf |
| 31 | +selection today. |
| 32 | + |
| 33 | +That exclusion is nearly invisible for existing languages because their codebases are |
| 34 | +class-dominant. Scala is the first supported language where the excluded types *are* the |
| 35 | +architecture: a typical Scala repository is mostly traits, objects, and case classes. A |
| 36 | +faithful analyzer emitting `"trait"` and `"object"` would produce a repository whose |
| 37 | +traits and objects are filtered out of documentation *and* whose `n_oop` count is near |
| 38 | +zero, misfiring the heuristic into documenting loose methods instead of architectural |
| 39 | +units. |
| 40 | + |
| 41 | +## Goals / Non-Goals |
| 42 | + |
| 43 | +**Goals:** |
| 44 | + |
| 45 | +- Land Scala support without changing behavior for any already-supported language. |
| 46 | +- Keep the whole change inside the 14-file footprint in proposal.md — Impact. |
| 47 | +- Model Scala's distinctive constructs (traits, companion objects) so the dependency |
| 48 | + graph is correct rather than merely populated. |
| 49 | + |
| 50 | +**Non-Goals:** |
| 51 | + |
| 52 | +- Widening `OOP_TYPES`, adding duplicate-ID detection, or re-enabling the dispatch |
| 53 | + warning. See proposal.md — Non-Goals; each is pre-existing and gets its own change. |
| 54 | + Note that the `OOP_TYPES` gap is broader than PHP traits and Kotlin objects: C++ |
| 55 | + `type_alias` components are dropped from leaf selection too, even though `cpp.py` |
| 56 | + extracts them deliberately on the grounds that aliases are real API surface. Whichever |
| 57 | + change widens the set should cover all of these together. |
| 58 | +- Semantic analysis beyond syntactic extraction: no implicit resolution, no type |
| 59 | + inference, no macro expansion. |
| 60 | +- Treating `.sbt` as an analyzable source language (see Decision 5). |
| 61 | + |
| 62 | +## Decisions |
| 63 | + |
| 64 | +### 1. Use the two-level `component_type` / `node_type` split |
| 65 | + |
| 66 | +`component_type` carries the coarse type the pipeline gates on; `node_type` and |
| 67 | +`display_name` carry the faithful Scala construct: |
| 68 | + |
| 69 | +| Scala construct | `component_type` | `node_type` | `display_name` | |
| 70 | +| --- | --- | --- | --- | |
| 71 | +| `class` / `case class` | `class` | `class` | `class Foo` | |
| 72 | +| `trait` | `interface` | `trait` | `trait Foo` | |
| 73 | +| `object` / `package object` | `class` | `object` | `object Foo` | |
| 74 | +| Scala 3 `enum` | `class` | `enum` | `enum Foo` | |
| 75 | +| method | `method` | `method` | `method Foo.bar` | |
| 76 | +| top-level definition | `function` | `function` | `function bar` | |
| 77 | + |
| 78 | +*Why:* the coarse type keeps traits and objects leaf-eligible and counted toward |
| 79 | +`n_oop`, so the issue-#75 heuristic reads a Scala repository accurately — while |
| 80 | +`node_type` preserves what the construct actually is. No shared code changes, and no |
| 81 | +information is discarded. |
| 82 | + |
| 83 | +This is an established pattern, not a new one. `typescript.py:561-563` already splits |
| 84 | +the two, deriving `component_type` from a coarse `type` and `node_type` from a finer |
| 85 | +`include_functions` heuristic on existing repositories. That deserves a dedicated change |
| 86 | +`component_type`. The `Node` model carries all three fields plus `get_display_name()`. |
| 87 | + |
| 88 | +Critically, nothing recomputes `component_type` from `node_type`, so a deliberate split |
| 89 | +cannot be clobbered downstream: `ast_parser._determine_component_type` — which has its |
| 90 | +own competing whitelist including `enum`, `record`, `annotation`, and `delegate` — is |
| 91 | +dead code, defined at `ast_parser.py:148` and never called. |
| 92 | + |
| 93 | +*Alternative considered — remap onto the existing vocabulary and discard the construct* |
| 94 | +(emit `component_type="interface"` for a trait with no faithful `node_type`). Rejected |
| 95 | +once the split was found: it loses information for no benefit, since setting the extra |
| 96 | +fields is free. |
| 97 | + |
| 98 | +*Alternative considered — emit faithful `"trait"`/`"object"` as `component_type` and |
| 99 | +widen `OOP_TYPES`.* Semantically cleanest, and it would fix the latent PHP, Kotlin, and |
| 100 | +C++ gaps as a side effect. Rejected for this change: widening a globally shared set |
| 101 | +changes documentation output for several shipped languages and can flip the |
| 102 | +generated today reads from the coarse type, which the deferred `display_name` wiring |
| 103 | +with a regression audit, not a rider on a new-language change. |
| 104 | + |
| 105 | +*Alternative considered — emit faithful types as `component_type` and leave `OOP_TYPES` |
| 106 | +alone.* Rejected: it would ship Scala support that produces near-empty documentation for |
| 107 | +idiomatic Scala. |
| 108 | + |
| 109 | +*Scope caveat — `node_type` and `display_name` are currently inert.* Neither flows into |
| 110 | +generated documentation today: `get_display_name()` is never called, the MCP component |
| 111 | +export at `mcp/tools/analysis.py:375` emits `component_type`, and `node_type` is read |
| 112 | +only for the `artifact_file` check at `prompt_template.py:429` and a `"method"` test in |
| 113 | +`call_graph_analyzer.py`. So setting these fields preserves the construct in the graph |
| 114 | +and the exported artifacts at no cost, but it does **not** by itself make the docs say |
| 115 | +"trait" rather than "interface". Surfacing faithful labels in documentation means wiring |
| 116 | +`display_name` through the prompt path — a small change, deliberately not in this scope. |
| 117 | +This design leaves the seam clean for it, and for the deferred `OOP_TYPES` work. |
| 118 | + |
| 119 | +### 2. Suffix companion objects in the component identifier |
| 120 | + |
| 121 | +Component IDs follow `relpath::Name` / `relpath::Class.method`. In Scala a companion |
| 122 | +object is a *top-level peer* sharing its class's name, so `class Buffer` and |
| 123 | +`object Buffer` both claim `buffer.scala::Buffer`. Because components are stored by |
| 124 | +plain dict assignment with no collision check, the later-parsed declaration silently |
| 125 | +displaces the earlier one — the class disappears from the graph while its methods remain |
| 126 | +parented to the surviving object's node. |
| 127 | + |
| 128 | +The object therefore takes a suffixed identifier: `buffer.scala::Buffer, mirroring |
| 129 | +Scala's own JVM encoding of module classes. Members follow their owner |
| 130 | +(`buffer.scala::Buffer.push`, `buffer.scala::Buffer$.apply`). |
| 131 | + |
| 132 | +*Why:* both declarations survive as distinct components with their own source ranges and |
| 133 | +documentation, and every edge stays attributable to the declaration that produced it. |
| 134 | +The ` convention is one Scala developers already recognize from stack traces. |
| 135 | + |
| 136 | +*Alternative considered — merge the companion's members into the class as static |
| 137 | +members.* Closer to how developers think about companions, but it conflates two |
| 138 | +disjoint source ranges into one component, so the generated documentation would show one |
| 139 | +declaration's source text under a component holding the other's members. |
| 140 | + |
| 141 | +*Note:* this is a local encoding choice, not a fix for the general silent-overwrite |
| 142 | +defect, which is deferred. |
| 143 | + |
| 144 | +### 3. Use the dedicated `tree-sitter-scala` package |
| 145 | + |
| 146 | +*Why:* every one of the ten existing analyzers imports its own `tree_sitter_<lang>` |
| 147 | +module and constructs a `Parser` from it. Following that pattern keeps the new analyzer |
| 148 | +reviewable against its siblings. |
| 149 | + |
| 150 | +*Alternative considered — `tree_sitter_language_pack`,* already a dependency and already |
| 151 | +capable of Scala. Rejected as inconsistent with the established per-language pattern; |
| 152 | +switching parser sourcing is a repo-wide decision, not a Scala one. |
| 153 | + |
| 154 | +Version 0.26.2 introduces no `tree-sitter` core constraint in practice: its only core |
| 155 | +pin is the optional `core` extra (`tree-sitter~=0.22`), which the pinned 0.23.2 already |
| 156 | +satisfies. Wheels cover every CI platform. The grammar covers Scala 2 and 3, confirmed |
| 157 | +against its published node types, which include `trait_definition`, |
| 158 | +`object_definition`, `package_object`, `enum_definition`, `given_definition`, and |
| 159 | +`extension_definition`. |
| 160 | + |
| 161 | +### 4. Register the language at every gate, not just the analyzer |
| 162 | + |
| 163 | +The whitelists in `analysis_service.py` (both the filter and the reported |
| 164 | +supported-language list), the CLI's `SUPPORTED_EXTENSIONS` and language-detection map, |
| 165 | +and the MCP incremental `source_extensions` set are all independent allowlists. Each |
| 166 | +needs `.scala`/`.sc` or `"scala"` added. The Ruby precedent touched all of them; the |
| 167 | +original requirements write-up named only some. |
| 168 | + |
| 169 | +*Why it matters:* the analyzer being correct is not sufficient for the feature to work. |
| 170 | +Missing `repo_validator.py` alone makes the CLI reject a pure-Scala repository outright. |
| 171 | + |
| 172 | +### 5. Treat `.sbt` as a build artifact, never as analyzable source |
| 173 | + |
| 174 | +`build.sbt` and `project/*.sbt` join the manifest and build classifications in |
| 175 | +`artifact.py` alongside the existing `pom.xml`, `build.gradle`, and `build.gradle.kts` |
| 176 | +entries. `.sbt` is *not* added to `CODE_EXTENSIONS`. |
| 177 | + |
| 178 | +*Why:* `.sbt` files are syntactically Scala but semantically build configuration. |
| 179 | +Documenting them as application source would misrepresent the architecture, while |
| 180 | +leaving them unclassified means an sbt project's build story goes undocumented — a |
| 181 | +visible gap now that artifact-aware generation has landed. |
| 182 | + |
| 183 | +`project/*.scala` (sbt meta-build code) remains ordinary Scala source. Special-casing it |
| 184 | +adds a path-shaped exception for modest benefit. |
| 185 | + |
| 186 | +## Risks / Trade-offs |
| 187 | + |
| 188 | +**The Kotlin analyzer is a structural template, not a copy-paste source.** Kotlin's |
| 189 | +grammar uses `class_declaration` / `object_declaration` / `function_declaration`; Scala's |
| 190 | +uses `class_definition` / `object_definition` / `function_definition`. Every `node.type` |
| 191 | +comparison needs rewriting. → Treat Kotlin as the reference for *shape* (node |
| 192 | +extraction, then relationship extraction, module-path derivation, ID construction) and |
| 193 | +derive all node type names from the Scala grammar's own node types. |
| 194 | + |
| 195 | +**Scala 3 significant-indentation syntax may parse poorly.** This is the only unknown |
| 196 | +that could invalidate the extraction approach. → Spike first: parse representative |
| 197 | +Scala 2 and Scala 3 files and assert the tree contains no `ERROR` nodes before building |
| 198 | +extraction on top. Sequenced as the first task. |
| 199 | + |
| 200 | +**Scala's expression-oriented style may produce noisy call graphs.** Heavy chaining, |
| 201 | +higher-order functions, and for-comprehensions generate many call expressions against |
| 202 | +standard-library targets. → Follow the precedent of the existing analyzers and filter |
| 203 | +primitives and common built-ins, as Kotlin and PHP already do with their primitive sets; |
| 204 | +assert the exclusion in tests. |
| 205 | + |
| 206 | +**`.sc` is not exclusive to Scala** — SuperCollider and Scilab also use it. → Accepted: |
| 207 | +the file is parsed with the Scala grammar and a non-Scala file simply yields no |
| 208 | +components, matching how the other analyzers behave on unparseable input. |
| 209 | + |
| 210 | +**Decision 1's coarse type is still a compromise.** A Scala trait carries |
| 211 | +`component_type="interface"`, so any consumer reading only the coarse type sees an |
| 212 | +interface. → Largely mitigated by the split: `node_type="trait"` and |
| 213 | +`display_name="trait Foo"` keep the construct in the graph, so no information is lost |
| 214 | +and a future consumer can render it faithfully. The residual issue is that documentation |
| 215 | +would address. |
| 216 | +`ruby.py:608-611` sets `node_type` and a `"<type> <name>"` `display_name` alongside |
| 217 | +`subtype` (a TS type alias is `component_type="type"`, `node_type="type_alias"`). |
| 218 | + |
| 219 | +## Open Questions |
| 220 | + |
| 221 | +These can be answered during implementation without changing the specs, the approach, |
| 222 | +or the task breakdown: |
| 223 | + |
| 224 | +- **Scala 3 `given` definitions.** Typeclass instances are architecturally meaningful, |
| 225 | + but whether each is a *documentable component* is unclear. Leaning toward extracting |
| 226 | + named givens as `"class"` and skipping anonymous ones. Requires a real Scala 3 |
| 227 | + codebase to judge signal versus noise. |
| 228 | +- **Scala 3 `extension` blocks.** The methods inside are plausibly `"method"` components, |
| 229 | + but their owner is the extended type, which may live outside the repository. |
| 230 | +- **`type` aliases.** The existing `"type_alias"` component type sits outside |
| 231 | + `OOP_TYPES` and would be dropped; likely skip rather than mismap. |
| 232 | +- **`project/build.properties`.** Whether the sbt version pin is worth classifying as a |
| 233 | + build artifact, or is too granular to document. |
0 commit comments