fix(class): support nested generics in class names (#7648)#7957
Open
Develop-KIM wants to merge 2 commits into
Open
fix(class): support nested generics in class names (#7648)#7957Develop-KIM wants to merge 2 commits into
Develop-KIM wants to merge 2 commits into
Conversation
Class definitions using a nested generic type in the class name, e.g. `class People List~List~Person~~` or `class List~List~Person~~`, threw "Syntax error in text". The class-diagram lexer's `generic` state was flat and could not balance nested `~` delimiters, and `splitClassNameAndType` only kept the first `~`-separated segment, dropping the nested type. The lexer now tracks generic nesting depth and emits the full balanced generic type as a single GENERICTYPE token, and `splitClassNameAndType` converts nested generics to angle-bracket notation (`List~List~Person~~` -> `List<List<Person>>`). Nested generics in attribute/method positions were already handled and are unaffected. Fixes mermaid-js#7648 Fixes mermaid-js#7480 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: 3d4671d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
the
Type: Bug / Error
📝 WalkthroughWalkthroughClass diagram generic parsing now tracks nested tilde delimiters, and class type formatting recursively renders nested generics as escaped angle brackets. New tests cover nested, deeply nested, backticked, and fixture-based class definitions. ChangesNested generic class diagrams
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7957 +/- ##
===========================================
- Coverage 77.51% 74.78% -2.74%
===========================================
Files 564 543 -21
Lines 74906 67400 -7506
Branches 14617 12539 -2078
===========================================
- Hits 58065 50405 -7660
- Misses 15842 16947 +1105
+ Partials 999 48 -951
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/mermaid/src/diagrams/class/classDb.ts`:
- Around line 103-110: Update ClassDB.formatGenericType to parse generic
delimiters with a depth-aware scan, matching each opening tilde with its
corresponding closing tilde instead of using the first and last tilde; preserve
sibling generic arguments and recursively format each nested type. Add a
regression test covering Map<List<Person>,Set<Animal>>.
In `@packages/mermaid/src/diagrams/class/classDiagram.spec.ts`:
- Around line 1474-1483: Update the test case in `classDiagram.spec.ts` to
assert that parsing the nested-generics sample preserves `Person`’s inline
member generic, checking the stored or rendered member text for
`List~List~string~~` rather than only asserting that parsing does not throw.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e855beac-12d9-47fc-b269-dc96c922eecd
📒 Files selected for processing (4)
.changeset/class-nested-generics.mdpackages/mermaid/src/diagrams/class/classDb.tspackages/mermaid/src/diagrams/class/classDiagram.spec.tspackages/mermaid/src/diagrams/class/parser/classDiagram.jison
Comment on lines
+103
to
+110
| private formatGenericType(type: string): string { | ||
| const firstTilde = type.indexOf('~'); | ||
| if (firstTilde === -1) { | ||
| return type; | ||
| } | ||
| const name = type.substring(0, firstTilde); | ||
| const inner = type.substring(firstTilde + 1, type.lastIndexOf('~')); | ||
| return `${name}<${this.formatGenericType(inner)}>`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Match nested delimiters by depth, not only first and last tilde.
formatGenericType mis-associates sibling nested generics. For example, class Container~Map~List~Person~,Set~Animal~~~ is rendered incorrectly because the recursive call consumes the first tilde and the final tilde, treating ,Set~Animal as part of Person.
Use a depth-aware scan matching the lexer’s delimiter rules, and add a regression test for Map<List<Person>,Set<Animal>>.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/mermaid/src/diagrams/class/classDb.ts` around lines 103 - 110,
Update ClassDB.formatGenericType to parse generic delimiters with a depth-aware
scan, matching each opening tilde with its corresponding closing tilde instead
of using the first and last tilde; preserve sibling generic arguments and
recursively format each nested type. Add a regression test covering
Map<List<Person>,Set<Animal>>.
Comment on lines
+1474
to
+1483
| it('should handle the full nested-generics sample from #7648', function () { | ||
| const str = | ||
| 'classDiagram\n' + | ||
| 'class Person {\n' + | ||
| ' ~AnotherInternalProperty : List~List~string~~\n' + | ||
| '}\n' + | ||
| 'class People List~List~Person~~'; | ||
|
|
||
| expect(() => parser.parse(str)).not.toThrow(); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert that the inline member generic is preserved.
The fixture contains List~List~string~~, but not.toThrow() does not verify the member’s stored or rendered text. Assert that Person retains the expected member so regressions in member-level generic handling are detected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/mermaid/src/diagrams/class/classDiagram.spec.ts` around lines 1474 -
1483, Update the test case in `classDiagram.spec.ts` to assert that parsing the
nested-generics sample preserves `Person`’s inline member generic, checking the
stored or rendered member text for `List~List~string~~` rather than only
asserting that parsing does not throw.
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
…rics Address review feedback on mermaid-js#7957. `formatGenericType` previously used the first and last tilde, which mis-associated sibling nested generics such as `Map~List~K~,List~V~~` (rendered `List<K<,List>>` instead of `List<K>,List<V>`). It now walks the string and, like the lexer, treats a `~` as an opening bracket when followed by a word character and a closing bracket otherwise. Added a sibling-generics regression test and an assertion that inline member generics are preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
1 participant
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
Class definitions that use a nested generic in the class name — e.g.
class People List~List~Person~~orclass List~List~Person~~— currently fail with "Syntax error in text".Nested generics in attribute and method positions (inside
{ }) already work, because those lines are consumed whole asMEMBERtokens. The bug is specific to the class-name / shorthand position, which flows through the lexer'sgenericstart-condition.Closes #7648
Closes #7480
Root cause
classDiagram.jison): thegenericstart-condition was flat —<generic>[~]popped on the first~, so it could not balance nested~delimiters.List~List~Person~~tokenized asALPHA GENERICTYPE ALPHA …, which noclassNamerule can assemble.splitClassNameAndType(classDb.ts): kept onlyid.split('~')[1], so even once parsed the nested inner type was dropped.Fix
genericlexer state now tracks nesting depth and captures the full balanced generic as a singleGENERICTYPEtoken. A~immediately followed by a word character opens a nested level; the matching~closes it.splitClassNameAndTypenow takes everything between the first and last~and converts nested generics to angle-bracket notation, e.g.List~List~Person~~→List<List<Person>>(emitted as</>to match how class labels are escaped elsewhere in the file).Single-level generics (
Class~T~) are unchanged.Before / after
class List~List~Person~~List<List<Person>>class Container~Map~String,List~Int~~~Container<Map<String,List<Int>>>class Class1~T~Class1<T>Class1<T>(unchanged)Tests
Added 4 regression tests in
classDiagram.spec.ts(nested class name, deeply nested, literal name, and the exact sample from #7648). They fail ondevelopand pass with this change; the full class-diagram suite (495 tests) stays green. Added a changeset.Summary
~delimiters and emit balancedGENERICTYPEtokens.List<List<Person>>.fix(class)changeset referencing issues#7648and#7480.