C# support

[mackowski]

I'm opening a ticket to indicate interest in support for the C# Language.
I can help with developing it if you can point me how to start and where to look.

[dlukeomalley]

Thank you for filing this @mackowski. Adding @aryx and @nbrahms to discuss what adding support for a new language looks like. We'd love to get more folks involved there.

[aryx]

Are you familiar with OCaml? the only remaining part to handle C# needs to be written in OCaml (in semgrep/semgrep-core/parsing/ like the Parse_ruby_tree_sitter.ml in this directory for example, but here it would be a Parse_csharp_tree_sitter.ml. We can generate the boilerplate for this file, but we still need to convert that in the generic AST (defined in pfff/h_program-lang/AST_generic.ml)

[mackowski]

I do not know OCaml, but it seems to be fun to learn it. Anyway, as I did not do it before I do not know how much time is needed to do it. I am ready to learn something new and help you with this project, but I also do not want to be a roadblock.
If you can generate the boilerplate in a new branch, I can try to start writing mapping, I will use other languages as examples. If you have capacity and willingness to do this faster do not wait for me, I will review the code to understand it better.

[aryx]

Sounds great! @mjambon can you add csharp in ocaml-tree-sitter-lang? I'll create the Parse_csharp_tree_sitter.ml boilerplate with a few cases filled in as a starting point.

[mjambon]

@aryx I just added csharp to ocaml-tree-sitter-lang. I parsed one sample program successfully on the first try but didn't run large-scale stats.

[aryx]

Ok, I'll create the Parse_csharp_tree_sitter.ml boilerplate with a few cases filled in as a starting point.

[ievans]

@aryx looks like you added the basic parser; did you want to follow up with @mackowski ?

[aryx]

No I didn't. I guess I need to add a few simple cases to be a better starting point and maybe some documentation on where to find the relevant AST definitions.

[aryx]

The main work @mackowski is to replace all the todo in https://github.com/returntocorp/semgrep/blob/develop/semgrep-core/parsing/Parse_csharp_tree_sitter.ml with the relevant construct defined in https://github.com/returntocorp/pfff/blob/master/h_program-lang/AST_generic.ml
Not trivial, but not impossible :)

[mackowski]

looks good! Next week I will be on a small trip without my laptop but after that I will start working on that :)

[Sjord]

I have started an attempt.

[Sjord]

I can't find anything in AST_Generic to represent an expression with an infix operator, such as 1 + 2. I would expect expr * operator * expr. There is AssignOp, but as I understand, that is for a += 1. Am I missing something?

[aryx]

Yes you need to convert that in a Call (IdSpecial (Op Plus), e1, e2)
You can look in the codebase of pffff for js_to_generic.ml, or java_to_generic.ml to see how we convert those languages to the generic AST.

[aryx]

Feel free to ask many many questions @Sjord. We know AST_generic.ml is not very well commented, so we know it's tedious to write those converters from tree-sitter CST to AST_generic.ml
Fantastic job btw, this looks great.

[Sjord]

C# has multidimensional arrays:

        // Three-dimensional array.
        int[, ,] array3D = new int[,,] { { { 1, 2, 3 }, { 4, 5, 6 } },
                                        { { 7, 8, 9 }, { 10, 11, 12 } } };
        // The same array with dimensions specified.
        int[, ,] array3Da = new int[2, 2, 3] { { { 1, 2, 3 }, { 4, 5, 6 } },
                                            { { 7, 8, 9 }, { 10, 11, 12 } } };

        // Accessing array elements.
        System.Console.WriteLine(array2D[0, 0]);

I was thinking of representing this as a TyArray with a Tuple, so that writing array2D[0, 0] is actually accessing array2D with the tuple (0, 0).

However, this presents a problem when parsing something like this:

int[, ,] array3D

This is an array with a tuple of (nothing, nothing, nothing), and that can't be represented as a Tuple as far as I know. Any ideas?

Edit: an alternative is to treat it as an array of arrays. C# also has those, but maybe it's not so bad if we mix up jagged arrays with multidimensional arrays.

[aryx]

