Support DD Style Includes #455

montymxb · 2025-10-23T09:32:18Z

Closes the last of #394 by:

supporting %INCLUDE ddname(member); includes, that resolve within libs
adding parser support for segmented ddnames in includes, ex. %include a.b.c(member)
%include member; includes that resolve within the first matching ddname(member) entry that is reachable from libs
adding ddname style lib entries, such as cpy/a.b.c, where one or more a.b.c(...) files exist within cpy. Collectively they constitute a single ddname entry that members can be resolved within
- these, along with recursive lib resolution, are what allow %include member to work
supporting resolution of ddnames in recursive libs
updating search to support member resolution with & without a ddname
refactor existing IncludeItem types to split up member & file cases
support process group lib resolution without tripping on ddnames
numerous other changes to support these includes across the stack

The later case is in supplement to our existing file resolution strategy, where %include abc would resolve to a file named abc with one of the supported extensions. This is still the case, but we'll first attempt to match to a member of an existing ddname entry before trying files.

This does not add:

quick fix support for resolving unresolved %include ddname(member); or %include member entries, but the existing quick fix support for files works as is
proper range for unresolved ddname(member) diagnostics (diagnostic is still on the member at the moment). The regular member case works as expected.

There's a lot of changes here for a relatively small feature, so I'm very open to feedback for issues, improvements, thoughts, etc.. I've done my best to try and document new additions & logic clearly to make it clear why changes were made at various stages, but feel free to make further suggestions if there's anything else that's missing or unclear in terms of documented behavior.

Some other notes:

Noticed syntax highlighting needs to be improved for some ddname includes, such as %INCLUDE D.E.F. This should be spun out into a separate issue, could benefit from semantic highlighting support.

- parse '%include a.b.c(member);' style includes - support '%include ddname(member);' includes - support '%include member;' includes using ddnames - support ddname libs entries (prefix entries) in config - update search to support file & member lookups (also w/ ddname) - update include item types for members & file includes - update fs providers to support search & read cases - split IncludeItem type in ast for file & member cases Signed-off-by: Benjamin Friedman Wilson <[email protected]>

github-actions · 2025-10-23T09:33:44Z

PR Preview Action v1.6.0
🚀 View preview at https://zowe.github.io/zowe-pli-language-support/pr/pr-455/
Built to branch `previews` at 2025-10-23 09:33 UTC. Preview will be ready when the GitHub Pages deployment is complete.

ssmifi

Thanks for the extensive work. The system is getting quite complex/detailed. I have a couple of questions mostly regarding future maintainability. We can also discuss in the internal meeting tomorrow.

In general, I would be quite happy if we could provide more tests for the different file systems because I have the feeling that things can break quite easily as the system grows. Same is true for the different clients, at least eventually.

ssmifi · 2025-10-27T09:10:29Z

packages/language/src/preprocessor/instruction-interpreter.ts

+        idempotent: instruction.idempotent,
+      };
+    } else {
+      continue;


Question: Is this continue only executed if the ast node is incomplete? Perhaps we should add a comment here.

Yep, that's it. If for any reason we're lacking a legitimate entry we shouldn't proceed to resolve it. I'll add a note to clarify the purpose.

ssmifi · 2025-10-27T09:16:03Z

packages/language/src/preprocessor/instruction-interpreter.ts

        unresolvedFile: item.fileName,
        entryUri: context.entryUri.toString(),
      };
