Use new formatting options in roslyn itself.#53957
Use new formatting options in roslyn itself.#53957CyrusNajmabadi merged 47 commits intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
- Need to reference stable package version (cc @JoeRobich)
- Have we verified that 16.10 is listed as the minimum supported development version?
src/Compilers/.editorconfig
Outdated
| dotnet_diagnostic.RS0102.severity = none | ||
|
|
||
| # IDE0059: Unnecessary assignment to a value | ||
| dotnet_diagnostic.IDE0059.severity = none |
There was a problem hiding this comment.
this was unfortunate. but without this the compiler had tons of warnings here that touched a lot of core code. i think we should take a followup PR that enables this in isolation.
There was a problem hiding this comment.
@sharwell @jcouv for thoughts on this. I really don't like the idea of disabling this (as i think it reveals many interesting cases where the code seems fishy). But i don't want to have this PR go in witha bunch of complex code changes that have to be closely reviewed and which carries the risk of a subtle bug being introduced.
There was a problem hiding this comment.
Is IDE0059 new for 3.10? I throught we disabled it for the compiler code a long time ago.
There was a problem hiding this comment.
Not sure. @mavasani ? The diagnostics look appropriate. But there are a lot of them and I don't think we should do this without explicit compiler desire ahead of time.
There was a problem hiding this comment.
but without this the compiler had tons of warnings
This cannot be correct. Our editorconfig does not set IDE0059 to warning for the compiler folder.
src/Compilers/.editorconfig
Outdated
| dotnet_diagnostic.IDE2001.severity = none | ||
| dotnet_diagnostic.IDE2002.severity = none | ||
| dotnet_diagnostic.IDE2003.severity = none | ||
| dotnet_diagnostic.IDE2004.severity = none |
There was a problem hiding this comment.
compiler was disabling the RS.... diagnostics above. It seems liek a pity, but i think we have to do the same with the IDE2XXX diagnostics until we can decide if we want to be consistent here across the codebase or not.
| { | ||
| bool isNoPiaLocalType; | ||
| return _metadataDecoder.GetSymbolForTypeHandleOrThrow(handle, out isNoPiaLocalType, allowTypeSpec: true, requireShortForm: false); | ||
| return _metadataDecoder.GetSymbolForTypeHandleOrThrow(handle, out _, allowTypeSpec: true, requireShortForm: false); |
There was a problem hiding this comment.
the EE is not under the compiler and doesn't ahve its own editorconfig. So i did fix these all up. THere were few enough cases that i felt ok doing this.
| expr: "this.get_P()", | ||
| resultProperties: out resultProperties, | ||
| resultProperties: out _, | ||
| error: out error); |
There was a problem hiding this comment.
most fixes were in tests.
.editorconfig
Outdated
| dotnet_diagnostic.IDE2000.severity = warning | ||
|
|
||
| dotnet_style_allow_statement_immediately_after_block_experimental = true:none | ||
| dotnet_diagnostic.IDE2003.severity = none |
There was a problem hiding this comment.
would like to set this to 'false'. but will come in a followup PR.
There was a problem hiding this comment.
Can we set it to false:suggestion for this PR, and just not fix the current reports yet?
|
@sharwell does thsi need anything else (other than reviews of course) to be able to move forward? |
No description provided.