VB LDM 2014-04-16

Topics: VB Language Design
Coordinator
Apr 27, 2014 at 7:10 AM
Edited Oct 31, 2014 at 7:12 AM

#Warning Disable

REQUEST

Please add #pragma to VB so we can suppress warnings. This will be especially useful for Roslyn Diagnostics, where people will find many more warnings in their code.

RESOLUTION

Add two new preprocessor directives:
#Disable Warning 40008, BC42358, "BC42356"
#Enable Warning 40008, BC42358, "BC42356"

ALTERNATIVE SYNTAXES CONSIDERED

#pragma disable <id>
#pragma enable <id>

#Disable <id>  ' sounds like you're disabling code, not warnings
#Enable <id>

#Ignore <id>
#End Ignore  ' block structure is worse than toggles

#Disable Warning <id>
#Enable Warning <id>

DISCUSSION

The tradition in VB and its rich history of quick-fixes is that you resolve warnings by FIXING YOUR CODE, e.g. by doing whatever the quickfix says, adding an explicit cast, adding a return statement, ...

Q. Do people really use #pragma much in C# today? Hard to say. With new Roslyn diagnostics, will more people start using #pragma in C# ? Will people write Roslyn diagnostics that have a much higher false-positive rate than existing rules, for people who wish to be more careful, safe in the knowledge that it's easy to suppress?

Q. Is there a downside to adding #pragma to VB for suppressing warnings? - no, not really

Q. Should we add a feature which suppresses the warning merely for the next line, rather than for the rest of the file? -- This isn't feasible in C# where lines don't really exist semantically. It is feasible for VB. Disadvantage: would confusing for LINQ, and not refactoring-safe. Advantage: if the lightbulb/quickfix appears for a warning, and when you chose the option to "suppress warning" then it only has to insert a single line to "suppress-warning-on-next-line", rther than having to insert #ignore before and #restore after. Conclusion: No we won't add this next-line-suppression feature.

Q. Is it good to disable/enable on a "block" basis like #Ignore / #End Ignore ? Or on a toggle basis like #Disable Warning / #Enable Warning ? Well, with blocks you can't do overlapping regions. And you can't easily disable warnings for an entire method but then re-enable them for a small critical region inside (even though that's a weird scenario). We believe overall that toggles are nicer. They have a lot of engineering, thought and experience behind them.

Q. For the <id>, should quotation marks be obligatory? - No. You can use any identifier or integer. It's only if the warning-id has spaces or other weird characters that you'd need quotation marks.

Q. Should we allow pure integers for the <id>, with the expectation that they disable intrinsic VB compiler warnings? All user-defined warnings (that come through Roslyn diagnostics) would have error codes that aren't pure integers. -- Yes, let's allow integers, just like C#, and just like the VB command-line. It will be a common case.

Q. Will the Roslyn diagnostics APIs allow people to define error codes that are just pure numbers, even though they'll be impossible to disable them? -- No real need. Diagnostic-authors will learn soon enough not to do this.

Q. Should we allow you to disable/enable command-separated lists of warnings in a single #Disable Warning / #Enable Warning directive? -- Yes.

Q. Preprocessor directives look ugly because they're always left-aligned. Can we make it so preprocessor directives can be indented? There's a uservoice request for it. -- Not discussed; left for another day.

Back-compat break with interface implementations

We also discussed another topic to do with ambiguity between unrelated interfaces. Discussion is in this thread:
https://roslyn.codeplex.com/discussions/570975
Coordinator
Nov 11, 2014 at 6:59 AM

Update: VB LDM 2014.07.01

We re-visited #Disable Warning. The summary for VB is:
  • Use the syntax #Disable Warning id1, id2, ...
  • The warning IDs must parse as per the same rules of VB identifiers, and are case-insensitive. That rules out plain numbers #Disable Warning 4008 (instead you must disable BC4008) and it rules out strings #Disable Warning "Don't Use Capitals!". That means you can't disable warnings that have fancy error codes with punctuation and so on.
  • This will guide users to the thought that they can right-click, find all references, and opens up the mental door to other refactorings/tooling
  • We will restrict what identifiers can be produced at the production side too, when authoring analyzers
  • We used to give BC2026 “that warning can’t be configured” when you try to nowarn or warnaserror on non-existent warnings (and also on certain named warnings e.g. 2026). But that wouldn’t work for diagnostic warning codes. So let’s no longer emit BC2026 ever. Instead, if you try to disable or enable a non-existing warning, simply do nothing.
  • Stretch goal: the IDE could grey out all identifiers that have been disabled but are never actually generated by the code
C# will be mostly similar, but
  • C# will support disabling warnings/errors by number as well as by identifier, and is case sensitive. It has to support disabling numbers for back-compat. If you disable warning number 4008, that's equivalent to disabling identifier CS4008.
  • As with VB, we stop validating whether the warning code is a legal compiler warning ID (both on the command-line and inside #pragma) - CS1691 will no longer be reported for C#.
Things that fall out for free:
  • No changes required to disallow Hexadecimal, Octal, Long etc. literals
  • No need to worry about making BC2026 unerrorable
For the future:
  • Update lightbulb suppression to always generate #pragma / #disable with identifiers. (Do we need to check that the id will be a legal identifier before spitting the code?)
  • Fix /nowarn and /warnaserror command-line switches to support custom diagnostics
  • Implement grey out for “unused” ids in #pragma, #Disable (plus corresponding code fix)
  • Have some way for users to get from a reported warning id / from a warning id used in #pragma or #Disable to a description of the diagnostic
  • Support for qualified identifiers that use dots (only if customers ask for it)