[Analyzers] Update .editorconfig with rules to relax IDE errors #36095
Conversation
…ng as errors in VS Set severity for IDE0005, IDE0008, IDE0016, IDE0018, IDE0019, IDE0021, IDE0022, IDE0023, IDE0025, IDE0027, IDE0028, IDE0029, IDE0031, IDE0032, IDE0034, IDE0036, IDE0039, IDE0042, IDE0044, IDE0045, IDE0046, IDE0047, IDE0057, IDE0051, IDE0052, IDE0054, IDE0055, IDE0056, IDE0057, IDE0059, IDE0060, IDE0061, IDE0063, IDE0071, IDE0073, IDE0074, IDE0075, IDE0077, IDE0078, IDE0083, IDE0090, IDE0100, IDE0130, IDE160, IDE180, IDE0200, IDE0240, IDE0250, IDE0251, IDE0260, IDE0270, IDE0290, IDE0300, IDE0301, IDE0305, IDE1005, IDE1006, CA1859, CA2022, CA2263
crutkas
added
the
Needs-Review
src/.editorconfig
Outdated
|
|
||
| # IDE0008: Use explicit type instead of 'var' | ||
| dotnet_diagnostic.IDE0008.severity = suggestion |
There was a problem hiding this comment.
Shouldn't many / all of these be silent instead of suggestion? Documentation
Here's an example - my implicitly-typed variables still have distracting markers (not as distracting as red squigglies, but still).
There was a problem hiding this comment.
I would defer to @crutkas for this one. We can surely tweak, but I wonder about those who have all info's filtered out and use the suggestions to improve incrementally. At the same time, I can understand it in the situations where squiggles are everywhere.
There was a problem hiding this comment.
silent will still allow changes, for example via light-bulb actions.
IMO, the following should be silent and not suggestion. Making them suggestion indicates that performing the suggested refactoring will improve the code, when in fact the opposite is likely true:
# IDE0008: Use explicit type instead of 'var'
# IDE0061: Use block body for methods
# IDE0160: Use block-scoped namespace
There was a problem hiding this comment.
That makes sense! I'm currently in the car for the next 3.5 hours, but if you want to make those changes that would be appreciated.
There was a problem hiding this comment.
While trying to fix this, I noticed that almost all of the comments are incorrect, i.e. the style codes don't seem to correspond to the actual ones listed here. Could you first address this please?
There was a problem hiding this comment.
Great catch. I thought I fixed them all from reorganizing the comments 😅. I'll fix these in a bit since I just stopped to grab a quick bite to eat
There was a problem hiding this comment.
There we go. Fixed. I know exactly what happened and I'm glad you caught it. That's what I get for asking Copilot to sort 🙃
There was a problem hiding this comment.
I made a commit to make the following style rules silent instead of suggestion:
- Use explicit type instead of 'var'
- Use expression body for ...
- Use block-scoped namespace
Please roll it back if it's not okay or needs more discussion!
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
Made the following style rules `silent` instead of `suggestion`: - Use explicit type instead of 'var' - Use expression body for ... - Use block-scoped namespace
| dotnet_diagnostic.IDE1006.severity = suggestion |
There was a problem hiding this comment.
Looking here we have no cases where this isn't the case currently. Should this be a warning or error then as this is a common convention I don't think we'd want to stray from?
There was a problem hiding this comment.
There's only about 8 or so instances where this affects anything, so this is another one that can be ripped out. Seriously, thanks again for reviewing ^_^.
|
|
||
| # IDE0290: Use primary constructor | ||
| dotnet_diagnostic.IDE0290.severity = suggestion |
There was a problem hiding this comment.
This one's bit me a bit already, maybe silent? It's a nice pattern in some circumstances, but a lot trickier to do well if adapting from an existing class.
There was a problem hiding this comment.
Yeah this can be a silenced instead of suggested. I was thinking that suggestion was needed in order for it to exist as a potential suggestion.
| dotnet_diagnostic.IDE0073.severity = suggestion |
There was a problem hiding this comment.
Warning/error? Don't we want to ensure this is the case for best practices with having headers on our files for licensing as an opensource project?
There was a problem hiding this comment.
Good find. This one can be pulled out and shouldn't have been added. I went through and grouped all the warning/errors that magically started appearing in the IDE that weren't previously.
The IDE0073 shows up due to the ImageResizer files having the file header of the original author
// Copyright (c) Brice Lambson
// The Brice Lambson licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information. Code forked from Brice Lambson's https://github.com/bricelam/ImageResizer/
Should we go ahead and change these to the proper header? Or do a suppression on just the ImageResizer related ones?
Summary of the Pull Request
Modified .editorconfig to add IDE and code analysis-based rules that aren't captured in Visual Studio 17.12
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed