Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/1898 fix diagnostics #1928

Merged
merged 53 commits into from
May 11, 2023
Merged

Feature/1898 fix diagnostics #1928

merged 53 commits into from
May 11, 2023

Commits on May 3, 2023

  1. Exploit unconstrained type parameter annotations

    This enables us to re-enable warnings CS8600, CS8602, CS8603, CS8604, and CS8632.
    
    When support for nullable reference types was added to Rx in 2020, you were not allowed to write `T?` if `T` was an unconstrained type parameter. Although that made sense - if `T` could be either a value type or a reference type, `T?` could mean fundamentally different things for different type arguments - in practice it was unhelpful because it meant there was no straightforward way to indicate "nullable in cases where T is a reference type", which is often what was required in practice.
    
    C# 9.0 added a feature that removed this limitation:
    https://github.com/dotnet/csharplang/blob/main/proposals/csharp-9.0/unconstrained-type-parameter-annotations.md
    
    This commit takes advantage of this language feature to better express some of the existing null handling.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    c764ce5 View commit details
    Browse the repository at this point in the history
  2. Re-enable CA1003 (no statics on generic types)

    The analyzer was producing a spurious diagnostic for TaskObservableMethodBuilder<T> - the rules for how method builders work require them to be generic, and to define a static Create method.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    cf30c81 View commit details
    Browse the repository at this point in the history
  3. Re-enable CA1003 (event delegate type)

    This analyzer wanted IEventSource<T>.OnNext and IEventPatternSource<T>.OnNext to use EventHandler<T>, but by design these takes the unusual step of using other delegate types for an event.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    176f34b View commit details
    Browse the repository at this point in the history
  4. Re-enable CA1014 and CA1019

    No longer causing warnings, possibly because changes in earlier commits have now resolved these issues.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    45a814a View commit details
    Browse the repository at this point in the history
  5. Re-enable CA1033

    Add suppression to ScheduledItem because this analyzer wanted us to make its IDisposable implementation overridable, which would not really be appropriate for that specialized base type.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    e38ea92 View commit details
    Browse the repository at this point in the history
  6. Re-enable CA1045, CA1051

    Suppressed spurious warnings on the TaskObservableMethodBuilder, which has to use ref because it's an async method builder.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    88dbb4e View commit details
    Browse the repository at this point in the history
  7. Re-enable CA1052

    Added suppression to CA1052 because it is designed to be inherited from, despite having no instance members.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    69e9097 View commit details
    Browse the repository at this point in the history
  8. Re-enable CA1063

    Add suppressions for the classes where this analyzer wants us to make breaking changes to the public API.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    e557cc8 View commit details
    Browse the repository at this point in the history
  9. Re-enable CA1067

    Suppress in the test code that this analyzer identifies, whihc doesn't need the recommended change.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    075316c View commit details
    Browse the repository at this point in the history
  10. Re-enable CA1068.

    Make suggested changes for internal methods. Add suppression for the two public methods for which this analyzer suggests a breaking change (and for the one internal method where the signature is determined by the ISchedulerLongRunning interface and can't be changed).
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    803d9ca View commit details
    Browse the repository at this point in the history
  11. Re-enable CA1507 (nameof)

    Took all recommended changes.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    747279b View commit details
    Browse the repository at this point in the history
  12. Re-enable CA1707

    This no longer seems to show up. Perhaps other changes have now addressed this.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    853908b View commit details
    Browse the repository at this point in the history
  13. Re-enable CA1711 (don't use Ex suffix.)

    Add suppressions for the public types that have existed for many years.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    499a57c View commit details
    Browse the repository at this point in the history
  14. Re-enable CA1716

    Suppress the one place this showed up. It was complaining about the use of 'error' as an argument name. It has been that way for years, and changing this name would break any code using named arguments.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    60e9834 View commit details
    Browse the repository at this point in the history
  15. Re-enable CA1720 (Identifier contains type name.)

    This didn't like Single, but that's a long-established LINQ method name, so we just have to suppress the warning.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    43f7072 View commit details
    Browse the repository at this point in the history
  16. Re-enable CA1806 (unused instances)

    Added suppressions in the various tests for which this was a spurious message, because they all expect the relevant constructor to throw.
    
    Fixed one actual bug! (ForkJoin_Nary_Immediate called SequenceEqual but never inspected the result. It should have been AssertEqual.)
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    e9b2654 View commit details
    Browse the repository at this point in the history
  17. Re-enable CA1815 (structs implement Equals)

    Suppressed on task builder where it's not required.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    9e04b52 View commit details
    Browse the repository at this point in the history
  18. Re-enable CA1816

    Add suppression for ScheduledItem for same reason as other IDisposable-related suppressions.
    idg10 committed May 3, 2023
    Configuration menu
    Copy the full SHA
    9330b97 View commit details
    Browse the repository at this point in the history

Commits on May 4, 2023

  1. Re-enable CA1822 in non-test projects

    Now disabled in test projects only, because it would take a lot of work to adjust the test projects to fit this rule, with little benefit.
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    e8e0606 View commit details
    Browse the repository at this point in the history
  2. Re-enable CA1825 (Array.Empty<T>())

    Remains disabled in test projects, because the impact on readability is not justified by the minimal performance benefits there.
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    6ccd0b6 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    28b05e3 View commit details
    Browse the repository at this point in the history
  4. Re-enable CA1845 (span-based string concatenation)

    Now disabled for tests only. This is a fairly low-level change that won't have a measurable impact on test performance, so we only want this enabled for the production libraries.
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    00408a2 View commit details
    Browse the repository at this point in the history
  5. Re-enable CA2016 (forward CancellationToken)

    A few tests were deliberately not forwarding it in order to test scenarios where that happens. These now do so explicitly to avoid the warning (or, in the one case where we specifically want to test an Rx overload that doesn't take a CancellationToken argument, we just suppress the message).
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    4c70a81 View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    2f63e8d View commit details
    Browse the repository at this point in the history
  7. Re-enable CA2109 (don't throw reserved exceptions)

    Added a couple of suppressions because for backwards compatibility, we need to continue to throw NullReferenceException in a couple of places.
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    e72541b View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    1901547 View commit details
    Browse the repository at this point in the history
  9. Re-enable CA2249

    Also add comment explaining why we're leaving CA2213 disabled.
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    6aa3340 View commit details
    Browse the repository at this point in the history
  10. Configuration menu
    Copy the full SHA
    16b4034 View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    5ffb2be View commit details
    Browse the repository at this point in the history
  12. Re-enable IDE0017 (object initializers)

    Set to silent suggest in .editorconfig, because the analyzer tends to be too keen: it suggests it even for individual properties, with the result that its 'simpler' suggestion is more verbose than what it replaces.
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    e8ac782 View commit details
    Browse the repository at this point in the history
  13. Re-enable expression-level IDExxx analyzers

    Enabled IDE0018, IDE0034, IDE0039, IDE0056, IDE0057, IDE0090 and IDE0180
    
    Also brought a few straggling non-var declarations into line with the (obviously wrong, but sadly established) policy of using var everywhere.
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    2fa607b View commit details
    Browse the repository at this point in the history
  14. Re-enable pattern analyzers

    IDE0019, IDE0020, IDE0038 and IDE0083.
    
    Used modern patterns where suggested.
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    596ba3c View commit details
    Browse the repository at this point in the history
  15. Configuration menu
    Copy the full SHA
    d3b34c2 View commit details
    Browse the repository at this point in the history
  16. Re-enable IDE0032 (auto properties)

    But we're downgrading it from suggest to silent, because in all the cases I looked at where this suggested a change, the existing use of fields made it easier to see how the internal state of the various objects worked.
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    1378a05 View commit details
    Browse the repository at this point in the history
  17. Re-enable IDE0037 (inferred member names)

    Also change a corresponding use of anonymous types to tuples.
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    aa2fe01 View commit details
    Browse the repository at this point in the history
  18. Re-enable modifier analyzers

    IDE0040 and IDE0044.
    
    Also added various missing private and readonly modifiers.
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    b92bb7e View commit details
    Browse the repository at this point in the history
  19. Re-enable private member analyzers

    IDE0051 and IDE0052.
    idg10 committed May 4, 2023
    Configuration menu
    Copy the full SHA
    3cbec48 View commit details
    Browse the repository at this point in the history

Commits on May 5, 2023

  1. Re-enable IDE0060 (remove unused parameters)

    Added suppressions in cases where unused parameters are present by design.
    idg10 committed May 5, 2023
    Configuration menu
    Copy the full SHA
    4776b87 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    e4c0ccd View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    8f5749d View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    ad8a973 View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    873edad View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    4d0d04c View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    ca4f3c4 View commit details
    Browse the repository at this point in the history

Commits on May 9, 2023

  1. Re-enable IDE0076 (invalid suppression target)

    Removed a load of CA1062 suppressions because that warning applies to public members (it checks that arguments are checked) but it had been applied to a load of non-public members.
    idg10 committed May 9, 2023
    Configuration menu
    Copy the full SHA
    fd731bd View commit details
    Browse the repository at this point in the history
  2. Re-enable IDE0077 (legacy target format in suppressions)

    Updated the SuppressMessage attributes in question to use the modern format.
    idg10 committed May 9, 2023
    Configuration menu
    Copy the full SHA
    8b38288 View commit details
    Browse the repository at this point in the history

Commits on May 10, 2023

  1. Fix unnecessary suppressions

    Move over more comprehensively to Roslyn analyzers. (It's not clear whether the old style analyzers were still running before. But some of the suppressions were causing problems.)
    idg10 committed May 10, 2023
    Configuration menu
    Copy the full SHA
    05b956b View commit details
    Browse the repository at this point in the history

Commits on May 11, 2023

  1. Configuration menu
    Copy the full SHA
    ed3e1eb View commit details
    Browse the repository at this point in the history
  2. Re-enable IDE0280 (use nameof)

    Tweaks to scoping rules in C# 11.0 mean we can now use `nameof` in a few cases where it used not to work. (Method parameter names are now in scope in parameter and return: attributes.)
    idg10 committed May 11, 2023
    Configuration menu
    Copy the full SHA
    7a68291 View commit details
    Browse the repository at this point in the history
  3. Remove IDE1056 suppression

    I think this one was a typo - currently there is no such diagnostic.
    idg10 committed May 11, 2023
    Configuration menu
    Copy the full SHA
    f7735aa View commit details
    Browse the repository at this point in the history
  4. Upgrade analysis levels to 7.0

    idg10 committed May 11, 2023
    Configuration menu
    Copy the full SHA
    a25aa21 View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    a89b22b View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    c27ef2e View commit details
    Browse the repository at this point in the history