Copilot commented on code in PR #3717: URL: https://github.com/apache/incubator-kie-tools/pull/3717#discussion_r3801287812
########## packages/drools-lsp/drools-completion/src/main/java/org/drools/completion/DrlFileGrouping.java: ########## @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.drools.completion; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.logging.Logger; + +/** + * A workspace's resolved DRL file groups: group name to the files that compile + * together, and the reverse lookup used on the completion path. + * + * <p>Immutable. Rebuilt when the workspace root changes or a config file is + * edited, never mutated in place. + */ +final class DrlFileGrouping { + + private static final Logger logger = Logger.getLogger(DrlFileGrouping.class.getName()); + + static final DrlFileGrouping EMPTY = new DrlFileGrouping(Map.of(), Map.of()); + + private final Map<String, WorkspaceSiblingResolver.Group> groupsByName; + private final Map<Path, String> groupByFile; + + private DrlFileGrouping(Map<String, WorkspaceSiblingResolver.Group> groupsByName, + Map<Path, String> groupByFile) { + this.groupsByName = groupsByName; + this.groupByFile = groupByFile; + } + + boolean isEmpty() { + return groupsByName.isEmpty(); + } + + /** Files in {@code groupName}, in declaration order; empty when unknown. */ + List<Path> filesFor(String groupName) { + WorkspaceSiblingResolver.Group group = groupsByName.get(groupName); + return (group == null) ? List.of() : group.files(); + } + + /** The group {@code file} belongs to, or {@code null} when it is ungrouped. */ + String groupFor(Path file) { + return (file == null) ? null : groupByFile.get(file.toAbsolutePath().normalize()); + } + + Map<String, WorkspaceSiblingResolver.Group> asMap() { + return groupsByName; + } + + /** + * Resolves declarations into concrete file lists. + * + * <p>Declarations sharing a name are merged rather than shadowing one + * another — several config files legitimately contribute to one group, and + * silently keeping only the first hides half of it. Their patterns and files + * are unioned and a warning names the group, because a merge is just as + * often two groups that accidentally share a name. + * + * <p>A file claimed by more than one group is indexed to the first that + * claims it, so lookup stays deterministic; the ambiguity is surfaced to the + * user through {@link #asMap()} rather than resolved silently here. + */ + static DrlFileGrouping resolve(List<KieBaseDecl> declarations, List<Path> drlFiles, Path workspaceRoot) { + if (declarations.isEmpty()) { + return EMPTY; + } + Map<String, KieBaseDecl> merged = mergeByName(declarations); + + PackageIndex packages = new PackageIndex(workspaceRoot); + boolean anySelectsByPackage = merged.values().stream().anyMatch(KieBaseDecl::selectsByPackage); + + Map<String, WorkspaceSiblingResolver.Group> groupsByName = new LinkedHashMap<>(); + Map<Path, String> groupByFile = new LinkedHashMap<>(); + + for (KieBaseDecl decl : merged.values()) { + Set<Path> files = new LinkedHashSet<>(decl.files()); + if (anySelectsByPackage && decl.selectsByPackage()) { + for (Path drl : drlFiles) { + if (KieBasePackages.matchesWithIncludes(decl, merged, packages.packageOf(drl))) { + files.add(drl); Review Comment: A kmodule-derived declaration is applied to every DRL in the workspace. In a multi-module workspace, a `packages="com.example.*"` kbase in module A therefore claims matching DRLs from module B, although each `kmodule.xml` only governs its own KIE module. Global name merging also collapses common kbase names across modules. Preserve the descriptor/module resource root on kmodule declarations and restrict matching/includes and identity to that module. ########## packages/drools-lsp/drools-lsp-server/src/main/java/org/drools/lsp/server/DroolsLspServer.java: ########## @@ -263,6 +296,166 @@ public CompletableFuture<InitializeResult> initialize(InitializeParams params) { return CompletableFuture.supplyAsync(() -> initializeResult); } + /** + * Returns the workspace's DRL file groups, keyed by name, so a client can + * show which group the open file is in and offer the rest. + * + * <p>The client asks rather than reading the config files itself: the server + * already resolves kmodule descriptors, config files and adopted manifests, + * and a second implementation of that in the client is a second place for it + * to be wrong. + */ + @JsonRequest("drools/fileGroups") + public CompletableFuture<Map<String, FileGroupingProtocol.FileGroup>> fileGroups() { + return CompletableFuture.supplyAsync(() -> { + Map<String, FileGroupingProtocol.FileGroup> groups = new LinkedHashMap<>(); + WorkspaceSiblingResolvers.active().resolveAllGroups().forEach((name, group) -> { + List<String> uris = new ArrayList<>(group.files().size()); + for (Path file : group.files()) { + uris.add(file.toUri().toString()); + } + Path declaredIn = group.declaredIn(); + groups.put(name, new FileGroupingProtocol.FileGroup(uris, group.kind(), + declaredIn == null ? null : declaredIn.toUri().toString())); + }); + return groups; + }); + } + + /** + * Pins a document to a named group, overriding what the configuration + * resolves it to. A file can belong to several groups, so this is how the + * user settles which one the editor works in. + */ + @JsonNotification("drools/setFileGroup") + public void setFileGroup(FileGroupingProtocol.FileGroupParams params) { + if (params == null || params.getUri() == null) { + return; + } + try { + WorkspaceSiblingResolvers.active() + .setGroupOverride(Paths.get(URI.create(params.getUri())), params.getGroup()); + // Pinning changes what is in scope, and diagnostics here are pulled + // rather than pushed, so nothing would re-ask on its own. + refreshDiagnostics(); + } catch (Exception e) { + logger.log(Level.WARNING, "Failed to pin " + params.getUri() + " to a DRL file group", e); + } + } + + /** Re-reads the workspace's grouping configuration after a config file changes. */ + @JsonNotification("drools/reloadFileGroups") + public void reloadFileGroups() { + WorkspaceSiblingResolvers.active().reload(); + notifyFileGroupsChanged(); + } + + /** + * Replaces the grouping declared in the editor's settings, so a user editing + * {@code drools.lsp.grouping} sees the effect without restarting the server. + */ + @JsonNotification("drools/setGroupingConfig") + public void setGroupingConfig(FileGroupingProtocol.GroupingConfigParams params) { + JsonObject config = (params == null) ? null : params.getConfig(); + WorkspaceSiblingResolvers.active().setSettingsConfig(config == null ? null : config.toString()); + notifyFileGroupsChanged(); + } + + /** + * Tells the client the group map changed, so it can re-read it. + * + * <p>Sent through the raw endpoint because this is a custom method the + * {@link LanguageClient} interface does not declare. A client that is not an + * lsp4j proxy — a test double, say — simply does not get told. + */ + private void notifyFileGroupsChanged() { + if (client instanceof Endpoint endpoint) { + endpoint.notify("drools/fileGroupsChanged", null); + } Review Comment: `client` is the remote proxy created as `Launcher<LanguageClient>` in `Main.java:60` and `DroolsLspTCPLauncher.java:67`; that proxy implements `LanguageClient`, not `Endpoint`, so this branch is false in the real server. Consequently `drools/fileGroupsChanged` is never sent, and a startup request made before the async scan completes stays stale. Expose the custom notification on a client interface used by both launchers (or retain the launcher's raw remote endpoint) and invoke it directly. ########## packages/drools-lsp/drools-completion/src/main/java/org/drools/completion/KieBasePackages.java: ########## @@ -0,0 +1,147 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.drools.completion; + +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +/** + * Decides whether a DRL package belongs to a {@link KieBaseDecl}, following the + * {@code packages} attribute semantics of {@code kmodule.xml}. + * + * <p><strong>This is a port, not an invention.</strong> The normative + * implementation is + * {@code org.drools.compiler.kie.builder.impl.KieBuilderImpl#isPackageInKieBase} + * (and {@code #isPackageInKieBaseOrIncludedKieBases} for {@code includes}). It is + * reproduced here rather than called because {@code drools-completion} carries no + * dependency on {@code drools-compiler}, and pulling the rule compiler into the + * language-server process to answer a string-matching question is not a trade + * worth making. Any change to the compiler's rules should be mirrored here. + * + * <p>Supported pattern forms, evaluated in declaration order: + * <ul> + * <li>{@code *} — every package.</li> + * <li>{@code com.example.rules} — that package exactly, or any package ending + * in {@code .com.example.rules}. An unqualified {@code rules} therefore + * matches {@code com.example.rules} as a suffix.</li> + * <li>{@code com.example.*} — {@code com.example} and everything beneath it. + * Matched against the package name with a build-tree prefix + * ({@code src.main.resources.}, {@code BOOT-INF.classes.}) stripped, and + * again with the group's own name stripped, so patterns may be written + * relative to the group.</li> + * <li>A leading {@code !} negates any of the above.</li> + * </ul> + * + * <p><strong>The first pattern that matches decides, sign included.</strong> A + * negation only excludes if no earlier pattern already matched, so + * {@code ["com.example.*", "!com.example.internal.*"]} does <em>not</em> exclude + * {@code com.example.internal} — the exclusion has to be listed first. This is + * the compiler's behavior and the most common way a {@code packages} attribute + * surprises its author. + */ +final class KieBasePackages { + + /** + * Prefixes stripped from a package name before wildcard matching, so that a + * pattern works whether the package was derived from a source tree or from + * an already-packaged artifact. Mirrors {@code SUPPORTED_RESOURCES_ROOTS}. + */ + private static final String[] RESOURCE_ROOT_PREFIXES = {"src.main.resources.", "BOOT-INF.classes."}; + + private KieBasePackages() { + } + + /** + * Returns whether {@code packageName} is claimed by {@code kbase}'s own + * {@code packages} patterns, ignoring {@code includes}. A declaration with + * no patterns claims every package, as an empty {@code packages} attribute + * does in {@code kmodule.xml}. + */ + static boolean matches(KieBaseDecl kbase, String packageName) { + if (kbase.packages().isEmpty()) { + return true; + } + String pkgName = (packageName == null) ? "" : packageName; + + for (String candidate : kbase.packages()) { + boolean negated = candidate.startsWith("!"); + String pattern = negated ? candidate.substring(1) : candidate; + + if (pattern.equals("*") || pkgName.equals(pattern) || pkgName.endsWith("." + pattern)) { + return !negated; + } + if (pattern.endsWith(".*")) { + String stem = pattern.substring(0, pattern.length() - 2); + String relative = stripResourceRoot(pkgName); + if (isAtOrUnder(relative, stem)) { + return !negated; + } + // A pattern may also be written relative to the group's own name. + String selfPrefix = kbase.name() + "."; + if (relative.startsWith(selfPrefix) + && isAtOrUnder(relative.substring(selfPrefix.length()), stem)) { + return !negated; + } + } + } + return false; + } + + /** + * As {@link #matches}, but also consults the groups named by + * {@code kbase.includes()}, transitively. {@code allByName} supplies the + * workspace's other declarations; unknown include names are ignored, and + * include cycles terminate. + */ + static boolean matchesWithIncludes(KieBaseDecl kbase, Map<String, KieBaseDecl> allByName, String packageName) { + return matchesWithIncludes(kbase, allByName, packageName, new HashSet<>()); + } + + private static boolean matchesWithIncludes(KieBaseDecl kbase, Map<String, KieBaseDecl> allByName, + String packageName, Set<String> visited) { + if (!visited.add(kbase.name())) { + return false; + } + if (matches(kbase, packageName)) { + return true; + } Review Comment: An explicitly listed group has `selectsByPackage == false` and an empty package list. If another group includes it, this recursive call currently treats that empty list as “match every package,” so the including group claims every DRL in the workspace. Only evaluate a declaration's own package patterns when it actually selects by package. ########## packages/drools-lsp/drools-completion/src/main/java/org/drools/completion/ConfiguredGroupingResolver.java: ########## @@ -0,0 +1,265 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.drools.completion; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.logging.Logger; + +/** + * The {@link WorkspaceSiblingResolver} shipped with the server: it groups DRL + * files the way the project's own configuration says they compile together. + * + * <p>Before this existed the SPI had no implementation that could be installed + * without running custom Java inside the server process, which meant grouping + * other than by directory required shipping a patched server. Configuration is + * now enough. + * + * <p>Groups are resolved from two tiers, in precedence order: + * <ol> + * <li>{@value WorkspaceScan#CONFIG_FILE_NAME} — explicit groups, and existing + * rule manifests adopted through its {@code sources}.</li> + * <li>{@code META-INF/kmodule.xml} — the {@code <kbase>} declarations the + * project already builds with.</li> + * </ol> + * + * <p>The kmodule tier is what makes grouping work with no configuration at all + * for a conventional project: it reads the same {@code packages} attribute the + * compiler uses to decide which resources form a knowledge base, so the editor's + * idea of scope matches the build's by construction rather than by convention. + * + * <p>Files that no group claims — and workspaces with no configuration at all — + * fall through to the registry's same-directory default, so installing this + * resolver can only ever add grouping, never take it away. + */ +public final class ConfiguredGroupingResolver implements WorkspaceSiblingResolver { + + private static final Logger logger = Logger.getLogger(ConfiguredGroupingResolver.class.getName()); + + /** Group chosen by the user for a specific file, overriding what the config says. */ + private final Map<Path, String> overrides = new ConcurrentHashMap<>(); + + private volatile DrlFileGrouping grouping = DrlFileGrouping.EMPTY; + private volatile Path workspaceRoot; + + /** + * Grouping declared in the editor's settings rather than a file, as raw + * JSON. Takes precedence over anything on disk, and lets a workspace be + * grouped without committing a config file — the file remains the option + * for a team that wants the grouping shared and reviewed. + */ + private volatile String settingsConfig; + + /** + * The workspace's files as reported by the client, or {@code null} when it + * reported none and this resolver has to find them itself. + */ + private volatile List<Path> providedFiles; + + @Override + public void setWorkspaceRoot(Path workspaceRoot) { + this.workspaceRoot = (workspaceRoot == null) ? null : workspaceRoot.toAbsolutePath().normalize(); + // Overrides are keyed by absolute path and pin a file to a group by + // name; a new workspace shares neither, so they are dropped with it. + overrides.clear(); + reload(); + } + + /** + * Replaces the grouping declared in the editor's settings and reloads. + * {@code null} or blank clears it, falling back to whatever is on disk. + */ + @Override + public void setSettingsConfig(String json) { + this.settingsConfig = json; + reload(); + } + + @Override + public void setWorkspaceFiles(List<Path> files) { + this.providedFiles = (files == null) ? null : List.copyOf(files); + reload(); + } + + /** + * Rebuilds the grouping from disk. Called when the workspace root is set and + * whenever a config file changes, so an edit takes effect without a restart. + */ + @Override + public void reload() { + Path root = workspaceRoot; + grouping = load(root, settingsConfig, providedFiles); + DrlFileGrouping current = grouping; Review Comment: Reloads can run concurrently (the initialization task is asynchronous and LSP notifications/file watchers can trigger additional reloads). Each call snapshots different settings/files, performs a potentially long scan, and then assigns unconditionally, so an older scan finishing last can overwrite newer configuration. Serialize state updates and reloads, or publish only when a generation/snapshot is still current. This issue also appears on line 135 of the same file. ########## packages/drools-lsp/drools-completion/src/main/java/org/drools/completion/DrlFileGrouping.java: ########## @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.drools.completion; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.logging.Logger; + +/** + * A workspace's resolved DRL file groups: group name to the files that compile + * together, and the reverse lookup used on the completion path. + * + * <p>Immutable. Rebuilt when the workspace root changes or a config file is + * edited, never mutated in place. + */ +final class DrlFileGrouping { + + private static final Logger logger = Logger.getLogger(DrlFileGrouping.class.getName()); + + static final DrlFileGrouping EMPTY = new DrlFileGrouping(Map.of(), Map.of()); + + private final Map<String, WorkspaceSiblingResolver.Group> groupsByName; + private final Map<Path, String> groupByFile; + + private DrlFileGrouping(Map<String, WorkspaceSiblingResolver.Group> groupsByName, + Map<Path, String> groupByFile) { + this.groupsByName = groupsByName; + this.groupByFile = groupByFile; + } + + boolean isEmpty() { + return groupsByName.isEmpty(); + } + + /** Files in {@code groupName}, in declaration order; empty when unknown. */ + List<Path> filesFor(String groupName) { + WorkspaceSiblingResolver.Group group = groupsByName.get(groupName); + return (group == null) ? List.of() : group.files(); + } + + /** The group {@code file} belongs to, or {@code null} when it is ungrouped. */ + String groupFor(Path file) { + return (file == null) ? null : groupByFile.get(file.toAbsolutePath().normalize()); + } + + Map<String, WorkspaceSiblingResolver.Group> asMap() { + return groupsByName; + } + + /** + * Resolves declarations into concrete file lists. + * + * <p>Declarations sharing a name are merged rather than shadowing one + * another — several config files legitimately contribute to one group, and + * silently keeping only the first hides half of it. Their patterns and files + * are unioned and a warning names the group, because a merge is just as + * often two groups that accidentally share a name. + * + * <p>A file claimed by more than one group is indexed to the first that + * claims it, so lookup stays deterministic; the ambiguity is surfaced to the + * user through {@link #asMap()} rather than resolved silently here. + */ + static DrlFileGrouping resolve(List<KieBaseDecl> declarations, List<Path> drlFiles, Path workspaceRoot) { + if (declarations.isEmpty()) { + return EMPTY; + } + Map<String, KieBaseDecl> merged = mergeByName(declarations); + + PackageIndex packages = new PackageIndex(workspaceRoot); + boolean anySelectsByPackage = merged.values().stream().anyMatch(KieBaseDecl::selectsByPackage); + + Map<String, WorkspaceSiblingResolver.Group> groupsByName = new LinkedHashMap<>(); + Map<Path, String> groupByFile = new LinkedHashMap<>(); + + for (KieBaseDecl decl : merged.values()) { + Set<Path> files = new LinkedHashSet<>(decl.files()); Review Comment: Explicit `files` entries seed the group without the build-output filtering used by `WorkspaceScan.classify`. A config or adopted manifest that lists `target/classes/...` therefore includes compiled DRL copies even though the PR states build outputs are filtered regardless of discovery source. Apply the same shared build-output predicate to declared files before adding them. ########## packages/drools-lsp/drools-completion/src/main/java/org/drools/completion/ConfiguredGroupingResolver.java: ########## @@ -0,0 +1,265 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.drools.completion; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.logging.Logger; + +/** + * The {@link WorkspaceSiblingResolver} shipped with the server: it groups DRL + * files the way the project's own configuration says they compile together. + * + * <p>Before this existed the SPI had no implementation that could be installed + * without running custom Java inside the server process, which meant grouping + * other than by directory required shipping a patched server. Configuration is + * now enough. + * + * <p>Groups are resolved from two tiers, in precedence order: + * <ol> + * <li>{@value WorkspaceScan#CONFIG_FILE_NAME} — explicit groups, and existing + * rule manifests adopted through its {@code sources}.</li> + * <li>{@code META-INF/kmodule.xml} — the {@code <kbase>} declarations the + * project already builds with.</li> + * </ol> + * + * <p>The kmodule tier is what makes grouping work with no configuration at all + * for a conventional project: it reads the same {@code packages} attribute the + * compiler uses to decide which resources form a knowledge base, so the editor's + * idea of scope matches the build's by construction rather than by convention. + * + * <p>Files that no group claims — and workspaces with no configuration at all — + * fall through to the registry's same-directory default, so installing this + * resolver can only ever add grouping, never take it away. + */ +public final class ConfiguredGroupingResolver implements WorkspaceSiblingResolver { + + private static final Logger logger = Logger.getLogger(ConfiguredGroupingResolver.class.getName()); + + /** Group chosen by the user for a specific file, overriding what the config says. */ + private final Map<Path, String> overrides = new ConcurrentHashMap<>(); + + private volatile DrlFileGrouping grouping = DrlFileGrouping.EMPTY; + private volatile Path workspaceRoot; + + /** + * Grouping declared in the editor's settings rather than a file, as raw + * JSON. Takes precedence over anything on disk, and lets a workspace be + * grouped without committing a config file — the file remains the option + * for a team that wants the grouping shared and reviewed. + */ + private volatile String settingsConfig; + + /** + * The workspace's files as reported by the client, or {@code null} when it + * reported none and this resolver has to find them itself. + */ + private volatile List<Path> providedFiles; + + @Override + public void setWorkspaceRoot(Path workspaceRoot) { + this.workspaceRoot = (workspaceRoot == null) ? null : workspaceRoot.toAbsolutePath().normalize(); + // Overrides are keyed by absolute path and pin a file to a group by + // name; a new workspace shares neither, so they are dropped with it. + overrides.clear(); + reload(); + } + + /** + * Replaces the grouping declared in the editor's settings and reloads. + * {@code null} or blank clears it, falling back to whatever is on disk. + */ + @Override + public void setSettingsConfig(String json) { + this.settingsConfig = json; + reload(); + } + + @Override + public void setWorkspaceFiles(List<Path> files) { + this.providedFiles = (files == null) ? null : List.copyOf(files); + reload(); + } + + /** + * Rebuilds the grouping from disk. Called when the workspace root is set and + * whenever a config file changes, so an edit takes effect without a restart. + */ + @Override + public void reload() { + Path root = workspaceRoot; + grouping = load(root, settingsConfig, providedFiles); + DrlFileGrouping current = grouping; + if (!current.isEmpty()) { + logger.info("DRL file grouping active: " + current.asMap().size() + " group(s) under " + root); + } + } + + @Override + public List<Path> resolveSiblings(Path currentFile) { + if (currentFile == null) { + return List.of(); + } + Path normalized = currentFile.toAbsolutePath().normalize(); + + String groupName = overrides.get(normalized); + if (groupName == null) { + groupName = grouping.groupFor(normalized); + } + if (groupName == null) { + return WorkspaceSiblingResolvers.sameDirectorySiblings(currentFile); + } + List<Path> files = grouping.filesFor(groupName); + if (files.isEmpty()) { + // A pin can name a group that a later config edit removed. + return WorkspaceSiblingResolvers.sameDirectorySiblings(currentFile); + } + List<Path> siblings = new ArrayList<>(files.size()); + for (Path file : files) { + if (!file.equals(normalized)) { + siblings.add(file); + } + } + return Collections.unmodifiableList(siblings); + } + + @Override + public Map<String, Group> resolveAllGroups() { + return grouping.asMap(); + } + + @Override + public void setGroupOverride(Path file, String groupName) { + if (file == null) { + return; + } + Path normalized = file.toAbsolutePath().normalize(); + if (groupName == null || groupName.isBlank()) { + overrides.remove(normalized); + return; + } + overrides.put(normalized, groupName.trim()); + logger.fine(() -> "Pinned " + normalized.getFileName() + " to DRL file group '" + groupName.trim() + "'"); + } + + // ── loading ────────────────────────────────────────────────────────────── + + /** Resolves the workspace's groups. Never {@code null}. */ + private static DrlFileGrouping load(Path workspaceRoot, String settingsConfig, List<Path> providedFiles) { + if (workspaceRoot == null) { + return DrlFileGrouping.EMPTY; + } + // Use the client's file list when it gave one; walking is the fallback. + WorkspaceScan scan = (providedFiles == null) + ? WorkspaceScan.of(workspaceRoot) + : WorkspaceScan.ofProvided(providedFiles); + + List<KieBaseDecl> declarations = new ArrayList<>(); + List<KBaseConfigFile.SourceSpec> sources = new ArrayList<>(); + + // Settings first, so they win the first-wins index over anything on disk. + KBaseConfigFile.Parsed fromSettings = KBaseConfigFile.parseInline(settingsConfig, workspaceRoot); + declarations.addAll(fromSettings.declarations()); + sources.addAll(fromSettings.sources()); + + for (Path configFile : scan.configFiles()) { + KBaseConfigFile.Parsed parsed = KBaseConfigFile.parse(configFile); + declarations.addAll(parsed.declarations()); + sources.addAll(parsed.sources()); + } + declarations.addAll(adopt(sources, workspaceRoot)); + + for (Path kmoduleFile : scan.kmoduleFiles()) { + declarations.addAll(KModuleParser.parse(kmoduleFile)); + } + + if (declarations.isEmpty()) { + logger.fine(() -> "No DRL file grouping declared under " + workspaceRoot + + "; falling back to same-directory grouping"); + return DrlFileGrouping.EMPTY; + } + return DrlFileGrouping.resolve(declarations, scan.drlFiles(), workspaceRoot); + } + + /** + * Resolves each adopted source's globs and reads the manifests they match. + * All sources share one glob walk. + */ + private static List<KieBaseDecl> adopt(List<KBaseConfigFile.SourceSpec> sources, Path workspaceRoot) { + if (sources.isEmpty()) { + return List.of(); + } + Set<String> fileGlobs = new LinkedHashSet<>(); + Set<String> directoryGlobs = new LinkedHashSet<>(); + for (KBaseConfigFile.SourceSpec source : sources) { + fileGlobs.add(source.includeGlob()); + directoryGlobs.addAll(source.pathsRelativeTo()); + } + WorkspaceScan.GlobMatches matches = WorkspaceScan.matching( + workspaceRoot, List.copyOf(fileGlobs), List.copyOf(directoryGlobs)); Review Comment: Even when the client supplies its filtered workspace file list, adopted manifests and their path roots are rediscovered by this independent server-side walk. A manifest under a user-excluded directory can therefore still affect grouping, contradicting the contract that client-withheld files are outside the project. Make adopted-source discovery honor the provided file view, or have the client enumerate the resolved source globs. This issue also appears on line 251 of the same file. ########## packages/drools-lsp/drools-completion/src/main/java/org/drools/completion/DrlPackageReader.java: ########## @@ -0,0 +1,185 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.drools.completion; + +import java.nio.file.Path; + +/** + * Determines the DRL package a file belongs to, which is what + * {@code kmodule.xml} {@code packages} patterns are matched against. + * + * <p>Mirrors {@code KieBuilderImpl#packageNameForFile}: the {@code package} + * declaration inside the file wins, and its location on disk is the fallback. + * The declaration is found by a comment-aware scan rather than a full parse — + * this runs once per DRL in the workspace, and the answer is a single + * dotted name. + */ +final class DrlPackageReader { + + /** + * Directory segments that mark the start of a package hierarchy. Matched in + * order; the last occurrence in the path wins, so a nested module resolves + * against its own root. + */ + private static final String[] PACKAGE_ROOTS = { + "/src/main/resources/", + "/src/test/resources/", + "/target/classes/", + "/target/test-classes/", + "/BOOT-INF/classes/", + }; + + private DrlPackageReader() { + } + + /** + * Returns the package named by the file's {@code package} declaration, or + * {@code null} when it has none. Declarations inside comments and string + * literals are ignored. + */ + static String declaredPackage(String drlText) { + if (drlText == null || drlText.isEmpty()) { + return null; + } + int at = indexOfPackageKeyword(drlText); + if (at < 0) { + return null; + } + int from = at + "package".length(); + int semicolon = drlText.indexOf(';', from); + int newline = drlText.indexOf('\n', from); + int end; + if (semicolon > 0) { + end = (newline > 0) ? Math.min(semicolon, newline) : semicolon; + } else { + end = newline; + } + if (end < 0) { + end = drlText.length(); + } + String name = drlText.substring(from, end).trim(); + return name.isEmpty() ? null : name; + } + + /** + * Locates the {@code package} keyword outside comments and string literals, + * requiring it to stand as a whole word. Returns {@code -1} when absent. + */ + private static int indexOfPackageKeyword(String text) { + boolean inLineComment = false; + boolean inBlockComment = false; + char stringDelimiter = 0; + + for (int i = 0; i < text.length(); i++) { + char c = text.charAt(i); + + if (inLineComment) { + if (c == '\n') { + inLineComment = false; + } + continue; + } + if (inBlockComment) { + if (c == '*' && i + 1 < text.length() && text.charAt(i + 1) == '/') { + inBlockComment = false; + i++; + } + continue; + } + if (stringDelimiter != 0) { + if (c == '\\') { + i++; + } else if (c == stringDelimiter) { + stringDelimiter = 0; + } + continue; + } + if (c == '/' && i + 1 < text.length()) { + char next = text.charAt(i + 1); + if (next == '/') { + inLineComment = true; + i++; + continue; + } + if (next == '*') { + inBlockComment = true; + i++; + continue; + } + } + if (c == '"' || c == '\'') { + stringDelimiter = c; + continue; + } + if (c == 'p' && text.startsWith("package", i) + && isWordBoundaryBefore(text, i) + && isWordBoundaryAfter(text, i + "package".length())) { + return i; + } Review Comment: Whole-word boundaries do not establish declaration syntax. For a DRL without a declaration, `import com.package.Type;` matches here and `declaredPackage` returns `.Type`, causing incorrect kbase membership. The referenced `KieBuilderImpl` searches for `"package "`; likewise require whitespace after the keyword so qualified-name segments cannot match. ########## packages/drl-vscode-extension/src/fileGrouping.ts: ########## @@ -0,0 +1,355 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import * as vscode from "vscode"; +import { LanguageClient } from "vscode-languageclient/node"; + +/** + * Shows which group of DRL files the open document compiles with, and lets the + * user pin it to a different one. + * + * The group map comes from the server (`drools/fileGroups`) rather than being + * re-derived here. The server already reads kmodule descriptors, the + * `drl-lsp-kbases.json` config and any manifests it adopts; parsing those a + * second time in the client would only create a second thing to keep correct. + */ + +type Logger = { + info: (msg: string) => void; + error: (msg: string) => void; +}; + +/** + * One group as the server reports it. `kind` is set only when the server can be + * more specific than "group" — "KIE base" for a group read from a kmodule.xml — + * so a project that never declared a kmodule is never shown kmodule vocabulary. + */ +type FileGroup = { + /** Normalized paths, for membership tests. */ + files: string[]; + kind?: string; + /** The file that declared the group, for answering "why is this file here?". */ + declaredIn?: string; +}; + +/** Group name to what the server last reported for it. */ +let groups = new Map<string, FileGroup>(); +/** Document fsPath to the group the user pinned it to. Persisted per workspace. */ +let overrides = new Map<string, string>(); +let statusItem: vscode.StatusBarItem | undefined; +let log: Logger = { info: () => undefined, error: () => undefined }; + +const OVERRIDES_STATE_KEY = "drools.fileGroupOverrides"; +const CONFIG_FILE_GLOB = "**/{drl-lsp-kbases.json,kmodule.xml}"; Review Comment: This watcher covers only the main config and kmodule descriptor. Files adopted through `sources[].include` can have arbitrary names, so editing, creating, or deleting an adopted manifest never reloads its groups, despite the live-reload behavior. Register watchers for the active source globs (or forward a suitable broader file-change stream to the server). This issue also appears on line 346 of the same file. ########## packages/drl-vscode-extension/README.md: ########## @@ -53,11 +53,71 @@ Language support for [DRL (Drools Rule Language)](https://kie.apache.org/docs/10 - Hover tooltips for DRL/Java types with doc-comment rendering - Reference-count code lens for DRL declared types +## File Grouping + +Scopes completion, navigation and validation to the files a rule compiles with, so `declare` types, imports, functions and `global` declarations resolve across the group. Resolved in this order: + +1. The `drools.lsp.grouping` setting +2. `drl-lsp-kbases.json` anywhere in the workspace — same content as the setting, for committed grouping +3. `META-INF/kmodule.xml` — the `packages` and `includes` attributes the build already uses +4. The containing directory of the active file, if none of the above + +Changes apply without a restart. The status bar shows the active group, and pins one when a file matches several; pins persist per workspace, and the tooltip names the declaring file. Groups from a `kmodule.xml` show as **KIE base**, all others as **DRL group**. + +### Declaring groups + +Use `packages`/`includes` for `kmodule.xml` semantics, or `files` for an explicit ordered list. Relative paths resolve against the workspace root in the setting, against the file's own directory in `drl-lsp-kbases.json`. + +```json +{ + "kbases": [ + { + "name": "validation", + "packages": ["!com.example.validation.internal.*", "com.example.validation.*"], + "includes": ["shared"] + }, + { "name": "legacy", "files": ["rules/Types.drl", "rules/Enums.drl"] } + ] +} +``` + +> In `packages`, the first matching pattern decides — including its sign. List exclusions before the wildcard they carve out of. + +### Adopting an existing manifest + +`sources` allows rule group definitions with alternate syntax. `aliases` maps each canonical key to the key(s) to alternates; several may collapse onto one. Review Comment: The phrase “maps each canonical key to the key(s) to alternates” is grammatically incorrect; use “maps each canonical key to one or more alternate keys.” ########## packages/drl-vscode-extension/src/fileGrouping.ts: ########## @@ -0,0 +1,355 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import * as vscode from "vscode"; +import { LanguageClient } from "vscode-languageclient/node"; + +/** + * Shows which group of DRL files the open document compiles with, and lets the + * user pin it to a different one. + * + * The group map comes from the server (`drools/fileGroups`) rather than being + * re-derived here. The server already reads kmodule descriptors, the + * `drl-lsp-kbases.json` config and any manifests it adopts; parsing those a + * second time in the client would only create a second thing to keep correct. + */ + +type Logger = { + info: (msg: string) => void; + error: (msg: string) => void; +}; + +/** + * One group as the server reports it. `kind` is set only when the server can be + * more specific than "group" — "KIE base" for a group read from a kmodule.xml — + * so a project that never declared a kmodule is never shown kmodule vocabulary. + */ +type FileGroup = { + /** Normalized paths, for membership tests. */ + files: string[]; + kind?: string; + /** The file that declared the group, for answering "why is this file here?". */ + declaredIn?: string; +}; + +/** Group name to what the server last reported for it. */ +let groups = new Map<string, FileGroup>(); +/** Document fsPath to the group the user pinned it to. Persisted per workspace. */ +let overrides = new Map<string, string>(); +let statusItem: vscode.StatusBarItem | undefined; +let log: Logger = { info: () => undefined, error: () => undefined }; + +const OVERRIDES_STATE_KEY = "drools.fileGroupOverrides"; +const CONFIG_FILE_GLOB = "**/{drl-lsp-kbases.json,kmodule.xml}"; +const GROUPING_SETTING = "drools.lsp.grouping"; + +/** + * The `drools.lsp.grouping` setting, or undefined when unset or empty. Sent to + * the server as an object rather than a JSON string, so it arrives as structured + * JSON instead of a quoted, escaped string. + */ +export function groupingSetting(): object | undefined { + const value = vscode.workspace.getConfiguration().get<object>(GROUPING_SETTING); + return !value || Object.keys(value).length === 0 ? undefined : value; +} + +/** Files the grouping layer needs to know about. */ +const WORKSPACE_FILE_GLOB = "**/{*.drl,kmodule.xml,drl-lsp-kbases.json}"; + +/** + * Enumerates the workspace files the server should consider, as URIs. + * + * The client does this rather than the server walking the filesystem, because + * `findFiles` already applies the user's `files.exclude`, `search.exclude` and + * ignore files. A server-side walk can only approximate that with a hardcoded + * list of directory names to skip, which goes stale and silently drops files. + */ +export async function enumerateWorkspaceFiles(): Promise<string[]> { + if (!vscode.workspace.workspaceFolders?.length) { + return []; + } + const found = await vscode.workspace.findFiles(WORKSPACE_FILE_GLOB); + return found.map((uri) => uri.toString()); +} + +function normalize(p: string): string { + return p.replace(/\\/g, "/").toLowerCase(); +} Review Comment: Lowercasing every path conflates distinct files on case-sensitive filesystems (for example, `rules/Foo.drl` and `rules/foo.drl`), so the status UI can report both as members of the same groups. Apply case folding only on platforms where path lookup is case-insensitive. ########## packages/drl-vscode-extension/src/fileGrouping.ts: ########## @@ -0,0 +1,355 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import * as vscode from "vscode"; +import { LanguageClient } from "vscode-languageclient/node"; + +/** + * Shows which group of DRL files the open document compiles with, and lets the + * user pin it to a different one. + * + * The group map comes from the server (`drools/fileGroups`) rather than being + * re-derived here. The server already reads kmodule descriptors, the + * `drl-lsp-kbases.json` config and any manifests it adopts; parsing those a + * second time in the client would only create a second thing to keep correct. + */ + +type Logger = { + info: (msg: string) => void; + error: (msg: string) => void; +}; + +/** + * One group as the server reports it. `kind` is set only when the server can be + * more specific than "group" — "KIE base" for a group read from a kmodule.xml — + * so a project that never declared a kmodule is never shown kmodule vocabulary. + */ +type FileGroup = { + /** Normalized paths, for membership tests. */ + files: string[]; + kind?: string; + /** The file that declared the group, for answering "why is this file here?". */ + declaredIn?: string; +}; + +/** Group name to what the server last reported for it. */ +let groups = new Map<string, FileGroup>(); +/** Document fsPath to the group the user pinned it to. Persisted per workspace. */ +let overrides = new Map<string, string>(); +let statusItem: vscode.StatusBarItem | undefined; +let log: Logger = { info: () => undefined, error: () => undefined }; + +const OVERRIDES_STATE_KEY = "drools.fileGroupOverrides"; +const CONFIG_FILE_GLOB = "**/{drl-lsp-kbases.json,kmodule.xml}"; +const GROUPING_SETTING = "drools.lsp.grouping"; + +/** + * The `drools.lsp.grouping` setting, or undefined when unset or empty. Sent to + * the server as an object rather than a JSON string, so it arrives as structured + * JSON instead of a quoted, escaped string. + */ +export function groupingSetting(): object | undefined { + const value = vscode.workspace.getConfiguration().get<object>(GROUPING_SETTING); + return !value || Object.keys(value).length === 0 ? undefined : value; +} + +/** Files the grouping layer needs to know about. */ +const WORKSPACE_FILE_GLOB = "**/{*.drl,kmodule.xml,drl-lsp-kbases.json}"; + +/** + * Enumerates the workspace files the server should consider, as URIs. + * + * The client does this rather than the server walking the filesystem, because + * `findFiles` already applies the user's `files.exclude`, `search.exclude` and + * ignore files. A server-side walk can only approximate that with a hardcoded + * list of directory names to skip, which goes stale and silently drops files. + */ +export async function enumerateWorkspaceFiles(): Promise<string[]> { + if (!vscode.workspace.workspaceFolders?.length) { + return []; + } + const found = await vscode.workspace.findFiles(WORKSPACE_FILE_GLOB); + return found.map((uri) => uri.toString()); +} + +function normalize(p: string): string { + return p.replace(/\\/g, "/").toLowerCase(); +} + +function activeDrlUri(): vscode.Uri | undefined { + const uri = vscode.window.activeTextEditor?.document.uri; + return uri && uri.fsPath.toLowerCase().endsWith(".drl") ? uri : undefined; +} + +/** Every group containing `uri`, in the order the server reported them. */ +function groupsContaining(uri: vscode.Uri): string[] { + const target = normalize(uri.fsPath); + const matches: string[] = []; + for (const [name, group] of groups) { + if (group.files.includes(target)) { + matches.push(name); + } + } + return matches; +} + +/** The noun for a group, defaulting to wording that needs no Drools background. */ +function labelFor(name: string): string { + return groups.get(name)?.kind ?? "DRL group"; +} + +/** A tooltip line naming the declaring file, or "" when the server gave none. */ +function provenanceOf(name: string): string { + const declaredIn = groups.get(name)?.declaredIn; + if (!declaredIn) { + return ""; + } + const fsPath = vscode.Uri.parse(declaredIn).fsPath; + return `\nDeclared in ${vscode.workspace.asRelativePath(fsPath)}`; +} + +function updateStatusItem(): void { + if (!statusItem) { + return; + } + const uri = activeDrlUri(); + if (!uri) { + statusItem.hide(); + return; + } + + const containing = groupsContaining(uri); + const pinned = overrides.get(uri.fsPath); + const active = pinned ?? containing[0]; Review Comment: A persisted pin is trusted even after its group disappears, so the status bar continues to show a deleted group as active while the server falls back to another scope. Treat a pin as active only when `groups.has(pinnedName)` (and ideally remove/persist invalid entries after refreshing the map). -- 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]
