Proposal: #pragma warning disable once

[thomaslevesque]

Currently when we need to disable a warning for a given statement, we have to enclose it between #pragma warning disable and #pragma warning restore directives:

#pragma warning disable FOO123
var x = Foo("bar");
#pragma warning restore FOO123

This is quite verbose and impedes code readability.

Proposal: introduce #pragma warning disable once directive that disables a warning for the following statement only:

#pragma warning disable once FOO123
var x = Foo("bar");

The idea is borrowed from ReSharper's warning suppression comments:

// ReSharper disable once InconsistentNaming

[sharwell]

Conceptually I'm not against the idea of simplifying this case. However, the proposed syntax leaves a lot of questions for me (i.e. it might not be intuitive and/or there could be a better way to approach this):

1. What happens if there are more than one instance of the specified ID(s) on the following line?
2. What happens if there are no instances of the specified ID(s) on the following line?
3. What happens if the warning isn't on the immediately following line, but appears later?
   • Special case: Proposal: Conditional Compilation in XML Comments #295
   • Special case: multi-line tokens such as verbatim string literals, where the warning is reported after the first line of the token
   • Special case: multiple disable once directives on consecutive lines

[lcsondes]

Here's what MSVC does with its very similar feature for C++:

1. All of them are suppressed

#pragma warning(suppress:4100)
void foo(int x,	int y) // 2 warnings suppressed
{
}

2. Nothing

#pragma warning(suppress:4100) // OK
void foo()
{
}

3. Only the next line is suppressed

#pragma warning(suppress:4100)
int foo(int x,
        int y) // warning C4100: 'y': unreferenced parameter
{
    return x;
}

#pragma warning(suppress:4100)
void foo(int
x) // warning C4100: 'x': unreferenced parameter
{
}

Special case: XML comments don't apply to C++

Special case: I'm not sure I understood the scenario, but here's my attempt at it

#pragma warning(suppress:4100)
void foo(const char* x = R"( no warning for x
)", int y = 0) // warning C4100: 'y': unreferenced parameter
{
}

Special case: Multiple suppressions in a row count as one, anything else spends the suppression

#pragma warning(suppress:4100)
#pragma warning(suppress:4001)
void foo(int x) // Suppressed
{
}

#pragma warning(suppress:4100)
#pragma pack(1)
void foo(int x) // warning C4100: 'x': unreferenced parameter
{
}

#pragma warning(suppress:4100)
#ifdef __cplusplus
void foo(int x) // warning C4100: 'x': unreferenced parameter
#endif
{
}

#pragma warning(suppress:4100)

void foo(int x) // warning C4100: 'x': unreferenced parameter
{
}

[HaloFour]

Ya know, I don't think I've ever used #pragma warning disable. Out of curiosity what are the common scenarios where you'd want to disable a warning rather than address it?

[sharwell]

@HaloFour Three most common cases:

1. Bug (or false positive) in the analyzer which caused it
2. During multi-targeting, one target results in the warning while others do not, and suppressing is easier than conditional compilation
3. The change would be a breaking change according to the versioning policy of the product

Also one project I worked on looked at the "most recently changed date" for methods and applied different testing requirements, so changes to old code could result in unexpected developer overhead to address secondary policies (in this case, a global suppressions file was used to target the warning locations).

[HaloFour]

@sharwell

Ah, I see, so something that's more commonly triggered via analyzers where you might be applying stricter and more pedantic rule sets that you might want to shut-up on an individual basic, just like Resharper. That does make sense.

When dealing with vanilla C# compiler warnings I don't think I've ever encountered a situation where the compiler couldn't be satiated with what is often better/shorter code anyway. 95% of the time it seems to be the result of the fact that other developers don't seem to realize that you can omit the variable name in a catch statement.

[thomaslevesque]

What happens if there are more than one instance of the specified ID(s) on the following line?

If that line has a single statement, all instances are suppressed; the directive applies to the whole statement.

What happens if there are no instances of the specified ID(s) on the following line?

Nothing. The directive applies to the next statement, not to the next occurrence of the warning.

What happens if the warning isn't on the immediately following line, but appears later?

Blank lines would have no effect. Only statements should be considered. The directive would apply to the first statement following it.

Special case: #295

That's a tricky one... I'm not sure what it should do in this case.

Special case: multi-line tokens such as verbatim string literals, where the warning is reported after the first line of the token

As I said, lines don't matter, only statements do.

Special case: multiple disable once directives on consecutive lines

They would all apply to the next statement.

[thomaslevesque]

Ya know, I don't think I've ever used #pragma warning disable. Out of curiosity what are the common scenarios where you'd want to disable a warning rather than address it?

In addition to the scenarios mentioned by @sharwell:

