Fixes stylecop warnings in resource manager project.

[valadas]

• Added stylecop reference in project
• Added stylecop analyzer to project
• Fixes about 1000 stylecop warnings in the resource manager project.

Summary

[bdukes]

Did you add the StyleCop.Analyzers reference manually? I was expecting to see it in a packages.config file.

[SkyeHoefling]

This is great, thanks for the hard work in fixing this. If there are 0 build warnings in this project, can you turn on 'Treat Build Warnings as Errors' in the csproj? If there are still build warnings due to deprecation notices, please cite them here in the PR and I can try and provide some guidance

[valadas]

There are still 2 or 3 warnings, I will post them shortly here

[valadas]

@ahoefling So the 2 warnings I get are:

Description	File	Line
Found conflicts between different versions of the same dependent assembly. In Visual Studio, double-click this warning (or select it and press Enter) to fix the conflicts; otherwise, add the following binding redirects to the "runtime" node in the application configuration file: <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1"><dependentAssembly><assemblyIdentity name="Newtonsoft.Json" culture="neutral" publicKeyToken="30ad4fe6b2a6aeed" /><bindingRedirect oldVersion="0.0.0.0-10.0.0.0" newVersion="10.0.0.0" /></dependentAssembly></assemblyBinding>	C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Current\Bin\Microsoft.Common.CurrentVersion.targets	2106

I find that strange because I removed the existing Newtonsoft.Json reference and re-added it from the main packages folder.

Severity	Code	Description	Project	File	Line	Suppression State
Warning	CS0618	'IPortalController.GetCurrentPortalSettings()' is obsolete: 'Deprecated in Platform 9.4.2. Scheduled removal in v11.0.0. Use GetCurrentSettings instead.'	Dnn.Modules.ResourceManager	D:\dnnvaladas\Dnn.Platform\DNN Platform\Modules\ResourceManager\Components\ThumbnailsManager.cs	73	Active

And well this one, I need ActiveTab which is not available on IPortalSettings but exists in PortalSettings.

@bdukes I added the existing stylecop.json file to the project and then I went to references and added an analyzer reference to the main packages folder stylcop folder and then 2 dlls, one is the analyzer and the other dll is the codefix helpers. Is that correct?

[valadas]

Another thing I was thinking while working in there is that there are a lot of public methods on something I believe should never be used by 3rd parties, I believe this is a module and not a public api thing, should I try and go change everything possible from public to protected ? Is that a good idea before it is released so it protects us a bit from breaking changes as we push this module forward in the future ?

There are also a couple of methods that have 0 references to them, I was thinking to remove them... Thoughts ?

[valadas]

Oh and yeah, related to that... Interface members have no access modifiers, so the implementation defines the access modifiers on the methods as needed. So what happens if a 3rd party tries to dependency inject that but then it ends up the members are protected ? Should we make the actual whole interface protected to prevent 3rd parties from depending on stuff we don't really want to recommend consuming ?

[bdukes]

@valadas are there interfaces in this project that you're concerned about? You don't need to worry about someone implementing interface members with the "wrong" access modifier. When you're using a reference with the interface type (e.g. IMyThing thing = GetMyThing())) you will always have access to the members of that interface.

Feel free to remove methods with no references. As we mentioned in the TAG meeting, since this isn't published on NuGet, we're not concerned about it exposing public members, and we're free to adjust visibility to internal if desired.

For now, instead of PortalSettings.ActiveTab can you use TabController.CurrentPage?

The stylecop.json reference looks great, but can you add StyleCop.Analyzers package as a NuGet reference instead of just an assembly reference?

[SkyeHoefling]

@valadas

Interface members have no access modifiers, so the implementation defines the access modifiers on the methods as needed. So what happens if a 3rd party tries to dependency inject that but then it ends up the members are protected ? Should we make the actual whole interface protected to prevent 3rd parties from depending on stuff we don't really want to recommend consuming ?

By design the interface specification in .NET defines public members that other classes that inherit the interface are required to implement. The interface members define a public contract and should always be public. You can use explicit interface implementations to restrict the interface members at the class level. However, that technique only hides the members, if you cast it to the interface you will instantly gain access to it because interface members are public. This is not an anti-pattern it is working as designed. It is a core pillar of object oriented programming to depend on abstractions which is exactly what an interface is accomplishing for us. When you change your interface implementation in the future, the code that depends on the interface does not need to change.

Thanks @bdukes for providing a good explanation to this question as well 💪

IPortalController.GetCurrentPortalSettings()' is obsolete: 'Deprecated in Platform 9.4.2. Scheduled removal in v11.0.0. Use GetCurrentSettings instead.

The current abstractions of IPortalSettings needs some ❤ and it isn't fully complete. To remove the warning I would follow @bdukes recommendation of using TabController.CurrentPage. If that doesn't work, you could cast the returned IPortalSettings object as a `PortalSettings. It really is a hacky way to solve the problem, but since the current abstractions integration needs some work, I think it would be okay as a workaround.

Found conflicts between different versions of the same dependent assembly.

This is most likely due to NuGet being corrupted for the project. Try the following steps to resolve this problem.

1. Unload the csproj and manually delete the Newtonsoft.Json reference
2. Open up the local web.config in the project directory and remove the binding redirects that reference Newtonsoft.Json
3. Open the packages.config in the project directory and remove the package reference line to Newtonsoft.Json
4. Right-Click at the solution level and select Manage NuGet Packages
5. Navigate to the installed packages tab and find Newtonsoft.Json
6. Install the matching version number in your current project by selecting it in the projects checkbox list.

This technique will clean up any broken references and allow the Visual Studio tooling to install the package reference correctly

[valadas]

For now, instead of PortalSettings.ActiveTab can you use TabController.CurrentPage?
Thanks, that works

[SkyeHoefling]

@valadas I don't think I saw the csproj updated to treat warnings as build errors. Would you like me to submit a PR for this?

I am really happy that we removed all build warnings, but without a build tool in place to prevent it from happening we will likely get back to having build warnings again

[valadas]

@ahoefling Yeah, I would be happy with that sure.