Update class member handling

[jgreywolf]

📑 Summary

The intent of this PR is to streamline the logic for parsing member statements for classes, and to make it then easier to fix other issues. This PR includes:

• fixes around comments

• properly handling different scenarios around generics

• initial work to not show members box if there are no attributes or methods

• handle leading newlines

• changed name from "member" to "attribute" to be explicit and distinct from members as a whole

• resolves Cannot have Generic of Generics in Class diagram #3287

• resolves Comments in classDiagram should be allowed inside of Class definitions #1645

• resolves Property strings on class diagrams #3189

• resolves error in parser when string to parse is \nclassDiagram #3243

• partial Method return not displayed in Class Diagram when using JavaScript computed method names #2185

📏 Design Decisions

Added a new class for members specifically, and all parsing of that member is handled within the class itself. Created a more thorough regex specific for methods to capture any of the expected values - or have an empty result if that value was not matched. There has been talk about whether or not the type or name of an attribute or method parameter should go first - so handling is set to take however the value is typed for that section

📋 Tasks

Make sure you

• 📖 have read the contribution guidelines
• 💻 have added unit/e2e tests (if appropriate)
• 📓 have added documentation (if appropriate)
• 🔖 targeted develop branch

[sidharthv96]

Looks like the unit tests caught a valid issue.

[jgreywolf]

Looks like the unit tests caught a valid issue.

I think this is something to be discussed - In theory I think that this test is invalid. I say in theory - because when creating a class member if the the first character is a ~, it basically gets stripped out, and will not be present in the text that is sent to the function.

However, if something else decides to call that function which is outside of classes, then that convention would not be "forced"

[sidharthv96]

Ok, so if it's an invalid test, let's remove it.

[jgreywolf]

Ok, so if it's an invalid test, let's remove it.

Yes - I had left it in for now to have the discussion ;)

[codecov-commenter]

Codecov Report

Merging #4534 (05c2a11) into develop (ed819e9) will decrease coverage by 3.60%.
Report is 4 commits behind head on develop.
The diff coverage is 65.42%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4534      +/-   ##
===========================================
- Coverage    77.05%   73.45%   -3.60%     
===========================================
  Files          146      146              
  Lines        14573    14487      -86     
  Branches       592      612      +20     
===========================================
- Hits         11229    10642     -587     
- Misses        3225     3704     +479     
- Partials       119      141      +22     

Flag	Coverage Δ
e2e	79.18% <81.17%> (-4.74%)	⬇️
unit	45.34% <7.54%> (-0.22%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
...ges/mermaid/src/diagrams/class/classRenderer-v2.ts	79.85% <ø> (ø)
packages/mermaid/src/diagrams/common/common.ts	71.03% <41.50%> (-3.97%)	⬇️
packages/mermaid/src/diagrams/class/classTypes.ts	86.95% <86.95%> (ø)
packages/mermaid/src/dagre-wrapper/nodes.js	81.45% <100.00%> (-8.17%)	⬇️
packages/mermaid/src/diagrams/class/classDb.ts	82.35% <100.00%> (+8.55%)	⬆️

... and 23 files with indirect coverage changes

[jgreywolf]

@knsv @sidharthv96 Please review

[aloisklink]

I'm not knowledgeable in class diagrams, so please take my review with a grain of salt!

Adding the new ClassMember class is a change I love! It can be pretty difficult for me to understand some of diagram parsing/DB code, so this definitely makes it easier to understand.

Before merging, is there any chance you can add some TSDoc comments to the ClassMember class? Ideally every publicly available method and field should have a short description.

[sidharthv96]

@jgreywolf the tests failures look like valid errors. So please fix those.

[jgreywolf]

@jgreywolf the tests failures look like valid errors. So please fix those.

Ill have to wait for the new actions to finish, because I pushed a small update. You say multiple? Thats frustrating - all looked good here :(

[jgreywolf]

@sidharthv96 errors went away after rerunning.

[sidharthv96]

@jgreywolf we can merge this once you review #4805

[sidharthv96]

Nice job fixing all these issues! Loved the new approach.

[mermaid-bot]

@jgreywolf, Thank you for the contribution!
You are now eligible for a year of Premium account on MermaidChart.
Sign up with your GitHub account to activate.