Home icon indicating copy to clipboard operation
Home copied to clipboard

Apply StyleCop to IoT bindings and core libraries

Open josesimoes opened this issue 3 years ago • 9 comments

Details about Problem

We now have a usable pattern and rules for using StyleCop in nf projects. Need to add it to IoT bindings and the class libaries.

Description

This task it's a bit tedious, granted, but has to be done. It's not complex, just a matter of adding a file and a couple of changes to the projects and then fixing the warnings from StyleCop.

Please check this PR for reference.

nanoFramework area: IoT Bindings

josesimoes avatar Jul 22 '22 09:07 josesimoes

Info: There is a script for syncing all projects in path: /StyleCop/syncSettingsOverEveryDevice.ps1 Please use it, if you want apply stylecop to project.

torbacz avatar Jul 26 '22 08:07 torbacz

For the core libs, More consise documentation and perhaps sample PR needed (since the one provided in the description is too broad to be of help). Although a great start is: https://github.com/nanoframework/nanoFramework.IoT.Device/tree/develop/StyleCop .

networkfusion avatar Jul 27 '22 23:07 networkfusion

I applied StyleCop on library I am working on and and have a problem with below line, is this too new language feature usage that StyleCop complains with SA0102 : CSharp.CsParser : A syntax error has been discovered in file ...? It looks like it doesn't like the "is" here and treats it as cast operator.

I have the same syntax in other line and it is not complaining at the same time. When I change the first occurence then I got the error in second.

if (value.DegreesCelsius is < -55 or > 125)

when bigger picture is

public Temperature TemperatureHighAlarm
        {
            get
            {
                return Temperature.FromDegreesCelsius(_tempHighAlarm);
            }
            set
            {
                if (value.DegreesCelsius is < -55 or > 125)
                {
                    throw new ArgumentOutOfRangeException(nameof(TemperatureHighAlarm),
                        "High alarm temperature has to be between -55 and 125 degrees");
                }
                _tempHighAlarm = value.DegreesCelsius;
            }
        }

rkalwak avatar Jul 28 '22 18:07 rkalwak

@rkalwak Unfortunately it does not support some C# features and the analysis stops when meets this kind of code. As for workaround I suggest to replace if (value.DegreesCelsius is < -55 or > 125) with if (value.DegreesCelsius < -55 || value.DegreesCelsius> 125)

PS. You can extract -55 and 125 to consts to keep code cleaner :)

torbacz avatar Jul 28 '22 21:07 torbacz

if (value.DegreesCelsius is < -55 or > 125)

Yes, the .NET Framework 4.x StyleCop that we are using here is not perfect for this. We will move to SDK base and we then should be able to use the modern one for .NET 5+ and .NET Core 3+ which is handling those elements properly. I'll try to check what we can do. You can as well release the rule with a pragma for a block of code. Good temporary workaround as well.

Ellerbach avatar Jul 29 '22 08:07 Ellerbach

I think the best approach to work on style cop fixes is:

  1. Replace in syncSettingsOverEveryDevice.ps1 script $projectWhiteList = ("AD5328", "4Relay", "Ads1115", "Adxl345", "Adxl357", "Ags01db", "Ahtxx") with $projectWhiteList
  2. Run script.
  3. Check git status for changed files.
  4. Discard all changes from project you don't want to fix.
  5. Fix projects one by one and push only one to repository (PR should be opened as draft to check if build is passing after applying fixes)

torbacz avatar Jul 31 '22 19:07 torbacz

Duplicate? https://github.com/nanoframework/Home/issues/1074

networkfusion avatar Aug 03 '22 16:08 networkfusion

Still several bindings to be worked out!

josesimoes avatar Oct 12 '22 16:10 josesimoes

Few more to go.

torbacz avatar Nov 13 '22 21:11 torbacz