Hmm, yes not super easy to represent right now. We could extend the generic AST at some point to better handle those.
In the mean time when a construct does not fit, you can use one of the OtherXxx constructor and Ox_Todo,
for example here could return for those multidimensional array something like
OtherType (OT_Todo, [T (TyArray ((lbracket_tok, None, rbracket_tok), type_elt)])

And later when we improve the generic AST we can transform those OtherType in something more precise.

[aryx]

How are things progressing @Sjord ? Anything we can do to help?
Do you think we could merge what you wrote?

[Sjord]

Progress is slow but steady. The parser can currently parse most common features. We started with 329 todo's. I have resolved 201 of those, and added another 30. The TODO's I added are mostly when I am not sure if what I'm doing is the correct way.

I ran into a couple of upstream issues in treesitter. The treesitter developers are very fast and helpful with resolving these, but I could use your help to get these fixes into semgrep-core.

I am currently only working on the parser, and testing by running semgrep-core -dump_ast. Presumably other parts of semgrep also need some work before semgrep works on C#. I think semgrep currently doesn't even call this parser. I could use some help with that.

The parser is currently half-finished. We could merge it in the current state. It wouldn't break anything, but it is not really complete. Do you have a review process? Perhaps it is a good idea to start reviewing already, before the whole thing is done.

[aryx]

Fantastic! We can definitely help later to update to the latest tree-sitter-csharp and also to extend the grammar to allow semgrep-specific constructs (metavariables, ellipsis). In the mean time, I think the easiest way to go forward and for you to make a Pull Request on our repo so we can review it and merge it.

[cfguimaraes]

@Sjord congratulations, you have done a lot, the community praises you.
I'm here to offer my help.
I see that the work you have done is far away from my knowledge, but I believe I can help with testing, or if some suggest, I could learn to help.

P.S. Sorry for my English, I'm brazilian guy that made a cheaper course :)

[Sjord]

Thanks, @cfguimaraes. You can help me improve the parser by creating small C# test files that have a construct that is currently not implemented. There are two ways to find those:

1. Search in Parse_csharp_tree_sitter.ml for todo env. The first occurence is here, in a function called attribute_target_specifier. Search in the docs how attribute target specifiers work, and create a little C# file similar to the files here.
2. Try to parse actual C# files until you encounter a file that can't be parsed correctly. Then reduce it to a minimal test case.

You can run semgrep-core -dump_ast -lang cs somefile.cs to check whether the file parses. What you are looking for is:

Fatal error: exception (Failure "not implemented")

If you have a minimal test case, this makes it easier for me to implement correct parsing.

What's also needed is semgrep integration tests, like these but for C#. However, I think C# is currently not sufficiently implemented to make passable tests, and I am not really sure how to write these. @aryx, can you comment on that?

[aryx]

For the semgrep integration tests you can follow https://semgrep.dev/docs/status/#maturity-definitions and try at least the tests for the "Alpha" category. You can imitate the tests for lua/ which has currently the "Alpha" state.
If something does not work we probably need to extend the semgrep-grammar for C# at https://github.com/returntocorp/semgrep-grammars/blob/master/src/semgrep-c-sharp/grammar.js
(and again, to possibly imitate what we did for the other languages in his semgrep-grammars/src/semgrep-xxx directories.

[cfguimaraes]

I've been looking at these links, I believe I can help you right now with option 2, parsing files and making it reproducible.
As I learn more about Semgrep I can write tests, no problem =D
And who knows someday I could help you with the parser.

Talk to you as soon as I found this exception in our codebase (is plenty of scenarios)

[ievans]

@cfguimaraes I am excited that you are interested in helping! If you would like even faster responses, you can also join us on Slack: https://r2c.dev/slack. Most of the developers are there and we are very friendly!

[Sjord]

We really need the latest tree-sitter-c-sharp to make semgrep work correctly, even for alpha functionality. I started updating it in semgrep-grammars, but this broke the build after the PR was merged, and I don't fully understand why.

[aryx]

What is the error message? What is broken?

[Sjord]

The CircleCI image here showed failed instead of passed, but when I tried to reproduce it I got different errors each time. However, I think I solved it in my PR.

[Sjord]

@mjambon, could you perhaps import the latest upstream C# grammar? The version in semgrep-c-sharp is pretty old and contains several serious bugs.

[aryx]

@mjambon I can have a go at it. Good way to test for me the new ocaml-tree-sitter organization.

[cfguimaraes]

Hello all :)

@Sjord when running locally in windows via docker I have got these exceptions:

FormaPagamentoService.cs:1:0: Fatal Error: Common.Impossible

Fatal error: exception Common.Impossible
error invoking semgrep with:
        /usr/local/bin/semgrep-core -lang cs -dump_ast FormaPagamentoService.cs
        Command '['/usr/local/bin/semgrep-core', '-lang', 'cs', '-dump_ast', 'FormaPagamentoService.cs']' returned non-zero exit status 2.
An error occurred while invoking the semgrep engine; please help us fix this by creating an issue at https://github.com/returntocorp/semgrep
PagamentoCartaoService.cs:1:0: Fatal Error: AST_generic_helpers.NotAnExpr

Fatal error: exception AST_generic_helpers.NotAnExpr
error invoking semgrep with:
        /usr/local/bin/semgrep-core -lang cs -dump_ast PagamentoCartaoService.cs
        Command '['/usr/local/bin/semgrep-core', '-lang', 'cs', '-dump_ast', 'PagamentoCartaoService.cs']' returned non-zero exit status 2.
An error occurred while invoking the semgrep engine; please help us fix this by creating an issue at https://github.com/returntocorp/semgrep

I know this isn't what you are want me to look for Fatal error: exception (Failure "not implemented"). But I'm curious if I should ignore or report these as separate?

[Sjord]

Fatal error: exception Common.Impossible

This is probably because of a bug in the tree-sitter grammar. It doesn't currently make much sense to create a test case for this, until we have an updated grammar.

Fatal error: exception AST_generic_helpers.NotAnExpr

This is new to me. This could be interesting to have a test case for.

[cfguimaraes]

I found what is the cause of the Error you are interested in.

I've made the minimum reproducible code that I can

Example
{
    public class ExampleClass
    {
        public async Task<ApiResponseTransacao> Method(Request request)
        {
            (int x, int y) = request.A.B().Value;
        }
    }
}

Sorry for the ugly names, it's because of my NDA.

In the code above Request is a DTO Class, request.A is a string property and request.B is an extension method that returns an (int,int)? Nullable tuple.

I want to know from you what is the best way you would like to receive these snippets. And of course,

My next step is learn more about @aryx suggestion on https://semgrep.dev/docs/status/#maturity-definitions to known a little more about definitions and after copy some implementation of integration tests write integration tests :)

@Sjord if you want, I can make a project with this code.

[cfguimaraes]

Important too, I've been running docker this way
docker run --rm -v "${PWD}:/src" returntocorp/semgrep --dump-ast -l cs file.cs

[Sjord]

Thanks, @cfguimaraes. Yes, I think a new GitHub project with code snippets is the easiest way. Then I will add them to the semgrep parsing tests as soon as I implement the parsing code for them.

[aryx]

update to latest tree-sitter-csharp here: semgrep/ocaml-tree-sitter-semgrep#154

[aryx]

And here is the PR to update semgrep: #2567 (incomplete).

[aryx]

@Sjord there are lots of changes in the latest tree-sitter-csharp. Would you mind fix #2567 ?

[cfguimaraes]

@Sjord sorry for the late reply.
I see that you already have evolved since our last talk.
I've submitted the samples to GitHub but didn't mentioned here.

The link with samples is https://github.com/cfguimaraes/semgrep-AST_generic_helpers.NotAnExpr

[Sjord]

#3121

[mschwager]

Can we close this out now that C# has reached alpha: #3121? If we'd like to track additional improvements we should create new, specific issues (e.g. beta, GA, parse errors, etc).

[mschwager]

Closing out - feel free to open new, specific issues for additional C# work!

Thanks for all the hard work here!