bafolts / tplant

Typescript to plantuml
https://segmentationfaults.com/tplant/default.html
GNU General Public License v3.0
266 stars 34 forks source link

refactor formatter and add mermaid support #73

Closed loopingz closed 3 years ago

loopingz commented 3 years ago

I wanted to use mermaid classDiagram so I did an early implementation/refactoring of Formatter, which allow to keep former PlantUML but also to use Mermaid

I haven't yet added the unit test for mermaid format but would like some feedback before I do

bafolts commented 3 years ago

This looks great to me. Add unit tests and lets ship!

loopingz commented 3 years ago

The linter fail on GitHub Actions but not on my side:

no-use-before-declare is deprecated. Since TypeScript 2.9. Please use the built-in compiler checks instead.
Warning: no-function-constructor-with-string-args rule is deprecated. Replace your usage with the TSLint function-constructor rule.
Warning: no-reserved-keywords rule is deprecated. Replace your usage with the TSLint variable-name rule.
Warning: no-increment-decrement rule is deprecated. Replace your usage with the TSLint increment-decrement rule.
Warning: no-unnecessary-bind rule is deprecated. Replace your usage with the TSLint unnecessary-bind rule.

This is this error on Github Actions

ERROR: /home/runner/work/tplant/tplant/src/Factories/ParameterFactory.ts:18:17 - This assertion is unnecessary since it does not change the type of the expression.
ERROR: /home/runner/work/tplant/tplant/src/Factories/PropertyFactory.ts:15:17 - This assertion is unnecessary since it does not change the type of the expression.

And without the cast tsc fails on my side.

Any ideas?

bafolts commented 3 years ago

If I remove my node_modules directory and run npm run install with the following change both npm run build and npm run lint pass for me on this branch.

diff --git a/src/Factories/ParameterFactory.ts b/src/Factories/ParameterFactory.ts
index 95a896c..4b41e2d 100644
--- a/src/Factories/ParameterFactory.ts
+++ b/src/Factories/ParameterFactory.ts
@@ -15,7 +15,7 @@ export namespace ParameterFactory {
         result.parameterType = checker.typeToString(
             checker.getTypeOfSymbolAtLocation(
                 parameterSymbol,
-                <ts.Declaration> parameterSymbol.valueDeclaration
+                parameterSymbol.valueDeclaration
             ),
             declaration
         );
loopingz commented 3 years ago

My VSCode is complaining but not the linter so all good with the latest change. If you like/want I can refactor the test of PlantUML with the same type of result files test instead of the string[].join(os.EOL)

loopingz commented 3 years ago

If you are good with this one, I can create another PR with test refactoring if you want it. The two other changes I'd like to work on is: