feat(aclass): propose ABAP OO parser package (Chevrotain, Ch)#111
Merged
ThePlenkov merged 12 commits intomainfrom Apr 21, 2026
Merged
feat(aclass): propose ABAP OO parser package (Chevrotain, Ch)#111ThePlenkov merged 12 commits intomainfrom
ThePlenkov merged 12 commits intomainfrom
Conversation
New change proposal for a Chevrotain-based parser of `.clas.abap` / `.intf.abap` source files — symmetric to `@abapify/acds` for CDS and reverse of `@abapify/abap-ast`'s emitter. Scope (MVP): * Class / interface DEFINITION + IMPLEMENTATION headers * Sections (PUBLIC / PROTECTED / PRIVATE) * Member declarations: METHODS, DATA, CLASS-DATA, TYPES (incl. STANDARD/SORTED/HASHED TABLE, BEGIN/END OF), CONSTANTS, EVENTS, ALIASES, INTERFACES * Inheritance (INHERITING FROM), create visibility, FOR TESTING * Method IMPLEMENTATIONS with opaque body slice (start/end offsets + raw text); body statements are NOT parsed * ABAPDoc lines attached to the following declaration Deferred: method body statement parsing, top-level DATA/TYPES, FORM/PERFORM, macros, includes, CDS annotations inside classes, select/update/insert statements, walker helpers, semantic validators. Primary driver: a `print(parse(src)) === src` roundtrip test against every file `@abapify/openai-codegen` emits for the petstore3 sample, closing the codegen output-contract loop. Structure of the proposal: * `proposal.md` — why / what / impact / non-goals * `specs/aclass/spec.md` — invariants, grammar coverage table, AST shape, testing contract, release gates * `tasks.md` — Wave 0 (skeleton + lexer) → Wave 1 (parser + visitor) → Wave 2 (fixtures + roundtrip) → Wave 3 (docs + consumer wiring) Branch starts from main; no existing package is modified by this proposal. First code PR will land Wave 0 only. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 32 minutes and 22 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR introduces the Changes
Sequence DiagramsequenceDiagram
participant Source as ABAP Source
participant Lexer as AclassLexer
participant StatementSplitter as Statement Splitter
participant Parser as Recursive Descent Parser
participant ASTBuilder as AST Builder
participant Result as ParseResult
Source->>Lexer: tokenize(source)
Lexer-->>Source: tokens[], errors[]
Source->>StatementSplitter: split into dot-terminated statements
StatementSplitter-->>Source: statement[]
Source->>Parser: dispatch by leading keyword
Parser->>ASTBuilder: build typed nodes (ClassDef, ClassImpl, InterfaceDef, etc.)
alt Recognized shape
ASTBuilder-->>Parser: TypedMember (Method, Attribute, Type, etc.)
else Unrecognized shape
ASTBuilder-->>Parser: RawMember (opaque source slice)
end
Parser->>Result: normalize errors, construct AbapSourceFile
Result-->>Source: { ast, errors }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for adt-cli canceled.
|
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
|
View your CI Pipeline Execution ↗ for commit 8fe3a79
☁️ Nx Cloud last updated this comment at |
First code commit for `@abapify/aclass`. Lands the package scaffold (package.json, tsconfig, tsdown, vitest, eslint), the ABAP OO lexer (`src/tokens.ts`), the public `tokenize()` entry point, and 12 smoke tests covering every tricky aspect of ABAP tokenisation. What's in the lexer: - 50+ keywords covering CLASS / INTERFACE definitions and implementations, sections, member declarations (METHODS, DATA, TYPES, CONSTANTS, EVENTS, ALIASES, INTERFACES), inheritance modifiers, method signature clauses (IMPORTING/EXPORTING/CHANGING/ RETURNING/RAISING), type-ref fragments (TYPE / REF TO / LIKE), and table-type declarations (STANDARD / SORTED / HASHED / KEY / UNIQUE / NON-UNIQUE). - Case-insensitive keyword matching (ABAP is case-insensitive). - Compound keywords (`CLASS-DATA`, `CLASS-METHODS`, `CLASS-EVENTS`, `NON-UNIQUE`, `READ-ONLY`) declared before their prefixes so the lexer doesn't split them at the hyphen. - `INTERFACES` (plural, member) declared before `INTERFACE` (definition keyword) so `INTERFACES zif_foo.` tokenises correctly. - ABAPDocLine (`"! …`) captured as a token while regular line comments (`" …`) are skipped. - Star-comments only recognised when the `*` is at column 1; a custom pattern with `line_breaks: false` rejects mid-line `*`. - Multi-char operators `=>`, `->`, `::` declared before their single-char prefixes. - Identifiers accept `/` (SAP namespaces like `/ui2/cl_json`). Tests (12, all green): minimal class header tokenisation, case-insensitivity invariance, `CLASS` vs `CLASS-DATA/METHODS/EVENTS` disambiguation, `INTERFACE` vs `INTERFACES`, `UNIQUE` vs `NON-UNIQUE`, ABAPDoc vs line-comment discrimination, star-comment column-1 rule, mid-line `*` error, static `=>` and instance `->` access, qualified tilde `~` references, method signature keyword sequence, and public API surface. Also: * `packages/aclass/AGENTS.md` — agent guide mirroring `packages/acds/AGENTS.md`. * Root `AGENTS.md` — new "Package-Level Guides" entry, `aclass` added to the foundation-packages list. Next: Wave 1 (CstParser + visitor + typed AST). Refs openspec/changes/add-aclass-parser. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
ThePlenkov
temporarily deployed
to
unprotected-branch-pipeline
GitHub Actions
Inactive
Wave 1 — parser + visitor + typed AST:
* `src/ast.ts` (200 lines) — typed nodes for every declaration the
ABAP OO surface grammar recognises: AbapSourceFile, ClassDef,
ClassImpl, InterfaceDef, Section, MethodDecl, MethodParam,
AttributeDecl, TypeDecl (alias + structure), ConstantDecl,
InterfaceStmt, AliasDecl, MethodImpl (opaque body), RawMember
fallback. Every node carries a SourceSpan. TypeRef is a sum of
BuiltinTypeRef, NamedTypeRef (handles `=>` and `~` qualification),
RefToTypeRef, TableTypeRef (STANDARD/SORTED/HASHED with verbatim
keyClause).
* `src/parser.ts` — pragmatic statement-based parser. ABAP is a
statement-terminated language so rather than a full Chevrotain
CstParser we (a) tokenise with the Wave 0 lexer, (b) split the
token stream on `Dot`, (c) classify each statement by its leading
keyword, (d) build typed AST nodes or fall back to `RawMember`.
Trade-off vs CstParser: we lose automatic recovery but gain
simpler code, direct offset access for spans, and easy fallback for
any shape we haven't taught. Never throws — returns `{ ast, errors }`.
* `src/index.ts` — public `parse()` export.
Wave 2 — real-world corpus + invariants:
* `tests/parse-interface.test.ts` (14 tests): empty interface, TYPES
alias, STANDARD TABLE OF qualified row, METHODS with all four
slots, OPTIONAL params, ABAPDoc on members, BEGIN OF/END OF
structure; plus CLASS header modifiers (FINAL / INHERITING FROM /
CREATE), CLASS IMPLEMENTATION with verbatim body preservation,
`INTERFACES` member, `RawMember` fallback for unrecognised kinds,
and error-path guarantees (never throws for malformed input,
returns errors instead).
* `tests/fixtures.test.ts` (7 tests): parses every `.clas.abap` /
`.intf.abap` under samples/petstore3-client/generated/abapgit/src/
with zero lex errors and asserts the expected top-level kinds per
file.
* `tests/roundtrip.test.ts` (6 tests): semantic roundtrip.
zif_petstore3 has 19 operations and all METHODS parse with a
RAISING clause; zif_petstore3_types has all 6 schemas; zcx error
class inherits cx_static_check and is FINAL; zcl_petstore3 has a
DEFINITION + IMPLEMENTATION pair with 19+ methods;
parse-twice idempotence across every fixture; every MethodImpl
body is non-empty and appears verbatim in the original source.
Gates green: 39 tests pass, typecheck clean, build produces
dist/index.mjs + dist/index.d.mts, ESLint clean.
Next: Wave 3 — wire the parser into openai-codegen as a CI gate
(every generated file MUST parse cleanly) and regenerate/redeploy
petstore3 against live TRL.
Refs openspec/changes/add-aclass-parser.
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Wires `@abapify/aclass` into `@abapify/openai-codegen` as a
dev-dependency + CI gate. Every `.clas.abap` / `.intf.abap` that the
generator ships MUST now round-trip cleanly through the structural
parser: zero lex errors, and interface files contain zero `RawMember`
fallbacks (the generator's interface output is required to stay inside
MVP structural grammar).
Details:
* `packages/openai-codegen/package.json` — add `@abapify/aclass` as a
workspace dev-dep. It ships to tests only; the published generator
bundle does not depend on it at runtime.
* `packages/openai-codegen/tests/aclass-parse-gate.test.ts` — 11 new
assertions: per-file lex-error freedom, interface structural
completeness (no RawMember), 19-method contract on
`zif_petstore3.intf.abap`, ClassDef+ClassImpl pair on
`zcl_petstore3.clas.abap`, and `cx_static_check` inheritance on
`zcx_petstore3_error.clas.abap`.
Lexer extensions (to support parsing generator method bodies):
* `packages/aclass/src/tokens.ts` — add 13 single-char tokens for
characters that appear in expression-heavy method bodies but didn't
exist in Wave 0 (because Wave 0 covered only header/declaration
territory):
- `Hash (#)` — inferred-type placeholder in `VALUE #(...)`, `NEW #(...)`
- `Pipe (|)`, `LBrace ({)`, `RBrace (})` — string templates `|...|`
and their `{ expr }` interpolation
- `Plus`, `Minus`, `Star`, `Slash`, `Lt`, `Gt`, `Question`, `At`,
`Ampersand` — arithmetic / comparison / misc operators that turn
up inside method bodies
The parser doesn't inspect method-body contents (they stay as opaque
source slices via `MethodImpl.body`), but the lexer still has to
accept every character in the stream or parsing aborts with a lex
error.
Smoke-test update: the "mid-line `*` rejected" test now checks that a
mid-line `*` tokenises as `Star` (positive behaviour) instead of
being rejected, since the `Star` token now exists.
Live verification on TRL (us10, CB9980002179):
- `adt deploy --source generated/abapgit --package ZPEPL --activate --unlock` → OK
- activation pre-audit returns `activationExecuted="false"
generationExecuted="true"` with no errors
- `adt aunit -c ZCL_PS3_CLIENT_TESTS --format json` → 3/3 pass
Tests: 153 openai-codegen passing, 39 aclass passing, 115 abap-ast
passing. Typecheck + build + lint clean on all three packages.
Refs openspec/changes/add-aclass-parser.
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
ThePlenkov
temporarily deployed
to
unprotected-branch-pipeline
GitHub Actions
Inactive
CodeQL `js/incomplete-sanitization`: the previous `word.replace(/-/g, '\\-')` only escaped the hyphen but left every other RegExp special character unescaped. If a future keyword contained e.g. `.` or `*` it would leak pattern semantics into the emitted token regex. Switch to a complete `REGEX_META` character class that covers every metacharacter RegExp recognises, and replace with `'\\$&'` so the exact matched character is escaped. Net effect on the current keyword set (all plain ASCII letters + hyphen) is zero — existing tokenisation behaviour unchanged. Tests: aclass 39/39, openai-codegen 153/153, abap-ast 115/115. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
ThePlenkov
temporarily deployed
to
unprotected-branch-pipeline
GitHub Actions
Inactive
ESLint `no-useless-escape`: inside a character class `[…]`, `-` only needs escaping when it would otherwise be part of a range. Placing it at the end of the class makes it literal; the leading backslash was noise. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
ThePlenkov
temporarily deployed
to
unprotected-branch-pipeline
GitHub Actions
Inactive
Code Review SummaryStatus: No New Issues Found | Recommendation: Merge Incremental review of changes since 222a213. Significant feature additions and improvements. Changes in This Commit
Code Quality AssessmentThe new code follows established patterns:
Issues Status (Unchanged)
Fix these issues in Kilo Cloud Reviewed by minimax-m2.5-20260211 · 261,394 tokens |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Responses to the inline review comments on PR #111 + the two findings my own /github-pr-review surfaced that bots missed. ## Parser correctness ### `consumeTypeRef` + `parseFieldRun` + `consumeMethodParam`: accept keyword tokens as identifier-shaped names (Devin 🔴, two threads) ABAP grammar lets many reserved words serve as identifiers in declaration positions — `DATA data TYPE i.`, `REF TO data`, struct fields named `type` / `data`, parameters named `value`. The lexer classifies those as keyword tokens; the parser now reinterprets any token whose `image` is identifier-shaped (`/^[A-Za-z_][A-Za-z0-9_/]*$/`) as a name via a new `isNameLike()` type-guard. Applied to: - type-reference head (`REF TO data`, `TYPE <keyword-named-type>`) - qualified type-reference parts (`zif_y=>data`) - structure-field names in `TYPES: BEGIN OF / END OF` - method-parameter names (plain and `VALUE(…)` forms) New tests (`tests/parse-interface.test.ts`): parameter named `data`; `REF TO data`; struct fields named `type`/`data`/`value`; qualified type ref with keyword tail. ### `MethodImpl.bodySpan.startLine/startColumn` (Copilot 🟠) Previously pointed at the `METHOD` keyword. Now resolves the first token at/after `bodyStart` via new `Cursor.firstTokenAtOrAfter()` so the span refers to the body content. New tests cover a normal body and the empty-body edge case. ### `parse()` top-level try/catch (my review) Documented contract: "never throws for malformed input". `Cursor .current()` still throws past-EOF. Wrapped the parse loop in a try/catch that turns any unexpected exception into a normalised `ParseError` so the contract holds even if a future refactor forgets an `eof()` guard. ### Remove `void T;` marker (Copilot 🟡) Dropped the unused `import * as T from './tokens'` and the corresponding `void T;` silencer. It added noise and would mask real unused-import issues in future edits. ## Reusable consumer gate New `@abapify/aclass/assert.ts`: - `assertCleanParse(source, fileLabel?)` — throws `AclassParseError` with file:line messages if lex or parse errors exist. - `AclassParseError` carries `{ source, errors }` for tools that want to inspect the diagnostics programmatically. Refactored `packages/openai-codegen/tests/aclass-parse-gate.test.ts` to consume the helper instead of inlining filter logic. Future packages that need the same invariant (e.g. `adt-plugin-abapgit`) can reach for the same helper without reimplementing it. ## Docs / spec alignment ### `openspec/changes/add-aclass-parser` - **`proposal.md`** — describe the parser as a statement splitter (which is what shipped) instead of the Chevrotain `CstParser` form the original proposal described. Rename `cdsClassDef` / `cdsClassImplementation` / `cdsInterfaceDef` to `classDef` / `classImpl` / `interfaceDef` (amazon-q). - **`specs/aclass/spec.md`** — fix the run-on `SourceSpan` docstring (amazon-q). Move `errors: ParseError[]` out of `AbapSourceFile` and into a new `ParseResult` block — the impl always returned `{ ast, errors }` and the spec was the only place they ever lived together (Copilot). - **`tasks.md`** — flip every item to `[x]`; Waves 0–3 are all shipped in this PR. Fix the reversed INTERFACE/INTERFACES keyword ordering note so it matches both the lexer source and `packages/aclass/AGENTS.md` (Copilot). ### `packages/aclass/AGENTS.md` - **"Current status"** — rewrite from "Wave 0 only" to "Waves 0–3 shipped" with a brief per-wave breakdown (Copilot). - **"Architecture"** — replace the `Chevrotain CstParser + Visitor` diagram with the actual statement-splitter pipeline and a short rationale for the choice. - **Key-files table** — list the real source files (`src/parser.ts`, `src/assert.ts`) instead of phantom `src/visitor.ts` / CstParser references. - **Anti-patterns table** — drop the "no dynamic RegExp per keyword" row (Copilot). The `kw()` helper intentionally uses `new RegExp(word, 'i')` with `REGEX_META` escaping; the guidance was contradicting the shipped code. ### PR body Rewritten to reflect what actually landed (Waves 0–3 + live TRL verification). Previously described as "docs-only" which was false from `fc76f504` onward (Copilot). ## Tests & live verification - `aclass`: 48/48 (5 suites) - `openai-codegen`: 153/153 (15 suites) - `abap-ast`: 115/115 (no regressions) - Typecheck ✅ / Lint ✅ / Build ✅ - Live TRL (CB9980002179, us10): deploy → activation pre-audit clean → `adt aunit -c ZCL_PS3_CLIENT_TESTS` → 3/3 pass. ## Intentionally deferred (tracked in `openspec/changes/add-aclass-parser/tasks.md`) - **Fixture corpus location** (my review, low) — stays as live generator output; documented as deliberate coupling. - **`bun.lock` in diff** — expected side-effect of adding the `@abapify/aclass` workspace dev-dep to `@abapify/openai-codegen`. - **Byte-exact roundtrip printer** — spec task; current structural roundtrip is sufficient for the parse-gate invariant. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…loud duplication gate
SonarCloud quality gate flagged `new_duplicated_lines_density` at
3.47% (threshold 3.0%). The only duplicated-block in the aclass
parser was the shared `<head> <name> TYPE <typeref>` preamble in
`parseAttributeDecl` (DATA / CLASS-DATA) and the simple-form branch
of `parseTypeDecl` (TYPES `<x>` TYPE `<y>`).
Extracted `parseNameTypeStatement(c, { name, type })` that collects
the statement, validates the name + TYPE keyword, consumes the type
ref, and returns the shared bag `{ nameTok, typeRes, toks, stmt,
tailIdx }` — `tailIdx` points at the token after the type ref so
callers can still inspect trailing modifiers like `READ-ONLY`.
No behaviour change — both `parseAttributeDecl` and the simple
`parseTypeDecl` path produce identical AST shapes as before. 48/48
aclass tests still pass; 153/153 openai-codegen tests still pass.
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Addresses the 5 CodeRabbit review comments on PR #111 that arrived after the initial fix batch. ## Typed `EventDecl` node (`ast.ts:102` thread) `EVENTS name [EXPORTING VALUE(p) TYPE t].` and `CLASS-EVENTS …` now parse into a proper `EventDecl` AST node instead of falling through to `RawMember`. New interface wired into the `ClassMember` union. `parser.ts` gains a `parseEventDecl()` that reuses `consumeMethodParam()` for exporting parameters so the shape mirrors `MethodDecl.exporting`. ## Stricter fixture gate (`fixtures.test.ts:37` thread) The fixture test previously filtered to lex-only errors. Tightened to `expect(errors).toEqual([])` — the fixture contract is "these generated files parse cleanly", so any new diagnostic (missing ENDCLASS, unknown shape, …) flips the gate red. ## Test-data refresh (`parse-interface.test.ts:211` thread) `EVENTS` used to be the example of an unknown member. Now that it's typed, the old test was locking in stale behaviour. Split into: - a positive `EventDecl` test (name + isClassEvent + exporting param). - a genuinely-unknown `WILDCARD something_unrecognised_by_mvp.` test that still validates the `RawMember` fallback path. ## Diagnostic for unterminated classes (`parse-interface.test.ts:237` thread) Previously `parse('CLASS x DEFINITION.\n PUBLIC SECTION.\n')` silently returned an empty-errors AST. That's a real information leak. `parseClassDef` / `parseClassImpl` now emit an "unterminated CLASS …" diagnostic when `ENDCLASS.` never arrives, and the test requires at least one error with `severity === 'error'`. ## Forward class declarations (tripping the stricter gate) The stricter fixture gate immediately caught that `zcl_petstore3.clas.locals_def.abap` contains CLASS lcl_http DEFINITION DEFERRED. — a valid ABAP forward declaration with no body and no ENDCLASS. `parseClassDef` now short-circuits when it sees `DEFERRED` or `LOAD` in the modifier list, producing an empty-sections `ClassDef` and requiring no ENDCLASS. Same file also uses method names like `METHODS to IMPORTING target TYPE REF TO data.` — `to` tokenises as the `To` keyword (part of `REF TO`), so `isNameLike()` is now applied to method-name positions in both `parseMethodDecl` and `parseMethodImpl` too. ## abaplint second opinion (`aclass-parse-gate.test.ts:44` thread) Parse-gate now runs every generator output through BOTH `aclass` AND `@abaplint/core`. We gate only on `parser_error` keys — abaplint's default rule set includes stylistic rules (`description_empty`, `in_statement_indentation`, `global_class` filename check) that aren't relevant to structural parseability. ## Tests - `aclass`: 49/49 (5 suites — added EventDecl tests, unterminated-class diagnostic test, real-unknown RawMember test) - `openai-codegen`: 159/159 (15 suites — +6 abaplint-gate assertions on top of the existing 5 aclass-gate assertions) - `abap-ast`: 115/115 (no regressions) - typecheck ✅ / lint ✅ Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Responses to the 3 Devin findings that arrived after my previous fix batch. Root cause of both: `parseMethodDecl` / `parseMethodImpl` consumed only a single identifier token as the method name. ## 🔴 Method name = ABAP keyword (e.g. `to`) Both `parseMethodImpl` and `parseMethodDecl` used the strict `nameTok.tokenType.name !== 'Identifier'` check. The word `to` is the `To` keyword (used in `REF TO`, `OF TABLE`, …), so `METHOD to.` — present in `zcl_petstore3.clas.locals_imp.abap:152` — silently dropped the method from the AST. Fix: both sites now use the existing `isNameLike()` type-guard (same approach already applied to attribute / parameter / struct-field names in the previous commit). ## 🟡 Qualified method names (`zif_foo~bar`) truncated `parseMethodImpl` read only `header.tokens[1]`, so `METHOD zif_petstore3~add_pet.` landed with `name: 'zif_petstore3'`. All 19 interface method implementations in the generated `zcl_petstore3.clas.abap` were affected; the body was fine (offset- based slicing) but the AST `MethodImpl.name` was wrong, breaking any consumer that indexes methods by name. Fix: extracted a `readQualifiedName(toks, start)` helper that walks `<name>(~|=>)<name>…` chains (reusing the same pattern `consumeTypeRef` uses for qualified type names), and applied it to both `parseMethodImpl` and `parseMethodDecl` (the latter also handles `METHODS zif_foo~bar REDEFINITION.`). ## Tests - New test: `MethodImpl` of `METHOD zif_petstore3~add_pet.` yields `name === 'zif_petstore3~add_pet'`. - New test: `METHODS zif_foo~bar REDEFINITION.` yields `name === 'zif_foo~bar'` with `isRedefinition === true`. Verified on the petstore3 corpus: the 20 `MethodImpl` nodes in `zcl_petstore3.clas.abap` now show `constructor` + 19 fully-qualified `zif_petstore3~<op>` names. Live TRL (CB9980002179, us10): deploy → activate → 3/3 AUnit pass. Tests: aclass 51/51, openai-codegen 159/159, abap-ast 115/115. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces
@abapify/aclass— a lexer + statement parser + typed AST for ABAP OO (.clas.abap/.intf.abap) — and wires it into@abapify/openai-codegenas a CI parse-gate. Symmetric to@abapify/acds(CDS parser) and the reverse direction of@abapify/abap-ast(ABAP emitter).Live verification on TRL (BTP Steampunk): deploy → activation pre-audit clean →
adt aunit -c ZCL_PS3_CLIENT_TESTS3/3 pass.What landed (Waves 0–3)
packages/aclass/{package.json, tsconfig, tsdown.config, vitest.config, eslint.config},src/tokens.ts,src/lex.ts,src/errors.ts,tests/lex.test.tssrc/ast.ts,src/parser.ts(statement splitter — not a Chevrotain CstParser — see AGENTS.md for rationale),tests/parse-interface.test.tstests/fixtures.test.ts,tests/roundtrip.test.tssrc/assert.ts(assertCleanParse,AclassParseError),packages/openai-codegen/tests/aclass-parse-gate.test.tsScope
In (structural):
DATA data TYPE i.,REF TO data, struct fields namedtype/data, etc.); the parser accepts any identifier-shaped token image.Out (explicitly deferred): method body statement parsing, top-level DATA/TYPES outside classes, FORM/PERFORM, macros, includes, CDS annotations inside classes, SELECT/UPDATE/INSERT, walker helpers, semantic validators.
Tests
aclass: 48/48 (lex smoke, per-topic grammar, keywords-as-names, petstore3 fixtures, structural roundtrip, body-span accuracy,assertCleanParsehelper)openai-codegen: 153/153 (includes newaclass-parse-gate.test.ts— 11 assertions — every generated.clas.abap/.intf.abapmust parse cleanly)abap-ast: 115/115 (no regressions)Live verification
Out of scope for this PR (tracked in
openspec/changes/add-aclass-parser/tasks.md/ spec)print(parse(src)) === srcroundtrip (current test is structural)walkDefinitions,walkMembers, …)aclass → abap-ast/abap-ast → aclassCLASS-EVENTSparameter types,ALIASES … IMPLEMENTED BYRisks / open questions
packages/aclass/AGENTS.md; the wave plan intasks.mdoriginally called for a CstParser, but the statement-splitter approach made the parser half the size with no loss of capability for the structural subset. If a future need for byte-exact recovery / partial CST inspection emerges, the CstParser can be slotted in behind the sameparse()entry point.packages/aclass/AGENTS.md.Generated with Devin
Summary by CodeRabbit
Release Notes
@abapify/aclass, a new parser for ABAP class and interface definitions. Parses.clas.abapand.intf.abapsource files into a typed Abstract Syntax Tree, with method bodies preserved as opaque source slices. Includes standardized error reporting and clean-parse validation for integration workflows.