project .pliplugin program config must win over user-scope pli.pgm_conf (#803)

project .pliplugin program config wins over user-scope pli.pgm_conf

Signed-off-by: Wagner Laranjeiras <wagner.laranjeiras@typefox.io>

Author: Wagner-Laranjeiras

packages/language/src/config/schema.ts

@@ -105,6 +105,13 @@ export interface GroupRecord extends ProcessGroup {
   computedLibs: LibsEntry[];
 }
 
+/**
+ * Where a {@link ProgramRecord} was loaded from. Project (`.pliplugin/`)
+ * entries take precedence over user-scope `settings.json` entries when a file
+ * matches program configs from both sources — see `getProgramConfig`.
+ */
+export type ProgramOrigin = "project" | "settings";
+
 /**
  * A {@link ProgramConfig} together with its merged compiler options. The
  * pli-options strings (this config's plus the bound process group's) are
@@ -118,6 +125,12 @@ export interface ProgramRecord extends ProgramConfig {
    * compiler-options translator doesn't double-report them.
    */
   issues: Diagnostic[];
+  /**
+   * Configuration source this record came from. Absent is treated as
+   * `"project"` (highest precedence) so non-merge callers (quick-fix add,
+   * tests, `.pliplugin/`-only parse) keep binding at project precedence.
+   */
+  origin?: ProgramOrigin;
 }
 
 /**

packages/language/src/workspace/plugin-configuration-provider.ts

@@ -35,6 +35,7 @@ import {
   plainItem,
   ProcessGroup,
   ProgramConfig,
+  ProgramOrigin,
   ProgramRecord,
 } from "../config/schema";
 import { isBoolean, isNumber, isStringArray } from "../utils/types";
@@ -65,6 +66,7 @@ export {
   type ProcessGroup,
   type ProgramConfig,
   type ProgramEntry,
+  type ProgramOrigin,
   type ProgramRecord,
 } from "../config/schema";
 
@@ -186,6 +188,26 @@ interface ConfigSource {
   document: TextDocument;
 }
 
+/**
+ * Precedence of a program-config match against a file, **lowest value wins**.
+ * Used by {@link PluginConfigurationProvider.getProgramConfig} to pick the
+ * best match rather than the first hit, so a user-scope `settings.json` entry
+ * can never shadow a project `.pliplugin/` entry that also matches:
+ *
+ *  1. source — project (`.pliplugin/`) beats settings;
+ *  2. specificity — an exact path beats a glob (within the same source).
+ *
+ * Ties beyond this are broken by insertion order (project entries are merged
+ * first). `ProjectExact` is the best possible rank, so matching it lets the
+ * lookup stop early.
+ */
+const ProgramMatchRank = {
+  ProjectExact: 0,
+  ProjectGlob: 1,
+  SettingsExact: 2,
+  SettingsGlob: 3,
+} as const;
+
 /**
  * Plugin configuration provider for loading '.pliplugin/pgm_conf.json' and '.pliplugin/proc_grps.json' (when they exist),
  * processing their contents, and making those settings available to the language server.
@@ -473,23 +495,44 @@ export class PluginConfigurationProvider {
       (source): source is ConfigSource => source !== undefined,
     );
 
-    // Parse pgm_conf sources and merge by resolved program URI.
-    const mergedPrograms = new Map<string, ProgramConfig>();
-    for (const source of pgmSources) {
+    // Parse pgm_conf sources and merge by resolved program URI, tagging each
+    // entry with its origin (project `.pliplugin/` vs. user-scope settings).
+    //
+    // Two layers of precedence work together:
+    //  1. Here, `.pliplugin/` sources are processed FIRST (the reverse of
+    //     `pgmSources`) and the `has` guard keeps the first entry per key, so a
+    //     project entry wins any exact-key collision with settings.
+    //  2. `getProgramConfig` then prefers project entries over settings entries
+    //     at lookup time (see its ranking). Together these guarantee a settings
+    //     program entry can never shadow a project entry that also matches the
+    //     file — whether the settings entry is a broad glob (e.g. `**/*`) or an
+    //     exact path (e.g. `a.pli`, which a plain `Map.get` would otherwise let
+    //     win via a direct key hit).
+    const projectPgmConfUri = plipluginPgmConfUri.toString();
+    const mergedPrograms = new Map<
+      string,
+      { config: ProgramConfig; origin: ProgramOrigin }
+    >();
+    for (const source of [...pgmSources].reverse()) {
       this.knownPgmConfUris.add(source.uri.toString());
       const result = parseProgramConfigs(source.text, source.uri, source.entry);
       diagnostics.push(...result.diagnostics);
       if (result.config) {
+        const origin: ProgramOrigin =
+          source.uri.toString() === projectPgmConfUri ? "project" : "settings";
         for (const config of result.config) {
           const resolvedUri = this.resolveProgramPath(
             config.program.value,
             this.workspacePath,
           );
-          mergedPrograms.set(resolvedUri.toString(), config);
+          const key = resolvedUri.toString();
+          if (!mergedPrograms.has(key)) {
+            mergedPrograms.set(key, { config, origin });
+          }
         }
       }
     }
-    this.setProgramConfigs(
+    this.applyProgramConfigs(
       this.workspacePath,
       Array.from(mergedPrograms.values()),
     );
@@ -866,10 +909,28 @@ export class PluginConfigurationProvider {
   public setProgramConfigs(
     workspaceUri: URI,
     programConfigs: ProgramConfig[],
+  ): void {
+    // Direct callers (quick-fix add, `.pliplugin/`-only parse, tests) are all
+    // project-scoped, so their entries bind at project precedence.
+    this.applyProgramConfigs(
+      workspaceUri,
+      programConfigs.map((config) => ({ config, origin: "project" as const })),
+    );
+  }
+
+  /**
+   * Rebuilds the program-config map from the given entries, each tagged with
+   * the source it came from ({@link ProgramOrigin}). Program paths are
+   * normalized and resolved relative to the workspace (unless absolute), then
+   * post-processed so abstract options are built.
+   */
+  private applyProgramConfigs(
+    workspaceUri: URI,
+    entries: Array<{ config: ProgramConfig; origin: ProgramOrigin }>,
   ): void {
     this.programConfigs.clear();
 
-    for (const config of programConfigs) {
+    for (const { config, origin } of entries) {
       const resolvedUri = this.resolveProgramPath(
         config.program.value,
         workspaceUri,
@@ -881,6 +942,7 @@ export class PluginConfigurationProvider {
         ...config,
         abstractOptions: { options: [], tokens: [], issues: [], comments: [] },
         issues: [],
+        origin,
       });
     }
     this.postProcessProgramConfigs();
@@ -996,11 +1058,14 @@ export class PluginConfigurationProvider {
 
   /**
    * Returns the program config for the given program URI.
-   * Lookup order:
-   * 1. Exact match against registered config keys.
-   * 2. Glob pattern match (using minimatch) against decoded URIs.
    *
-   * @see https://github.com/isaacs/minimatch
+   * Rather than returning the first hit, this picks the *highest-precedence*
+   * match (see {@link ProgramMatchRank}) so a user-scope `settings.json` entry
+   * can never shadow a project `.pliplugin/` entry that also matches. This is
+   * what fixes the exact-path settings case: a plain `Map.get(uri)` direct hit
+   * would return a settings `program: "a.pli"` entry even when a project glob
+   * (e.g. `*.pli`) also matches; ranking lets the project glob win instead.
+   *
    * @param program Name of the program to get a config for
    * @returns Associated program config, or undefined if not found
    */
@@ -1010,32 +1075,60 @@ export class PluginConfigurationProvider {
     // the path alone is sufficient.
     // Note that we need to decode the URI
     const uri = program.toString(true);
-    const direct = this.programConfigs.get(uri);
-    if (direct) {
-      return direct;
-    }
-    // fallback to glob matching
+
+    let best: ProgramRecord | undefined;
+    let bestRank = Number.POSITIVE_INFINITY;
     for (const [pattern, config] of this.programConfigs.entries()) {
-      if (pattern === uri) {
-        continue; // already checked
+      const rank = this.programMatchRank(uri, pattern, config);
+      if (rank === undefined || rank >= bestRank) {
+        continue; // no match, or not better than what we already have
       }
+      best = config;
+      bestRank = rank;
+      if (rank === ProgramMatchRank.ProjectExact) {
+        break; // best possible rank
+      }
+    }
+    return best;
+  }
+
+  /**
+   * Precedence of a single program-config entry as a match for `uri`, or
+   * `undefined` when its pattern does not match the file. Lower ranks win;
+   * see {@link ProgramMatchRank}.
+   *
+   * @param uri Decoded program URI being resolved.
+   * @param pattern The program config's key (an exact path or a glob).
+   * @param record The candidate program config.
+   */
+  private programMatchRank(
+    uri: string,
+    pattern: string,
+    record: ProgramRecord,
+  ): number | undefined {
+    const isExact = pattern === uri;
+    if (!isExact) {
       try {
         // attempt match on decoded URI
-        if (
-          minimatch(uri, decodeURIComponent(pattern), {
-            nocase: true,
-          })
-        ) {
-          return config;
+        if (!minimatch(uri, decodeURIComponent(pattern), { nocase: true })) {
+          return undefined;
         }
       } catch (e) {
         console.error(
-          `Invalid glob pattern "${pattern}" for program "${program}": ${e}`,
+          `Invalid glob pattern "${pattern}" for program "${uri}": ${e}`,
         );
+        return undefined;
       }
     }
-    // no match
-    return undefined;
+    // Absent origin is treated as "project" (see ProgramRecord.origin).
+    if (record.origin === "settings") {
+      return isExact
+        ? ProgramMatchRank.SettingsExact
+        : ProgramMatchRank.SettingsGlob;
+    }
+    return isExact
+      ? ProgramMatchRank.ProjectExact
+      : ProgramMatchRank.ProjectGlob;
   }
 
   /**

packages/language/test/fourslash/preprocessor/include/program-config-precedence-exact-settings.ts

@@ -0,0 +1,61 @@
+/**
+ * This program and the accompanying materials are made available under the terms of the
+ * Eclipse Public License v2.0 which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-v20.html
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Copyright Contributors to the Zowe Project.
+ *
+ */
+
+// A project `.pliplugin/pgm_conf.json` entry must win over a user-scope
+// `pli.pgm_conf` entry even when the settings entry is an EXACT path match for
+// the file. Here the settings bind `main.pli` (exact) to `DFLT` (no libs),
+// while the project binds `*.pli` (glob) to `default` (has the `cpyla` lib).
+//
+// This is the harder sibling of `program-config-precedence-over-settings.ts`:
+// a plain `Map.get(uri)` direct lookup would return the settings exact-path
+// entry and the include would fail with IBM1848I. `getProgramConfig` instead
+// ranks project matches above settings matches, so the project glob wins and
+// the include resolves.
+
+/// <reference path="../../framework.ts" />
+
+// @filename: .vscode/settings.json
+//// {
+////   "pli.pgm_conf": { "pgms": [{ "pgroup": "DFLT", "program": "main.pli" }] },
+////   "pli.proc_grps": { "pgroups": [{ "name": "DFLT", "libs": [], "include-extensions": [".pli"] }] }
+//// }
+
+// @filename: .pliplugin/pgm_conf.json
+//// { "pgms": [{ "program": "*.pli", "pgroup": "default" }] }
+
+// @filename: .pliplugin/proc_grps.json
+//// {
+////   "pgroups": [
+////     {
+////       "name": "default",
+////       "libs": [<|missing:"cpy"|>, "cpyla"],
+////       "include-extensions": [".pli"]
+////     }
+////   ]
+//// }
+
+// @filename: cpyla/b.pli
+//// DECLARE LIB_VAR FIXED;
+
+// @filename: main.pli
+//// %INCLUDE "b.pli";
+
+// The include resolves via the project `default` group (which lists `cpyla`),
+// not the settings `DFLT` group that exact-matches `main.pli` with no libs.
+preprocessor.expectTokens(`
+  DECLARE LIB_VAR FIXED;
+`);
+
+// The project `default` group is the active source: its missing `cpy` lib is
+// flagged on proc_grps.json, while `cpyla` resolves.
+verify.expectDiagnosticsAt("missing", {
+  message: code.LSP.PluginConfiguration.UnresolvedEntry.message("cpy"),
+});

packages/language/test/fourslash/preprocessor/include/program-config-precedence-over-settings.ts

@@ -0,0 +1,60 @@
+/**
+ * This program and the accompanying materials are made available under the terms of the
+ * Eclipse Public License v2.0 which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-v20.html
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Copyright Contributors to the Zowe Project.
+ *
+ */
+
+// A project `.pliplugin/pgm_conf.json` entry must win over a broader user-scope
+// `pli.pgm_conf` glob when binding a file to its process group. Here the user
+// settings bind every file (`**/*`) to `DFLT` (which has no libs), while the
+// project binds `*.pli` to `default` (which has the `cpyla` lib). `main.pli`
+// must resolve against `default`, so the include is found — otherwise it would
+// pick the empty settings group and fail with IBM1848I.
+//
+// Regression test for the include that "could not be found" even though the
+// resolvable lib was listed in the project proc_grps.json.
+
+/// <reference path="../../framework.ts" />
+
+// @filename: .vscode/settings.json
+//// {
+////   "pli.pgm_conf": { "pgms": [{ "pgroup": "DFLT", "program": "**/*" }] },
+////   "pli.proc_grps": { "pgroups": [{ "name": "DFLT", "libs": [], "include-extensions": [".pli"] }] }
+//// }
+
+// @filename: .pliplugin/pgm_conf.json
+//// { "pgms": [{ "program": "*.pli", "pgroup": "default" }] }
+
+// @filename: .pliplugin/proc_grps.json
+//// {
+////   "pgroups": [
+////     {
+////       "name": "default",
+////       "libs": [<|missing:"cpy"|>, "cpyla"],
+////       "include-extensions": [".pli"]
+////     }
+////   ]
+//// }
+
+// @filename: cpyla/b.pli
+//// DECLARE LIB_VAR FIXED;
+
+// @filename: main.pli
+//// %INCLUDE "b.pli";
+
+// The include resolves via the project `default` group (which lists `cpyla`),
+// not the settings `DFLT` group (which has no libs).
+preprocessor.expectTokens(`
+  DECLARE LIB_VAR FIXED;
+`);
+
+// The project `default` group is the active source: its missing `cpy` lib is
+// flagged on proc_grps.json, while `cpyla` resolves.
+verify.expectDiagnosticsAt("missing", {
+  message: code.LSP.PluginConfiguration.UnresolvedEntry.message("cpy"),
+});