nanoframework / Home

:house: The landing page for .NET nanoFramework repositories.
https://www.nanoframework.net
MIT License
861 stars 79 forks source link

Apply StyleCop to IoT bindings and core libraries #1101

Open josesimoes opened 2 years ago

josesimoes commented 2 years ago

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

torbacz commented 2 years ago

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

networkfusion commented 2 years ago

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 .

rkalwak commented 2 years ago

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;
            }
        }
torbacz commented 2 years ago

@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 :)

Ellerbach commented 2 years ago

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.

torbacz commented 2 years ago

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)
networkfusion commented 2 years ago

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

josesimoes commented 1 year ago

Still several bindings to be worked out!

torbacz commented 1 year ago

Few more to go.