daniellansun commented on PR #2936: URL: https://github.com/apache/groovy/pull/2936#issuecomment-5846110519
> AI read: > > > I think the strategic instinct is right: a language service backed by _the_ > > compiler rather than a re-implementation of it is the one thing no other Groovy tool can offer, > > and the "community servers lag the language" problem is real and measurable. Several of the > > design refusals are right too — no build-tool invocation from the server, refuse-to-guess > > rename, `@Grab` and `@ASTTest` off by default, advertising only what's implemented. > > Given the concerns about the overall direction for this PR, we could look at a first > > Minimal Viable Product (MVP) for this work. One concrete suggestion is to > > drop the public extension SPI from the first cut and keep the hook > > points as internal seams. Mostly agreeing with Jochen here: > > > > * The only in-tree consumer is the STC pass, which is core behaviour rather than an extension. > > * It's already accreting — three pairs of telescoping overloads on a brand-new interface, with > > no compatibility burden yet to justify carrying both. > > * It leaks the transport: the signatures use LSP4J types, but LSP4J is declared `implementation` > > rather than `api`, so an implementor can't compile against the published metadata without > > adding LSP4J themselves — and any in-process consumer would need a JSON-RPC library to ask a > > question about an AST. > > > > Adding an API later is easy; withdrawing one isn't. > > I don't think that costs you the reuse story so much as relocate it. The durable asset looks to > > me like a **language-services library, with the LSP server as its first client**: given sources > > and a classpath, answer "what is at this position", "what can follow here", "what is wrong", > > "where is this used" — in plain Groovy and JDK types, in-process, no JSON-RPC, no file-URI > > assumption, with a session context. That's how rust-analyzer (`ide` under the LSP crate) and > > Kotlin (Analysis API under the tooling) are built, and your code is already nearly that shape: > > `feature/*` and `compile/*` are the library, `protocol/*` is the adapter. The SPI lets strangers > > push things into our server; the library lets our own tools — Console, groovysh, a notebook > > kernel, an MCP server — pull answers out, and that's the side with customers today. I think it > > also gives Jochen's "what is its role" question a sharper answer than we've managed so far. > > I have notes on two other things which I'll write up on GROOVY-12411 rather than lengthen this > > thread: a trust question around classpath inference from workspace contents that I'd want closed > > before any release, and a set of tie-ins to tools we already have. The one I'd flag now as worth > > aiming at is surfacing type-checking extension results as editor diagnostics — `NullChecker`, > > `PurityChecker`, `SqlInjectionChecker`, groovy-verify — since that's the most differentiating > > thing a compiler-backed server can do, and something IDEA can't model without running them. > > None of this is a request to make the PR bigger — mostly the opposite. Happy to help with any of > > the pieces. Thank you, Paul. That MVP cut is what we have now implemented. The public `GroovyLspExtension` SPI is gone from this first cut: no `ServiceLoader`, no `GroovyLspSession`, and no LSP4J types on a published extension surface. The only in-tree consumer was the STC pass; that now runs as core `StaticTypeCheckingPass` after `SEMANTIC_ANALYSIS`. Adding a public API later remains easy; withdrawing one is not. The reuse story is the in-process library you described. `feature/*` and `compile/*` answer questions about sources; `protocol/*` is the LSP adapter. Console, groovysh, a notebook kernel, or a later MCP process can pull from that engine without JSON-RPC. We are not inviting other language servers to push plugins into this process. The shape matches what lathe does with a compiler core, an LSP server, and an MCP server as separate clients of the same engine. Type-checker diagnostics (`NullChecker`, `PurityChecker`, …) and the classpath-inference trust notes stay on GROOVY-12411 as follow-ups. This change makes the PR smaller. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
