Missing analyzer in visual studio?

[swegele]

I updated to CSLA 5.2 and today 5.3 and I get many errors from analyzer

CSLA0011 - CSLA business objects should not be created outside of a ObjectFactory instance.

But I only see analyzer numbers 1 to 10 in the list, so I cannot disable it.
(see screenshots). Am I missing something?

Version and Platform
CSLA version: 5.3.0
OS: Windows
Platform: ASP.NET Webforms and Core
IDE: Visual Studio 2019 (latest)

[rockfordlhotka]

Have you closed and reopened VS?

[swegele]

Have you closed and reopened VS?

Yes, I first noticed this when I did a branch where I upgraded from 4.6.x to 5.2. I ended up just deleting the csla analyzers from the References/Analyzers section in all projects.
But then they came back when I did the Nuget update to 5.3 yesterday.

I do have ruleset files in the projects from when I had previously changed some of the CSLA analyzer rules to type none. I wonder if that might have an effect on rules showing up versus when there is no preexisting ruleset file. Shooting in the dark there.

[rockfordlhotka]

@JasonBock ???

[JasonBock]

I don't know why CSLA0011 would not show up in the list. I just created a project and referenced the Csla package and this is what I see:

[JasonBock]

Side question: Are you automatically disabling all of the CSLA analyzers? I really hope you're not ;)

[swegele]

Side question: Are you automatically disabling all of the CSLA analyzers? I really hope you're not ;)

@JasonBock - No, previously ~4.6.x I had only selected the couple that did not apply in our scenario, changing them to not block us with errors. But when I could no longer see the new rules throwing errors, the only way around it was to delete all the analyzers entirely which obviously is worrisome.

Also, WOW I guess I'm not seeing any of the rules above 10. Hmmm this is weird.

[JasonBock]

I had only selected the couple that did not apply in our scenario, changing them to not block us with errors.

Which ones were they? I'm curious to know what they are and why they would block you.

[swegele]

@JasonBock I have to delete the CSLA Nuget reference from one of my business layer projects and then put it back in to get the analyzers back. Consequently I will delete the ruleset file first as well to see if that not being there makes them show up properly. Will comment with results shortly.

FYI - upon deleting the CSLA 5.3 Nuget reference from the project I noticed this in the Output window -> Package Manager

Attempting to gather dependency information for package 'Csla.5.3.0' with respect to project 'Business Layer\xxxx.Common.BL', targeting '.NETFramework,Version=v4.7.2'
Resolving actions to uninstall package 'Csla.5.3.0'
Resolved actions to uninstall package 'Csla.5.3.0'
Executing script file 'C:\xxxx\packages\Csla.5.3.0\tools\uninstall.ps1'...
Error HRESULT E_FAIL has been returned from a call to a COM component.
At C:\xxxx\packages\Csla.5.3.0\tools\uninstall.ps1:14 char:17

