O spec feature

[supravi96]

Changes

Added comprehensive O-spec (Output Specification) parsing support to the fixed-form RPG parser

Changes Made - New Function: parseOLine()
Implemented a complete parser for fixed-form RPG Output specifications that extracts all standard O-spec fields according to IBM i RPG column positions
Current outline view

After outline view update

Checklist

• have tested my change
• updated relevant documentation
• Remove any/all console.logs I added
• eslint is not complaining
• have added myself to the contributors' list in the README
• for feature PRs: PR only includes one feature enhancement.

[bobcozzi]

Can you elaborate on why this feature was necessary and what it was used for? Thanks.

[supravi96]

@bobcozzi we have implemented this pr to improve parser wrt to o spec as a requirement in another extension., also at the moment o spec symbols are not part of outline, this pr will also add o spec symbols to outline.

[SanjulaGanepola]

What exactly is this really long description? These views have limited horizontal space (unless dragged to be bigger)

[supravi96]

@SanjulaGanepola we were not sure about what all fields are required or matter the most in the description, as it was not mentioned in the requirement. could you please suggest what is the correct or important field as a description anandsha20 ??

[bobcozzi]

Thank you for the contribution. I pulled the branch locally, ran the full test suite (430 tests pass, 1 skipped — no regressions), and did a detailed code review. The feature concept is valid and welcome, but there are several issues that need to be addressed before this can be merged.

---

Bugs

1. Dead code / unreachable branch in detectOSpecType (language/models/fixed.ts)

if (endPos && (fieldName || constant)) {
  if (!endPos) {       // unreachable: outer guard already ensures endPos is truthy
    return 'OXF';
  }
  return 'OF';
}

The if (!endPos) can never be true inside if (endPos && ...). The OXF path intended here is dead — it will never be reached. The OXF classification does work in the separate if (fieldName) block further down, but the logic here is silently dropped.

2. parseOLine has no TypeScript type annotations (language/models/fixed.ts)

