spgennard / vscode_cobol

Visual Studio Code Extension for COBOL, JCL and MF Directive Files
MIT License
36 stars 16 forks source link

How to debug "no definition found" for copybooks? #218

Closed GitMensch closed 4 years ago

GitMensch commented 4 years ago

Most copybooks are found (so I can use "show definition" on the copy statement and I think its contents are known to the editor, too. But for some copybooks I get a "no definition found" message.

Things I've checked:

spgennard commented 4 years ago

You can start by giving collating the actual COPY statement used, as not all forms of the COPY verb are understood. For example copy replacing is beyond the scope of the scanner/extension.

GitMensch commented 4 years ago

I should have added this to the info above - the ones I inspect are plain copies:

       WORKING-STORAGE SECTION.
       01  W-PRGM-ID.
           05 W-PRGM            PIC  X(08)    VALUE "prog".
           05 W-VERS            PIC  9(03)    VALUE 001.
      *
           COPY  CWSTD.
           COPY CWSPEC.
       LINKAGE SECTION.
           COPY CWINOUT.
           COPY CDSTD.

All copybooks but CWINOUT are "found", some are in the same folder as the source, some aren't, If I swap CDSTD and CWINOUT I still get the same result - CWINOUT not found.

GitMensch commented 4 years ago

Cool INout :-)

GitMensch commented 4 years ago

There is another copybook that is never found:

           COPY CWSPEC.
       LINKAGE SECTION.
           COPY CWINOUT.
           COPY CDSTD.
           COPY CDWPS.
           COPY CDSTD2.

The CDWPS copybook (which consists only of comments, 670 lines of those in its header and 175 COPY statements + a FILLER) is never found. All copybooks I've tested that are contained in there (starting with the same prefix) are also not found, so maybe it is something about the name here, too?

All of those copybooks with the prefix are placed in the same directory of he program they are used in.

spgennard commented 4 years ago

The way you describe the copybook, it won't have any symbols in it (because the filler is not a symbol).

Just the same name, as above with cut-paste... the source definition provider finds the copybook:

image

and when you open it.. parsed it as COBOL and show the single "COPY" in the outline view (my cutdown)

image

GitMensch commented 4 years ago

There's one difference in my setup to yours: he copybook extension is .lib which is configured via "files.associations: {"*.lib": "COBOL"}" and "coboleditor.copybookexts": ["lib"] `but this works in general so I don't think that's an issue either

In the outline of CDWPS all 175 copybooks (each starting with CDWPS) are shown here, too, but again their definitions aren't found.

Hm, I'm updating to 1.49.3 and see if that makes any difference - I guess it won't. In that case you'd likely need test bed, don't you?

spgennard commented 4 years ago

It looke like the copybook CDWPS has no definitions, it has copy statements and a filler. The filler is not a symbol, so can't be included in the definitions... So you need to be clear what you are expecting to work because it sounds like it is working.

GitMensch commented 4 years ago

I'm expecting to see the "* Comment Line" as definition like in your screenshot, but instead get nothing (no hover) and when manually executing "go to definition (F12)" on the COPY line, which normally opens the copybook in a preview tab I get

no definition found for "CDWPS".

This is not about the definition of the copybook's content (the "filler") but the copybook itself. Was this a better description of my issue?

spgennard commented 4 years ago

Changing the extension to .lib and amending the file.associations, as follows:

        "coboleditor.copybookexts": [
            "cpy",
            "scr",
            "CPY",
            "SCR",
            "cbl",
            "CBL",
            "ccp",
            "dds",
            "ss",
            "wks",
            "lib"
        ],
        "files.associations": {
            "*.lib" : "COBOL"
        }

It still works:

image

GitMensch commented 4 years ago

We already know:

I guess it is related to the place the copybook is - in a sub-folder. I've created a self-contained testbed (= workspace) for this, here we go: 218_reproducer.zip

spgennard commented 4 years ago

The mod1 directory is unknown to the extension, so it is never searched.

TIP: If you look at the COBOL output panel, you can see the exact search paths used.

        "coboleditor.copybookdirs": [
            "mod1"
        ],
GitMensch commented 4 years ago

Thanks for checking that.

The mod1 directory is unknown to the extension, so it is never searched.

Didn't we previously came to the conclusion that all folders in the workspace should be searched by default? It is already done for the metadata parsing (and of course by vscode itself, for example via quick open).

I think there are 2 scenarios:

1 use bitlang.cobol just for looking at some files, in this case it is likely one places standard directories (all other than ".", which should be "relative to the file") into coboleditor.copybookdirs 2 use bitlang.cobol to actually work with it, in this case one would very likely define a workspace, which then also includes all copybook folders in the workspace itself (direct or "below" the folders. like in this case); one may additionally use coboleditor.copybookdirs in this case to point to "common copybooks", for example copies shipped with the COBOL compiler

