search

microsoft / kiota

OpenAPI based HTTP Client code generator
https://aka.ms/kiota/docs
MIT License
2.84k stars 194 forks source link

add auto-generated comment in CSharp to avoid triggering static analysis #2886

Closed HowardvanRooijen closed 1 year ago

HowardvanRooijen commented 1 year ago

A big ask but I was wondering if it would be possible to either generate StyleCop compliant code OR to add // <auto-generated /> headers to generated C# files so that StyleCop could automatically ignore these files?

I was also wondering if it would be possible to specify a .NET version - thus if you're targeting .NET 6 / 7 / 8 you don't need all the debug conditionals to deal with .NET Standard / .NET Core 3.1 if you're not supporting them.

darrelmiller commented 1 year ago

@HowardvanRooijen Thanks for the feedback. I remember you telling me about the comment issue and I completely forgot to create an issue for it. I agree that it would be really nice if we didn't need all the conditionals.

HowardvanRooijen commented 1 year ago

On the StyleCop front, we have some standard nuget "recommended practices" packages we pull into projects to configure them for our standard build / deployment process which includes StyleCop.

My current workaround is adding exclusions to the projects to remove the 3000+ rule violations generated. It's quite a long list!

SA1629;SA1201;SA1134;SA1514;SA1615;SA1600;SA1602;CS1591;SA1128;SA1128;SA1611;SA1633;SA1612;SA1117;SA1116;SA1623;
baywet commented 1 year ago

Hi @HowardvanRooijen Thanks for using Kiota and for reaching out. You'll find some context in #2594 and #2073 as to the conditional compilation, the fact that kiota doesn't have language specific options and more.

As for linting/static analysis in general, all those rules can be configured differently by the target projects, additional ones can be added, which means there's no "right" version of the generated code. Of course there are some more widely adopted than others we might consider. @darrelmiller the only reason why we need conditional compilation is to keep compatibility with .net framework. Changing this now would be a breaking change.

With that in mind, having the auto-generated comment at the top of the files or changing the extension to .g.cs see this important note

This is something that could be added at the following locations: https://github.com/microsoft/kiota/blob/1ff6537bea89442dabacf4be341d57ca644d120e/src/Kiota.Builder/Writers/CSharp/CodeClassDeclarationWriter.cs#L35 https://github.com/microsoft/kiota/blob/1ff6537bea89442dabacf4be341d57ca644d120e/src/Kiota.Builder/Writers/CSharp/CodeEnumWriter.cs#L21

Is this something you'd be willing to submit a pull request for?

HowardvanRooijen commented 1 year ago

I absolutely understand that StyleCop is a fickle and user configurable tool (I maintained it when it got OSS'd on CodePlex back in the day!), so totally understand your position on that. Although I still think the OOTB settings are a good baseline.

And yes, I'd be happy to have a go at implementing it and issuing a PR for the header. I'll try and figure out when I can make time to investigate!

sebastienlevert commented 1 year ago

@HowardvanRooijen are you still willing to help with implementing these changes? We'd love to get your contribution! @andrueastman would be a good candidate to help with this one also. Thanks!

HowardvanRooijen commented 1 year ago

I am - it's just finding the window in my month where I have capacity (the joys of a tech small business owner!).

samarthasthana commented 1 year ago

+1 on adding the auto generated header on top of the classes generated using kiota. We are currently trying to port over to kiota lib for code generation and our project is using StyleCop.Analyzers version 1.1.0+ and we are running into this issue.

If the header // is added the stylecop is able to identify the class and not show warnings.

Also, curious to know if the code generated can actually honor the stylecop rules when generating code. It would make the generated classes more readable and comparable to the non generated classes.

baywet commented 1 year ago

@samarthasthana the challenge with "generating the rules that comply with Stylecop" is that projects can have their rules be configured different ways, or even use different linters/static analysis tools. So even if we get compliant with one tool/set of rules, it might conflict with another.

samarthasthana commented 1 year ago

Agreed, it would make sense to choose one code analysis tool and be in compliance just so that the generated code is more readable and navigable. Adding the autogenerated header on top would still be required so that the target project's code analyzer does not flag the code.

samarthasthana commented 1 year ago

Created the Pull Req 3034 Kindly review. cc @sebastienlevert , @baywet, @darrelmiller , @andrueastman