• CS0169 on a field that is actually used by reflection.
• Code Analysis or FxCop naming rules when there's a good reason to violate the rule.
• CA1006 when there's no way around it (e.g. Expression<Func<T>>)

[jnm2]

Ya know, I don't think I've ever used #pragma warning disable. Out of curiosity what are the common scenarios where you'd want to disable a warning rather than address it?

Not to mention the three or four I typically put in for pinvoke structs.

[aluanhaddad]

I had to use this a number of times recently before support for generic patterns was added in 7.1.

Ultimately though I tend to use the suppression file approach.

As for scope, I think it would be better to say something like

# pragma warning disable next line FOO123

Which is used by JavaScript and TypeScript tools such as eslint and tslint.

As for multi line verbatim string-literals, I'm not sure if the same approach could be used but such tools don't have issues with ECMAScript multi-line strings so perhaps a similar approach could be used.

Just throwing ideas out there.

[gautelo]

Another use case is when the framework or a library will call certain optional methods through reflection. One example is the mstest ClassInitialize method which require a TestContext argument. When you don't use that argument you get IDE0060. I expect there might be similar cases for asp.net global asax things and/or console host args etc. I guess interfaces for these things would remove this problem, but as it stands this pops up.

[goblinfactory]

Isn't the problems described above already resolved for C++ using #pragma warning supress? What I mean, is that they've already worked through all the edge cases, and this was their final conclusion, and most likely well reasoned and battle tested.

We should adopt the same pattern for C# for the same reasons they did. imho. Below are examples of how it works in C++, and I see no practical reason we shouldn't follow the same convention? It's very readable, and the concept of push and pop solves problems of really bad bugs creeping in by having a piece of code inadvertently creating a coupling (changing behavior) of other code using pragmas.

example of supress warning 1234 in C++

#pragma warning (disable : 4507 34; once : 4385; error : 164; supress : 1234 )

C++ supress docs here
https://docs.microsoft.com/en-us/cpp/preprocessor/warning?view=vs-2019

important point is this text

suppress | Pushes the current state of the pragma on the stack, disables the specified 
warning for the next line, and then pops the warning stack
 so that the pragma state is reset.

[goblinfactory]

if we are going to deviate from the C++ convention, then I'd propose that
when the #pragma warning disable appears before any namespace declaration, that the suppression applies only for that file, and only for that namespace. This will allow us to move the pragma warning to the top of the file, and it will be invisible to anyone scanning the code. these are pre-processor commands to compilers that humans do not need to concern when reading code to understand it's intent.

example

#pragma warming (disable: 1234)
using System;

namespace Foo {
  public class Bar ...
}

[msschl]

Any news on this?

[CyrusNajmabadi]

@msschl No. This is just a discussion, and has no champion/proposal from an LDM member to move it along.

[msschl]

Performing a simple search for #pragma warning restore in the dotnet/runtime repo results in 781 results. I think this proposal should be brought forward.

[lcsondes]

1.3k since (searching for "#pragma warning restore" with the quotes)

[lonix1]

From my dupe issue: it would be nice to also have (in addition to syntax shown above) the ability to suppress warnings like you find in other tools/languages, where the suppression is inline. That reduces noise to a minimum. Something like:

SomethingThatTriggersX();              #pragma warning disable-line X

[huoyaoyuan]

And there's another question: what is a line? Multi-line statement isn't that rare. The associated span of the warning can also cross multiple lines.

Disabling the next warning is also a bad idea. If the closed warning disappears and another warning happens, the new one can be eaten.

[lcsondes]

Warnings come with line numbers attached, and CallerLineNumberAttribute exists, so it must be defined somehow. Alternatively, whatever MSVC considers a line for its one-liner #pragma warning(suppress:x) works for the overwhelming majority of cases, so that's a good start.

I collected a few examples here: #1070 (reply in thread)

Documentation: https://learn.microsoft.com/en-us/cpp/preprocessor/warning

[lonix1]

That may be true, and unfortunately there are other problems noted above by sharwell as well. So instead of reinventing the wheel, maybe the best idea is to examine how javascript / typescript tooling does it.

[theunrepentantgeek]

I'm an enthusiastic fan of Static Analysis tooling and I universally use inline SuppressMessage attributes, not #pragma.

• Limited scope (usually just a single method)
• Zero noise within the method
• Built in support for recording a reason for the suppression (great for future readers)

Do we need to improve #pragma at all?

[lonix1]

That is before the line of interest, not inline. The idea in this issue is to have easy to use one-line pragmas (equivalent to suppressions but less noisy), as well as an inline option.

[snebjorn]

[SuppressMessage] is great. But it can't be used on namespaces or directives. See dotnet/roslyn#60361