Concerning 1 - I see the issue that the copybook in "mod1" is not found, even when "PROG.cob" actually is in "mod1", so it is available relative to the source (I guess currently only "relative to the workspace" is checked). Concerning 2 - Obviously sub-folders are included in many areas, also concerning this extension, but not yet in resolving copy.

One may say "only defined folders should looked in", but then I'd vote for a boolean setting `coboleditor.copybooksInWorkspace to be able to define "search the complete workspace, too".

What do you think about those two points?

spgennard commented 4 years ago

1 use bitlang.cobol just for looking at some files, in this case it is likely one places standard directories (all other than ".", which should be "relative to the file") into coboleditor.copybookdirs

The "mod1" is part of the workspace but is not a workspace folder, it is under the "src" workspace folder.

If "mod1" was a workspace folder then it would be found, however, because you have a tree structure with nested folders then you have rightly decided not to include it.

If the source code used "COPY xx in 'mod1'" then it works both in the editor and when compiled too.

One may say "only defined folders should looked in", but then I'd vote for a boolean setting `coboleditor.copybooksInWorkspace to be able to define "search the complete workspace, too".

To do this I would have to traverse the filesystem and this would be a massive performance issue.

The extension tries to avoid doing any filesystem "stat" or filesystem traversal because it is horribly slow.

By using the copybookdirs it does it in a controlled way and setups up the paths once, so the performance issue is slightly mitigated.

One of my long term aims is to get rid of any filesystem related APIs, so the extension can then be used from a browser environment, where I can only use the vscode file system apis. At this point, the use of copybookdirs will not be possible

GitMensch commented 4 years ago

I see. But what about the metadata?

1 if contained in metadata -> use this as reference (no stat necessary at all) 2 if not: fallback to stat

Shouldn't this improve the overall performance as it reduces the stat (as long as metadata is available/up-to-date)?

spgennard commented 4 years ago

Accessing the metadata requires the use of stat/file access (needs to know if it is out of date), so it does at least two 'stat's, the filesystem file (which is not part of the workspace)

GitMensch commented 4 years ago

I've added all module names to coboleditor.copybookdirs now, result:

Invalid CopyBook directories (35)
=> Z:\home\me\src\mod1
=> Z:\home\me\src\mod2
=> Z:\home\me\src\mod3
=> Z:\home\me\src\mod4
=> Z:\home\me\src\mod5
=> Z:\home\me\src\mod6
=> Z:\home\me\src\mod7
=> Z:\home\me\src\mod8
=> X:\src\mod1
=> X:\src\mod2
=> X:\src\mod3
=> X:\src\mod4
=> X:\src\mod5
=> X:\src\mod6
=> X:\src\mod7
=> X:\src\mod8
=> Z:\project1\cfg\mod1
=> Z:\project1\cfg\mod2
=> Z:\project1\cfg\mod3
=> Z:\project1\cfg\mod4
=> Z:\project1\cfg\mod5
=> Z:\project1\cfg\mod6
=> Z:\project1\cfg\mod7
=> Z:\project1\cfg\mod8
...

It looks like coboleditor.copybookdirs is not really made for workspace folders :-(

Accessing the metadata requires the use of stat/file access (needs to know if it is out of date), so it does at least two 'stat's, the filesystem file (which is not part of the workspace)

What about a new coboleditor.assume_current_metadata, which is likely useful to set if coboleditor.process_metadata_cache_on_start true? With this new setting metadata would be updated:

spgennard commented 4 years ago

(sorry my lunch time is nearly over.. so I'll need to be quick)

coboleditor.assume_current_metadata Doing this on save, is a non-starter because it would require a complete parse (which might not have been done) and would stall the "save" process. I investigated this and it is very problematic, so I dropped this as an idea.

It looks like coboleditor.copybookdirs is not really made for workspace folders :-( No, I disagree.. you are saying the "mod1"directory may be below this workspace folder and if you have a lot of workspace folders.. then only one will be valid and many may not be.

You are focusing on the verbosity of the diagnostic information as a negative rather than looking at the reason, which is the sturcture of the project.

The ideal project structure is one that has the following characteristics:

GitMensch commented 4 years ago

For my understanding: What is the metadata caching then used for? What would one miss if it is deactivated? And should parse copybooks have an effect on metadata processing (if those are not stored) at all?

spgennard commented 4 years ago

For my understanding: What is the metadata caching then used for? What would one miss if it is deactivated?

When parse_copybooks_for_references is off and metadata caching is on.

The metadata processing provides "goto/peek definition":

And should parse copybooks have an effect on metadata processing (if those are not stored) at all? No, it is used for navigation in the current document (and copybooks), which might not have been saved