drccrd opened a new pull request, #3998: URL: https://github.com/apache/incubator-kie-tools/pull/3998
Closes https://github.com/apache/incubator-kie-tools/issues/3713 ### What changes Drools merges every DRL file sharing a package into one namespace, so an `import` declared in any file of the package is legal in every file of it. The unknown-type lint read only the current document's imports, so a type imported by a sibling was flagged as unknown even though it compiles. The lint now unions the imports contributed by same-package siblings. Siblings come from the active `WorkspaceSiblingResolver`, so grouping follows whatever the workspace configured rather than assuming a directory; open unsaved buffers shadow their on-disk counterparts. A sibling that declares a *different* package contributes nothing, and a document with no package declaration merges with nothing. Resolution itself is unchanged: sibling imports are threaded into `resolveFqcn` through a new five-argument overload, and the existing four-argument overload delegates to it with an empty collection, so hover, completion and go-to-definition behave exactly as before. ### One parse, not two The issue asks whether this can be done with a single parse rather than two. It can, and is. `DRLDeclaredTypeParser` gains a `FileInfo` (package name, imports, declared types) served by `cachedFileInfo`, sharing the one mtime-keyed cache that already backed declared-type lookup. A sibling is therefore parsed once per change and that parse answers both the declares walk and the imports walk. While there, `DRLLintHelper`'s local package-name extractor was folded into `DRLDeclaredTypeParser.extractPackageName` — the two were identical and in the same package. ### Companion fix: local wildcard imports `resolveFqcn` has a wildcard-import branch guarded by `endsWith(".*")`, and for the current document that branch was unreachable. The grammar carries the `.*` of a wildcard import as a separate `(DOT MUL)` outside `drlQualifiedName`, so `DRLCompletionHelper.extractImports` yielded the bare package name and nothing ever matched `.*`. Local wildcard imports only ever resolved through the unambiguous-class-index fallback, which means a simple name shared by two classpath types resolved for the compiler but not for the LSP. `extractImports` now reconstructs the suffix, mirroring the sibling-side extractor this PR adds. Two consequences worth stating rather than leaving in the diff: - **This changes hover/completion resolution for ambiguous names**, in the direction of matching the compiler: under `import com.acme.model.*`, a simple name that also exists in another package now resolves to the wildcard's package instead of resolving to nothing. The branch still verifies through the class index, so a wildcard cannot resolve a type its package does not provide. - It also removes a latent false positive in completion sort priority. That check is exact-FQCN membership, so the bare package `com.acme.model` would match a class whose FQCN is literally that (a class named `model` in package `com.acme`) and boost it as though explicitly imported. `com.acme.model.*` cannot collide with any FQCN. ### Deliberately out of scope Classes reachable *via* a wildcard import still get no completion sort boost — the priority check is exact-FQCN equality, so they rank alongside entirely unimported classes. That is unchanged by this PR and needs the sort check to test package prefixes against `.*` entries, which is a completion-ranking decision with no bearing on the lint. Happy to file it separately. ### Verification `mvn test` in `packages/drools-lsp`: drools-completion 379, drools-lsp-server 52, all green, `BUILD SUCCESS`. Nine of those are new — seven covering the sibling-import behaviour (exact import, wildcard-through-class-index, different-package sibling, unsaved-buffer import, unsaved buffer *removing* an on-disk import, classpath gating, and a package-less document) and two covering local wildcard resolution and its package boundary. No client files are touched. The wildcard fix was written test-first: the new assertion failed with `expected: "com.acme.model.Order" but was: null` before the one-line change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
