search

dbolin / Apex.Analyzers

Roslyn powered analyzers for C# to support convention defined architecture
MIT License
15 stars 2 forks source link

Helpers could be exposed publicly (or via separate NuGet package) #30

Closed mwelsh1118 closed 4 years ago

mwelsh1118 commented 4 years ago

We're using your immutability analyzers to detect mutability problems in our code. We have our own set of analyzers that build on top of yours. Specifically, we are trying to prevent static state that is mutable. Our analyzers use similar logic to what is defined in your Helpers class. Could this be made public or exposed as a separate Apex.Analyzers.Helpers package?

dbolin commented 4 years ago

Not opposed to this (I'd probably make it a separate package unless you have reasons why it would be better not to). I'd have to think about what should be exposed there and what's really internal.

What methods would you use right now? E.g. would you use anything other than IsImmutableType()?

mwelsh1118 commented 4 years ago

I think exposing IsImmutableType() would be sufficient for our purposes. Also agree that a separate package is cleaner.

mwelsh1118 commented 4 years ago

@dbolin, Let me know if you'd like me to break out a separate project (Apex.Analyzers.Helpers?). Happy to send you a PR if you haven't started on this already.

dbolin commented 4 years ago

Yeah, not sure what I would like the name to be, since Helpers is just kind of vague. The namespace prefix should be Apex.Analyzers.Immutable... since this is for the immutability rules (I don't have any other analyzers yet, but if I do they would be in separate packages).

mwelsh1118 commented 4 years ago

Apex.Analyzers.Immutable.Primitives? Apex.Analyzers.Immutable.TypeInfo? Apex.Analyzers.Immutable.Metadata?

dbolin commented 4 years ago

I'll go with Apex.Analyzers.Immutable.Semantics - still not quite accurate, but closer than anything else I can think of now.

dbolin commented 4 years ago

https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/1624#issuecomment-151197400

Somehow when you install the analyzer, the consuming project needs to reference the analyzer's dependencies also as an analyzer, or it can't load them. I haven't figured out a way to make this happen automatically. I might just reference the same cs files from both projects if there isn't another way to do this.

dbolin commented 4 years ago

This also affects analyzers that consume the Semantics package, so this needs a good solution if this is to work at all.

dbolin commented 4 years ago

Ended up including it in the package e.g. <None Include="$(OutputPath)\Apex.Analyzers.Immutable.Semantics.dll" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" />

mwelsh1118 commented 4 years ago

That is working great! Thanks for the help!