Suppress netstandard1.x warning for Lucene.Net.CodeAnalysis projects

[paulirwin]

Is there an existing issue for this?

• I have searched the existing issues

Task description

The .NET 9 SDK now adds a warning that targeting netstandard1.x is no longer recommended:

warning NETSDK1215: Targeting .NET Standard prior to 2.0 is no longer recommended. See https://aka.ms/dotnet/dotnet-standard-guidance for more details.

This applies to our Lucene.Net.CodeAnalysis.CSharp and .VisualBasic projects, which target netstandard1.3. We should suppress this warning since Visual Studio 2017 requires it, and it is under extended support until 2027.

[paulirwin]

@NightOwl888 in light of #1041, should this target the individual targets that our other projects support instead?

Edit: after I posted this, I realized I should research what Microsoft recommends. They recommend targeting .NET Standard 2.0, so these would be an exception to #1041. Docs: https://learn.microsoft.com/en-us/dotnet/csharp/roslyn-sdk/tutorials/how-to-write-csharp-analyzer-code-fix

[NightOwl888]

Right. These code analyzers are to support IDEs as well as MSBuild, so they should have broad support. The reason why we downgraded to netstandard1.3 was to support VS2017 (something Microsoft needed). I am not sure whether that is still the case. But I also don't see any real need to change it. We can test netstandard1.3 with the latest .NET core and always keep it up to date. So, perhaps it would be better to suppress the warning on these projects.

We have several open tasks that involve making new code analyzers, but for those we will need a separate assembly. The existing analyzers are specifically for end users and we will need a different assembly for supporting Lucene.NET development. I was originally thinking of making an installer for the development analyzers, but it makes more sense to version it and put it on NuGet. It can then be a dependency of all Lucene.Net projects declared as PrivateAssets=analyzers. Ideally, we would create a different repo (similar to lucenenet-site) where it is developed and versioned, since it will have a different release schedule than Lucene.NET. I created a repo here: https://github.com/NightOwl888/lucenenet-codeanalysis-dev. But some of those analyzers probably shouldn't be enabled by default. It is named that way to be migrated to Apache. But, maybe we shouldn't because then we would have to comply with their release policy (having a release vote) for things that we need to develop Lucene.NET.

[paulirwin]

Is there a good reason to keep it on netstandard1.x? I don't think those older IDE versions would even support our codebase currently. Microsoft recommends netstandard2.0 at the link above (see my edit), so I would suggest we go with that instead of suppressing the warning.

As for the dev-targeted analyzers, my current thought is I think those should be versioned in our repo if at all possible.

[NightOwl888]

Is there a good reason to keep it on netstandard1.x? I don't think those older IDE versions would even support our codebase currently. Microsoft recommends netstandard2.0 at the link above (see my edit), so I would suggest we go with that instead of suppressing the warning.

These analyzers are for users, not for our codebase. There is no reason why someone who is stuck on VS2017 couldn't load our library, but they will get a bunch of load failures if they don't suppress the analyzers when they are on an IDE that isn't compatible.

• Lucene.Net NuGet (4.8.0-beta00013) does not compile under Visual 2017 - Lucene.Net.CodeAnalysis.Lucene1000_TokenStreamOrItsIncrementTokenMethodMustBeSealedCSAnalyzer #394

So, this may create usability issues if we upgrade again. Microsoft also specifically requested this feature (although I couldn't find the conversation when I searched) and I don't know whether they still need it or not.

IMO, it is less trouble to suppress an irrelevant warning than to deal with usability issues (again). Not to mention, I think there may be more to it than just changing the target framework.

As for the dev-targeted analyzers, my current thought is I think those should be versioned in our repo if at all possible.

If there were an exception to the 72 hour vote rule, we could. Otherwise, this would become an obstacle for releasing our analyzer changes in a reasonable timeframe.

These dev analyzers won't be embedded in our .nupkg like the user analyzers are. The IDE requires a different assembly version for each release due to caching, so it is easier to deal with as a package reference than trying to bootstrap the latest version into the current build. It will need to be a publicly available package reference so anyone who wants to contribute can depend on it. We would also need to have independent versioning and releases from Lucene.NET so we can update the analyzers between Lucene.NET releases easily. So, it will require an independent build from our current one. It just seems like it would be easier to keep this in a separate repo so we can push it out to NuGet and then bump the dependency version in this repo much like we would for RandomizedTesting or Spatial4n.

[paulirwin]

Regarding Visual Studio 2017, unfortunately that is still under extended support until 2027. https://learn.microsoft.com/en-us/visualstudio/productinfo/vs-servicing#older-versions-of-visual-studio

I feel bad for anyone still having to use that, but I guess theoretically it is possible to still be using 2017 when 4.8 launches with our netfx target. So I updated the title and description of this item to suppress the warning instead.

As far as the dev analyzers go, we can create a separate issue for that, but it is possible to install analyzers within your project without having them be NuGet packages, and just regular project references instead. Then that way we don't need to worry about releases or versioning, and they will evolve with the code base, since they are intended for use by Lucene.NET developers working on this codebase. Say we put in #985 into a dev-focused analyzer referenced by all of our projects. Then that will help prevent future contributors from making that mistake. I tested this with the Roslyn sample analyzer and it just uses a local project reference just fine.

[paulirwin]

Also #985 might be a user-focused analyzer rule... might be a bad example. I just picked one.

[NightOwl888]

I tested this with the Roslyn sample analyzer and it just uses a local project reference just fine.

Yes, it works fine the first time you try it. And then when you try to update it, it doesn't. It requires a new assembly version or it won't override the old version (at least in Visual Studio). Maybe there is a way to make it work locally and refresh for updates, but a NuGet package and a separate build with the versioning setup correctly will save from having to reinvent the wheel just to make it happen. And there is also a simple way to roll back if the latest version doesn't load for some reason.

[paulirwin]

I think that might have been an older Visual Studio bug that's now (at least somewhat) fixed. I tried changing a rule's behavior, and in Rider it updates if you clean and rebuild. In Visual Studio 2022 latest, if you clean/rebuild and then close and reopen VS it picks up on the new change without needing to change the version number.

That might be a minor annoyance to someone actively developing the analyzer rules or switching to an older branch where those rules might be outdated (at least in Visual Studio), but I'd imagine those won't change often enough for that to warrant needing to publish them to nuget just for working around that. I think the common case would be that the rules, once developed, will not be changing often enough for this to matter that much, and that there's a fairly easy workaround (clean, rebuild, and reopen if using VS).

[paulirwin]

Also, now that Rider is free for non-commercial use, we could just suggest in the CONTRIBUTING file that if you're wanting to work on the dev analyzers, just use Rider 😄

[NightOwl888]

That might be a minor annoyance to someone actively developing the analyzer rules or switching to an older branch where those rules might be outdated (at least in Visual Studio), but I'd imagine those won't change often enough for that to warrant needing to publish them to nuget just for working around that. I think the common case would be that the rules, once developed, will not be changing often enough for this to matter that much, and that there's a fairly easy workaround (clean, rebuild, and reopen if using VS).

Not exactly. If someone pulls down the branch with changed rules in it and builds locally, it won't load the rules for that developer. They may not even be aware the rules have changed, but the rules don't take effect until the IDE is restarted. That is more than a minor annoyance. I often keep the IDE open for weeks at a time and would not get these updates that entire time.

A NuGet package avoids this hassle entirely. The developer pulls down a branch and NuGet restore will install the new analyzer with the rest of the dependencies. No restart required and the developer doesn't even have to be aware that it updated.