Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use Roslyn code analyzer to parse source code #1

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

vojtechkacmarik
Copy link

Roslyn Code Analyzers are used to C# code analysis.

I implemented C# code analysis using Roslyn.
The original implementation using NRefactory is still existed.
I done this as compatible solution, all of components are used using DI and implementation against interfaces.
I updated a few Unit Tests because I assume that they calculate the wrong values (I compared measured values againts R# Cyclomatic Complexity Extension and also Code Metrics Viewer, Tools included in VS).
The structure of the project is very similar to the original state.
Support for VS2017 is not added yet. I think it would be better to add this commit as separate task.

@elishalom elishalom self-assigned this Apr 29, 2017
@elishalom elishalom self-requested a review April 29, 2017 14:22
@elishalom
Copy link
Owner

Thanks for the effort. Moving to Roslyn is an essential step forward for this project.

One of the things I noticed is that the updated code computes cyclomatic complexity rather than simple complexity. Simple complexity is a metric introduced in Code-Complete, which counts decision points. I tend to keep it as the default metric since it got mainly positive feedbacks so far. We had in the past a feature request to be able to support cyclomatic complexity as well. It seems like we have a trigger to add this support now.

@vojtechkacmarik
Copy link
Author

I assume that code computes the same complexity value as before. Because all existing Unit Tests return the original expected values.
In a few cases I modified return value because I assumed that the computed value is wrong (maybe 2-3 examples).
Maybe it seems like different complexity because I renamed IComplexity to ICyclomaticComplexity (Calculator) but the measured process is very similar as origin.

Did you have some special reason to separate Parsing business logic and Calculation business logic to different projects? Currently when I use Roslyn it is unnecessarily complicated to parse C# code to some abstract representation (like ISyntaxNode etc.) and pass this instance to Calculator. I assume that it would be better use Roslyn to do everything at one time. I have a few problems to parse C# code in some cases (e.g. C# code is dirty and Roslyn analyzes uncompleted, not compiled code). Of course we can place component behind some interface.

Btw. thanks for adding support for VS2017. I will also add into my branch.

@vojtechkacmarik
Copy link
Author

Hi Eli,
I allowed myself to publish my version (fork) of Code Metrices which uses Roslyn. I have named it like 'Roslyn Code Metrices'. I have also mentioned all original notes, sources, licences etc. in my custom version of your tool.

Mainly I have wanted to share this tool for my colleagues (for use in VS2017 and C# 6.0 features).

I hope that it is no problem for you.
I have only good intention and I have spent lot of time to re-write this tool using Roslyn so I have wanted to publish it.
In case that you will not want to have published this tool please let me know and I will remove my version from VS Gallery. I don't want some problems with you.

Kind regards Vojtěch Kačmařík

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants