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

CA1801 is reported for event-handler method #2593

Closed
Tiergan opened this issue Jun 18, 2019 · 3 comments · Fixed by #2638
Closed

CA1801 is reported for event-handler method #2593

Tiergan opened this issue Jun 18, 2019 · 3 comments · Fixed by #2638

Comments

@Tiergan
Copy link

Tiergan commented Jun 18, 2019

Analyzer package

Microsoft.CodeAnalysis.FxCopAnalyzers

Package Version

v2.9.3

Diagnostic ID

CA1801

Repro steps

  1. Create a new UWP project
  2. Add button to MainPage.xaml
  3. Add event handler for button click
  4. Build project and change show issue generated to Build Only

Example:

XAML

<Page
    x:Class="App.MainPage"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:local="using:App"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    mc:Ignorable="d"
    Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">

    <Grid>
        <Button Click="Button_Click"/>
    </Grid>
</Page>

CS

    public sealed partial class MainPage : Page
    {
        public MainPage()
        {
            this.InitializeComponent();
        }
        private void Button_Click(object sender, RoutedEventArgs e)
        {
        }
    }

Expected behavior

CA1801 is not reported as the method is event handler.

Actual behavior

Similar to #2458, CA1801 is being reported. Although the sub type is not EventArgs, the intention is the same and should be ignored. It is also impossible to change RoutedEventArgs to EventArgs to mitigate this issue.

@sharwell
Copy link
Member

Related to #2338 and #2458

@sharwell
Copy link
Member

@mavasani Could the exclusion for EventArgs-derived types be failing because this is a UWP project?

@mavasani
Copy link
Contributor

mavasani commented Jun 26, 2019

@mavasani Could the exclusion for EventArgs-derived types be failing because this is a UWP project?

Yes, this is indeed correct. I don't believe the analyzer should be adjusted to bail out on this UWP specific issue. I believe there are 2 possible ways to address this:

  1. Users should mark parameters that are required just for signature constraint as discard parameter named _. I am also going to enhance the analyzer to treat names such as _1, _2 as discard symbol names, so multiple parameters can be made discards.
  2. The UWP library owners can author a DiagnosticSuppressor that detects this case and suppresses CA1801. The suppressor should ship along with the framework/library as a development dependency.

I am going to mark this issue as external/by-design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants