Enable more Roslynator rules #21192
Enable more Roslynator rules #21192
Conversation
| @@ -1052,6 +1061,9 @@ dotnet_diagnostic.RCS1159.severity = warning | |||
| # Unused type parameter. | |||
| dotnet_diagnostic.RCS1164.severity = warning | |||
|
|
|||
| # Use read-only auto-implemented property. | |||
| dotnet_diagnostic.RCS1170.severity = warning | |||
There was a problem hiding this comment.
it looks like this rule was implemented incorrectly. Private setters were made public.
I'm not sure how this rule exactly works as it affected so little code, but this could make it harder to work with magic. In trait infos we use auto-implemented properties for non-yaml fields
There was a problem hiding this comment.
Ah mb I misunderstood, but it did indeed remove the privateness from RadarWidget
There was a problem hiding this comment.
The rule wants to change public string SoundUp { get; private set; } to public string SoundUp { get; }. However, this breaks our use of reflection in FieldLoader/ObjectCreator as it sets SoundUp from YAML.
I manually changed it to a field, like other things that need to be loaded from YAML, as it was the most elegant workaround I could see. This seemed like a worthwhile tradeoff so we can still enable an otherwise useful rule.
There was a problem hiding this comment.
actually should have made these readonly to match other yaml defined values
| if (explicitMounts.TryGetValue(filename[..explicitSplit], out var explicitPackage)) | ||
| if (explicitPackage.Contains(filename[(explicitSplit + 1)..])) | ||
| return true; | ||
| if (explicitSplit > 0 && |
There was a problem hiding this comment.
It would be nice if we could be consistent about whether the && goes at the start or end of the line in these cases. Is there a rule for that?
There was a problem hiding this comment.
I'm not aware of one offhand, but that's not to say there isn't one somewhere.
I've generally wrapped after as I think that's the more common convention in the codebase, but even in this chageset I wrap before in FieldLoader as that's the more common convention in that file.
There was a problem hiding this comment.
I usually find the && or || nicer and more readable when placed at the start. Though it does depend on the length of the first line
| @@ -63,8 +63,8 @@ public sealed class RadarWidget : Widget, IDisposable | |||
| PlayerRadarTerrain playerRadarTerrain; | |||
| Player currentPlayer; | |||
|
|
|||
| public string SoundUp { get; private set; } | |||
| public string SoundDown { get; private set; } | |||
| public string SoundUp; | |||
There was a problem hiding this comment.
These should probably move up with the other widget fields?
Do we have a convention for widget fields to be readonly like we do for traits?
There was a problem hiding this comment.
Moved upwards.
A quick flick through some widgets and it seems read-write fields are more common, but there are some read-only ones littered about.
RoosterDragon
force-pushed
the
style-ros-med
branch
from
f82c0c8
to
3dff038
Compare
Following on from #21176.
Enforces 6 Roslynator rules across the project.
I have batched together a PR for these rules as they are hopefully uncontroversial. However fixes are arranged per commit so we can easily bikeshed over rules as desired.
See https://josefpihrt.github.io/docs/roslynator/analyzers for explanation of each rule.