export function parseOLine(lineNumber, lineIndex, content) {

Every other parser in this file (parseFLine, parseCLine, parseDLine, parsePLine, parseISpec) declares typed parameters. This function uses implicit any, losing all type safety. Please add (lineNumber: number, lineIndex: number, content: string).

3. position.range can be undefined silently

In the case 'O' handler:

currentItem.position = {
  path: fileUri,
  range: oSpec.filename?.range || oSpec.exceptName?.range || oSpec.type?.range
};

calculateToken returns undefined when the trimmed value is empty. If all three tokens are blank (a valid O-spec line with no meaningful identifier), position.range will be undefined. Downstream consumers (hover, go-to-definition) would receive undefined and could throw. The same pattern exists in the OAnd, OFC, and OXF branches. At minimum add a guard or a sensible fallback range.

---

Architectural concerns

4. lineType property name will render oddly in the outline (language/ile/parser.ts)

The prettyKeywords pattern used here is consistent with how I-spec input declarations already work in this codebase (e.g. { type: 'program' } → type(program)), so passing the O-spec keyword object to prettyKeywords is fine. However, the camelCase property name lineType gets lowercased by prettyKeywords and outputs as linetype(O) rather than something like LINETYPE(O). Consider renaming it to an all-caps key (e.g. LINETYPE) to stay consistent with how all other RPG keywords appear in the outline. This is not a blocker.

5. SymbolKind.String is applied uniformly but is wrong for most output symbols

Every output declaration uses SymbolKind.String regardless of line type. SymbolKind.String is only semantically correct for literal constant output lines (e.g. O 10 'CUSTOMER') — and per item 6 below, those should not be symbols at all. For the symbols that should appear in the outline, SymbolKind.String is the wrong choice:

• Record-type lines (filename + H/D/T/E type) → SymbolKind.Object or similar grouping kind
• Field-reference lines (actual variable names like CUSTNO) → SymbolKind.Field

The current code in documentSymbols.ts applies the same kind to all output symbols without checking which subtype they are. Once item 6 is fixed (constant-only lines removed), the remaining symbols should use appropriate kinds differentiated by their lineType.

6. Constant-only output lines must not be added as symbols

In the OF handler, the guard is:

if (oSpec.fieldName || oSpec.constantOrEdit) {
  currentItem.name = oSpec.fieldName?.value || oSpec.constantOrEdit?.value || '';

When fieldName is empty (e.g. O 10 'CUSTOMER REPORT'), the literal string 'CUSTOMER REPORT' — including the quote characters — becomes the declaration name and is added to the symbol cache. String literals are not named program symbols; they have no meaning in hover, go-to-definition, or autocomplete. Constant-only lines (where fieldName is blank and only constantOrEdit is present) should be skipped entirely — not stored with a placeholder name. Only lines with an actual field/variable name should produce a Declaration.

7. No parent-child nesting in the outline

Each O-spec line is added to currentScopeDefs as a flat sibling. For programs with many output lines this produces a flat, hard-to-navigate outline. The expected UX — matching how inputs, structs, and their subItems work — would nest field description lines (the PR's internal lineType: 'OF') as children under their parent record identification line (lineType: 'O'). Note: OF here is the PR's own internal classification label; Please either implement the parent-child relationship or file a follow-up issue and add a // TODO comment so it is not forgotten.

---

Test quality

8. All 25 tests only assert toBeGreaterThan(0)

Every test ends with:

expect(cache.outputs.length).toBeGreaterThan(0);

This passes even if the parser produces completely wrong names, positions, or field values. These tests confirm that something was added to the cache, not that the right things were added. Please add at least a few assertions for specific names, types, and counts. Example:

// In ospec1 — 3 field lines: CUSTNO, CUSTNAME, and one constant-only line
expect(cache.outputs.length).toBe(3);
expect(cache.outputs[0].name).toBe('CUSTNO');

Also, the comment // Made with Bob at the end of ospec.test.ts should be removed before merging.

---

Minor

• Trailing whitespace on the added lines in cache.ts — ESLint may flag these.
• connection.ts: The newly added outputs: doc.outputs will serialize all O-spec declarations over the LSP wire on every parse. For programs with many output lines this adds overhead. Confirm this is actually consumed by the client, or omit it.

---

Summary

The feature concept is valid and worth having in the codebase — O-spec parsing is a real gap for users working with legacy fixed-form RPG printer programs, and the implementation introduces no regressions against the existing test suite.

However, this PR was submitted before it was fully ready. The tests confirm only that something was parsed, not that it was parsed correctly. Items 3 and 6 in particular would cause real problems in production: item 3 can produce a runtime exception in the language server for programs with blank-identifier O-spec lines, and item 6 silently corrupts the symbol cache with string literals for any program that prints constants — which is essentially all of them.

Required before merge (blockers):

• Item 1: fix the dead code / unreachable branch in detectOSpecType
• Item 2: add TypeScript type annotations to parseOLine
• Item 3: guard against undefined position ranges
• Item 6: skip constant-only field lines entirely rather than storing the literal as a symbol name
• Item 8: strengthen at least a few tests with specific name/count assertions; remove the // Made with Bob comment

Can be addressed in follow-up issues:

• Item 5: differentiate SymbolKind by output line subtype
• Item 7: implement parent-child nesting in the outline (field description lines under their record identification parent)

Cosmetic only:

• Item 4: rename lineType to LINETYPE for outline consistency

The required items need to be addressed -- the concept is worth having, but not in its current state.

Dismissing — review posted prematurely before owner approval.

[supravi96]

@bobcozzi if you could provide some info regarding what fields are required in the desc and also give some examples of parent child nesting scenarios , then I will be able to improve this pr also considering other points that you have mentioned.

[supravi96]

@bobcozzi since review was marked as stale, let me know if the bugs that were pointed out in the stale review should be worked upon parallelly. Thanks

[supravi96]

@SanjulaGanepola @bobcozzi I have pushed some new changes while awaiting feedback. Though this pr still needs more improvement.

Please provide your feedback, as I will be away on medical leave, my team will take care of the pr in my absence.