I it would be best if we have the same abilities to disable warnings in C# as tools like ESLint does for JS/TS.

That means:

• disable inline
• disable next line
• disable arbitrary sections
• disable multiple rules in one statement

[johnnyreilly]

This is a great idea! I raised a separate issue as I wasn't aware of the discussion here. dotnet/roslyn#60668

There were 2 separate parts to my request:

1. Single line ignore
2. Lightbulb assistance in VS Code

Single line ignore

Rather than:

#pragma warning disable IDE0052
    private readonly ILogger<WeatherForecastController> _logger;
#pragma warning restore IDE0052

What if instead, something like this existed:

#pragma warning disable-line IDE0052
    private readonly ILogger<WeatherForecastController> _logger;

Lightbulb assistance in VS Code

In ESLint you get lightbulb assistance in VS Code to implement an ignore for you; see the "Disable no-var for this line" below:

Imagine if the lightbulb with the C# extension offered the option to ignore a lint; just as the ESLint menu does.

Alas at the moment, it doesn't:

The above lightbulb is for the following issue:

Private member 'WeatherForecastController._logger' can be removed as the value assigned to it is never read [WeatherService, WeatherService] csharp(IDE0052)

If it did offer this it could be quite a productivity boost

[rhys-vdw]

I think following the more explicit naming of ESLint of disable-next-line would be preferable.

#pragma warning disable-next-line IDE0052
    private readonly ILogger<WeatherForecastController> _logger;

[lcsondes]

I like the proposed #pragma warning disable once syntax, but #pragma warning suppress would not just be shorter, but more consistent with MSVC, too. I think this has a (very) minor benefit for people who work with solutions that contain .csprojs and .vcxprojs both, a situation I often find myself in. You won't see me complain if the former gets implemented, though. :)

disable-line and disable-next-line don't "feel" C# to me, with - being used as a word separator. disable once is not dissimilar to, e.g., yield return.

[jaredpar]

Per-line is what I've seen elsewhere

I agree but I'm pushing back on that being the correct approach. Consider that many times in our own code bases we want to suppress warnings that are in the middle of if conditions. I'm not eager to support a solution that leaves me writing code like this:

   if (condition &&
#pragma warning disable line CSXXXX
      condition2) { 

The implicit goal of this proposal is how to make warning suppression more presentable. There is no functional limitations today, just the presentation of suppressions. C# is not a line based language hence leaning into line based solutions does not seem like the right approach. We should be aiming to match the underlying language structures here to make the suppressions as visually pleasant as possible.

I'd still be googling "how do disable a warning for one line".

Every new syntax, even #pragma warning suppress line, begin with a learning curve. Even the "one line" solution would start with customers googling "how do I disable a warning for one line". The issue is whether the syntax once learned is sticky and can be easily recalled in the future.

[lcsondes]

This is twice as visually unpleasant, and as far as I know, your only precisely-targeted option right now:

   if (condition &&
#pragma warning disable CSXXXX
      condition2) { 
#pragma warning restore CSXXXX

I don't oppose a better approach, but until someone presents one, I don't think this proposal should get canned just because such a thing might be possibly invented one day in the future.

#pragma lives in its ugly line-based corner of the language, parsed more or less independently. It doesn't really affect the "main syntax" of the language, so the maintenance cost should be relatively low.

Thinking about other solutions (not just MSVC's similar #pragma, but clang-tidy // NOLINT, // NOLINTNEXTLINE, // ReSharper, the JavaScript examples brought up above, even Visual Basic), they all tend to be line-based, which could be a sign that this is either the easier problem to solve, the one that matches user expectations the best, or maybe both.

The originally-proposed once wording is fairly neutral, it doesn't commit itself to being a line forever. It could be maximally strict in the first iteration, and be taught to conform to other non-line-based language elements later as an improvement when/if someone comes up with those. This would enable future C# versions to have warning suppressions in more pleasant places, while already addressing half the ugliness from the code block at the top of this comment with a drop-in replacement for the first #pragma.

[jaredpar]

The originally-proposed once wording is fairly neutral, it doesn't commit itself to being a line forever

The actual word used is less interesting to me than the semantics. The original posting had once apply to next statement which is very much in line with my way of thinking. Not everything is a statement though, there are plenty of cases where a suppress should apply to a method declaration or entire method body. Hence I want to think broadly about how to solve all of these scenarios.

I'm on the fence as to whether or not solving "for this method body" is worth it or not. The existing disable / enable doesn't seem that distracting to me at least.

[lcsondes]

I can't recall a single instance where I wanted to suppress a warning for exactly a method body.

[CyrusNajmabadi]

A simple example I've run into recently is wanting to write a particular method with certain async/await characteristics and not have warnings from analyzers about not following their rules within that method.