Copilot commented on code in PR #3998:
URL:
https://github.com/apache/incubator-kie-tools/pull/3998#discussion_r4002308732
##########
packages/drools-lsp/drools-completion/src/main/java/org/drools/completion/DRLWorkspaceTypeIndex.java:
##########
@@ -256,6 +257,58 @@ static void forEachSiblingFile(Path documentPath,
Map<Path, String> openFiles,
}
}
+ /**
+ * Returns the imports contributed by sibling files that declare the
+ * <em>same</em> Drools package as the current document. Drools merges
every
+ * file sharing a package into one namespace, so an import declared in any
+ * same-package sibling is in scope here — a consumer that would otherwise
+ * flag such a type (the unknown-type lint) must honor them.
+ *
+ * <p>Siblings come from the active {@link WorkspaceSiblingResolver}
alone, so
+ * grouping stays whatever the workspace configured; open unsaved buffers
+ * shadow their on-disk counterpart, matching {@link #forEachSiblingType}.
+ * A null/blank {@code ownPackage} contributes nothing — a package-less
file
+ * merges with no other file. Wildcard imports keep their {@code .*}
suffix.
+ */
+ public static List<String> siblingImports(Path documentPath, String
ownPackage,
+ Map<Path, String> openFiles) {
+ if (documentPath == null || ownPackage == null ||
ownPackage.isBlank()) {
+ return List.of();
+ }
+ String pkg = ownPackage.trim();
+ Path docNorm = documentPath.toAbsolutePath().normalize();
+ Path dir = docNorm.getParent();
+ Set<Path> shadowed = new HashSet<>();
+ List<String> imports = new ArrayList<>();
+
+ // Layer 2: open unsaved siblings (buffer content shadows disk).
+ if (openFiles != null) {
+ for (Map.Entry<Path, String> e : openFiles.entrySet()) {
+ Path p = normalizedSibling(e.getKey(), docNorm, dir);
+ if (p == null) {
+ continue;
+ }
+ shadowed.add(p);
+ DRLDeclaredTypeParser.FileInfo info =
DRLDeclaredTypeParser.parseFileInfo(e.getValue());
Review Comment:
Open sibling buffers are still parsed twice per lint run.
`forEachSiblingType` first calls `parseDeclaredTypes(e.getValue())`, then this
call parses the same text again for imports; only disk files benefit from the
mtime cache. This contradicts the stated single-parse requirement and adds
repeated ANTLR work on each edit. Collect each buffer's `FileInfo` once and
reuse both `types` and `imports`.
##########
packages/drools-lsp/drools-completion/src/main/java/org/drools/completion/DRLWorkspaceTypeIndex.java:
##########
@@ -256,6 +257,58 @@ static void forEachSiblingFile(Path documentPath,
Map<Path, String> openFiles,
}
}
+ /**
+ * Returns the imports contributed by sibling files that declare the
+ * <em>same</em> Drools package as the current document. Drools merges
every
+ * file sharing a package into one namespace, so an import declared in any
+ * same-package sibling is in scope here — a consumer that would otherwise
+ * flag such a type (the unknown-type lint) must honor them.
+ *
+ * <p>Siblings come from the active {@link WorkspaceSiblingResolver}
alone, so
+ * grouping stays whatever the workspace configured; open unsaved buffers
+ * shadow their on-disk counterpart, matching {@link #forEachSiblingType}.
+ * A null/blank {@code ownPackage} contributes nothing — a package-less
file
+ * merges with no other file. Wildcard imports keep their {@code .*}
suffix.
+ */
+ public static List<String> siblingImports(Path documentPath, String
ownPackage,
+ Map<Path, String> openFiles) {
+ if (documentPath == null || ownPackage == null ||
ownPackage.isBlank()) {
+ return List.of();
+ }
+ String pkg = ownPackage.trim();
+ Path docNorm = documentPath.toAbsolutePath().normalize();
+ Path dir = docNorm.getParent();
+ Set<Path> shadowed = new HashSet<>();
+ List<String> imports = new ArrayList<>();
+
+ // Layer 2: open unsaved siblings (buffer content shadows disk).
+ if (openFiles != null) {
+ for (Map.Entry<Path, String> e : openFiles.entrySet()) {
+ Path p = normalizedSibling(e.getKey(), docNorm, dir);
+ if (p == null) {
+ continue;
+ }
Review Comment:
The open-buffer layer does not use the active resolver at all:
`normalizedSibling` accepts every same-directory buffer and rejects every
cross-directory buffer. With a configured KIE-base grouping, this can import a
type from a file the resolver excluded, while an open cross-directory sibling
fails to shadow its on-disk content. Determine membership from
`resolveSiblings(documentPath)` and use those normalized paths for both buffer
inclusion and disk shadowing.
##########
packages/drools-lsp/drools-completion/src/test/java/org/drools/completion/DRLLintHelperTest.java:
##########
@@ -799,4 +819,144 @@ void lowercasePropertyTailIsNotChased() {
+ "rule R\n when\n Animal( legs == PetKind.CAT.ordinal )\n
then\nend\n";
assertThat(lintUnknownTypes(text)).isEmpty();
}
+
+ // ── sibling imports (same-package) ───────────────────────────────────
+
+ private static final String USES_ORDER =
+ "package demo;\nrule R\n when\n Order( )\n then\nend\n";
+
+ /** Builds a class index from empty {@code .class} files for {@code
fqcns}. */
+ private static ClassIndex classIndexOf(Path tempDir, String... fqcns)
throws IOException {
+ Path classesDir = tempDir.resolve("classes");
+ Files.createDirectories(classesDir);
+ for (String fqcn : fqcns) {
+ Path classFile = classesDir.resolve(fqcn.replace('.', '/') +
".class");
+ Files.createDirectories(classFile.getParent());
+ Files.createFile(classFile);
+ }
+ return ClassIndex.build(Set.of(classesDir));
+ }
+
+ @Test
+ void siblingImportLegalizesPatternType(@TempDir Path tempDir) throws
IOException {
+ // A same-package sibling imports Order; the current file uses it as a
+ // pattern type without importing it itself. The exact sibling import
+ // makes it resolvable, so no unknown-type diagnostic fires.
+ Path current = tempDir.resolve("current.drl");
+ Files.writeString(current, USES_ORDER);
+ Files.writeString(tempDir.resolve("sibling.drl"),
+ "package demo;\nimport com.example.model.Order;\n");
+
+ List<Diagnostic> diags = DRLLintHelper.lintUnknownTypes(
+ Files.readString(current), current, Map.of(),
ClassIndex.empty(), members, true);
+
+ assertThat(diags).isEmpty();
+ }
+
+ @Test
+ void siblingWildcardImportLegalizesThroughClassIndex(@TempDir Path
tempDir) throws IOException {
+ // A wildcard sibling import (com.example.model.*) legalizes Order only
+ // when the class index confirms that package provides it. Two Order
+ // classes make the bare simple name ambiguous, so only the wildcard's
+ // package can disambiguate it — a path local wildcard imports never
+ // exercise (their extraction yields the bare package name).
Review Comment:
This comment is now outdated: the same PR reconstructs `.*` for local
imports, and the new completion tests verify that local wildcard imports do
exercise this resolution path. Describe this as the sibling wildcard path
rather than claiming local extraction still yields a bare package.
--
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]