• ... $project.Object.AnalyzerReferences.Remove($analyzerFilePa ...

•             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  

  • CategoryInfo : OperationStopped: (:) [], COMException
  • FullyQualifiedErrorId : System.Runtime.InteropServices.COMException

Removed package 'Csla.5.3.0' from 'packages.config'
Successfully uninstalled 'Csla.5.3.0' from xxxx
Executing nuget actions took 8.63 sec
Time Elapsed: 00:00:08.9731103
========== Finished ==========

[JasonBock]

The .ps file is automatically generated for an analyzer project....well, that's what happened with analyzer projects when this was created. Not sure if that's even needed anymore. It may be easier in the long run to look at different ways to reference the analyzers from CSLA but I'll defer that to @rockfordlhotka.

When you say "ruleset" file are you talking ".ruleset" or ".editorconfig"? The latter is what should be used now for analyzer configuration.

[swegele]

@JasonBock @rockfordlhotka Success...

• I removed the old .ruleset file from the project (that file has been there for years since way back when analyzers first started being added in CSLA that threw errors instead of just warnings)
• I removed the CSLA 5.3 nuget reference
• I added it back in and viola I now see all the analyzer rules 1 - 19

So I'm going to assume it was the presence of that pre-existing ruleset file, which as you say Jason is perhaps an older outdated way of handling customizing the analyzer behaviors.

I will answer your question in the next separate comment about what error analyzer rules we are disabling and why (at the risk of airing my dirty laundry :-)

[swegele]

@JasonBock Per your question...we change the following 3 analyzer rules to not throw an error...reasons below each are listed. We change them to none or warning.

CSLA0003 - CSLA business objects must have a public constructor with no arguments
We do not have any public constructors on our business objects. I remember when this need to allow public constructors came (back in the silverlight days). I didn't like it then and still don't today. Private constructors and the corresponding Public factory methods make a nice happy path for non-business layer devs who wish to consume the BL. For better or worse, we made a decision early on that we were not going to have our business objects being crated like that since we were not supporting any async only client side scenarios. I've often wondered if I will end up having to cave on that when blazor wasm inevitably warms my hearth.

CSLA0008 - Managed backing fields must be public, static and read-only
Adopted around the same time for silverlight...I also did not like this as it clutters up the view of a class in intellisense. IMHO this low-key invites some to mess with things they shouldn't even be seeing (were it not required by limitations in silverlight). Yes we can use the visible-false attributes...but that never seems to work for me in visual studio anyway.

CSLA0011 - CSLA business objects should not be created outside of a ObjectFactory instance.
Again, IMHO this is an optional pattern (even if recommended)...but it isn't required for any reason in our projects.

Does that help answer your question?

[swegele]

When you say "ruleset" file are you talking ".ruleset" or ".editorconfig"? The latter is what should be used now for analyzer configuration.

Now that I've removed the old .ruleset file...and then go to change the analyzer rule to warning from error...I see VS adds a new .editorconfig as you indicated. SWEET! One per solution instead of one in each project. I'm washing those old .ruleset files right out of my hair. Thanks guys! I think this was an issue with me needed to stay up with modern methods rather than a bug with CSLA analyzers. Honestly this was the first I heard of that .editorconfig. Ugh so much to know.

[JasonBock]

@swegele thanks for the comments about disabling analyzers. Here's my response:

CSLA0003 - Public, no argument constructors are necessary for serialization. I understand your point, but this analyzer is trying to prevent issues that may happen in distributed scenarios. Now, one thought that came to mind is that your C# code may not have any constructors, in which case I believe the C# compiler will add a public no-argument constructor in. This may be something that I haven't considered, so I'll put in an issue to track that.
CSLA0008 - This is one that is needed for managed backing fields to work correctly.
CSLA0011 - The DataPortal should control the creation of business objects, not your code. Doing new Customer() in your code may work, but it also may lead to unintended or unexpected behaviors as well.

(As a side note, the docs on the analyzers may need updates to explain these in more detail.)

All of the analyzers I run by @rockfordlhotka before I do any significant work, because I want to make sure they're enforcing rules and conventions that should (or should not) be done in CSLA, and that they're applicable for any developer using CSLA. He can add more clarity around why these analyzers are there.

[rockfordlhotka]

• CSLA003 - as we've moved steadily away from BinaryFormatter/NDCS toward MobileFormatter it can be a major challenge if your classes don't have a default public ctor
• CSLA0011 - this is really related to 003, because while your type has a public ctor, you should never call it!! The use of the new keyword is a source of serious errors because it bypasses all the standard lifetime management provided by the data portal
• CSLA008 - see Jason's comment

[swegele]

@JasonBock @rockfordlhotka I'd welcome a little more feedback and your thoughts.

CSLA003 & CSLA011- Each of our business objects have a default (no-parameter) PRIVATE constructor. The only time it is ever called is within that object by itself in it's own encapsulated static factory method. Are you saying this is a problem? Rocky you are talking about serialize/deserializing?

CSLA008 - Can you define "properly" a little more. NONE of our managed backing PropertyInfo fields are public and we are working fine for many years. What am I missing?

Thanks guys for the guidance...hopefully what you say might help others too.

[rockfordlhotka]

CSLA003 & CSLA011 - MobileFormatter requires a public default constructor, and we've been moving away from BinaryFormatter b/c it doesn't work everywhere, so eventually your code will probably need public default constructors.

CSLA008 - most of the modern data binding or UI helper functionality in Csla.Xaml, Csla.Blazor, etc. relies on access to the static property descriptor fields for each property. You can avoid making them public, but then you'll need to use string names to reference your properties instead of using strongly typed descriptors. And I'm not 100% sure all the latest UI helpers even have string overloads, because I never use that technique.

[swegele]

Okie Dokie...gotta go do some housekeeping in my code then :-)
Thanks again both of you for the feedback and help...and of course for the years of great work on this framework!

[JasonBock]

One last thing to keep in mind....analyzers can have associated code fixes. For some of the CSLA analyzers, I've created code fixes where it's made sense. Depending on the mood of VS :), the code fix can be applied for all instances of the analyzer violation:

Notice that you get "Fix all occurrences in:" as an option. If "Solution" or "Project" shows up, you can do the fixes as a one-and-done operation.

[swegele]

@JasonBock
First pass was quick, change the managed backing PropertyInfos to public. Find/replace and viola (2 minutes)

Second pass not so quick. I spent 6 hours yesterday (cutting / pasting) through the ~600 places where the ctors were private instead of public. No way I could see to automate that. Then it was on to ~150 Command objects where the private ctors had parameters. Had to add a public default ctor to those. It was almost midnight and I've got that done but my eyes are ready to fall out of their sockets and I look like dracula!

Next up...replace, mostly in the child objects, where I did manual creation with ctor setting this.IsNew or this.IsOld etc, and instead now have the DataPortal.Create do the work. I think that is also gonna take a day or two.

Question: If I call DataPortal.Create or CreateChild, does there HAVE TO BE a corresponding minimum method (new style) like below?

[Create] [RunLocal] private void Create() { }

or

[CreateChild] [RunLocal] private void Create() { }

I know the RunLocal is optional. But I'd swear it used to be that if you called a DataPortal.Create that it would work even if you didn't have a default instance of the old style method:
override protected DataPortal_Create(...)

[rockfordlhotka]

The old DataPortal_Create method does exist in the base classes, so technically you can rely on that. I don't remember if there's a default DataPortal_ChildCreate method or not...

I recommend implementing your own as you show @swegele, because that's explicit (and allows you to use RunLocal).