Project

General

Profile

Feature #3882

changes to allow the front-end to be used for 4GL syntax checking

Added by Greg Shah over 7 years ago. Updated 15 days ago.

Status:
New
Priority:
Normal
Target version:
-
Start date:
Due date:
% Done:

0%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
production:
No
env_name:
topics:

Related issues

Related to Conversion Tools - Feature #1757: update ANTLR to latest version New
Related to Testing - Support #6861: tests for valid and invalid 4GL syntax (parsing) New
Related to Conversion Tools - Feature #11423: implement centralized status/error tracking for conversion/dev tools New
Related to Conversion Tools - Feature #11528: implement COMPILE stmt New
Related to Conversion Tools - Feature #11741: implement a fully featured Language Server Protocol (LSP) server for 4GL code New
Related to Conversion Tools - Feature #3881: output actual file and line/column for all outputs in conversion/analytics Review

History

#1 Updated by Greg Shah over 7 years ago

The existing front end is written with the assumption that all 4GL code is valid on input. Customers that switch to FWD develop changes in FWD. Enable the front end for easy use as a syntax check and improve the error output so that it is feasible to use it as the only syntax checking facility.

#2 Updated by Greg Shah over 3 years ago

#3 Updated by Greg Shah over 1 year ago

  • Related to Support #6861: tests for valid and invalid 4GL syntax (parsing) added

#4 Updated by Greg Shah 4 months ago

  • Related to Feature #11423: implement centralized status/error tracking for conversion/dev tools added

#5 Updated by Greg Shah 3 months ago

#6 Updated by Greg Shah 20 days ago

  • Related to Feature #11741: implement a fully featured Language Server Protocol (LSP) server for 4GL code added

#7 Updated by Greg Shah 20 days ago

  • Assignee set to Paula Păstrăguș

The first step on this task is to make the list of requirements. The results should work with both the command line tooling envisioned here as well as the LSP in #11741.

#8 Updated by Paula Păstrăguș 15 days ago

I currently have the following requirements in mind. These may be refined or expanded as implementation progresses, so I'll add any additional requirements that become necessary.

1. Structured diagnostic type
Introduce a structured diagnostic representation containing the origin file, start and end position, severity, code, phase, and message. No information that matters should exist only inside the message string, the message should be a projection of the structured diagnostic data.

2. Diagnostic sink
Introduce a single interface through which all diagnostic producers report their findings. Provide a collecting implementation initially, the service from #11423 can become another implementation later.

3. Nothing swallowed
Migrate all front-end diagnostic producers from System.err to the diagnostic sink, including cases that currently catch errors without rethrowing them, with SymbolResolver being a particularly important example. The caller must be able to enumerate the complete set of diagnostics.

4. Origin positions with explicit mapping state
Diagnostics must point to positions in the developer's original .p, .cls, or .i source file, rather than positions in the .cache. The mapping state must be explicit: EXACT, APPROXIMATE, EXPANSION_SITE, or UNMAPPED for example. When mapping fails, we should report that rather than providing a confidently incorrect position.

5. Report every error in a file
Implement parser error recovery so that an error is recorded, the parser resynchronizes at statement boundaries, and processing continues. (this would typically require antlr4 in place)

6. Invalid-syntax corpus
Create a corpus of intentionally broken 4GL files with expected diagnostic codes and positions. Tests should assert against the code and position, not the diagnostic message text. This is necessary to demonstrate that requirements 4 and 5 work correctly.

7. Check mode that writes nothing
Add a check mode that only parses/analyzes the source and reports diagnostics, without making any persistent changes to the project. The front-end may still build and use the AST internally, but it should not persist ASTs or generate/update files in the project's output tree. If cache data is required for the check, it should be written to a temporary/scratch location rather than the project, or kept in memory where feasible.

8. CLI rendering and exit codes
Provide CLI output in a deterministic format such as:

file:line:col: severity: message [code]

Diagnostics should be grouped by file and deterministically ordered. Exit codes should distinguish between:
  • clean
  • warnings
  • errors
  • tool failure

9. In-memory input
Allow source code to be provided directly as text together with a nominal path. Also support an overlay so that an unsaved .i file is visible to the file that includes it. This is relatively inexpensive to support now but would be structurally difficult to retrofit later, and it will be required for #11741 and eventually for LSP integration.

10. Warm context with no global state on the check path
Resolve schema, keywords, profile, etc. once and reuse them during checking. Static/global state used by the check path should become context-scoped. This requirement is limited to the syntax-checking path and should not turn into a broader refactoring of the entire conversion process.

I may add further requirements as I get deeper into the implementation, particularly if I identify additional constraints needed for the future VS Code/LSP integration.

The intended progression is essentially: 4GL source -> standalone syntax checker -> structured diagnostics -> LSP integration -> VS Code extension

#9 Updated by Greg Shah 15 days ago

I think this is a good start on the plan.

Comments/questions:

