dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.12k stars 4.04k forks source link

Cannot turn off "'if' statement can be simplified" #27197

Closed davkean closed 5 years ago

davkean commented 6 years ago

Version Used: Version 15.8.0 Preview 2.0 [27723.0.d15.8]

Steps to Reproduce:

  1. Try flipping the setting back and forth from false to true:suggestion with a VS restart in between. The setting is being cached someone I cannot figure out.

.editorconfig

root = true

[*.{cs,vb}]
dotnet_style_prefer_conditional_expression_over_return = false
    class Program
    {
        public override bool Equals(object obj)
        {
            if (obj is string other)
            {
                return Equals(other);
            }

            return false;
        }
    }
}

Expected Behavior: No suggestion

Actual Behavior:

Message IDE0046 'if' statement can be simplified    ConsoleApp372   C:\Users\davkean\Source\Repos\ConsoleApp372\Program.cs  9   Active
Neme12 commented 6 years ago

Isn't this option true/false?

davkean commented 6 years ago

Whoops made a mistake in the repro and updated it. It still repros with false.

davkean commented 6 years ago

I can't repro above anymore - I went to attach a debugger and the repro disappeared. There's some sort of caching logic here that I cannot figure out - someone is caching that this thing as true and I'm not sure where this is being cached. Blowing away .vs directory didn't solve the issue. Updated the repro which does show it.

My change here: https://github.com/dotnet/project-system/pull/3583 still repros the issue.

CyrusNajmabadi commented 6 years ago

Tagging @mavasani @heejaechang, i've also observed this. I've def changed settings around code-style options, but not had things update in VS.

It it possible this isn't kicking off a high-pri update for the documents that are open or that we've reported diagnostics for? Is that something we could potentially do?

heejaechang commented 6 years ago

if the option is changed through editorconfig, currently editorconfig has a few bugs where it doesn't recognize changes until VS is closed and reopened.

we are working on it so should be fixed soon.

..

in case you want more detail, first, editorconfig API we use has some bugs where it doesn't let us know changes until either file is closed and reopened. or some cases (like solution is closed with files opened), editorconfig APIs get stuck and let us know only old data until VS is closed and reopened.

current options (including editor config options) doesn't update roslyn version which cause analyzer cache not detecting option changes and use old cached data.

there could be more issues, but these are things I can remember on top of my head.

all these should be fixed once we finish work to adapt recently checked in compiler editorconfig work to IDE.

now, option change will change roslyn version. and that will fix all the issue above since new roslyn version will case cache to be invalidated, will wake up solution crawlers and etc.

davkean commented 6 years ago

@heejaechang I still cannot turn off this setting despite:

dotnet_style_prefer_conditional_expression_over_return = false:none
dotnet_style_prefer_conditional_expression_over_assignment = false:none

-or-

dotnet_style_prefer_conditional_expression_over_return = false
dotnet_style_prefer_conditional_expression_over_assignment = false

What does it take to get these 500 messages out of my error list?

heejaechang commented 6 years ago

tagging @sharwell he knows about editorconfig

sharwell commented 6 years ago

What does it take to get these 500 messages out of my error list?

Setting IDE0046 to None in your rule set file always works. Will look into it ignoring settings.

sharwell commented 6 years ago

@davkean This should be fixed in the internal dogfood build minimum version d15.8.27814.01.

sharwell commented 6 years ago

Duplicate of #15003

Fixed by #27280

Please let me know if you reproduce the issue after the indicated version.

davkean commented 6 years ago

This still occurs on Version 15.8.0 Preview 4.0 [27815.3001.d15.8stg]. I have 500 of these issues still in my error list.

NickCraver commented 6 years ago

Confirming what @davkean is seeing - I have the same behavior in 15.8 Preview 3, .editorconfig has no impact.

sharwell commented 5 years ago

I'm not able to reproduce this. I'm guessing it was an intermediate state caused by the use of preview builds. It should be fixed in 15.8 and newer.

Try flipping the setting back and forth from false

false is not a valid option for this setting. It will fail to parse and fall back to the default value of true:silent. If you want to use false, you should set a severity as well (e.g. false:none or false:silent).