+    } else if (isMemberIncludeItem(item)) {


Minor: Isn't it a MemberIncludeItem automatically?

Based on the type it is, but for the same reason as the continue usage above we may not always have a complete object, which would prevent us from producing a good data payload here.

ssmifi · 2025-10-27T09:19:08Z

packages/language/src/preprocessor/instruction-interpreter.ts

-      if (!absPathRegex.test(lib)) {
+
+    // construct the appropriate file name or partial name for members
+    let fileNameOrPartial: string;


Minor: If we provide a naming function outside of this function, it could be reused in the diagnostics above.

Totally agree with this, I'll factor that out & reuse

ssmifi · 2025-10-27T09:26:45Z

packages/language/src/preprocessor/instruction-interpreter.ts

+    // construct the appropriate file name or partial name for members
+    let fileNameOrPartial: string;
+    // used to denote whether we're working with a pure member (requires special lookup handling)
+    let isPureMember = false;


Minor: Could we describe what this means in more detail? Is this always located in ddLib? Add an example to the comment that shows how a pure member looks like.

Can do! Yeah it's quite complicated, but I can boil it down to a some comments that should help factor clarify what each case would look like.

packages/language/src/workspace/file-system-provider.ts

ssmifi · 2025-10-27T09:44:23Z

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

+   * Only contains directory entries, not DD name entries, which are partial.
+   * Used to find process groups from a lib URI
+   */
+  $computedLibsSet: Set<string>;


Question: Couldn't computedLibs be a set from the start? Feels a bit odd to have two computed libs "sets" that only partially overlap.

We have the other one as an array since we need to retrieve those values for directory look-ups. This one is just for checking whether a dir entry is present that's an actual directory (and not a ddname entry). We could implement the same check using the array alone (I had this initially at some point), but we have to walk through it & skip the ddname entries. This just avoids the redundant checking.

ssmifi · 2025-10-27T09:52:05Z

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

+              }
+            } catch (parentError) {
+              // parent directory also failed to read, skip this lib
+              console.warn(`Failed to resolve library entry "${lib}"`);


Question: Will this eventually (maybe in some function further down the stack) result in a proper diagnostic or popup? I can imagine that mistakes can happen quite easily in these configurations and be confusing without proper notification.

Yep 100% we should have this be a diagnostic. The question is still where to put it, since it's derived from the config again, but having at least some feedback that there's an issue outside of the output log would be nice. I'm not sure if I want to put it up in this PR however, would it make sense to divide up as a follow-up task?

ssmifi · 2025-10-27T09:59:25Z

packages/vscode-extension/src/common/file-search.ts

+      const dir = await readDirSafe(start);
+      const segment = segments[i];
+      const fsSegment = dir.find(
+        (d) => d.toLowerCase() === segment.toLowerCase(),


Question: Same as before, can we just assume, everything is lowercase?

In general, no we can't assume that, you're right about that. But this implementation is just leveraged in the browser at the moment with the vfs, where we're doing case insensitive look-ups (at least I'm pretty sure that's the case). However this code is in the common section, which implies it probably should not make the browser based assumption, which means this wouldn't work if used elsewhere on an ext4 fs & such 🤔 . So this probably should be changed, I'll get back to it next week most likely.

ssmifi · 2025-10-27T10:01:55Z

packages/language/test/fourslash/preprocessor/include/from-lib-path-recursive.ts

 /// <reference path="../../framework.ts" />

 // trigger a reparsing of the config, so we can pick up the sub libs folder
+// TODO @montymxb Oct 17th, 2025: Replace with a fourslash utility to trigger config reload


Question: How would that look like? What do you have in mind here? Perhaps this should be included in this PR to help manage the file system's complexity?

This came up as a request by @msujew in a previous PR for testing configs, since it's becoming a pattern to re-trigger a config reload by rewriting the file again. It's still on my TODO list, but while implementing this PR the same tactic is used in these 4slash tests as well, so I just wanted to extend the same mention here.

My plan is to do that as a separate task so it doesn't expand this one further. There's about 3 or 4 tests that leverage this right now, and they work as is, it would just be an improvement to the testing infra. The problem is that we need to await async handling for the plugin configuration provider to reload fully, as far as I'm aware of we can't do that yet.

ssmifi · 2025-10-27T11:12:38Z

packages/language/test/fourslash/preprocessor/include/test-ddmember-include-members.ts

+// @filename: cpy2/MYLIB(m1)
+//// DECLARE LIB_VAR1 FIXED;
+
+// @filename: cpy2/A.B.C(m2)


Comment: This looks rather confusing by mixing different file systems. Should be ok in the test system, because it is just a name, but is this also a realistic case, e.g., on zOS?

It is an actual use case that was presented some weeks ago, but it's also confusing at first glance too. The file A.B.C(m2) is synced over from zOS to the user's machine, and corresponds to a lib entry on the mainframe (akin to A/B/C/m2). So both the unix & mainframe lib resolution strategies need to be present as far as I'm aware. There will be cases where large quantities of these ddname entries need to be recursively resolved from a given libs entry.

msujew

I think we really need some documentation in addition to the code/tests that explain how each style of include is supposed to be resolved. I also believe that we can consolidate some of the include resolution logic, because this has become a lot of "if this, then that" on multiple levels (i.e. search - with its various implementations, URI resolution in the interpreter, etc.).

msujew · 2025-10-27T09:22:48Z

packages/language/src/syntax-tree/ast.ts

+/**
+ * Include item by file name
+ */
+export type IncludeItemFile = IncludeItem & {
+  fileName: string | null;
+};
+
+/**
+ * Include item by member name + optional ddname
+ */
+export type IncludeItemMember = IncludeItem & {


Suggestion: I'm not a big fan of breaking with the convention that we have throughout the file of having one SyntaxKind per type. IMO these should be distinct interfaces. Also, please add them to the SyntaxNode union type, which currently only contains IncludeItem.

That's fine, I can split up those up further, probably for the best. Early on this was causing problems as the same node type takes on entirely different semantics depending on how it's constructed.

msujew · 2025-10-27T09:43:32Z

packages/vscode-extension/src/language/main.ts

+      // member lookup w/ dd path (partial lib file as entry)
+      const uri = URI.parse(options.ddPath + `(${options.member})`);


Suggestion: Isn't this just the normal, non-global file search using a <ddPath>(<member>) syntax? IMO we can represent this using the normal PathSearch object.

Note that using fs.access will perform a case-sensitive match on the file system. This might fail on Linux, since PL/I is generally case insensitive. That why we use glob even for simple file lookups (for the nocase option).

Yes it is just the full path combined in this case, that can be done via a regular path search, I can swap that over.

It probably will fail. The ext4 case on Linux is something that needs further checking for sure, @ssmifi mentioned this as well too, there are a number of points where I'm too aggressively lower-casing things to get stuff to behave in the tests, and I haven't yet verified how that will behave in practice on linux, I need to go over that still.

msujew · 2025-10-27T11:48:24Z

packages/language/test/fourslash/preprocessor/include/test-ddmember-includes-registered-libs.ts

+
+// @filename: main.pli
+//// %INCLUDE m1;
+//// %INCLUDE m2;


Suggestion: Can we also test that the diagnostic marker appears here?

Yep definitely, I can add a check for that.

msujew · 2025-10-27T12:09:15Z

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

-                // also add to the full libs list
-                computedLibs.add(`${lib}/${fileName}`);
+          try {
+            const entries = await FileSystemProviderInstance.readDir(libUri);


Suggestion: Add a stat method to the FileSystemProvider to resolve this.

msujew · 2025-10-27T12:15:59Z

packages/language/src/preprocessor/instruction-interpreter.ts

+        if (isPureMember) {
+          // attempt an early resolution for any DDName that introduces this member
+          const memberMatch = await FileSystemProviderInstance.search({
+            dirPath: resolveLibFileUri(lib.dir),
+            member: fileNameOrPartial,
+          });
+          if (memberMatch) {
+            return memberMatch;
+          }
+        }


Question: Is this a performance optimization? It seems like that, because otherwise it will just fall through to the standard search.

It's a requested case for include resolution order in case we have a file cpy/member.pli and cpy/ddname(member). I had to hash this out a bit, but it appears that in such a case we should be resolving the ddname w/ a matching member first when present over the literal member file entry. And when we can't we just fall through as you've noted. But the question points out that this isn't well documented, I'll add some additional comments to clarify the intent.

montymxb requested review from msujew and ssmifi October 23, 2025 09:32

ssmifi reviewed Oct 27, 2025

View reviewed changes

msujew reviewed Oct 27, 2025

View reviewed changes

		// member lookup w/ dd path (partial lib file as entry)
		const uri = URI.parse(options.ddPath + `(${options.member})`);

Support DD Style Includes #455

Are you sure you want to change the base?

Support DD Style Includes #455

Uh oh!

Conversation

montymxb commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 23, 2025

Built to branch previews at 2025-10-23 09:33 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

ssmifi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

montymxb Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msujew left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

montymxb commented Oct 23, 2025 •

edited

Loading

Built to branch `previews` at 2025-10-23 09:33 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

montymxb Oct 28, 2025 •

edited

Loading