1. I don't want to have an independent implementation of the diagnostic sink from #11423. The implementation should BE done as #11423. It can evolve over time. Consider that #11423 doesn't exist yet.

2. For the actual "origin positions" (item 4), we have already done quite a bit of work in this regard. Please see #3881. We should finish that work first.

3. We are assuming that this work builds on ANTLRv4.

4. Please provide more detail on what is meant by "Also support an overlay so that an unsaved .i file is visible to the file that includes it.".

5. Please provide more detail on "Resolve schema, keywords, profile, etc. once and reuse them during checking. Static/global state used by the check path should become context-scoped.".

#10 Updated by Paula Păstrăguș 15 days ago

Greg Shah wrote:

4. Please provide more detail on what is meant by "Also support an overlay so that an unsaved .i file is visible to the file that includes it.".

By "overlay", I mean that the syntax checker should be able to use the in-memory version of a file instead of the version currently saved on disk.

For example, if main.p includes common.i, and common.i has been modified in the editor but not yet saved, the checker should be able to see and use the modified in-memory content when checking main.p

#11 Updated by Paula Păstrăguș 15 days ago

Greg Shah wrote:

5. Please provide more detail on "Resolve schema, keywords, profile, etc. once and reuse them during checking. Static/global state used by the check path should become context-scoped.".

By this, I mean that the syntax-checking path should not rely on mutable process-global state. The front-end currently has several pieces of state stored statically, for example in ScanDriver, AstGenerator, and SymbolResolver. This works reasonably well for the current batch conversion model, where a process typically handles one project and then exits, but it can cause problems for a long lived syntax checking service.

For the syntax-checking path, state that belongs to a particular check or project should instead be owned by an explicit check context. This includes things such as the schema, keywords, profile, AST registry, and other mutable state required during checking. These resources should be initialized/resolved once for the context and then reused while checking the relevant files, rather than being repeatedly initialized or shared through static fields.

The reason for this is the future LSP use case. The server may check multiple files from different projects, potentially concurrently, and the result of one check should not depend on which files were checked previously or in what order. Re-checking the same file should also produce the same diagnostics without accumulating state from previous checks.

As acceptance criteria, checking the same file repeatedly should produce identical diagnostics, checking files in different orders should not change the results, and two independent project contexts should be able to run concurrently without interfering with each other.

#12 Updated by Paula Păstrăguș 15 days ago

Greg, please take a look at this testcase:

  DEFINE VARIABLE i AS INTEGER   NO-UNDO.
  DEFINE VARIABLE c AS CHARACTER NO-UNDO.

  /* ---- valid: establishes a clean starting state ---- */
  ASSIGN i = 10.
  DISPLAY "start".

  /* ==== ERROR 1 : IF without the required THEN ==== */
  IF i > 5 DISPLAY "greater".

  /* ---- valid: did recovery resume here? ---- */
  ASSIGN c = "recovered after 1".
  DISPLAY c.

  /* ==== ERROR 2 : unknown statement keyword (DISPLAY misspelled) ==== */
  DISPALY "this is not a statement".

  /* ---- valid: did recovery resume here? ---- */
  ASSIGN c = "recovered after 2".
  DISPLAY c.

  /* ==== ERROR 3 : DEFINE VARIABLE with no AS or LIKE clause ==== */
  DEFINE VARIABLE j NO-UNDO.

  /* ---- valid: did recovery resume here? ---- */
  ASSIGN c = "recovered after 3".
  DISPLAY c.

  /* ==== ERROR 4 : unbalanced parenthesis in an expression ==== */
  ASSIGN i = (3 + 4.

  /* ---- valid: did recovery resume here? ---- */
  ASSIGN c = "recovered after 4".
  DISPLAY c.

  /* ==== ERROR 5 : DO block opened and never closed (detected at EOF) ==== */
  DO:
     DISPLAY "inside a block that is never terminated".

With trunk/ANTLR2, the first three errors are reported, while with 10804b/ANTLR4 only the first one is reported. In OE, only the first error is reported as well.

I want to clarify what the syntax checker is expected to report when there are multiple syntax errors in the same file. Should we follow OE's behavior and report only the first error, or should we report as many syntax errors as possible..? (aka 5)

My concern is that if we rely on ANTLR4's default error reporting, we may not match OE's error reporting behavior. This is also different from the current ANTLR2 behavior, which reports multiple errors rather than stopping at the first one.

#13 Updated by Greg Shah 15 days ago

If we can recover and keep going, we should. But it won't always be possible and even if we do try, it can be wrong because of the broken code in the middle.

For now, assume that we will enable some level of recovery but the success of it will vary and that is OK. We don't have to match the OE behavior on this. Our objective is to be as useful as possible from the developer's perspective.

#14 Updated by Greg Shah 13 days ago

  • Related to Feature #3881: output actual file and line/column for all outputs in conversion/analytics added

Also available in: Atom PDF